Re: [PATCH v2 5/5] mmc: sdhci-esdhc-imx: fix suspend/resume error handling

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Jun 25, 2026 at 06:59:34PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> Fix several error handling issues in sdhci_esdhc_suspend/resume:
>
> 1. Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
>    to simplify error handling. If it fails, the device is unclocked
>    and accessing hardware registers would cause a kernel panic.
>
> 2. Make pinctrl_pm_select_sleep_state() and mmc_gpio_set_cd_wake()
>    failures non-fatal in suspend path. These failures only mean
>    slightly higher power consumption or missing CD wakeup, but should
>    not block system suspend.
>
> 3. Check pm_runtime_force_resume() return value in resume. If it
>    fails (clock enable failure), return immediately since accessing
>    hardware registers on an unclocked device would cause a panic.
>
> 4. Make mmc_gpio_set_cd_wake(false) call in resume not check return
>    value since it always returns 0.
>
> 5. Always return 0 on success path instead of propagating non-fatal
>    warning return values.

each patch fix one problem.

Frank

>
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c4a22e42628e..4d6818c95809 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;
>
>  	if ((imx_data->socdata->flags & ESDHC_FLAG_STATE_LOST_IN_LPMODE) &&
>  		(host->tuning_mode != SDHCI_TUNING_MODE_1)) {
> @@ -2094,10 +2096,12 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  		 */
>  		ret = pinctrl_pm_select_sleep_state(dev);
>  		if (ret)
> -			return ret;
> +			dev_warn(dev, "Failed to select sleep pinctrl state\n");
>  	}
>
>  	ret = mmc_gpio_set_cd_wake(host->mmc, true);
> +	if (ret)
> +		dev_warn(dev, "Failed to enable cd wake\n");
>
>  	/*
>  	 * Make sure invoke runtime_suspend to gate off clock.
> @@ -2105,7 +2109,7 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  	 */
>  	pm_runtime_force_suspend(dev);
>
> -	return ret;
> +	return 0;
>  }
>
>  static int sdhci_esdhc_resume(struct device *dev)
> @@ -2121,12 +2125,12 @@ static int sdhci_esdhc_resume(struct device *dev)
>  			dev_warn(dev, "Failed to restore pinctrl state\n");
>  	}
>
> -	pm_runtime_force_resume(dev);
> -
> -	ret = mmc_gpio_set_cd_wake(host->mmc, false);
> +	ret = pm_runtime_force_resume(dev);
>  	if (ret)
>  		return ret;
>
> +	mmc_gpio_set_cd_wake(host->mmc, false);
> +
>  	/* re-initialize hw state in case it's lost in low power mode */
>  	sdhci_esdhc_imx_hwinit(host);
>
> @@ -2153,7 +2157,7 @@ static int sdhci_esdhc_resume(struct device *dev)
>
>  	pm_runtime_put_autosuspend(dev);
>
> -	return ret;
> +	return 0;
>  }
>
>  static int sdhci_esdhc_runtime_suspend(struct device *dev)
> --
> 2.34.1
>
>