* Re: [PATCH v2] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration
2026-04-08 6:26 [PATCH v2] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration Shawn Lin
@ 2026-04-08 6:39 ` Adrian Hunter
2026-04-19 0:04 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: Adrian Hunter @ 2026-04-08 6:39 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson; +Cc: linux-mmc, linux-rockchip, Stable
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration
2026-04-08 6:26 [PATCH v2] mmc: sdhci-of-dwcmshc: Disable clock before DLL configuration Shawn Lin
2026-04-08 6:39 ` Adrian Hunter
@ 2026-04-19 0:04 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2026-04-19 0:04 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson
Cc: oe-kbuild-all, linux-mmc, linux-rockchip, Adrian Hunter,
Shawn Lin, Stable
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
^ permalink raw reply [flat|nested] 3+ messages in thread