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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aC39X1f6fJErfkDN@lizhi-Precision-Tower-5810>
Date: Wed, 21 May 2025 12:20:47 -0400
From: Frank Li <Frank.li@....com>
To: ziniu.wang_1@....com
Cc: haibo.chen@....com, adrian.hunter@...el.com, ulf.hansson@...aro.org,
	linux-mmc@...r.kernel.org, shawnguo@...nel.org,
	s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
	imx@...ts.linux.dev, s32@....com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mmc: sdhci-esdhc-imx: optimize clock loopback
 selection with dummy pad support

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