Re: [PATCH 2/4] 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] 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