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

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

[ziniu.wang_1]

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

This series fixes several issues with SDIO devices that keep power
during system suspend on i.MX platforms.

Patch 1 removes unnecessary mmc_card_wake_sdio_irq() check from tuning
save/restore paths.

Patch 2 restores DLL override configuration for DDR modes on resume,
which is lost when sdhci_esdhc_imx_hwinit() clears ESDHC_DLL_CTRL.

Patch 3 restores pinctrl before pm_runtime_force_resume() to ensure
proper pin configuration before DDR_EN is set, avoiding CRC errors.

Patch 4 unconditionally disables usdhc interrupt during suspend to
prevent "irq xxx: nobody cared" warnings during resume.

Luke Wang (4):
  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: restore pinctrl before restoring ios timing on
    resume
  mmc: sdhci-esdhc-imx: disable irq during suspend to fix unhandled
    interrupt

 drivers/mmc/host/sdhci-esdhc-imx.c | 57 ++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 18 deletions(-)

-- 
2.34.1

[PATCH 1/4] 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")
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 1/4] 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 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return values from `pm_runtime_get_sync()` and `pm_runtime_force_resume()` can lead to kernel panics via unclocked hardware register accesses.
- [High] Error paths in system suspend and resume callbacks leak PM runtime usage counters, bypass PM state changes, and leave the host IRQ permanently disabled.
--

commit 3bcc9d5490c53923eb1abbde361cd46732c4aeb4
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 suspend and
resume paths, as tuning values should be preserved when the card keeps power
regardless of the SDIO IRQ wake flag.

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

[Severity: High]
This is a pre-existing issue, but does the earlier call to
pm_runtime_get_sync() in sdhci_esdhc_suspend() need its return value checked?

If pm_runtime_get_sync() fails, for example due to a clock enable failure, it
returns a negative error code but leaves the device unclocked. When the code
reaches this point and calls sdhc_esdhc_tuning_save(), it will blindly access
hardware registers. On i.MX platforms, accessing an unclocked peripheral
triggers an asynchronous external abort, resulting in a kernel panic.

Additionally, returning early further down in sdhci_esdhc_suspend() if
pinctrl_pm_select_sleep_state() fails:

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

bypasses decrementing the usage counter, leaking the runtime PM reference. It
also leaves the host IRQ permanently disabled because it skips
pm_runtime_force_suspend() and the PM core aborts system suspend, bypassing
the resume callback.

Also, later in this function:

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

If mmc_gpio_set_cd_wake() fails, returning an error here after calling
pm_runtime_force_suspend() seems to break the PM core's state tracking,
confusing the PM core into skipping the resume callback while the device is
fully suspended.

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

[Severity: High]
This is a pre-existing issue, but similar to the suspend path, should we
check the return value of pm_runtime_force_resume() called earlier in
sdhci_esdhc_resume()?

If pm_runtime_force_resume() fails, the subsequent calls to
sdhci_esdhc_imx_hwinit() and sdhc_esdhc_tuning_restore() here will blindly
access hardware registers on an unclocked device, which can trigger a kernel
panic.

Also, earlier in this function:

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

and later:

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

Returning early on these failures skips enable_irq() and
pm_runtime_put_autosuspend(), which leaves the IRQ permanently disabled and
leaks the PM usage counter.

>  
>  	pm_runtime_put_autosuspend(dev);

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

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6526d65538de..a944351dbcdf 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
@@ -1425,15 +1442,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 +2132,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 2/4] 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] Early error returns in suspend/resume paths bypass mandatory resource cleanup, PM usage counter balancing, and hardware state restoration.
--

commit 10711dfe4cc676e64053bf9eecc1ce9d4fa7a61e
Author: Luke Wang <ziniu.wang_1@nxp.com>
Subject: mmc: sdhci-esdhc-imx: restore DLL override for DDR modes on resume

This commit restores the DLL override configuration on the resume path for
SDIO devices operating in DDR mode that keep power during system suspend.
It extracts the DLL override setup into a helper function and calls it
during resume to ensure the configuration is not lost.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6526d65538de5..a944351dbcdff 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c

[ ... ]

> @@ -2123,9 +2132,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 do the suspend and resume error paths
bypass mandatory resource cleanup?

Looking at the start of sdhci_esdhc_resume():

sdhci_esdhc_resume() {
    ...
    ret = pinctrl_pm_select_default_state(dev);
    if (ret)
        return ret;
    ...

If this fails, it returns early and bypasses pm_runtime_force_resume() and
pm_runtime_put_autosuspend() at the end of the function, leaving the
device runtime-suspended and its IRQ disabled.

A similar issue appears to exist in sdhci_esdhc_suspend():

sdhci_esdhc_suspend() {
    ...
    /* The irqs of imx are not shared. It is safe to disable */
    disable_irq(host->irq);

    if (device_may_wakeup(dev)) {
        ...
    } else {
        ret = pinctrl_pm_select_sleep_state(dev);
        if (ret)
            return ret;
    }
    ...

If pinctrl_pm_select_sleep_state() fails, it returns early without
re-enabling the IRQ that was just disabled or reverting the PM runtime
usage counter.

Could these error paths permanently hang the SDHCI controller until the
next reboot?

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

[PATCH 3/4] 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. When DDR_EN is set while the pinctrl is still
muxed to GPIO or other non-uSDHC function, the loopback clock from the
external pad is not valid, resulting in an incorrect internal sampling
point being selected. This causes persistent read CRC errors on subsequent
data transfers, even after the pinctrl is later configured correctly.

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

Fix this by restoring the default pinctrl state before
pm_runtime_force_resume(). The timing-specific pin configuration will
be set later by esdhc_change_pinstate() during runtime resume.

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, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a944351dbcdf..a3688c94cf58 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2114,6 +2114,10 @@ static int sdhci_esdhc_resume(struct device *dev)
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret)
+		return ret;
+
 	pm_runtime_force_resume(dev);
 
 	ret = mmc_gpio_set_cd_wake(host->mmc, false);
-- 
2.34.1

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

[sashiko-bot]

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

New issues:
- [High] The newly added early return on `pinctrl_pm_select_default_state()` failure skips critical PM state restoration, IRQ re-enablement, and leaks the runtime PM usage counter.
- [Medium] Unconditional transition to default pinctrl state on resume overrides the active pin state retained for wakeup-enabled SDIO devices.

Pre-existing issues:
- [High] Preexisting early returns in both the suspend and resume paths leak the runtime PM usage counter and leave the device's IRQ permanently disabled.
--

commit 36afd1c0e24c2aa82ce799711cc1629a34958ef4
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. When DDR_EN is set during
pm_runtime_force_resume() without the default pinctrl state, the loopback
clock is invalid and causes read CRC errors. This patch restores the default
pinctrl state before pm_runtime_force_resume() to fix this issue.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a944351dbcdff..a3688c94cf580 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2114,6 +2114,10 @@ static int sdhci_esdhc_resume(struct device *dev)
>  	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
>  	int ret;
>  
> +	ret = pinctrl_pm_select_default_state(dev);

[Severity: Medium]
Does this unconditional transition to the default pinctrl state override the
active pin state retained for wakeup-enabled SDIO devices?

During suspend, if a device is a wakeup source, it intentionally avoids changing
the pinctrl state to sleep, retaining its active high-speed state.

If we unconditionally select the default state here, could it glitch the SD
bus pins for powered SDIO cards by forcing them out of the retained state
before switching back to high-speed later?

> +	if (ret)
> +		return ret;

[Severity: High]
If pinctrl_pm_select_default_state() fails and we return early, does this skip
important PM state restoration?

It looks like returning here bypasses pm_runtime_force_resume(), enable_irq(),
and the final pm_runtime_put_autosuspend(). Could this permanently leave the
device with its hardware unresumed, its runtime PM usage count elevated, and
its interrupt disabled?

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

[Severity: High]
This is a pre-existing issue, but does this early return leak the PM usage
counter and leave the IRQ disabled?

If mmc_gpio_set_cd_wake() fails, we return early without calling enable_irq()
or pm_runtime_put_autosuspend(), potentially rendering the device unusable.

There appears to be a similar pre-existing early return on pinctrl failure in
sdhci_esdhc_suspend() that skips balancing pm_runtime_get_sync() and
disable_irq().

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

[PATCH 4/4] 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")
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 a3688c94cf58..9afb74cac559 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -2076,9 +2076,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");
@@ -2127,10 +2128,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

Re: [PATCH 4/4] mmc: sdhci-esdhc-imx: disable irq during suspend to fix unhandled interrupt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced disable_irq() on suspend and resume early error paths permanently breaks the controller.
--

commit e3505d4371bebecedf44dae6f5061d01e6084213
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: disable irq during suspend to fix unhandled interrupt

This commit fixes an "irq xxx: nobody cared" warning by unconditionally
disabling the usdhc interrupt during suspend and re-enabling it during
resume.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a3688c94cf580..9afb74cac5599 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2076,9 +2076,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");

[Severity: High]
If pinctrl_pm_select_sleep_state() fails in the else branch below, does this
leave the IRQ permanently disabled?

If device_may_wakeup(dev) is false, sdhci_esdhc_suspend() can return early:

    if (device_may_wakeup(dev)) {
        ...
    } else {
        ret = pinctrl_pm_select_sleep_state(dev);
        if (ret)
            return ret;
    }

Since the PM core won't invoke the resume callback if suspend fails, it looks
like the disabled IRQ state is leaked. Should we re-enable the IRQ before
returning the error?

[ ... ]

> @@ -2127,10 +2128,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);

[Severity: High]
Does this also affect early error returns in sdhci_esdhc_resume()?

Earlier in this function, there are error returns:

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

Now that disable_irq() is unconditional during suspend, do we need to ensure
the IRQ is re-enabled even if we return early here, or is it expected that
the controller remains broken if resume fails?

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