Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
dwmshc: add rk3528 support
To:
tech@openbsd.org
Cc:
aoglmf@gmail.com, kettenis@openbsd.org
Date:
Fri, 27 Mar 2026 16:30:50 +1000

Download raw body.

Thread
  • Jonathan Matthew:

    dwmshc: add rk3528 support

dwmshc(4) attaches to the RK3528 emmc controller, because the device tree
says it's compatible with the 3588 variant, and sd(4) attaches successsfully
and looks like it'll work, but i/o operations to the device fail.

To make it actually work, it needs a few different settings to the 3568
and 3588.  I decided that just adding more if-else blocks was making it
a bit hard to follow, so I extracted the different register values to a
per-model structure.

For the RK3528 controller, we have to manually set the tap values to twice
the DLL lock value rather than letting the controller do it.  It doesn't
work otherwise.  This means the write to the RXCLK register has to happen
after the call to dwmshc_dll_wait() in order to get a valid read of the
DLL lock value.

I've tested this on RK3588 without problems, but not RK356x yet.

ok?

Index: dwmshc.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/dwmshc.c,v
diff -u -p -r1.8 dwmshc.c
--- dwmshc.c	15 Jul 2024 09:56:30 -0000	1.8
+++ dwmshc.c	27 Mar 2026 05:44:38 -0000
@@ -102,6 +102,7 @@
 #define  EMMC_DLL_TXCLK_TX_TAP_NUM_SHIFT	0
 #define  EMMC_DLL_TXCLK_TX_TAP_NUM_MASK		0x1f
 #define  EMMC_DLL_TXCLK_TX_TAP_NUM_90_DEG	0x8
+#define  EMMC_DLL_TXCLK_TX_TAP_NUM_RK3528	0xc
 #define  EMMC_DLL_TXCLK_TX_TAP_NUM_DEFAULT	0x10
 #define  EMMC_DLL_TXCLK_TX_TAP_VALUE_SHIFT	8
 #define  EMMC_DLL_TXCLK_TX_TAP_VALUE_MASK	0xff
@@ -157,10 +158,18 @@
 #define  EMMC_DLL_STATUS1_DLL_STRBIN_DELAY_VALUE_MASK \
 						0xff
 
+struct dwmshc_params {
+	const char		*compatible;
+	uint32_t		 txclk_tapnum;
+	uint32_t		 rxclk;
+	int			 dll_tap_value;
+};
+
 struct dwmshc_softc {
 	struct sdhc_softc	 sc_sdhc;
 
 	int			 sc_node;
+	const struct dwmshc_params *sc_params;
 
 	bus_space_tag_t		 sc_iot;
 	bus_space_handle_t	 sc_ioh;
@@ -193,14 +202,43 @@ struct cfdriver dwmshc_cd = {
 	NULL, "dwmshc", DV_DULL
 };
 
+static const struct dwmshc_params dwmshc_hw_params[] = {
+	{
+	  .compatible = "rockchip,rk3528-dwcmshc",
+	  .txclk_tapnum = EMMC_DLL_TXCLK_TX_TAP_NUM_RK3528,
+	  .rxclk = EMMC_DLL_RXCLK_RX_CLK_OUT_SEL,
+	  .dll_tap_value = 1
+	},
+	{
+	  .compatible = "rockchip,rk3568-dwcmshc",
+	  .txclk_tapnum = EMMC_DLL_TXCLK_TX_TAP_NUM_DEFAULT,
+	  .rxclk = EMMC_DLL_RXCLK_RX_CLK_OUT_SEL |
+	      EMMC_DLL_RXCLK_RX_CLK_SRC_SEL,
+	  .dll_tap_value = 0
+	},
+	{
+	  .compatible = "rockchip,rk3588-dwcmshc",
+	  .txclk_tapnum = EMMC_DLL_TXCLK_TX_TAP_NUM_DEFAULT,
+	  .rxclk = EMMC_DLL_RXCLK_RX_CLK_OUT_SEL,
+	  .dll_tap_value = 0
+	}
+};
+
 int
 dwmshc_match(struct device *parent, void *match, void *aux)
 {
 	struct fdt_attach_args *faa = aux;
+	int i;
+
+	for (i = 0; i < nitems(dwmshc_hw_params); i++) {
+		if (OF_is_compatible(faa->fa_node,
+		    dwmshc_hw_params[i].compatible))
+			return 1;
+	}
 
-	return (OF_is_compatible(faa->fa_node, "rockchip,rk3568-dwcmshc") ||
-	    OF_is_compatible(faa->fa_node, "rockchip,rk3588-dwcmshc"));
+	return 0;
 }
+
 static void
 dwmshc_attach(struct device *parent, struct device *self, void *aux)
 {
@@ -209,7 +247,7 @@ dwmshc_attach(struct device *parent, str
 	struct fdt_attach_args *faa = aux;
 	uint64_t capmask = 0;
 	uint16_t capset = 0;
-	int bus_width;
+	int bus_width, i;
 	uint32_t freq;
 
 	sc->sc_node = faa->fa_node;
@@ -228,6 +266,14 @@ dwmshc_attach(struct device *parent, str
 		return;
 	}
 
+	for (i = 0; i < nitems(dwmshc_hw_params); i++) {
+		if (OF_is_compatible(sc->sc_node,
+		    dwmshc_hw_params[i].compatible)) {
+			sc->sc_params = &dwmshc_hw_params[i];
+			break;
+		}
+	}
+
 	pinctrl_byname(sc->sc_node, "default");
 
 	clock_set_assigned(sc->sc_node);
@@ -320,7 +366,8 @@ static void
 dwmshc_clock_post(struct sdhc_softc *sdhc, int freq, int timing)
 {
 	struct dwmshc_softc *sc = (struct dwmshc_softc *)sdhc;
-	uint32_t txclk_tapnum = EMMC_DLL_TXCLK_TX_TAP_NUM_DEFAULT;
+	uint32_t txclk_tapnum = sc->sc_params->txclk_tapnum;
+	uint32_t dll_val;
 
 	clock_set_frequency(sc->sc_node, 0, freq * 1000);
 
@@ -341,11 +388,6 @@ dwmshc_clock_post(struct sdhc_softc *sdh
 	delay(1);
 	dwmshc_wr4(sc, EMMC_DLL_CTRL, 0);
 
-	if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-dwcmshc"))
-		dwmshc_wr4(sc, EMMC_DLL_RXCLK, EMMC_DLL_RXCLK_RX_CLK_OUT_SEL |
-		    EMMC_DLL_RXCLK_RX_CLK_SRC_SEL);
-	else
-		dwmshc_wr4(sc, EMMC_DLL_RXCLK, EMMC_DLL_RXCLK_RX_CLK_OUT_SEL);
 	dwmshc_wr4(sc, EMMC_DLL_CTRL, EMMC_DLL_CTRL_DLL_START |
 	    0x5 << EMMC_DLL_CTRL_DLL_START_POINT_SHIFT |
 	    0x2 << EMMC_DLL_CTRL_DLL_INCREMENT_SHIFT);
@@ -353,6 +395,16 @@ dwmshc_clock_post(struct sdhc_softc *sdh
 	if (dwmshc_dll_wait(sc) != 0)
 		return;
 
+	dll_val = 0;
+	if (sc->sc_params->dll_tap_value != 0) {
+		dll_val = EMMC_DLL_RXCLK_RX_TAP_VALUE_SEL |
+		    ((dwmshc_rd4(sc, EMMC_DLL_STATUS0) &
+		    EMMC_DLL_STATUS0_DLL_LOCK_VALUE_MASK) * 2);
+		dwmshc_wr4(sc, EMMC_DLL_RXCLK, EMMC_DLL_RXCLK_RX_CLK_OUT_SEL |
+		    dll_val);
+	}
+	dwmshc_wr4(sc, EMMC_DLL_RXCLK, sc->sc_params->rxclk | dll_val);
+
 	dwmshc_wr4(sc, EMMC_AT_CTRL, EMMC_AT_CTRL_TUNE_CLK_STOP_EN |
 	    EMMC_AT_CTRL_PRE_CHANGE_DLY_LT4 |
 	    EMMC_AT_CTRL_POST_CHANGE_DLY_LT4);
@@ -377,11 +429,12 @@ dwmshc_clock_post(struct sdhc_softc *sdh
 
 	dwmshc_wr4(sc, EMMC_DLL_TXCLK, EMMC_DLL_TXCLK_TX_CLK_OUT_SEL |
 	    EMMC_DLL_TXCLK_TX_TAP_NUM_SEL |
-	    txclk_tapnum << EMMC_DLL_TXCLK_TX_TAP_NUM_SHIFT);
+	    txclk_tapnum << EMMC_DLL_TXCLK_TX_TAP_NUM_SHIFT |
+	    dll_val);
 	dwmshc_wr4(sc, EMMC_DLL_STRBIN, EMMC_DLL_STRBIN_DELAY_ENA |
 	    EMMC_DLL_STRBIN_TAP_NUM_SEL |
 	    (EMMC_DLL_STRBIN_TAP_NUM_90_DEG <<
-	     EMMC_DLL_STRBIN_TAP_NUM_SHIFT));
+	     EMMC_DLL_STRBIN_TAP_NUM_SHIFT) | dll_val);
 }
 
 static int