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

[Luke Wang (OSS)]

> -----Original Message-----
> From: Frank Li (OSS) <frank.li@xxxxxxxxxxx>
> Sent: Friday, June 26, 2026 12:36 AM
> To: Luke Wang (OSS) <ziniu.wang_1@xxxxxxxxxxx>
> Cc: adrian.hunter@xxxxxxxxx; ulfh@xxxxxxxxxx; Bough Chen
> <haibo.chen@xxxxxxx>; Frank Li <frank.li@xxxxxxx>;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; dl-S32 <S32@xxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxx
> wrote:
> > From: Luke Wang <ziniu.wang_1@xxxxxxx>
> >
> > 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.

Thanks, 
Luke

> 
> Frank
> >
> > Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM
> logic")
> > Signed-off-by: Luke Wang <ziniu.wang_1@xxxxxxx>
> > ---
> >  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
> >
> >