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

[sashiko-bot@kernel.org]

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