Re: [PATCH 1/4] mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked return values from `pm_runtime_get_sync()` and `pm_runtime_force_resume()` can lead to kernel panics via unclocked hardware register accesses.
- [High] Error paths in system suspend and resume callbacks leak PM runtime usage counters, bypass PM state changes, and leave the host IRQ permanently disabled.
--

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

mmc: sdhci-esdhc-imx: remove unnecessary mmc_card_wake_sdio_irq check for tuning save/restore

This commit removes the mmc_card_wake_sdio_irq() check from the suspend and
resume paths, as tuning values should be preserved when the card keeps power
regardless of the SDIO IRQ wake flag.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -2064,8 +2064,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 * to save the tuning delay value just in case the usdhc
>  	 * lost power during system PM.
>  	 */
> -	if (mmc_card_keep_power(host->mmc) && mmc_card_wake_sdio_irq(host->mmc) &&
> -	    esdhc_is_usdhc(imx_data))
> +	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
>  		sdhc_esdhc_tuning_save(host);

[Severity: High]
This is a pre-existing issue, but does the earlier call to
pm_runtime_get_sync() in sdhci_esdhc_suspend() need its return value checked?

If pm_runtime_get_sync() fails, for example due to a clock enable failure, it
returns a negative error code but leaves the device unclocked. When the code
reaches this point and calls sdhc_esdhc_tuning_save(), it will blindly access
hardware registers. On i.MX platforms, accessing an unclocked peripheral
triggers an asynchronous external abort, resulting in a kernel panic.

Additionally, returning early further down in sdhci_esdhc_suspend() if
pinctrl_pm_select_sleep_state() fails:

	ret = pinctrl_pm_select_sleep_state(dev);
	if (ret)
		return ret;

bypasses decrementing the usage counter, leaking the runtime PM reference. It
also leaves the host IRQ permanently disabled because it skips
pm_runtime_force_suspend() and the PM core aborts system suspend, bypassing
the resume callback.

Also, later in this function:

	ret = mmc_gpio_set_cd_wake(host->mmc, true);
	pm_runtime_force_suspend(dev);
	return ret;

If mmc_gpio_set_cd_wake() fails, returning an error here after calling
pm_runtime_force_suspend() seems to break the PM core's state tracking,
confusing the PM core into skipping the resume callback while the device is
fully suspended.

>  
>  	if (device_may_wakeup(dev)) {

[ ... ]

> @@ -2124,8 +2123,7 @@ 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) && mmc_card_wake_sdio_irq(host->mmc) &&
> -	    esdhc_is_usdhc(imx_data))
> +	if (mmc_card_keep_power(host->mmc) && esdhc_is_usdhc(imx_data))
>  		sdhc_esdhc_tuning_restore(host);

[Severity: High]
This is a pre-existing issue, but similar to the suspend path, should we
check the return value of pm_runtime_force_resume() called earlier in
sdhci_esdhc_resume()?

If pm_runtime_force_resume() fails, the subsequent calls to
sdhci_esdhc_imx_hwinit() and sdhc_esdhc_tuning_restore() here will blindly
access hardware registers on an unclocked device, which can trigger a kernel
panic.

Also, earlier in this function:

	ret = pinctrl_pm_select_default_state(dev);
	if (ret)
		return ret;

and later:

	ret = mmc_gpio_set_cd_wake(host->mmc, false);
	if (ret)
		return ret;

Returning early on these failures skips enable_irq() and
pm_runtime_put_autosuspend(), which leaves the IRQ permanently disabled and
leaks the PM usage counter.

>  
>  	pm_runtime_put_autosuspend(dev);

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