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

["Luke Wang (OSS)" <ziniu.wang_1@oss.nxp.com>]

> -----Original Message-----
> From: Frank Li (OSS) <frank.li@oss.nxp.com>
> Sent: Friday, June 26, 2026 12:40 AM
> To: Luke Wang (OSS) <ziniu.wang_1@oss.nxp.com>
> Cc: adrian.hunter@intel.com; ulfh@kernel.org; Bough Chen
> <haibo.chen@nxp.com>; Frank Li <frank.li@nxp.com>;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> imx@lists.linux.dev; linux-mmc@vger.kernel.org; dl-S32 <S32@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 5/5] mmc: sdhci-esdhc-imx: fix suspend/resume error
> handling
> 
> 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.

I will split this patch in v3

Thanks,
Luke

> 
> 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
> >
> >