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

[PATCH v6 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 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 | 84 ++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 28 deletions(-)

-- 
2.34.1

[PATCH v6 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

[PATCH v6 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

[PATCH v6 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")
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 v6 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")
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

[PATCH v6 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")
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 v6 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")
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

[PATCH v6 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")
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7818f351a58a..6cb84690fde4 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2097,10 +2097,12 @@ static int sdhci_esdhc_suspend(struct device *dev)
 		 */
 		ret = pinctrl_pm_select_sleep_state(dev);
 		if (ret)
-			return ret;
+			dev_warn(dev, "Failed to select sleep pinctrl state\n");
 	}
 
 	ret = mmc_gpio_set_cd_wake(host->mmc, true);
+	if (ret)
+		dev_warn(dev, "Failed to enable cd wake\n");
 
 	/*
 	 * Make sure invoke runtime_suspend to gate off clock.
@@ -2108,7 +2110,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

[PATCH v6 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")
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 6cb84690fde4..f4bfb14423e4 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2126,12 +2126,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);
 
@@ -2158,7 +2158,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 v6 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] Missing cleanup of the PM usage counter and IRQ state on error paths in `sdhci_esdhc_suspend()` and `sdhci_esdhc_resume()`.
--

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

mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend
    
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.

> 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 we are modifying the PM reference
handling here, does the rest of sdhci_esdhc_suspend() properly clean up the
PM usage counter and IRQ state on its error paths?

For example, further down in sdhci_esdhc_suspend(), if
pinctrl_pm_select_sleep_state() fails:

		ret = pinctrl_pm_select_sleep_state(dev);
		if (ret)
			return ret;

Does this early return exit the function without dropping the PM reference
acquired above or re-enabling IRQs?

Similarly, if mmc_gpio_set_cd_wake() fails:

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

	pm_runtime_force_suspend(dev);

	return ret;

If this fails, the function returns an error, which causes the PM core to
abort system suspend and skip the device's resume callback.

Does this leak the PM reference and leave the device with a permanently
disabled IRQ?

Also, in sdhci_esdhc_resume(), if mmc_gpio_set_cd_wake() fails:

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

Could this early return skip enable_irq() and pm_runtime_put_autosuspend()
at the end of the function, permanently breaking the device state?

[ ... ]

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

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

[sashiko-bot]

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

New issues:
- [High] Returning early on resume failure leaks the PM usage counter and desyncs the IRQ depth, causing a permanent device lockup.

Pre-existing issues:
- [High] Missing synchronize_irq() in runtime suspend causes a race condition with the threaded IRQ handler, leading to a kernel panic.
--

--- Patch [8]: [PATCH v6 8/8] mmc: sdhci-esdhc-imx: fix resume error handling ---
commit 656597a370f7029432d32936bfe95d2095dee2e6
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 the
resume path. It returns immediately on failure to prevent accessing hardware
registers on an unclocked device, while skipping enable_irq() and IRQ wakeup
teardown.

> 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
> @@ -2126,12 +2126,12 @@ static int sdhci_esdhc_resume(struct device *dev)
[ ... ]
> -	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: High]
Will returning early here leak the PM usage counter and leave the IRQ
permanently disabled?

sdhci_esdhc_suspend() increments the PM usage counter via
pm_runtime_resume_and_get() and the IRQ disable depth via disable_irq().

If pm_runtime_force_resume() fails and we return early, we skip the calls
to enable_irq() and pm_runtime_put_autosuspend() at the end of the function.

On subsequent suspend/resume cycles, disable_irq() and
pm_runtime_resume_and_get() would be called again, stacking the counters.
Even if the hardware failure is transient, a subsequent successful resume
would only decrement the counters once, potentially leaving the device IRQ
disabled until reboot.

Is there a way to balance the suspend operations even when resume fails?

[ ... ]
> @@ -2158,7 +2158,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)
>  {
[ ... ]
>  	sdhci_runtime_suspend_host(host);
>  
>  	if (host->tuning_mode != SDHCI_TUNING_MODE_3)
>  		mmc_retune_needed(host->mmc);
>  
>  	imx_data->actual_clock = host->mmc->actual_clock;
>  	esdhc_pltfm_set_clock(host, 0);
>  	clk_disable_unprepare(imx_data->clk_per);

[Severity: High]
This is a pre-existing issue, but does this runtime suspend path need to
wait for threaded IRQ handlers to finish before disabling hardware clocks?

The core requests the interrupt with IRQF_SHARED and a threaded handler
(sdhci_thread_irq). sdhci_runtime_suspend_host() masks interrupts at the
controller and calls synchronize_hardirq(), but it does not wait for the
threaded handler to complete.

If a card event triggers the threaded handler just before suspend, it
can run concurrently and read hardware registers (for example via
sdhci_card_event() calling sdhci_get_cd()). 

Because the clocks are disabled shortly after, a concurrent register read
by the threaded handler could cause a kernel panic.

Should synchronize_irq() be called here before powering down the clocks?

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] On non-uSDHC boards, requesting high-speed UHS timings via a malformed device tree triggers a NULL pointer dereference in the pinctrl subsystem because `pins_100mhz` and `pins_200mhz` are checked with `IS_ERR()` instead of `IS_ERR_OR_NULL()`.
--

--- Patch [3]: [PATCH v6 3/8] mmc: sdhci-esdhc-imx: fix esdhc_change_pinstate() to allow default state restore ---
commit dd1c1cd99a70d068cd94016148812cfb2f68ca96
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: fix esdhc_change_pinstate() to allow default state restore

This commit moves the checks for pins_100mhz and pins_200mhz into their
respective switch cases in esdhc_change_pinstate(). This prevents the checks
from unconditionally blocking the default case and allows devices without
high-speed pin states to restore their default pin configuration.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 685e508c61279..2da5c8ab3e622 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;

[Severity: High]
This is a pre-existing issue, but will IS_ERR() properly catch a NULL
pointer if pins_100mhz is uninitialized?

For non-uSDHC boards, imx_data->pins_100mhz can be NULL. If a malformed
device tree specifies UHS timings, IS_ERR(NULL) evaluates to false. Does
this result in pinctrl being set to NULL and later passed to
pinctrl_select_state(), causing a kernel panic?

Would it be safer to use IS_ERR_OR_NULL() here and for pins_200mhz below?

>  		pinctrl = imx_data->pins_100mhz;
>  		break;

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

Re: [PATCH v6 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 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced IRQ disable/enable during system suspend/resume when SDIO wakeup fails to enable or is not applicable.
- [Medium] Missing `pm_runtime_put` on error paths in `sdhci_esdhc_suspend()` leads to permanent runtime PM usage count leaks.
--

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

mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume
    
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.

> 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)
>  	 * 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;

[Severity: High]
This is a pre-existing issue, but I noticed a potential IRQ unbalance during
suspend/resume.

If device_may_wakeup(dev) is true but the controller doesn't support or
successfully enable in-band IRQ wakeups (for example, standard SD cards
using GPIO CD), sdhci_enable_irq_wakeups() returns false and
host->irq_wake_enabled is set to false.

In sdhci_esdhc_suspend(), the device IRQ is unconditionally disabled:

sdhci_esdhc_suspend() {
    ...
    disable_irq(host->irq);
    ret = sdhci_enable_irq_wakeups(host);
    ...
}

If host->irq_wake_enabled is false, will the IRQ remain disabled on resume?
Looking at sdhci_esdhc_resume(), it appears enable_irq() is bypassed if the
wakeup enablement failed:

sdhci_esdhc_resume() {
    ...
    if (host->irq_wake_enabled) {
        sdhci_disable_irq_wakeups(host);
        enable_irq(host->irq);
    }
    ...
}

Could this leave the SDHCI controller's main interrupt permanently disabled
after a suspend/resume cycle, stalling all future MMC/SD card transactions?

[Severity: Medium]
This is another pre-existing issue, but does sdhci_esdhc_suspend() leak the
runtime PM usage counter on error paths?

sdhci_esdhc_suspend() starts by incrementing the PM usage counter:

sdhci_esdhc_suspend() {
    ...
    pm_runtime_get_sync(dev);
    ...

Later, it can return early without dropping the acquired PM reference:

    ...
    ret = pinctrl_pm_select_sleep_state(dev);
    if (ret)
        return ret;
    ...
}

If operations like pinctrl_pm_select_sleep_state() or
mmc_gpio_set_cd_wake() fail, the function returns an error code and system
suspend is aborted. Since the PM core skips the resume callback for devices
whose suspend failed, wouldn't the matching pm_runtime_put_autosuspend() be
skipped, leaving the PM usage counter permanently leaked and preventing the
controller from entering runtime suspend again?

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

Re: [PATCH v6 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:

New issues:
- [Medium] Spurious pinctrl warnings on every system resume for platforms without pinctrl configuration.

Pre-existing issues:
- [Critical] Unchecked return value of pm_runtime_force_resume() may lead to unclocked register access and system crash.
- [High] Error paths in system suspend and resume callbacks fail to balance the runtime PM usage counter.
- [High] esdhc_change_pinstate() uses IS_ERR() instead of IS_ERR_OR_NULL(), leading to a potential NULL pointer dereference on misconfigured DTs.
--

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

mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume

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. This patch restores the pinctrl
state based on current timing mode before pm_runtime_force_resume() is
called.

> 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);

[Severity: High]
This is a pre-existing issue, but does esdhc_change_pinstate() fail to
catch a NULL pointer on misconfigured device trees?

In esdhc_change_pinstate(), it uses IS_ERR() instead of IS_ERR_OR_NULL():

	case MMC_TIMING_UHS_DDR50:
		if (IS_ERR(imx_data->pins_100mhz))
			return -EINVAL;
		pinctrl = imx_data->pins_100mhz;

If a misconfigured device tree forces UHS capabilities, the MMC core
enables UHS timings, but pins_100mhz may remain NULL. Since IS_ERR(NULL)
evaluates to false, it assigns NULL to pinctrl.

This NULL pointer is directly passed to the pinctrl subsystem:

	return pinctrl_select_state(imx_data->pinctrl, pinctrl);

This causes a panic when dereferenced by pinctrl_commit_state().

> +		if (ret)
> +			dev_warn(dev, "Failed to restore pinctrl state\n");

[Severity: Medium]
Is this warning going to trigger on every system resume for platforms
without pinctrl?

Platforms without pinctrl correctly initialize imx_data->pinctrl to an
ERR_PTR in probe(), treating it as non-fatal.

When esdhc_change_pinstate() is called, it returns -EINVAL if pinctrl
is an ERR_PTR:

	if (IS_ERR(imx_data->pinctrl))
		return -EINVAL;

This causes dev_warn() to spam the kernel log on unaffected hardware on
every system resume.

> +	}
> +
>  	pm_runtime_force_resume(dev);

[Severity: Critical]
This is a pre-existing issue, but does ignoring the return value of
pm_runtime_force_resume() risk an unclocked register access?

If the runtime resume callback fails to enable hardware clocks, the error
is returned but completely ignored here.

The driver then proceeds to access memory-mapped registers unconditionally
in sdhci_esdhc_imx_hwinit(). Accessing peripheral registers without clocks
enabled on i.MX platforms causes a Synchronous External Abort,
deterministically crashing the system.

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

[Severity: High]
This is a pre-existing issue, but do the error paths in the system suspend
and resume callbacks fail to balance the runtime PM usage counter?

In sdhci_esdhc_resume(), if mmc_gpio_set_cd_wake() fails, the function
returns early:

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

This bypasses pm_runtime_put_autosuspend(), leaking the usage counter
incremented during suspend.

Similarly, in sdhci_esdhc_suspend(), if pinctrl_pm_select_sleep_state()
fails, it returns early without calling pm_runtime_put() to balance
the preceding pm_runtime_get_sync().

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

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

[Frank Li]

On Mon, Jul 13, 2026 at 12:38:59PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> 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")
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@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
>
>

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

[Frank Li]

On Mon, Jul 13, 2026 at 12:39:03PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> 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")
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 7818f351a58a..6cb84690fde4 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2097,10 +2097,12 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  		 */
>  		ret = pinctrl_pm_select_sleep_state(dev);
>  		if (ret)
> -			return ret;
> +			dev_warn(dev, "Failed to select sleep pinctrl state\n");
>  	}
>
>  	ret = mmc_gpio_set_cd_wake(host->mmc, true);
> +	if (ret)
> +		dev_warn(dev, "Failed to enable cd wake\n");
>
>  	/*
>  	 * Make sure invoke runtime_suspend to gate off clock.
> @@ -2108,7 +2110,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 */
>  	pm_runtime_force_suspend(dev);
>
> -	return ret;
> +	return 0;

if other error path set ret as not zero, the real error may missed, I suggest
don't set it let

	if (pinctrl_pm_select_sleep_state(dev))
		dev_warn(...)

	still leave ret here.

Frank

>  }
>
>  static int sdhci_esdhc_resume(struct device *dev)
> --
> 2.34.1
>
>

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

[Frank Li]

On Mon, Jul 13, 2026 at 12:39:04PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> 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")
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@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 6cb84690fde4..f4bfb14423e4 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2126,12 +2126,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);
>
> @@ -2158,7 +2158,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 v6 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend

[Luke Wang (OSS)]

> -----Original Message-----
> From: Frank Li (OSS) <frank.li@oss.nxp.com>
> Sent: Monday, July 13, 2026 10:53 PM
> To: Luke Wang (OSS) <ziniu.wang_1@oss.nxp.com>
> Cc: adrian.hunter@intel.com; ulfh@kernel.org; Bough Chen
> <haibo.chen@nxp.com>; Frank Li <frank.li@nxp.com>;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> imx@lists.linux.dev; linux-mmc@vger.kernel.org; dl-S32 <S32@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v6 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-
> blocking in suspend
> 
> On Mon, Jul 13, 2026 at 12:39:03PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> > 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")
> > Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> > index 7818f351a58a..6cb84690fde4 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -2097,10 +2097,12 @@ static int sdhci_esdhc_suspend(struct device
> *dev)
> >  		 */
> >  		ret = pinctrl_pm_select_sleep_state(dev);
> >  		if (ret)
> > -			return ret;
> > +			dev_warn(dev, "Failed to select sleep pinctrl state\n");
> >  	}
> >
> >  	ret = mmc_gpio_set_cd_wake(host->mmc, true);
> > +	if (ret)
> > +		dev_warn(dev, "Failed to enable cd wake\n");
> >
> >  	/*
> >  	 * Make sure invoke runtime_suspend to gate off clock.
> > @@ -2108,7 +2110,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
> >  	 */
> >  	pm_runtime_force_suspend(dev);
> >
> > -	return ret;
> > +	return 0;
> 
> if other error path set ret as not zero, the real error may missed, I suggest
> don't set it let
> 
> 	if (pinctrl_pm_select_sleep_state(dev))
> 		dev_warn(...)
> 
> 	still leave ret here.

Thanks for the review.

The fatal error path (pm_runtime_resume_and_get()) already returns
early, so by the time we reach the end of the function, ret can only
hold a non-fatal (warn-only) value. Keeping "return ret" would turn
these intentionally non-fatal errors back into fatal ones and block
system suspend, which is exactly what this patch tries to avoid.

To address your concern about ret being polluted, I'll drop the
assignment for the non-fatal calls so they no longer touch ret:

	if (!sdhci_enable_irq_wakeups(host))
		dev_warn(dev, "Failed to enable irq wakeup\n");

	if (pinctrl_pm_select_sleep_state(dev))
		dev_warn(dev, "Failed to select sleep pinctrl state\n");

	if (mmc_gpio_set_cd_wake(host->mmc, true))
		dev_warn(dev, "Failed to enable cd wake\n");

	pm_runtime_force_suspend(dev);
	return 0;

This keeps the semantics clear. Will send v7 patch.

Thanks, 
Luke

> 
> Frank
> 
> >  }
> >
> >  static int sdhci_esdhc_resume(struct device *dev)
> > --
> > 2.34.1
> >
> >

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

[Adrian Hunter]

On 13/07/2026 07:38, 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 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 | 84 ++++++++++++++++++++----------
>  1 file changed, 56 insertions(+), 28 deletions(-)
> 

Already acked patches 1 and 2, but now for all the rest:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>