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

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

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

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

All patches now carry a Fixes tag pointing at v6.16. Patches 2-8 fix
regressions introduced by commit 676a83855614 ("mmc: host:
sdhci-esdhc-imx: refactor the system PM logic") in v6.16; patch 1 fixes
commit c63d25cdc59a ("mmc: sdhci-esdhc-imx: Save tuning value when card
stays powered in suspend"), also in v6.16.

Changes since v6:
- Patch 7: Drop "ret =" for non-fatal calls (sdhci_enable_irq_wakeups,
  pinctrl_pm_select_sleep_state, mmc_gpio_set_cd_wake) so they no longer
  touch ret. Call them directly in if() to keep semantics clear and
  avoid any risk of a non-fatal return value polluting the fatal error
  path. (Frank Li)

Changes since v5:
- Added Fixes tags to all remaining patches (patch 3, 6, 7, 8) so the
  whole series is properly tagged for stable, as requested by Ulf.
  No functional changes.

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 | 91 +++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 33 deletions(-)

-- 
2.34.1

[PATCH v7 1/8] mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

The tuning save/restore during system PM is conditioned on
mmc_card_wake_sdio_irq(), but this check is unrelated to whether
tuning values need to be preserved. The actual requirement is that
the card keeps power during suspend and the controller is a uSDHC.

SDIO devices using out-of-band GPIO wakeup maintain power during
suspend but do not set the SDIO IRQ wake flag. In this case the
tuning delay values are not saved/restored.

Remove the unnecessary mmc_card_wake_sdio_irq() condition from both
the suspend save and resume restore paths.

Fixes: c63d25cdc59a ("mmc: sdhci-esdhc-imx: Save tuning value when card stays powered in suspend")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 18ecddd6df6f..6526d65538de 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2064,8 +2064,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	 * to save the tuning delay value just in case the usdhc
 	 * lost power during system PM.
 	 */
-	if (mmc_card_keep_power(host->mmc) && mmc_card_wake_sdio_irq(host->mmc) &&
-	    esdhc_is_usdhc(imx_data))
+	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
 		sdhc_esdhc_tuning_save(host);
 
 	if (device_may_wakeup(dev)) {
@@ -2124,8 +2123,7 @@ static int sdhci_esdhc_resume(struct device *dev)
 	 * restore the saved tuning delay value for the device which keep
 	 * power during system PM.
 	 */
-	if (mmc_card_keep_power(host->mmc) && mmc_card_wake_sdio_irq(host->mmc) &&
-	    esdhc_is_usdhc(imx_data))
+	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
 		sdhc_esdhc_tuning_restore(host);
 
 	pm_runtime_put_autosuspend(dev);
-- 
2.34.1

Re: [PATCH v7 1/8] mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced `disable_irq()` and `enable_irq()` during suspend/resume for devices with out-of-band wakeup, causing the SDHCI IRQ to remain permanently disabled after system resume.
- [High] The system suspend callback `sdhci_esdhc_suspend()` ignores the return value of `pm_runtime_force_suspend()`, leading to an unbalanced `pm_runtime_enable()` and PM state machine corruption on failure.
- [High] The runtime resume callback `sdhci_esdhc_runtime_resume()` leaks hardware clocks and PM QoS requests if `cqhci_resume()` fails.
--

--- Patch [1]: [PATCH v7 1/8] mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore ---
commit e08eb8101cb72edc4b3a76b418077811fe74beba
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore

This commit removes the mmc_card_wake_sdio_irq() check from the tuning
delay save and restore paths during system PM. The check is removed
because tuning values need to be preserved based on whether the card keeps
power, regardless of if it sets the SDIO IRQ wake flag.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index d0fa83f67a806..6e06b3eeacce9 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2064,8 +2064,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 * to save the tuning delay value just in case the usdhc
>  	 * lost power during system PM.
>  	 */
> -	if (mmc_card_keep_power(host->mmc) && mmc_card_wake_sdio_irq(host->mmc) &&
> -	    esdhc_is_usdhc(imx_data))
> +	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
>  		sdhc_esdhc_tuning_save(host);
>  
>  	if (device_may_wakeup(dev)) {

[Severity: High]
This isn't a bug introduced by this patch, but can the host IRQ remain
permanently disabled after a system resume for devices with out-of-band wakeup?

In sdhci_esdhc_suspend(), the irq is unconditionally disabled and wakeups are
enabled if the device may wake up:

    disable_irq(host->irq);
    if (device_may_wakeup(dev)) {
        if (!sdhci_enable_irq_wakeups(host))
            ...

For SDIO devices using out-of-band GPIO wakeup, mmc_card_wake_sdio_irq()
evaluates to false. This causes sdhci_enable_irq_wakeups() to return false
immediately without setting host->irq_wake_enabled = true.

If enable_irq() in sdhci_esdhc_resume() is explicitly gated behind 
host->irq_wake_enabled, does this leave the IRQ permanently disabled after
the first suspend/resume cycle, requiring a system reboot or unbinding to
recover?


[Severity: High]
This is a pre-existing issue, but does sdhci_esdhc_suspend() ignore the return
value of pm_runtime_force_suspend()?

If pm_runtime_force_suspend() fails, it handles cleanup by calling
pm_runtime_enable(dev) and returns an error code. Since sdhci_esdhc_suspend()
ignores this return value, the PM core considers the device successfully
suspended.

Upon system resume, sdhci_esdhc_resume() calls pm_runtime_force_resume(dev).
Since the device failed to suspend and never set needs_force_resume, this skips
the active resume step and executes its out path, which unconditionally calls
pm_runtime_enable(dev). 

Does this second pm_runtime_enable() drive dev->power.disable_depth below 0,
triggering an unbalanced pm_runtime_enable kernel warning and corrupting the
runtime PM state machine?

> @@ -2124,8 +2123,7 @@ static int sdhci_esdhc_resume(struct device *dev)
>  	 * restore the saved tuning delay value for the device which keep
>  	 * power during system PM.
>  	 */
> -	if (mmc_card_keep_power(host->mmc) && mmc_card_wake_sdio_irq(host->mmc) &&
> -	    esdhc_is_usdhc(imx_data))
> +	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
>  		sdhc_esdhc_tuning_restore(host);
>  
>  	pm_runtime_put_autosuspend(dev);

[Severity: High]
This isn't a bug introduced by this patch, but does sdhci_esdhc_runtime_resume()
leak hardware clocks and PM QoS requests if cqhci_resume() fails?

If the device supports Command Queuing Engine (CQE), the function calls
cqhci_resume():

    if (host->mmc->caps2 & MMC_CAP2_CQE)
        err = cqhci_resume(host->mmc);

    return err;

If cqhci_resume() returns an error, the function returns directly instead of
jumping to the disable_per_clk labels to call clk_disable_unprepare() for the
three clocks and cpu_latency_qos_remove_request(). 

Does this leave the hardware resources permanently active while the PM core
treats the device as runtime suspended or in an error state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=1

[PATCH v7 2/8] mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

sdhci_esdhc_imx_hwinit() unconditionally clears ESDHC_DLL_CTRL by
writing zero. For SDIO devices that keep power during system suspend
and operate in DDR mode, the card remains in DDR timing while the host
DLL override configuration is lost.

Extract the DLL override setup from esdhc_set_uhs_signaling() into
a helper esdhc_set_dll_override(), and call it on the resume path
when the card kept power and is using a DDR timing mode.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 39 +++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6526d65538de..7230d70e02ae 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1349,6 +1349,23 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
 	return pinctrl_select_state(imx_data->pinctrl, pinctrl);
 }
 
+static void esdhc_set_dll_override(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
+	u32 v;
+
+	if (!boarddata->delay_line)
+		return;
+
+	v = boarddata->delay_line << ESDHC_DLL_OVERRIDE_VAL_SHIFT |
+	    (1 << ESDHC_DLL_OVERRIDE_EN_SHIFT);
+	if (is_imx53_esdhc(imx_data))
+		v <<= 1;
+	writel(v, host->ioaddr + ESDHC_DLL_CTRL);
+}
+
 /*
  * For HS400 eMMC, there is a data_strobe line. This signal is generated
  * by the device and used for data output and CRC status response output
@@ -1404,7 +1421,6 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
 	u32 m;
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
 	/* disable ddr mode and disable HS400 mode */
 	m = readl(host->ioaddr + ESDHC_MIX_CTRL);
@@ -1425,15 +1441,7 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
 		m |= ESDHC_MIX_CTRL_DDREN;
 		writel(m, host->ioaddr + ESDHC_MIX_CTRL);
 		imx_data->is_ddr = 1;
-		if (boarddata->delay_line) {
-			u32 v;
-			v = boarddata->delay_line <<
-				ESDHC_DLL_OVERRIDE_VAL_SHIFT |
-				(1 << ESDHC_DLL_OVERRIDE_EN_SHIFT);
-			if (is_imx53_esdhc(imx_data))
-				v <<= 1;
-			writel(v, host->ioaddr + ESDHC_DLL_CTRL);
-		}
+		esdhc_set_dll_override(host);
 		break;
 	case MMC_TIMING_MMC_HS400:
 		m |= ESDHC_MIX_CTRL_DDREN | ESDHC_MIX_CTRL_HS400_EN;
@@ -2123,9 +2131,18 @@ static int sdhci_esdhc_resume(struct device *dev)
 	 * restore the saved tuning delay value for the device which keep
 	 * power during system PM.
 	 */
-	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
+	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data)) {
 		sdhc_esdhc_tuning_restore(host);
 
+		/*
+		 * Restore DLL override for DDR modes. hwinit unconditionally
+		 * clears ESDHC_DLL_CTRL, but the card is still in DDR mode.
+		 */
+		if (host->timing == MMC_TIMING_UHS_DDR50 ||
+		    host->timing == MMC_TIMING_MMC_DDR52)
+			esdhc_set_dll_override(host);
+	}
+
 	pm_runtime_put_autosuspend(dev);
 
 	return ret;
-- 
2.34.1

Re: [PATCH v7 2/8] mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] PM Runtime usage counter leak on system suspend error path.
--

--- Patch [2]: [PATCH v7 2/8] mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume ---
commit 6d9ae84daab041f83a8a4ceef937b796f9b804f4
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume

sdhci_esdhc_imx_hwinit() unconditionally clears ESDHC_DLL_CTRL by
writing zero. For SDIO devices that keep power during system suspend
and operate in DDR mode, the card remains in DDR timing while the host
DLL override configuration is lost.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6e06b3eeacce9..685e508c61279 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
[ ... ]
> @@ -2123,9 +2131,18 @@ static int sdhci_esdhc_resume(struct device *dev)

[Severity: High]
This is a pre-existing issue, but while reviewing the system PM logic here,
I noticed a PM runtime usage counter leak in the suspend error path.

In sdhci_esdhc_suspend(), pm_runtime_get_sync() increments the usage counter
early in the function, but the error path for pinctrl_pm_select_sleep_state()
returns without balancing it:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_suspend() {
    ...
	pm_runtime_get_sync(dev);
    ...
	if (device_may_wakeup(dev)) {
    ...
	} else {
    ...
		ret = pinctrl_pm_select_sleep_state(dev);
		if (ret)
			return ret;
    ...
}

If pinctrl_pm_select_sleep_state(dev) fails, does this permanently leak the
PM runtime usage counter, since the system suspend is aborted and the
corresponding resume callback will not run?

>  	 * restore the saved tuning delay value for the device which keep
>  	 * power during system PM.
>  	 */
> -	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
> +	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data)) {
>  		sdhc_esdhc_tuning_restore(host);
>  
> +		/*
> +		 * Restore DLL override for DDR modes. hwinit unconditionally
> +		 * clears ESDHC_DLL_CTRL, but the card is still in DDR mode.
> +		 */
> +		if (host->timing == MMC_TIMING_UHS_DDR50 ||
> +		    host->timing == MMC_TIMING_MMC_DDR52)
> +			esdhc_set_dll_override(host);
> +	}
> +
>  	pm_runtime_put_autosuspend(dev);
>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=2

[PATCH v7 3/8] mmc: sdhci-esdhc-imx: fix esdhc_change_pinstate() to allow default state restore

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

esdhc_change_pinstate() checks for pins_100mhz and pins_200mhz at the
top of the function and returns -EINVAL if either is not defined. This
prevents the default case from ever being reached, which means devices
with a sleep pinctrl state but without high-speed pin states (100mhz/
200mhz) can never restore their default pin configuration.

Move the IS_ERR checks for pins_100mhz and pins_200mhz into their
respective switch cases.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7230d70e02ae..ead4685d621a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1326,19 +1326,21 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
 
 	dev_dbg(mmc_dev(host->mmc), "change pinctrl state for uhs %d\n", uhs);
 
-	if (IS_ERR(imx_data->pinctrl) ||
-		IS_ERR(imx_data->pins_100mhz) ||
-		IS_ERR(imx_data->pins_200mhz))
+	if (IS_ERR(imx_data->pinctrl))
 		return -EINVAL;
 
 	switch (uhs) {
 	case MMC_TIMING_UHS_SDR50:
 	case MMC_TIMING_UHS_DDR50:
+		if (IS_ERR(imx_data->pins_100mhz))
+			return -EINVAL;
 		pinctrl = imx_data->pins_100mhz;
 		break;
 	case MMC_TIMING_UHS_SDR104:
 	case MMC_TIMING_MMC_HS200:
 	case MMC_TIMING_MMC_HS400:
+		if (IS_ERR(imx_data->pins_200mhz))
+			return -EINVAL;
 		pinctrl = imx_data->pins_200mhz;
 		break;
 	default:
-- 
2.34.1

[PATCH v7 4/8] mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

SDIO devices such as WiFi may keep power during suspend, so the MMC
core skips full card re-initialization on resume and directly restores
the host controller's ios timing to match the card. For DDR mode,
pm_runtime_force_resume() sets DDR_EN before the pin configuration is
restored from sleep state.

This is related to the SoC IP integration: switching pinctrl setting
(changing alt from GPIO to USDHC) impacts the internal loopback path.
If pinctrl configures the pad to GPIO function, once DDR_EN is set, the
DLL delay will be fixed based on the GPIO function loopback path. When
the pinctrl is later changed to USDHC function, the internal loopback
path changes, making the original fixed sample point no longer suitable
for the current loopback path. This causes persistent read CRC errors on
subsequent data transfers.

SD/eMMC running in DDR mode are unaffected as they are fully
re-initialized from legacy timing after resume.

Fix this by restoring the pinctrl state based on current timing mode
using esdhc_change_pinstate() before pm_runtime_force_resume(). This
ensures the correct pin configuration (e.g., 100/200MHz for UHS modes)
is applied before DDR_EN is set. Only restore for non-wakeup devices
since wakeup devices kept their active pin state during suspend.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index ead4685d621a..a54da5757acc 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2115,6 +2115,12 @@ static int sdhci_esdhc_resume(struct device *dev)
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
+	if (!device_may_wakeup(dev)) {
+		ret = esdhc_change_pinstate(host, host->timing);
+		if (ret)
+			dev_warn(dev, "Failed to restore pinctrl state\n");
+	}
+
 	pm_runtime_force_resume(dev);
 
 	ret = mmc_gpio_set_cd_wake(host->mmc, false);
-- 
2.34.1

Re: [PATCH v7 4/8] mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of `pm_runtime_get_sync()` is ignored in `sdhci_esdhc_suspend()`, which can lead to a synchronous external abort (kernel panic) if it fails.
- [High] The return value of `pm_runtime_force_resume(dev)` is ignored. If it fails, subsequent register accesses will cause a system crash.
- [High] Runtime PM usage count is leaked on the error paths of `sdhci_esdhc_suspend()` and `sdhci_esdhc_resume()`.
- [High] The IRQ remains permanently disabled if `sdhci_enable_irq_wakeups()` fails during system suspend.
--

--- Patch [4]: [PATCH v7 4/8] mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume ---
commit fff04de9eca73d5e5d595f7bfc0d534292f22cd5
Author: Luke Wang <ziniu.wang_1@nxp.com>
mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume

This commit fixes persistent read CRC errors on SDIO devices resuming in DDR
mode. It ensures the pinctrl state is correctly restored for the current timing
before enabling DDR mode during resume. Wakeup devices are skipped as they
keep their active pin state during suspend.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 2da5c8ab3e622..bc0864cb22b49 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2115,6 +2115,12 @@ static int sdhci_esdhc_resume(struct device *dev)
>  	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
>  	int ret;
>  
> +	if (!device_may_wakeup(dev)) {
> +		ret = esdhc_change_pinstate(host, host->timing);
> +		if (ret)
> +			dev_warn(dev, "Failed to restore pinctrl state\n");
> +	}
> +
>  	pm_runtime_force_resume(dev);

[Severity: High]
This is a pre-existing issue, but the return value of pm_runtime_force_resume()
is ignored here. If it fails, could the subsequent unclocked hardware register
accesses in sdhci_esdhc_imx_hwinit() cause a synchronous external abort?

>  
>  	ret = mmc_gpio_set_cd_wake(host->mmc, false);

[Severity: High]
This is a pre-existing issue, but if mmc_gpio_set_cd_wake() fails, the early
return skips the required pm_runtime_put_autosuspend() at the end of the
function:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_resume() {
    ...
	ret = mmc_gpio_set_cd_wake(host->mmc, false);
	if (ret)
		return ret;
    ...
	pm_runtime_put_autosuspend(dev);
	return 0;
}

Could this leak the runtime PM usage count and prevent the device from ever
entering runtime suspend again?


[Severity: High]
This is a pre-existing issue, but could a similar leak happen in the suspend
path if pinctrl_pm_select_sleep_state() fails?

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_suspend() {
	...
	pm_runtime_get_sync(dev);
	...
	} else {
		ret = pinctrl_pm_select_sleep_state(dev);
		if (ret)
			return ret;
	}
	...
}

Does the early return here leak the PM reference acquired by
pm_runtime_get_sync()?


[Severity: High]
This is a pre-existing issue, but the return value of pm_runtime_get_sync()
in sdhci_esdhc_suspend() is completely ignored:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_suspend() {
    ...
	pm_runtime_get_sync(dev);

	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
    ...
}

If pm_runtime_get_sync() fails to enable the clocks, could the subsequent
hardware register accesses via sdhc_esdhc_tuning_save() cause a kernel panic?


[Severity: High]
This is a pre-existing issue, but could the IRQ remain permanently disabled if
sdhci_enable_irq_wakeups() fails during suspend?

In sdhci_esdhc_suspend(), the IRQ is disabled unconditionally when the device
may wake up:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_suspend() {
    ...
	if (device_may_wakeup(dev)) {
		disable_irq(host->irq);
		ret = sdhci_enable_irq_wakeups(host);
    ...
}

But in sdhci_esdhc_resume(), it is only re-enabled if host->irq_wake_enabled is
true, which requires sdhci_enable_irq_wakeups() to have succeeded:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_resume() {
    ...
	if (host->irq_wake_enabled) {
		sdhci_disable_irq_wakeups(host);
		enable_irq(host->irq);
	}
    ...
}

If sdhci_enable_irq_wakeups() fails in the suspend path, will the device become
completely unresponsive to interrupts after system resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=4

[PATCH v7 5/8] mmc: sdhci-esdhc-imx: disable irq during suspend to fix unhandled interrupt

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

When using WIFI out-of-band wakeup, an "irq xxx: nobody cared" warning
occurs. This happens because the usdhc interrupt is not disabled during
system suspend when device_may_wakeup() returns false.

The sequence of events leading to this issue:
1. System enters suspend without disabling usdhc interrupt
(because device_may_wakeup() returns false for usdhc device)
2. WIFI out-of-band wakeup triggers system resume via GPIO interrupt
3. WIFI sends a Card interrupt before usdhc has fully resumed
4. usdhc is still in runtime suspend state and cannot handle the
interrupt properly
5. The unhandled interrupt triggers "nobody cared" warning

Fix this by unconditionally disabling the usdhc interrupt during suspend
and re-enabling it during resume, regardless of the wakeup capability.
This ensures no interrupts are processed during the suspend/resume
transition.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a54da5757acc..d980dea9cad1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2077,9 +2077,10 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
 		sdhc_esdhc_tuning_save(host);
 
+	/* The irqs of imx are not shared. It is safe to disable */
+	disable_irq(host->irq);
+
 	if (device_may_wakeup(dev)) {
-		/* The irqs of imx are not shared. It is safe to disable */
-		disable_irq(host->irq);
 		ret = sdhci_enable_irq_wakeups(host);
 		if (!ret)
 			dev_warn(dev, "Failed to enable irq wakeup\n");
@@ -2130,10 +2131,10 @@ static int sdhci_esdhc_resume(struct device *dev)
 	/* re-initialize hw state in case it's lost in low power mode */
 	sdhci_esdhc_imx_hwinit(host);
 
-	if (host->irq_wake_enabled) {
+	if (host->irq_wake_enabled)
 		sdhci_disable_irq_wakeups(host);
-		enable_irq(host->irq);
-	}
+
+	enable_irq(host->irq);
 
 	/*
 	 * restore the saved tuning delay value for the device which keep
-- 
2.34.1

[PATCH v7 6/8] mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

Replace pm_runtime_get_sync() with pm_runtime_resume_and_get() to
simplify error handling. pm_runtime_resume_and_get() automatically
drops the usage counter on failure, avoiding the need for a separate
pm_runtime_put_noidle() call. If it fails, the device is unclocked and
accessing hardware registers would cause a kernel panic, so return the
error immediately.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index d980dea9cad1..7818f351a58a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2061,7 +2061,9 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	 * 2, make sure the pm_runtime_force_resume() in sdhci_esdhc_resume() really
 	 *    invoke its ->runtime_resume callback (needs_force_resume = 1).
 	 */
-	pm_runtime_get_sync(dev);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
 
 	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
 		(host->tuning_mode != SDHCI_TUNING_MODE_1)) {
-- 
2.34.1

Re: [PATCH v7 6/8] mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked pm_runtime_get_sync() in sdhci_esdhc_imx_remove() leads to kernel panic on failure.
--

--- Patch [6]: [PATCH v7 6/8] mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend ---
commit f0a062e3623dc14c66d429be48e9ae36028fd188
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend

This patch replaces pm_runtime_get_sync() with pm_runtime_resume_and_get() in
sdhci_esdhc_suspend(). It properly returns an error if resuming fails,
preventing a kernel panic caused by accessing hardware registers when the
device is unclocked.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6e792bee25bfa..ec531a9451c4e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2061,7 +2061,9 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 * 2, make sure the pm_runtime_force_resume() in sdhci_esdhc_resume() really
>  	 *    invoke its ->runtime_resume callback (needs_force_resume = 1).
>  	 */
> -	pm_runtime_get_sync(dev);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but since the commit message notes that accessing
hardware registers while the device is unclocked causes a kernel panic, could
this same panic occur in sdhci_esdhc_imx_remove()?

Looking at the device removal path:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_imx_remove() {
	...
	pm_runtime_get_sync(&pdev->dev);
	dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
	...
}

If pm_runtime_get_sync() fails to resume the device here, execution proceeds to
readl() on the unclocked hardware. Should we also use
pm_runtime_resume_and_get() and check the return value in the remove path to
avoid a panic during unbind?

>  
>  	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
>  		(host->tuning_mode != SDHCI_TUNING_MODE_1)) {
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=6

[PATCH v7 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

Make pinctrl_pm_select_sleep_state() and mmc_gpio_set_cd_wake() failures
non-fatal in the suspend path. These failures only mean slightly higher
power consumption or missing CD wakeup capability, but should not block
system suspend.

Also change the function to always return 0 on the success path instead
of propagating non-fatal warning return values.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7818f351a58a..290a3172931b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2083,8 +2083,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	disable_irq(host->irq);
 
 	if (device_may_wakeup(dev)) {
-		ret = sdhci_enable_irq_wakeups(host);
-		if (!ret)
+		if (!sdhci_enable_irq_wakeups(host))
 			dev_warn(dev, "Failed to enable irq wakeup\n");
 	} else {
 		/*
@@ -2095,12 +2094,12 @@ static int sdhci_esdhc_suspend(struct device *dev)
 		 * other function like GPIO function to save power in PM,
 		 * which finally block the SDIO wakeup function.
 		 */
-		ret = pinctrl_pm_select_sleep_state(dev);
-		if (ret)
-			return ret;
+		if (pinctrl_pm_select_sleep_state(dev))
+			dev_warn(dev, "Failed to select sleep pinctrl state\n");
 	}
 
-	ret = mmc_gpio_set_cd_wake(host->mmc, true);
+	if (mmc_gpio_set_cd_wake(host->mmc, true))
+		dev_warn(dev, "Failed to enable cd wake\n");
 
 	/*
 	 * Make sure invoke runtime_suspend to gate off clock.
@@ -2108,7 +2107,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
 	 */
 	pm_runtime_force_suspend(dev);
 
-	return ret;
+	return 0;
 }
 
 static int sdhci_esdhc_resume(struct device *dev)
-- 
2.34.1

Re: [PATCH v7 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of `pm_runtime_force_suspend()` is ignored during system suspend, which can cause a kernel warning and a potential fatal crash on system resume if the suspend operation fails.
--

--- Patch [7]: [PATCH v7 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend ---
commit 343530f069b563d297215c212283dc394ffae3fd
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend

This commit modifies the suspend path to ensure that failures from certain
wakeup and pinctrl functions do not block the system suspend process. It
accomplishes this by treating these failures as non-fatal warnings and
always returning success.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c

[ ... ]

> @@ -2108,7 +2107,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 */
>  	pm_runtime_force_suspend(dev);
>  
> -	return ret;
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
pm_runtime_force_suspend() here?

If the driver's runtime suspend callback fails (for instance, due to a
cqhci_suspend error), pm_runtime_force_suspend() will return an error and
leave the device in the active state.

By returning 0 regardless of whether pm_runtime_force_suspend() succeeded,
the PM core is led to believe the device suspended successfully. On system
resume, pm_runtime_force_resume() would see the device is still active and
skip calling the runtime_resume callback.

Since the runtime_resume callback enables the clocks, skipping it means the
subsequent call to sdhci_esdhc_imx_hwinit() will attempt to access hardware
registers without clocks enabled. If the power domain was turned off during
sleep, wouldn't this unclocked register access trigger a synchronous external
abort and kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=7

[PATCH v7 8/8] mmc: sdhci-esdhc-imx: fix resume error handling

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

Check pm_runtime_force_resume() return value in resume. If it fails
(clock enable failure), return immediately since accessing hardware
registers on an unclocked device would cause a kernel panic.

The early return intentionally skips enable_irq() and
sdhci_disable_irq_wakeups() because the IRQ handler reads
SDHCI_INT_STATUS, which would also fault without clocks. The PM runtime
usage counter leak only affects this already-broken device instance and
is an acceptable tradeoff to preserve system stability.

Remove the return value check for mmc_gpio_set_cd_wake(host->mmc, false)
since disable_irq_wake() called internally always returns 0.

Also return 0 explicitly on the success path instead of propagating
stale return values.

Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic")
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 290a3172931b..18f4905c15b9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2123,12 +2123,12 @@ static int sdhci_esdhc_resume(struct device *dev)
 			dev_warn(dev, "Failed to restore pinctrl state\n");
 	}
 
-	pm_runtime_force_resume(dev);
-
-	ret = mmc_gpio_set_cd_wake(host->mmc, false);
+	ret = pm_runtime_force_resume(dev);
 	if (ret)
 		return ret;
 
+	mmc_gpio_set_cd_wake(host->mmc, false);
+
 	/* re-initialize hw state in case it's lost in low power mode */
 	sdhci_esdhc_imx_hwinit(host);
 
@@ -2155,7 +2155,7 @@ static int sdhci_esdhc_resume(struct device *dev)
 
 	pm_runtime_put_autosuspend(dev);
 
-	return ret;
+	return 0;
 }
 
 static int sdhci_esdhc_runtime_suspend(struct device *dev)
-- 
2.34.1

Re: [PATCH v7 8/8] mmc: sdhci-esdhc-imx: fix resume error handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Early return on clock enable failure leaks IRQ wake and disable depths for the host IRQ.
- [Medium] Unnecessary PM runtime usage counter leak on the error path.

Pre-existing issues:
- [Critical] The fix to prevent a kernel panic on unclocked register accesses is incomplete, leaving the unclocked device exposed to child MMC accesses.
- [High] Missing return value check for pm_runtime_get_sync() in sdhci_esdhc_imx_remove() leads to unclocked hardware register access and a kernel panic.
--

--- Patch [8]: [PATCH v7 8/8] mmc: sdhci-esdhc-imx: fix resume error handling ---
commit 2ddb161f38e3bf1f1836d2167b881e211d1fa57f
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: fix resume error handling

This commit adds a return value check for pm_runtime_force_resume() in
sdhci_esdhc_resume() to avoid unclocked hardware register access on failure.
It returns early if clock enablement fails, purposefully skipping IRQ
enablement. Additionally, it removes a redundant return check for
mmc_gpio_set_cd_wake() and explicitly returns 0 on success.

>     The early return intentionally skips enable_irq() and
>     sdhci_disable_irq_wakeups() because the IRQ handler reads
>     SDHCI_INT_STATUS, which would also fault without clocks. The PM runtime
>     usage counter leak only affects this already-broken device instance and
>     is an acceptable tradeoff to preserve system stability.

[Severity: Medium]
Could this usage counter leak be avoided entirely? Since the standard
kernel API pm_runtime_put_noidle() drops the usage counter without invoking
any idle callbacks or accessing hardware, would it be cleaner to use it
here instead of leaking the counter on the error path?

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 0824f598bf598..6ab59d3e346f9 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2123,12 +2123,12 @@ static int sdhci_esdhc_resume(struct device *dev)
>  			dev_warn(dev, "Failed to restore pinctrl state\n");
>  	}
>  
> -	pm_runtime_force_resume(dev);
> -
> -	ret = mmc_gpio_set_cd_wake(host->mmc, false);
> +	ret = pm_runtime_force_resume(dev);
>  	if (ret)
>  		return ret;

[Severity: Medium]
Does this early return bypass software-level cleanup of IRQ wake and
disable states? If pm_runtime_force_resume() fails, the function returns
early, bypassing enable_irq(host->irq) and sdhci_disable_irq_wakeups(host).

Because the PM core does not abort system resume on a device resume failure,
a subsequent system suspend will call the suspend callback again.
This unconditionally calls disable_irq() and sdhci_enable_irq_wakeups().

Can this cause the IRQ disable depth and wake depth to accumulate unbounded
on subsequent suspends?

[Severity: Critical]
This isn't a bug introduced by this patch, but does the early return leave
the unclocked device exposed to child MMC accesses?

Returning an error from a parent's system resume callback does not abort
the system resume process for children. The PM core continues and invokes
the resume callback for the child mmc_card device, which unconditionally
triggers sdhci_set_ios() on the host controller and accesses hardware
registers.

Because the SDHCI_DEVICE_DEAD flag is not set before returning early,
sdhci_set_ios() does not abort and accesses the unclocked hardware
registers, potentially triggering the same kernel panic this patch was
intended to fix. Would setting host->flags |= SDHCI_DEVICE_DEAD; before
returning prevent this?

[ ... ]

This isn't a bug introduced by this patch, but looking at
sdhci_esdhc_imx_remove() in the same file:

    pm_runtime_get_sync(&pdev->dev);
    dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);

[Severity: High]
Does this code ignore the return value of pm_runtime_get_sync()?

If resuming the device fails, the clocks remain disabled. The immediate
next instruction accesses SDHCI_INT_STATUS. Can this trigger a kernel panic
due to unclocked hardware access, similar to the one being fixed in the
resume path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715071818.272318-1-ziniu.wang_1@oss.nxp.com?part=8

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

[Ulf Hansson]

On Wed, Jul 15, 2026 at 9:16 AM <ziniu.wang_1@oss.nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> 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
>
> All patches now carry a Fixes tag pointing at v6.16. Patches 2-8 fix
> regressions introduced by commit 676a83855614 ("mmc: host:
> sdhci-esdhc-imx: refactor the system PM logic") in v6.16; patch 1 fixes
> commit c63d25cdc59a ("mmc: sdhci-esdhc-imx: Save tuning value when card
> stays powered in suspend"), also in v6.16.
>
> Changes since v6:
> - Patch 7: Drop "ret =" for non-fatal calls (sdhci_enable_irq_wakeups,
>   pinctrl_pm_select_sleep_state, mmc_gpio_set_cd_wake) so they no longer
>   touch ret. Call them directly in if() to keep semantics clear and
>   avoid any risk of a non-fatal return value polluting the fatal error
>   path. (Frank Li)
>
> Changes since v5:
> - Added Fixes tags to all remaining patches (patch 3, 6, 7, 8) so the
>   whole series is properly tagged for stable, as requested by Ulf.
>   No functional changes.
>
> 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 | 91 +++++++++++++++++++-----------
>  1 file changed, 58 insertions(+), 33 deletions(-)
>
> --
> 2.34.1

The v7 series applied for fixes and by adding stable tags to all of
the patches, thanks!

Kind regards
Uffe