Re: [PATCH v3 5/7] mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Hardware access without clock via ignored `pm_runtime_get_sync` failure in device removal path.
--

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

mmc: sdhci-esdhc-imx: use pm_runtime_resume_and_get() in suspend

Replace pm_runtime_get_sync() with pm_runtime_resume_and_get() to
simplify error handling. pm_runtime_resume_and_get() automatically
drops the usage counter on failure, avoiding the need for a separate
pm_runtime_put_noidle() call. If it fails, the device is unclocked and
accessing hardware registers would cause a kernel panic, so return the
error immediately.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c4a22e42628e4..97ee3f933bda7 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2060,7 +2060,9 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 * 2, make sure the pm_runtime_force_resume() in sdhci_esdhc_resume() really
>  	 *    invoke its ->runtime_resume callback (needs_force_resume = 1).
>  	 */
> -	pm_runtime_get_sync(dev);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but it shares the exact same root cause as the
issue fixed here in sdhci_esdhc_suspend().

In sdhci_esdhc_imx_remove(), the return value of pm_runtime_get_sync() is
ignored and followed immediately by a hardware read:

drivers/mmc/host/sdhci-esdhc-imx.c:sdhci_esdhc_imx_remove() {
	...
	pm_runtime_get_sync(&pdev->dev);
	dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
	...
}

As mentioned in the commit message for the suspend path, if the device fails
to resume (e.g., due to a clock enable failure), the hardware remains
unclocked, and accessing hardware registers could cause a kernel panic.

Should pm_runtime_resume_and_get() be used in sdhci_esdhc_imx_remove() as well
to avoid a potential kernel panic during driver removal if the resume fails?

>  
>  	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
>  		(host->tuning_mode != SDHCI_TUNING_MODE_1)) {

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