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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced `disable_irq()` on system resume if `sdhci_enable_irq_wakeups()` fails or returns false.
--

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

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

This commit extracts the DLL override setup into a helper function and
calls it during the resume path for SDIO devices operating in DDR timing
modes that keep power.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6526d65538de5..7230d70e02ae1 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 could this code lead to an unbalanced
disable_irq() on system resume?

Looking at sdhci_esdhc_suspend(), the IRQ is unconditionally disabled
if device_may_wakeup(dev) is true:

    disable_irq(host->irq);
    ret = sdhci_enable_irq_wakeups(host);

However, if sdhci_enable_irq_wakeups() fails or returns false (for example,
if no wake-capable SDIO card is present and CD IRQ wakeup is disabled),
host->irq_wake_enabled remains false.

During sdhci_esdhc_resume(), enable_irq() appears to be executed conditionally
based on host->irq_wake_enabled:

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

If host->irq_wake_enabled is false, would the re-enablement be skipped?
Could this permanently disable the SDHCI controller's main interrupt after a
system suspend/resume cycle, rendering the MMC controller unusable and
potentially leading to a system crash if it is the rootfs device?

>  	 * 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/20260707110303.3779799-1-ziniu.wang_1@oss.nxp.com?part=2