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

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

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@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.

Signed-off-by: Luke Wang <ziniu.wang_1@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_MASK, 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

[PATCH 2/2] mmc: sdhci-esdhc-imx: optimize clock loopback selection with dummy pad support

[ziniu.wang_1]

From: Luke Wang <ziniu.wang_1@nxp.com>

For legacy platforms without dummy pad:
When clock <= 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 0 (external clock
pad loopback) for better bus clock proximity.
When clock > 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 1 (internal clock
loopback) to avoid signal reflection noise at high frequency.

For i.MX94/95 with dummy pad support:
Keep ESDHC_MIX_CTRL_FBCLK_SEL at 0 for all speed mode. Hardware
automatically substitutes clock pad loopback with dummy pad loopback
when available, eliminating signal reflections while preserving better
bus clock proximity.

Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index c448a53530a5..5f1c45b2bd5d 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -212,6 +212,9 @@
 /* The IP does not have GPIO CD wake capabilities */
 #define ESDHC_FLAG_SKIP_CD_WAKE		BIT(18)
 
+/* the controller has dummy pad for clock loopback */
+#define ESDHC_FLAG_DUMMY_PAD		BIT(19)
+
 #define ESDHC_AUTO_TUNING_WINDOW	3
 
 enum wp_types {
@@ -348,6 +351,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
 	.quirks = SDHCI_QUIRK_NO_LED,
 };
 
+static struct esdhc_soc_data usdhc_imx95_data = {
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
+			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
+			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
+			| ESDHC_FLAG_DUMMY_PAD,
+	.quirks = SDHCI_QUIRK_NO_LED,
+};
+
 struct pltfm_imx_data {
 	u32 scratchpad;
 	struct pinctrl *pinctrl;
@@ -392,6 +404,8 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
 	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
 	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
 	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
+	{ .compatible = "fsl,imx94-usdhc", .data = &usdhc_imx95_data, },
+	{ .compatible = "fsl,imx95-usdhc", .data = &usdhc_imx95_data, },
 	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
 	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
 	{ /* sentinel */ }
@@ -1424,9 +1438,10 @@ 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)
+	if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD) &&
+	    (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;
@@ -1678,7 +1693,9 @@ static void sdhc_esdhc_tuning_restore(struct sdhci_host *host)
 		writel(reg, host->ioaddr + ESDHC_TUNING_CTRL);
 
 		reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
-		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL | ESDHC_MIX_CTRL_FBCLK_SEL;
+		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL;
+		if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD))
+			reg |= ESDHC_MIX_CTRL_FBCLK_SEL;
 		writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
 
 		writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,
-- 
2.34.1

Re: [PATCH 2/2] mmc: sdhci-esdhc-imx: optimize clock loopback selection with dummy pad support

[Frank Li]

On Wed, May 21, 2025 at 10:55:02AM +0800, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> For legacy platforms without dummy pad:
> When clock <= 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 0 (external clock
> pad loopback) for better bus clock proximity.
> When clock > 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 1 (internal clock
> loopback) to avoid signal reflection noise at high frequency.
>
> For i.MX94/95 with dummy pad support:
> Keep ESDHC_MIX_CTRL_FBCLK_SEL at 0 for all speed mode. Hardware
> automatically substitutes clock pad loopback with dummy pad loopback
> when available, eliminating signal reflections while preserving better
> bus clock proximity.

Add indents after : to better read

For legacy platforms without dummy pad:
   When clock <= 100Mhz: Set ...

For i.MX94/95 with dummy pad support:
   Set ESDHC_MIX_CTRL_FBCLK_SEL at 0 for all speed mode. ....

>
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c448a53530a5..5f1c45b2bd5d 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -212,6 +212,9 @@
>  /* The IP does not have GPIO CD wake capabilities */
>  #define ESDHC_FLAG_SKIP_CD_WAKE		BIT(18)
>
> +/* the controller has dummy pad for clock loopback */
> +#define ESDHC_FLAG_DUMMY_PAD		BIT(19)
> +
>  #define ESDHC_AUTO_TUNING_WINDOW	3
>
>  enum wp_types {
> @@ -348,6 +351,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
>  	.quirks = SDHCI_QUIRK_NO_LED,
>  };
>
> +static struct esdhc_soc_data usdhc_imx95_data = {
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> +			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
> +			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_DUMMY_PAD,
> +	.quirks = SDHCI_QUIRK_NO_LED,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -392,6 +404,8 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>  	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
>  	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> +	{ .compatible = "fsl,imx94-usdhc", .data = &usdhc_imx95_data, },
> +	{ .compatible = "fsl,imx95-usdhc", .data = &usdhc_imx95_data, },

You'd better mention "fsl,imx94-usdhc" and "fsl,imx95-usdhc" already in
binding doc after "---" to let maintainer known these already documented.

Frank

>  	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
>  	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
>  	{ /* sentinel */ }
> @@ -1424,9 +1438,10 @@ 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)
> +	if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD) &&
> +	    (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;
> @@ -1678,7 +1693,9 @@ static void sdhc_esdhc_tuning_restore(struct sdhci_host *host)
>  		writel(reg, host->ioaddr + ESDHC_TUNING_CTRL);
>
>  		reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
> -		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL | ESDHC_MIX_CTRL_FBCLK_SEL;
> +		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL;
> +		if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD))
> +			reg |= ESDHC_MIX_CTRL_FBCLK_SEL;
>  		writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
>
>  		writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,
> --
> 2.34.1
>

Re: [PATCH 2/2] mmc: sdhci-esdhc-imx: optimize clock loopback selection with dummy pad support

[Adrian Hunter]

On 21/05/2025 05:55, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> For legacy platforms without dummy pad:
> When clock <= 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 0 (external clock
> pad loopback) for better bus clock proximity.
> When clock > 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 1 (internal clock
> loopback) to avoid signal reflection noise at high frequency.
> 
> For i.MX94/95 with dummy pad support:
> Keep ESDHC_MIX_CTRL_FBCLK_SEL at 0 for all speed mode. Hardware
> automatically substitutes clock pad loopback with dummy pad loopback
> when available, eliminating signal reflections while preserving better
> bus clock proximity.
> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c448a53530a5..5f1c45b2bd5d 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -212,6 +212,9 @@
>  /* The IP does not have GPIO CD wake capabilities */
>  #define ESDHC_FLAG_SKIP_CD_WAKE		BIT(18)
>  
> +/* the controller has dummy pad for clock loopback */
> +#define ESDHC_FLAG_DUMMY_PAD		BIT(19)
> +
>  #define ESDHC_AUTO_TUNING_WINDOW	3
>  
>  enum wp_types {
> @@ -348,6 +351,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
>  	.quirks = SDHCI_QUIRK_NO_LED,
>  };
>  
> +static struct esdhc_soc_data usdhc_imx95_data = {
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> +			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
> +			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_DUMMY_PAD,
> +	.quirks = SDHCI_QUIRK_NO_LED,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -392,6 +404,8 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>  	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
>  	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> +	{ .compatible = "fsl,imx94-usdhc", .data = &usdhc_imx95_data, },
> +	{ .compatible = "fsl,imx95-usdhc", .data = &usdhc_imx95_data, },
>  	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
>  	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
>  	{ /* sentinel */ }
> @@ -1424,9 +1438,10 @@ 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)
> +	if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD) &&
> +	    (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;
> @@ -1678,7 +1693,9 @@ static void sdhc_esdhc_tuning_restore(struct sdhci_host *host)
>  		writel(reg, host->ioaddr + ESDHC_TUNING_CTRL);
>  
>  		reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
> -		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL | ESDHC_MIX_CTRL_FBCLK_SEL;
> +		reg |= ESDHC_MIX_CTRL_SMPCLK_SEL;
> +		if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD))
> +			reg |= ESDHC_MIX_CTRL_FBCLK_SEL;
>  		writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
>  
>  		writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,

Re: [PATCH 2/2] mmc: sdhci-esdhc-imx: optimize clock loopback selection with dummy pad support

[Ulf Hansson]

On Wed, 21 May 2025 at 04:53, <ziniu.wang_1@nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> For legacy platforms without dummy pad:
> When clock <= 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 0 (external clock
> pad loopback) for better bus clock proximity.
> When clock > 100MHz: Set ESDHC_MIX_CTRL_FBCLK_SEL to 1 (internal clock
> loopback) to avoid signal reflection noise at high frequency.
>
> For i.MX94/95 with dummy pad support:
> Keep ESDHC_MIX_CTRL_FBCLK_SEL at 0 for all speed mode. Hardware
> automatically substitutes clock pad loopback with dummy pad loopback
> when available, eliminating signal reflections while preserving better
> bus clock proximity.
>
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c448a53530a5..5f1c45b2bd5d 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -212,6 +212,9 @@
>  /* The IP does not have GPIO CD wake capabilities */
>  #define ESDHC_FLAG_SKIP_CD_WAKE                BIT(18)
>
> +/* the controller has dummy pad for clock loopback */
> +#define ESDHC_FLAG_DUMMY_PAD           BIT(19)
> +
>  #define ESDHC_AUTO_TUNING_WINDOW       3
>
>  enum wp_types {
> @@ -348,6 +351,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
>         .quirks = SDHCI_QUIRK_NO_LED,
>  };
>
> +static struct esdhc_soc_data usdhc_imx95_data = {
> +       .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> +                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +                       | ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
> +                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +                       | ESDHC_FLAG_DUMMY_PAD,
> +       .quirks = SDHCI_QUIRK_NO_LED,
> +};
> +
>  struct pltfm_imx_data {
>         u32 scratchpad;
>         struct pinctrl *pinctrl;
> @@ -392,6 +404,8 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>         { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>         { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
>         { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> +       { .compatible = "fsl,imx94-usdhc", .data = &usdhc_imx95_data, },
> +       { .compatible = "fsl,imx95-usdhc", .data = &usdhc_imx95_data, },
>         { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
>         { .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
>         { /* sentinel */ }
> @@ -1424,9 +1438,10 @@ 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)
> +       if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD) &&
> +           (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;
> @@ -1678,7 +1693,9 @@ static void sdhc_esdhc_tuning_restore(struct sdhci_host *host)
>                 writel(reg, host->ioaddr + ESDHC_TUNING_CTRL);
>
>                 reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
> -               reg |= ESDHC_MIX_CTRL_SMPCLK_SEL | ESDHC_MIX_CTRL_FBCLK_SEL;
> +               reg |= ESDHC_MIX_CTRL_SMPCLK_SEL;
> +               if (!(imx_data->socdata->flags & ESDHC_FLAG_DUMMY_PAD))
> +                       reg |= ESDHC_MIX_CTRL_FBCLK_SEL;
>                 writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
>
>                 writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,
> --
> 2.34.1
>

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

[Frank Li]

On Wed, May 21, 2025 at 10:55:01AM +0800, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@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?

Frank

>
> Signed-off-by: Luke Wang <ziniu.wang_1@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_MASK, 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
>

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

[Bough Chen]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: 2025年5月22日 0:10
> To: Luke Wang <ziniu.wang_1@nxp.com>
> Cc: Bough Chen <haibo.chen@nxp.com>; adrian.hunter@intel.com;
> ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> imx@lists.linux.dev; dl-S32 <S32@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@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@nxp.com wrote:
> > From: Luke Wang <ziniu.wang_1@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@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
> >

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

[Bough Chen]

> -----Original Message-----
> From: Bough Chen
> Sent: 2025年5月22日 11:53
> To: Frank Li <frank.li@nxp.com>; Luke Wang <ziniu.wang_1@nxp.com>
> Cc: adrian.hunter@intel.com; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; imx@lists.linux.dev; dl-S32
> <S32@nxp.com>; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] mmc: sdhci-esdhc-imx: refactor clock loopback
> selection logic
> 
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: 2025年5月22日 0:10
> > To: Luke Wang <ziniu.wang_1@nxp.com>
> > Cc: Bough Chen <haibo.chen@nxp.com>; adrian.hunter@intel.com;
> > ulf.hansson@linaro.org; linux-mmc@vger.kernel.org;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; imx@lists.linux.dev; dl-S32 <S32@nxp.com>;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@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@nxp.com wrote:
> > > From: Luke Wang <ziniu.wang_1@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().

sdhc_esdhc_tuning_restore() will also set back the ESDHC_MIX_CTRL_FBCLK_SEL..

Regards
Haibo Chen
> 
> Regards
> Haibo Chen
> >
> > Frank
> >
> > >
> > > Signed-off-by: Luke Wang <ziniu.wang_1@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
> > >

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

[Adrian Hunter]

On 21/05/2025 05:55, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@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.
> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.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_MASK, 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);
>  }
>  

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

[Ulf Hansson]

On Wed, 21 May 2025 at 04:53, <ziniu.wang_1@nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@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.
>
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>

Applied for next, thanks!

Kind regards
Uffe


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