Re: [PATCH] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration

[Adrian Hunter <adrian.hunter@intel.com>]

On 01/04/2026 06:39, Shawn Lin wrote:
> According to the ASIC design recommendations, the clock must be
> disabled before operating the DLL to prevent glitches that could
> affect the internal digital logic. In extreme cases, failing to
> do so may cause the controller to malfunction completely.
> 
> Adds a step to disable the clock before DLL configuration and
> re-enables it at the end.
> 
> Fixes: 08f3dff799d4 ("mmc: sdhci-of-dwcmshc: add rockchip platform support")
> Cc: <Stable@vger.kernel.org>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> This is bascially a code sync with the downstream vendor kernel which was been
> done this way and tested for some years to confirm it could fix the issues in
> all corner cases.
> 
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 6139516..e3ae334 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -783,12 +783,15 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  	extra |= BIT(4);
>  	sdhci_writel(host, extra, reg);
>  
> +	/* Disable clock while config DLL */
> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
>  	if (clock <= 52000000) {
>  		if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
>  		    host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
>  			dev_err(mmc_dev(host->mmc),
>  				"Can't reduce the clock below 52MHz in HS200/HS400 mode");
> -			return;
> +			goto enable_clk;
>  		}
>  
>  		/*
> @@ -808,7 +811,7 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  			DLL_STRBIN_DELAY_NUM_SEL |
>  			DLL_STRBIN_DELAY_NUM_DEFAULT << DLL_STRBIN_DELAY_NUM_OFFSET;
>  		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> -		return;
> +		goto enable_clk;
>  	}
>  
>  	/* Reset DLL */
> @@ -835,7 +838,7 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  				 500 * USEC_PER_MSEC);
>  	if (err) {
>  		dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
> -		return;
> +		goto enable_clk;
>  	}
>  
>  	extra = 0x1 << 16 | /* tune clock stop en */
> @@ -868,6 +871,9 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  		DLL_STRBIN_TAPNUM_DEFAULT |
>  		DLL_STRBIN_TAPNUM_FROM_SW;
>  	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> +
> +enable_clk:
> +	sdhci_enable_clk(host, 0);

Should this be 0?  If so, needs some explanation.

>  }
>  
>  static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)