Re: [PATCH v2 3/5] mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume

[Bough Chen <haibo.chen@oss.nxp.com>]

On Fri, Jun 26, 2026 at 06:03:05AM +0000, Luke Wang (OSS) wrote:
> 
> 
> > -----Original Message-----
> > From: Frank Li (OSS) <frank.li@oss.nxp.com>
> > Sent: Friday, June 26, 2026 12:36 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 3/5] mmc: sdhci-esdhc-imx: restore pinctrl before
> > restoring ios timing on resume
> > 
> > On Thu, Jun 25, 2026 at 06:59:32PM +0800, ziniu.wang_1@oss.nxp.com
> > wrote:
> > > From: Luke Wang <ziniu.wang_1@nxp.com>
> > >
> > > SDIO devices such as WiFi may keep power during suspend, so the MMC
> > > core skips full card re-initialization on resume and directly restores
> > > the host controller's ios timing to match the card. For DDR mode,
> > > pm_runtime_force_resume() sets DDR_EN before the pin configuration is
> > > restored from sleep state. When DDR_EN is set while the pinctrl is still
> > > muxed to GPIO or other non-uSDHC function, the loopback clock from the
> > > external pad is not valid, resulting in an incorrect internal sampling
> > > point being selected. This causes persistent read CRC errors on subsequent
> > > data transfers, even after the pinctrl is later configured correctly.
> > >
> > > SD/eMMC running in DDR mode are unaffected as they are fully re-
> > initialized
> > > from legacy timing after resume.
> > >
> > > Fix this by restoring the pinctrl state based on current timing mode
> > > using esdhc_change_pinstate() before pm_runtime_force_resume(). This
> > > ensures the correct pin configuration (e.g., 100/200MHz for UHS modes)
> > > is applied. Only restore for non-wakeup devices since wakeup devices
> > > kept their active pin state during suspend to avoid glitching the SD
> > > bus pins for powered SDIO cards.
> > 
> > pin state change should only impact driver strength, why cause glitch ?
> 
> You're right that switching driver strength alone won't cause a glitch. 
> The issue is more specific to the sleep pinctrl state: the uSDHC clock pin is
> low when the clock is stopped, but the sleep pinctrl enables a pull-up on that
> pin, driving it high during suspend. When we switch back to the uSDHC function
> pinctrl on resume, the pin transitions from high back to low, generating 
> a falling edge glitch.
> 
> I'll update the commit message in v3 to clarify this.

The glitch should be related to the SoC IP intergration, switch pinctrl setting
(change alt from GPIO to USDHC) impact the internal loopback path. If pinctrl
config the pad to GPIO function, once DDR_EN configed, the dll delay will fix
based on the GPIO function loopback path, but then change the pinctrl to function
USDHC, the internal loopback path change, the original fixed sample point maybe
not suitable for current loopback path.

Luke, please add this in the commit log.

Regards
Haibo Chen
> 
> Thanks, 
> Luke
> 
> > 
> > Frank
> > >
> > > Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM
> > logic")
> > > Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> > > ---
> > >  drivers/mmc/host/sdhci-esdhc-imx.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> > esdhc-imx.c
> > > index a944351dbcdf..7fcaecdd4ec6 100644
> > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > > @@ -2114,6 +2114,12 @@ static int sdhci_esdhc_resume(struct device
> > *dev)
> > >  	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> > >  	int ret;
> > >
> > > +	if (!device_may_wakeup(dev)) {
> > > +		ret = esdhc_change_pinstate(host, host->timing);
> > > +		if (ret)
> > > +			dev_warn(dev, "Failed to restore pinctrl state\n");
> > > +	}
> > >  	pm_runtime_force_resume(dev);
> > >
> > >  	ret = mmc_gpio_set_cd_wake(host->mmc, false);
> > > --
> > > 2.34.1
> > >
> > >