[PATCH 1/2] mmc: sdhci-esdhc-imx: refactor clock loopback selection logic

[Bough Chen]

> -----Original Message-----
> From: Frank Li <frank.li at nxp.com>
> Sent: 2025年5月22日 0:10
> To: Luke Wang <ziniu.wang_1 at nxp.com>
> Cc: Bough Chen <haibo.chen at nxp.com>; adrian.hunter at intel.com;
> ulf.hansson at linaro.org; linux-mmc at vger.kernel.org; shawnguo at kernel.org;
> s.hauer at pengutronix.de; kernel at pengutronix.de; festevam at gmail.com;
> imx at lists.linux.dev; dl-S32 <S32 at nxp.com>;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: Re: [PATCH 1/2] mmc: sdhci-esdhc-imx: refactor clock loopback
> selection logic
> 
> On Wed, May 21, 2025 at 10:55:01AM +0800, ziniu.wang_1 at nxp.com wrote:
> > From: Luke Wang <ziniu.wang_1 at nxp.com>
> >
> > i.MX reference manual specifies that internal clock loopback should be
> > used for SDR104/HS200/HS400 modes. Move ESDHC_MIX_CTRL_FBCLK_SEL
> > configuration into the timing selection function to:
> >
> > 1. Explicitly set internal loopback path for SDR104/HS200/HS400 modes
> > 2. Avoid redundant bit manipulation across multiple functions
> >
> > Preserve ESDHC_MIX_CTRL_FBCLK_SEL during system resume for SDIO
> > devices with MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ flag,
> as the
> > controller might lose register state during suspend while skipping
> > card re-initialization.
> 
> what's means?
> 
> controller lost power during suspend, so
> u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL) to get reset value and miss
> 
> set ~ESDHC_MIX_CTRL_FBCLK_SEL?

Miss set ESDHC_MIX_CTRL_FBCLK_SEL.

Previous SDIO3.0 device with MMC_PM_KEEP_POWER will set ESDHC_MIX_CTRL_FBCLK_SEL,
After system resume, need to re-config ESDHC_MIX_CTRL_FBCLK_SEL. But SDIO3.0 device with
MMC_PM_KEEP_POWER will not re-initialization, but will call set_ios-> set_uhs_signaling().

Regards
Haibo Chen
> 
> Frank
> 
> >
> > Signed-off-by: Luke Wang <ziniu.wang_1 at nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 27 ++++++++++++++-------------
> >  1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 7611682f10c3..c448a53530a5 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -728,23 +728,17 @@ static void esdhc_writew_le(struct sdhci_host
> *host, u16 val, int reg)
> >  		writel(new_val, host->ioaddr + ESDHC_VENDOR_SPEC);
> >  		if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
> >  			u32 v = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS);
> > -			u32 m = readl(host->ioaddr + ESDHC_MIX_CTRL);
> > -			if (val & SDHCI_CTRL_TUNED_CLK) {
> > +			if (val & SDHCI_CTRL_TUNED_CLK)
> >  				v |= ESDHC_MIX_CTRL_SMPCLK_SEL;
> > -			} else {
> > +			else
> >  				v &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
> > -				m &= ~ESDHC_MIX_CTRL_FBCLK_SEL;
> > -			}
> >
> > -			if (val & SDHCI_CTRL_EXEC_TUNING) {
> > +			if (val & SDHCI_CTRL_EXEC_TUNING)
> >  				v |= ESDHC_MIX_CTRL_EXE_TUNE;
> > -				m |= ESDHC_MIX_CTRL_FBCLK_SEL;
> > -			} else {
> > +			else
> >  				v &= ~ESDHC_MIX_CTRL_EXE_TUNE;
> > -			}
> >
> >  			writel(v, host->ioaddr + SDHCI_AUTO_CMD_STATUS);
> > -			writel(m, host->ioaddr + ESDHC_MIX_CTRL);
> >  		}
> >  		return;
> >  	case SDHCI_TRANSFER_MODE:
> > @@ -1082,7 +1076,6 @@ static void esdhc_reset_tuning(struct sdhci_host
> *host)
> >  		ctrl &= ~ESDHC_MIX_CTRL_AUTO_TUNE_EN;
> >  		if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) {
> >  			ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
> > -			ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL;
> >  			writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL);
> >  			writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
> >  		} else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING)
> { @@
> > -1177,8 +1170,7 @@ static void esdhc_prepare_tuning(struct sdhci_host
> *host, u32 val)
> >  		"warning! RESET_ALL never complete before sending tuning
> > command\n");
> >
> >  	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
> > -	reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL |
> > -			ESDHC_MIX_CTRL_FBCLK_SEL;
> > +	reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL;
> >  	writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
> >
> 	writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MAS
> K, val),
> >  	       host->ioaddr + ESDHC_TUNE_CTRL_STATUS); @@ -1432,6
> +1424,15
> > @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned
> timing)
> >  		break;
> >  	}
> >
> > +	if (timing == MMC_TIMING_UHS_SDR104 ||
> > +	    timing == MMC_TIMING_MMC_HS200 ||
> > +	    timing == MMC_TIMING_MMC_HS400)
> > +		m |= ESDHC_MIX_CTRL_FBCLK_SEL;
> > +	else
> > +		m &= ~ESDHC_MIX_CTRL_FBCLK_SEL;
> > +
> > +	writel(m, host->ioaddr + ESDHC_MIX_CTRL);
> > +
> >  	esdhc_change_pinstate(host, timing);  }
> >
> > --
> > 2.34.1
> >