Re: [PATCH v4 3/7] mmc: sdhci-esdhc-imx: restore pinctrl before restoring ios timing on resume
Adrian Hunter <[email protected]>
| Newsgroups | org.kernel.vger.linux-mmc,dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 03/07/2026 13:42, [email protected] wrote: > From: Luke Wang <[email protected]> > > 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. > > This is related to the SoC IP integration: switching pinctrl setting > (changing alt from GPIO to USDHC) impacts the internal loopback path. > If pinctrl configures the pad to GPIO function, once DDR_EN is set, the > DLL delay will be fixed based on the GPIO function loopback path. When > the pinctrl is later changed to USDHC function, the internal loopback > path changes, making the original fixed sample point no longer suitable > for the current loopback path. This causes persistent read CRC errors on > subsequent data transfers. > > 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 before DDR_EN is set. Only restore for non-wakeup devices > since wakeup devices kept their active pin state during suspend. > > Fixes: 676a83855614 ("mmc: host: sdhci-esdhc-imx: refactor the system PM logic") > Reviewed-by: Haibo Chen <[email protected]> > Signed-off-by: Luke Wang <[email protected]> > --- > 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 7230d70e02ae..3b1e63425a19 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -2113,6 +2113,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"); Sashiko has a point about this dev_warn(). See its first comment in: https://sashiko.dev/#/patchset/20260703104208.3426374-1-ziniu.wang_1%40oss.nxp.com?part=3 > + } > + > pm_runtime_force_resume(dev); > > ret = mmc_gpio_set_cd_wake(host->mmc, false);