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

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

[Shawn Lin]

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

Changes in v2:
- Add a comment about why passing zero to sdhci_enable_clk()

 drivers/mmc/host/sdhci-of-dwcmshc.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 6139516..5af35c9 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,16 @@ 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
+	/*
+	 * The sdclk frequency select bits in SDHCI_CLOCK_CONTROL are not functional
+	 * on Rockchip's SDHCI implementation. Instead, the clock frequency is fully
+	 * controlled via external clk provider by calling clk_set_rate(). Consequently,
+	 * passing 0 to sdhci_enable_clk() only re-enables the already-configured clock,
+	 * which matches the hardware's actual behavior.
+	 */
+	sdhci_enable_clk(host, 0);
 }
 
 static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
-- 
2.7.4


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

[Adrian Hunter]

On 08/04/2026 09:26, 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>

Missing colon below, otherwise:

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

> ---
> 
> Changes in v2:
> - Add a comment about why passing zero to sdhci_enable_clk()
> 
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 6139516..5af35c9 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,16 @@ 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

Missing colon

> +	/*
> +	 * The sdclk frequency select bits in SDHCI_CLOCK_CONTROL are not functional
> +	 * on Rockchip's SDHCI implementation. Instead, the clock frequency is fully
> +	 * controlled via external clk provider by calling clk_set_rate(). Consequently,
> +	 * passing 0 to sdhci_enable_clk() only re-enables the already-configured clock,
> +	 * which matches the hardware's actual behavior.
> +	 */
> +	sdhci_enable_clk(host, 0);
>  }
>  
>  static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

[kernel test robot]

Hi Shawn,

kernel test robot noticed the following build errors:

[auto build test ERROR on v7.0]
[cannot apply to linus/master ulf-hansson-mmc-mirror/next next-20260417]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shawn-Lin/mmc-sdhci-of-dwcmshc-Disable-clock-before-DLL-configuration/20260417-234134
base:   v7.0
patch link:    https://lore.kernel.org/r/1775629564-11267-1-git-send-email-shawn.lin%40rock-chips.com
patch subject: [PATCH v2] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20260419/202604190759.51VAkEgC-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260419/202604190759.51VAkEgC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604190759.51VAkEgC-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/mmc/host/sdhci-of-dwcmshc.c: In function 'dwcmshc_rk3568_set_clock':
>> drivers/mmc/host/sdhci-of-dwcmshc.c:830:1: error: unknown type name 'enable_clk'
     830 | enable_clk
         | ^~~~~~~~~~
>> drivers/mmc/host/sdhci-of-dwcmshc.c:838:31: error: expected ')' before numeric constant
     838 |         sdhci_enable_clk(host, 0);
         |                               ^~
         |                               )
>> drivers/mmc/host/sdhci-of-dwcmshc.c:796:17: error: label 'enable_clk' used but not defined
     796 |                 goto enable_clk;
         |                 ^~~~


vim +/enable_clk +830 drivers/mmc/host/sdhci-of-dwcmshc.c

   706	
   707	static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock)
   708	{
   709		struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
   710		struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
   711		struct rk35xx_priv *priv = dwc_priv->priv;
   712		u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
   713		u32 extra, reg;
   714		int err;
   715	
   716		host->mmc->actual_clock = 0;
   717	
   718		if (clock == 0) {
   719			/* Disable interface clock at initial state. */
   720			sdhci_set_clock(host, clock);
   721			return;
   722		}
   723	
   724		/* Rockchip platform only support 375KHz for identify mode */
   725		if (clock <= 400000)
   726			clock = 375000;
   727	
   728		err = clk_set_rate(pltfm_host->clk, clock);
   729		if (err)
   730			dev_err(mmc_dev(host->mmc), "fail to set clock %d", clock);
   731	
   732		sdhci_set_clock(host, clock);
   733	
   734		/* Disable cmd conflict check and internal clock gate */
   735		reg = dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3;
   736		extra = sdhci_readl(host, reg);
   737		extra &= ~BIT(0);
   738		extra |= BIT(4);
   739		sdhci_writel(host, extra, reg);
   740	
   741		/* Disable clock while config DLL */
   742		sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
   743	
   744		if (clock <= 52000000) {
   745			if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
   746			    host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
   747				dev_err(mmc_dev(host->mmc),
   748					"Can't reduce the clock below 52MHz in HS200/HS400 mode");
   749				goto enable_clk;
   750			}
   751	
   752			/*
   753			 * Disable DLL and reset both of sample and drive clock.
   754			 * The bypass bit and start bit need to be set if DLL is not locked.
   755			 */
   756			sdhci_writel(host, DWCMSHC_EMMC_DLL_BYPASS | DWCMSHC_EMMC_DLL_START, DWCMSHC_EMMC_DLL_CTRL);
   757			sdhci_writel(host, DLL_RXCLK_ORI_GATE, DWCMSHC_EMMC_DLL_RXCLK);
   758			sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
   759			sdhci_writel(host, 0, DECMSHC_EMMC_DLL_CMDOUT);
   760			/*
   761			 * Before switching to hs400es mode, the driver will enable
   762			 * enhanced strobe first. PHY needs to configure the parameters
   763			 * of enhanced strobe first.
   764			 */
   765			extra = DWCMSHC_EMMC_DLL_DLYENA |
   766				DLL_STRBIN_DELAY_NUM_SEL |
   767				DLL_STRBIN_DELAY_NUM_DEFAULT << DLL_STRBIN_DELAY_NUM_OFFSET;
   768			sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
   769			goto enable_clk;
   770		}
   771	
   772		/* Reset DLL */
   773		sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
   774		udelay(1);
   775		sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
   776	
   777		/*
   778		 * We shouldn't set DLL_RXCLK_NO_INVERTER for identify mode but
   779		 * we must set it in higher speed mode.
   780		 */
   781		extra = DWCMSHC_EMMC_DLL_DLYENA;
   782		if (priv->devtype == DWCMSHC_RK3568)
   783			extra |= DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
   784		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
   785	
   786		/* Init DLL settings */
   787		extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT |
   788			0x2 << DWCMSHC_EMMC_DLL_INC |
   789			DWCMSHC_EMMC_DLL_START;
   790		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
   791		err = readl_poll_timeout(host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
   792					 extra, DLL_LOCK_WO_TMOUT(extra), 1,
   793					 500 * USEC_PER_MSEC);
   794		if (err) {
   795			dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
 > 796			goto enable_clk;
   797		}
   798	
   799		extra = 0x1 << 16 | /* tune clock stop en */
   800			0x3 << 17 | /* pre-change delay */
   801			0x3 << 19;  /* post-change delay */
   802		sdhci_writel(host, extra, dwc_priv->vendor_specific_area1 + DWCMSHC_EMMC_ATCTRL);
   803	
   804		if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
   805		    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
   806			txclk_tapnum = priv->txclk_tapnum;
   807	
   808		if ((priv->devtype == DWCMSHC_RK3588) && host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
   809			txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES;
   810	
   811			extra = DLL_CMDOUT_SRC_CLK_NEG |
   812				DLL_CMDOUT_EN_SRC_CLK_NEG |
   813				DWCMSHC_EMMC_DLL_DLYENA |
   814				DLL_CMDOUT_TAPNUM_90_DEGREES |
   815				DLL_CMDOUT_TAPNUM_FROM_SW;
   816			sdhci_writel(host, extra, DECMSHC_EMMC_DLL_CMDOUT);
   817		}
   818	
   819		extra = DWCMSHC_EMMC_DLL_DLYENA |
   820			DLL_TXCLK_TAPNUM_FROM_SW |
   821			DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL |
   822			txclk_tapnum;
   823		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
   824	
   825		extra = DWCMSHC_EMMC_DLL_DLYENA |
   826			DLL_STRBIN_TAPNUM_DEFAULT |
   827			DLL_STRBIN_TAPNUM_FROM_SW;
   828		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
   829	
 > 830	enable_clk
   831		/*
   832		 * The sdclk frequency select bits in SDHCI_CLOCK_CONTROL are not functional
   833		 * on Rockchip's SDHCI implementation. Instead, the clock frequency is fully
   834		 * controlled via external clk provider by calling clk_set_rate(). Consequently,
   835		 * passing 0 to sdhci_enable_clk() only re-enables the already-configured clock,
   836		 * which matches the hardware's actual behavior.
   837		 */
 > 838		sdhci_enable_clk(host, 0);
   839	}
   840	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip