RE: [PATCH v5 0/8] mmc: sdhci-esdhc-imx: fix SDIO suspend/resume issues

> -----Original Message-----
> From: Ulf Hansson <[email protected]>
> Sent: Friday, July 10, 2026 9:20 PM
> To: Luke Wang (OSS) <[email protected]>
> Cc: [email protected]; [email protected]; Bough Chen
> <[email protected]>; Frank Li <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-S32 <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH v5 0/8] mmc: sdhci-esdhc-imx: fix SDIO suspend/resume
> issues
> 
> [You don't often get email from [email protected]. Learn why
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Tue, Jul 7, 2026 at 1:00 PM <[email protected]> wrote:
> >
> > From: Luke Wang <[email protected]>
> >
> > This series fixes several suspend/resume issues in the sdhci-esdhc-imx
> > driver, primarily affecting SDIO devices (e.g., WiFi).
> >
> > Issues fixed:
> > - Tuning delay values not saved/restored for out-of-band wakeup devices
> > - DLL override lost after resume for DDR modes
> > - Pinctrl not restored before DDR_EN is set, causing CRC errors
> > - Unhandled interrupt during resume causing "nobody cared" warning
> > - Various error handling issues in suspend/resume paths
> >
> > Changes since v4:
> > - New patch 3: Fix esdhc_change_pinstate() to move IS_ERR checks for
> >   pins_100mhz/pins_200mhz into their respective switch cases, so that
> >   legacy timing modes can still reach pinctrl_select_default_state().
> >   This avoids log spam on resume for devices without UHS pin states
> (Sashiko)
> >
> > Changes since v3:
> > - Patch 2: Remove unused 'boarddata' variable in esdhc_set_uhs_signaling()
> > - Patch 7: Expand commit message to document intentional skip of
> enable_irq()
> >   and sdhci_disable_irq_wakeups() on early return, and explain why the
> >   PM runtime usage counter leak is acceptable (Sashiko)
> >
> > Changes since v2:
> > - Patch 3: Updated commit message to explain the SoC IP integration
> >   detail about internal loopback path change when switching pinctrl
> >   from GPIO to USDHC function (per Bough Chen suggestion)
> > - Patch 5 (v2): Split into three separate patches (5/6/7) so each
> >   patch fixes one problem (per Frank Li suggestion)
> >
> > Changes since v1:
> > - Added patch 5 to fix error handling issues identified during review
> > - Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
> > - Make pinctrl and cd-wake failures non-fatal (dev_warn only)
> > - Use esdhc_change_pinstate() instead of pinctrl_pm_select_default_state()
> >   in resume to restore correct pin state based on timing mode
> > - Skip pinctrl restore for wakeup devices to avoid SD bus glitch
> > - Check pm_runtime_force_resume() return value in resume
> >
> > Luke Wang (8):
> >   mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq
> check
> >     for tuning save/restore
> >   mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume
> >   mmc: sdhci-esdhc-imx: fix esdhc_change_pinstate() to allow default
> >     state restore
> >   mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on
> >     resume
> >   mmc: sdhci-esdhc-imx: disable irq during suspend to fix unhandled
> >     interrupt
> >   mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend
> >   mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend
> >   mmc: sdhci-esdhc-imx: fix resume error handling
> >
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 84 ++++++++++++++++++++----------
> >  1 file changed, 57 insertions(+), 27 deletions(-)
> >
> > --
> > 2.34.1
> >
> 
> Looks like this entire series may be material for stable kernels, but
> perhaps it's easier to manage that through manual backports to do the
> quite extensive changes? I can certainly tag the patches for stable,
> but in that case we should have a fixes tag or specific kernel version
> for all patches in the series, can you please help with that.
> 
> I am leaving a few more days for reviews before I queue this up for
> next to get it tested.

Thanks a lot for the review and for offering to tag the patches for stable.

I have made sure every patch in the series now carries a proper Fixes tag.
This is the only change in v6, there are no functional changes.

The Fixes mapping is as follows:

  Patch 1 (remove mmc_card_wake_sdio_irq check for tuning save/restore):
    Fixes: c63d25cdc59a ("mmc: sdhci-esdhc-imx: Save tuning value when
    card stays powered in suspend")   -> first appeared in v6.16

  Patches 2-8 (all the DLL/pinctrl/irq/PM error-handling fixes):
    Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system
    PM logic")                         -> first appeared in v6.16

So the whole series only needs to reach v6.16 and later stable kernels.

I have verified the backport to v6.16.y. As you already suspected, the
extensive changes do not cherry-pick entirely cleanly: there are two
trivial, non-functional context conflicts:

  - v6.16 still has the explicit pm_runtime_mark_last_busy(dev) call in
    sdhci_esdhc_resume(), which should be kept when applying patch 2.
  - An unrelated later refactor of the suspend pinctrl path is not
    present in v6.16, so patch 7 applies to the plain unconditional
    pinctrl_pm_select_sleep_state() call there.

I am happy to send a dedicated 6.16.y backport if the automatic pick fails.

Thanks
Luke Wang

> 
> Kind regards
> Uffe