Re: [PATCH v2] mmc: rockchip_sdhci: Set xx_TAP_VALUE for RK3528

[Kever Yang]

On 2025/7/15 04:34, Jonas Karlman wrote:

eMMC erase and write support on RK3528 is somewhat unreliable, sometime
e.g. mmc erase and write commands will fail with an error.

Use the delay line lock value for half card clock cycle, DLL_LOCK_VALUE,
to set a manual xx_TAP_VALUE to fix the unreliable eMMC support.

This is only enabled for RK3528, remaining SoCs still use the automatic
tap value, (DLL_LOCK_VALUE * 2) % 256, same value we configure manually
for RK3528.

Signed-off-by: Jonas Karlman <jo...@kwiboo.se>

Reviewed-by: Kever Yang <kever.y...@rock-chips.com>

Thanks,
- Kever

---
Changes in v2:
- Rename flag and field value to function name, TAPVALUE_FROM_SW
- Simplify and make code more understandable with use of FIELD_GET/PREP,
   should be more clear that xx_TAP_VALUE is set to DLL_LOCK_VALUE * 2
---
  drivers/mmc/rockchip_sdhci.c | 27 ++++++++++++++++++++++-----
  1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index 761e3619329c..cca917da68e7 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -9,6 +9,7 @@
  #include <dm.h>
  #include <dm/ofnode.h>
  #include <dt-structs.h>
+#include <linux/bitfield.h>
  #include <linux/delay.h>
  #include <linux/err.h>
  #include <linux/libfdt.h>
@@ -86,6 +87,9 @@
  #define DLL_CMDOUT_SRC_CLK_NEG                BIT(28)
  #define DLL_CMDOUT_EN_SRC_CLK_NEG     BIT(29)
  #define DLL_CMDOUT_BOTH_CLK_EDGE      BIT(30)
+#define DLL_TAPVALUE_FROM_SW           BIT(25)
+#define DLL_TAP_VALUE_PREP(x)          FIELD_PREP(GENMASK(15, 8), (x))
+#define DLL_LOCK_VALUE_GET(x)          FIELD_GET(GENMASK(7, 0), (x))

#define DLL_LOCK_WO_TMOUT(x) \

        ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
@@ -93,6 +97,7 @@
  #define ROCKCHIP_MAX_CLKS             3

#define FLAG_INVERTER_FLAG_IN_RXCLK BIT(0)

+#define FLAG_TAPVALUE_FROM_SW          BIT(1)

struct rockchip_sdhc_plat {

        struct mmc_config cfg;
@@ -317,7 +322,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, 
u32 clock, bool enab
        struct sdhci_data *data = (struct sdhci_data 
*)dev_get_driver_data(priv->dev);
        struct mmc *mmc = host->mmc;
        int val, ret;
-       u32 extra, txclk_tapnum;
+       u32 extra, txclk_tapnum, dll_tap_value;

if (!enable) {

                sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
@@ -347,7 +352,15 @@ static int rk3568_sdhci_config_dll(struct sdhci_host 
*host, u32 clock, bool enab
                if (ret)
                        return ret;

- extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_ORI_GATE;

+               if (data->flags & FLAG_TAPVALUE_FROM_SW)
+                       dll_tap_value = DLL_TAPVALUE_FROM_SW |
+                                       
DLL_TAP_VALUE_PREP(DLL_LOCK_VALUE_GET(val) * 2);
+               else
+                       dll_tap_value = 0;
+
+               extra = DWCMSHC_EMMC_DLL_DLYENA |
+                       DLL_RXCLK_ORI_GATE |
+                       dll_tap_value;
                if (data->flags & FLAG_INVERTER_FLAG_IN_RXCLK)
                        extra |= DLL_RXCLK_NO_INVERTER;
                sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
@@ -361,19 +374,22 @@ static int rk3568_sdhci_config_dll(struct sdhci_host 
*host, u32 clock, bool enab
                                DLL_CMDOUT_BOTH_CLK_EDGE |
                                DWCMSHC_EMMC_DLL_DLYENA |
                                data->hs400_cmdout_tapnum |
-                               DLL_CMDOUT_TAPNUM_FROM_SW;
+                               DLL_CMDOUT_TAPNUM_FROM_SW |
+                               dll_tap_value;
                        sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CMDOUT);
                }

extra = DWCMSHC_EMMC_DLL_DLYENA |

                        DLL_TXCLK_TAPNUM_FROM_SW |
                        DLL_TXCLK_NO_INVERTER |
-                       txclk_tapnum;
+                       txclk_tapnum |
+                       dll_tap_value;
                sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);

extra = DWCMSHC_EMMC_DLL_DLYENA |

                        data->hs400_strbin_tapnum |
-                       DLL_STRBIN_TAPNUM_FROM_SW;
+                       DLL_STRBIN_TAPNUM_FROM_SW |
+                       dll_tap_value;
                sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
        } else {
                /*
@@ -663,6 +679,7 @@ static const struct sdhci_data rk3528_data = {
        .set_ios_post = rk3568_sdhci_set_ios_post,
        .set_clock = rk3568_sdhci_set_clock,
        .config_dll = rk3568_sdhci_config_dll,
+       .flags = FLAG_TAPVALUE_FROM_SW,
        .hs200_txclk_tapnum = 0xc,
        .hs400_txclk_tapnum = 0x6,
        .hs400_cmdout_tapnum = 0x6,