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

[Ulf Hansson]

On Wed, 8 Apr 2026 at 09:19, Shawn Lin <shawn.lin at rock-chips.com> 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 at vger.kernel.org>
> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
> Acked-by: Adrian Hunter <adrian.hunter at intel.com>

It's getting too late for fixes (unless we get an rc8), so I decided
to apply this for next instead, thanks!

Kind regards
Uffe


> ---
>
> Changes in v3:
> - Fix compile error while amending the patch file by mistake
> - Add Adrian's tag
>
> 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..0b2158a 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
>