Re: [PATCH v7 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] PM Runtime usage counter leak on system suspend error path.
--

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

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

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.

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

[Severity: High]
This is a pre-existing issue, but while reviewing the system PM logic here,
I noticed a PM runtime usage counter leak in the suspend error path.

In sdhci_esdhc_suspend(), pm_runtime_get_sync() increments the usage counter
early in the function, but the error path for pinctrl_pm_select_sleep_state()
returns without balancing it:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_suspend() {
    ...
	pm_runtime_get_sync(dev);
    ...
	if (device_may_wakeup(dev)) {
    ...
	} else {
    ...
		ret = pinctrl_pm_select_sleep_state(dev);
		if (ret)
			return ret;
    ...
}

If pinctrl_pm_select_sleep_state(dev) fails, does this permanently leak the
PM runtime usage counter, since the system suspend is aborted and the
corresponding resume callback will not run?

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