Re: [PATCH v5 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking 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] Ignoring `pm_runtime_force_suspend()` failure leads to an unbalanced PM state and potential system crash during resume.
--

--- Patch [7]: [PATCH v5 7/8] mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend ---
commit 4c281f4618f6273e8644ab8fb66535c0ebe7c6e2
Author: Luke Wang <ziniu.wang_1@nxp.com>

mmc: sdhci-esdhc-imx: make non-fatal errors non-blocking in suspend

This commit modifies the suspend path of the sdhci-esdhc-imx driver to
make failures in pinctrl_pm_select_sleep_state() and mmc_gpio_set_cd_wake()
non-fatal. It logs warnings instead of returning an error for these cases
and changes sdhci_esdhc_suspend() to always return 0 on completion.

> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 7818f351a58ae..6cb84690fde4e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
[ ... ]
> @@ -2108,7 +2110,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 */
>  	pm_runtime_force_suspend(dev);
>  
> -	return ret;
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does ignoring the return value of
pm_runtime_force_suspend() lead to an unbalanced PM state?

If pm_runtime_force_suspend() fails (for example, if cqhci_suspend() times
out and returns -ETIMEDOUT), the device's runtime PM status will remain
RPM_ACTIVE, but the suspend process continues.

Upon system resume, sdhci_esdhc_resume() calls pm_runtime_force_resume().
Because pm_runtime_force_resume() checks pm_runtime_status_suspended(), it
will see RPM_ACTIVE and assume the device is already active, skipping the
driver's runtime_resume callback.

Since the runtime_resume callback is skipped, cqhci_resume() is never called
to reinitialize the Command Queuing Engine (CQE), and clock enablement might
also be skipped. Subsequent accesses to the hardware in
sdhci_esdhc_imx_hwinit() or later MMC operations could then trigger a
synchronous external abort (if clocks are off) or lead to I/O errors
(due to an uninitialized CQE).

If the error from pm_runtime_force_suspend() is propagated instead, shouldn't
the driver also roll back the operations performed earlier in
sdhci_esdhc_suspend(), specifically disable_irq() and
pm_runtime_resume_and_get(), since the PM core will abort the suspend and
will not call sdhci_esdhc_resume() for this device?

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