Re: [PATCH v4 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] Unchecked `pm_runtime_get_sync()` in `sdhci_esdhc_imx_remove()` can lead to unclocked hardware register access and a kernel panic.
--

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

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

This patch replaces a call to pm_runtime_get_sync() with
pm_runtime_resume_and_get() during device suspend to simplify error handling.
It ensures that if the resume fails, the driver returns immediately instead
of proceeding to access unclocked hardware registers which would cause a
kernel panic.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index ade99dabdb5fd..30aef7bc1fc67 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2059,7 +2059,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, and not a bug introduced by this patch, but
since you are fixing unchecked runtime PM calls here, could a similar panic
happen during device removal?

Looking at sdhci_esdhc_imx_remove(), there is an unchecked call to
pm_runtime_get_sync() right before reading from the hardware:

sdhci_esdhc_imx_remove() {
    ...
    pm_runtime_get_sync(&pdev->dev);
    dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
    ...
}

If the runtime resume fails in the removal path, the clocks would remain
disabled.

Would the subsequent readl() trigger a bus fault for the same reason? 

Should we add error handling or switch to pm_runtime_resume_and_get() in
sdhci_esdhc_imx_remove() as well?

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