Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
Re: rkcomphy: add rk3528 support
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
tech@openbsd.org, kettenis@openbsd.org
Date:
Mon, 23 Mar 2026 15:22:46 +1000

Download raw body.

Thread
On Sun, Mar 22, 2026 at 07:17:48PM +0100, Mark Kettenis wrote:
> > Date: Sun, 22 Mar 2026 21:48:59 +1000
> > From: Jonathan Matthew <jonathan@d14n.org>
> > 
> > This adds support for the RK3528 variant of the combo phy,
> > currently only in PCIe mode.  It looks like it can do USB3 too
> > but I don't see any devices that do that.  Hopefully a TRM for
> > the 3528 will turn up at some point so we can do this properly.
> > 
> > The rkclock change just ignores attempts at setting the pcie inner
> > phy clock rate, since it's fixed at 100Mhz but the device tree
> > indicates the driver should set it.
> > 
> > With this I can get pcie and the second nic on a Radxa E20C working
> > with u-boot 2026.04rc4 and some device tree bits pulled from linux.
> > 
> > ok?
> 
> Can we implement that clock properly?

I was hoping to get away with just copying what u-boot does..
I think I've deciphered the linux macro maze correctly; the updated diff
still works, at least.

> > +
> > +	reg = HREAD4(sc, COMBO_PIPE_PHY_REG(31));
> > +	reg &= ~COMBO_PIPE_PHY_SSC_OFFSET_MASK;
> > +	reg &= ~COMBO_PIPE_PHY_SSC_DIR_MASK;
> > +	reg |= COMBO_PIPE_PHY_SSC_DIR_DOWN;
> > +	HWRITE4(sc, COMBO_PIPE_PHY_REG(31), reg);
> 
> I don't see writes to this register in the Linux driver.  Instead it
> writes to RK3528_PHYREG6 (which would be register 5 in our driver).

Right, I forgot to update the register number.  It appears the linux driver
uses 0 based numbering for the 3528 registers, and the numbers are also
base 16, unlike the other chips:

#define RK3528_PHYREG6                          0x18
 ..
#define RK3528_PHYREG40				0x100

so I've changed that to COMBO_PIPE_PHY_REG(6), and adjusted the 
other register numbers.

Index: rkclock_clocks.h
===================================================================
RCS file: /cvs/src/sys/dev/fdt/rkclock_clocks.h,v
diff -u -p -r1.68 rkclock_clocks.h
--- rkclock_clocks.h	12 Mar 2026 20:44:38 -0000	1.68
+++ rkclock_clocks.h	23 Mar 2026 05:19:35 -0000
@@ -297,6 +297,8 @@
 #define RK3528_CLK_MATRIX_200M_SRC	10
 #define RK3528_CLK_PWM0			111
 #define RK3528_CLK_PWM1			114
+#define RK3528_CLK_PPLL_100M_MATRIX	121
+#define RK3528_CLK_REF_PCIE_INNER_PHY	123
 #define RK3528_CLK_PPLL_125M_MATRIX	127
 #define RK3528_CCLK_SRC_EMMC		140
 #define RK3528_BCLK_EMMC		143
Index: rkclock.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/rkclock.c,v
diff -u -p -r1.96 rkclock.c
--- rkclock.c	12 Mar 2026 20:44:38 -0000	1.96
+++ rkclock.c	23 Mar 2026 05:19:35 -0000
@@ -176,6 +176,7 @@
 #define RK3528_CRU_GATE_CON(i)		(0x00800 + (i) * 4)
 #define RK3528_CRU_SOFTRST_CON(i)	(0x00a00 + (i) * 4)
 #define RK3528_PCIE_CRU_PLL_CON(i)	(0x20000 + (i) * 4)
+#define RK3528_PCIE_CLKSEL_CON(i)	(0x20300 + (i) * 4)
 
 /* RK3568 registers */
 #define RK3568_CRU_APLL_CON(i)		(0x0000 + (i) * 4)
@@ -3219,6 +3220,16 @@ const struct rkclock rk3528_clocks[] = {
 		SEL(9, 8), 0,
 		{ RK3528_CLK_MATRIX_100M_SRC, RK3528_CLK_MATRIX_50M_SRC,
 		  RK3528_XIN24M }
+	},
+	{
+		RK3528_CLK_PPLL_100M_MATRIX, RK3528_PCIE_CLKSEL_CON(1),
+		0, DIV(7, 2),
+		{ RK3528_PLL_PPLL }
+	},
+	{
+		RK3528_CLK_REF_PCIE_INNER_PHY, RK3528_PCIE_CLKSEL_CON(1),
+		SEL(13, 13), 0,
+		{ RK3528_CLK_PPLL_100M_MATRIX, RK3528_XIN24M }
 	},
 	{
 		RK3528_CLK_PPLL_125M_MATRIX, RK3528_CRU_CLKSEL_CON(60),
Index: rkcomphy.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/rkcomphy.c,v
diff -u -p -r1.2 rkcomphy.c
--- rkcomphy.c	27 Apr 2023 08:56:39 -0000	1.2
+++ rkcomphy.c	23 Mar 2026 05:19:35 -0000
@@ -36,9 +36,13 @@
 
 /* Combo PHY registers */
 #define COMBO_PIPE_PHY_REG(idx)			((idx) * 4)
+/* REG_004 */
+#define  COMBO_PIPE_PHY_GATE_TX_PCK_DLY_PLL_OFF	(1 << 3)
 /* REG_005 */
 #define  COMBO_PIPE_PHY_PLL_DIV_MASK		(0x3 << 6)
 #define  COMBO_PIPE_PHY_PLL_DIV_2		(0x1 << 6)
+#define  COMBO_PIPE_PHY_PLL_KVCO_MASK_RK3528	(0x7 << 10)
+#define  COMBO_PIPE_PHY_PLL_KVCO_VALUE_RK3528	(0x2 << 10)
 /* REG_006 */
 #define  COMBO_PIPE_PHY_TX_RTERM_50OHM		(0x8 << 4)
 #define  COMBO_PIPE_PHY_RX_RTERM_44OHM		(0xf << 4)
@@ -124,6 +128,7 @@ struct cfdriver rkcomphy_cd = {
 	NULL, "rkcomphy", DV_DULL
 };
 
+int	rkcomphy_rk3528_enable(void *, uint32_t *);
 int	rkcomphy_rk3568_enable(void *, uint32_t *);
 int	rkcomphy_rk3588_enable(void *, uint32_t *);
 
@@ -133,7 +138,8 @@ rkcomphy_match(struct device *parent, vo
 	struct fdt_attach_args *faa = aux;
 	int node = faa->fa_node;
 
-	return OF_is_compatible(node, "rockchip,rk3568-naneng-combphy") ||
+	return OF_is_compatible(node, "rockchip,rk3528-naneng-combphy") ||
+	    OF_is_compatible(node, "rockchip,rk3568-naneng-combphy") ||
 	    OF_is_compatible(node, "rockchip,rk3588-naneng-combphy");
 }
 
@@ -161,11 +167,87 @@ rkcomphy_attach(struct device *parent, s
 
 	sc->sc_pd.pd_node = faa->fa_node;
 	sc->sc_pd.pd_cookie = sc;
-	if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-naneng-combphy"))
+	if (OF_is_compatible(faa->fa_node, "rockchip,rk3528-naneng-combphy"))
+	    sc->sc_pd.pd_enable = rkcomphy_rk3528_enable;
+	else if (OF_is_compatible(faa->fa_node,
+	    "rockchip,rk3568-naneng-combphy"))
 	    sc->sc_pd.pd_enable = rkcomphy_rk3568_enable;
 	else
 	    sc->sc_pd.pd_enable = rkcomphy_rk3588_enable;
 	phy_register(&sc->sc_pd);
+}
+
+void
+rkcomphy_rk3528_pll_tune(struct rkcomphy_softc *sc)
+{
+	uint32_t reg;
+
+	reg = HREAD4(sc, COMBO_PIPE_PHY_REG(5));
+	reg |= COMBO_PIPE_PHY_GATE_TX_PCK_DLY_PLL_OFF;
+	HWRITE4(sc, COMBO_PIPE_PHY_REG(5), reg);
+
+	reg = HREAD4(sc, COMBO_PIPE_PHY_REG(6));
+	reg &= ~COMBO_PIPE_PHY_PLL_KVCO_MASK_RK3528;
+	reg |= COMBO_PIPE_PHY_PLL_KVCO_VALUE_RK3528;
+	HWRITE4(sc, COMBO_PIPE_PHY_REG(6), reg);
+
+	/* su_trim[6:4]=111, [10:7]=1001, [2:0]=000, swing 650mv */
+	HWRITE4(sc, COMBO_PIPE_PHY_REG(0x42), 0x570804f0);
+}
+
+int
+rkcomphy_rk3528_enable(void *cookie, uint32_t *cells)
+{
+	struct rkcomphy_softc *sc = cookie;
+	struct regmap *rm, *phy_rm;
+	int node = sc->sc_pd.pd_node;
+	uint32_t type = cells[0];
+	uint32_t grf, phy_grf, reg;
+
+	/* We only support PCIe for now. (maybe USB3 later?) */
+	switch (type) {
+	case PHY_TYPE_PCIE:
+		break;
+	default:
+		return EINVAL;
+	}
+
+	grf = OF_getpropint(node, "rockchip,pipe-grf", 0);
+	rm = regmap_byphandle(grf);
+	if (rm == NULL)
+		return ENXIO;
+
+	phy_grf = OF_getpropint(node, "rockchip,pipe-phy-grf", 0);
+	phy_rm = regmap_byphandle(phy_grf);
+	if (phy_rm == NULL)
+		return ENXIO;
+
+	clock_set_assigned(node);
+	clock_enable_all(node);
+
+	reg = HREAD4(sc, COMBO_PIPE_PHY_REG(6));
+	reg &= ~COMBO_PIPE_PHY_SSC_OFFSET_MASK;
+	reg &= ~COMBO_PIPE_PHY_SSC_DIR_MASK;
+	reg |= COMBO_PIPE_PHY_SSC_DIR_DOWN;
+	HWRITE4(sc, COMBO_PIPE_PHY_REG(6), reg);
+
+	switch (type) {
+	case PHY_TYPE_PCIE:
+		regmap_write_4(phy_rm, PIPE_PHY_GRF_PIPE_CON(0), 0xffff0110);
+		regmap_write_4(phy_rm, PIPE_PHY_GRF_PIPE_CON(1), 0xffff0000);
+		regmap_write_4(phy_rm, PIPE_PHY_GRF_PIPE_CON(2), 0xffff0101);
+		regmap_write_4(phy_rm, PIPE_PHY_GRF_PIPE_CON(3), 0xffff0200);
+		break;
+	}
+
+	regmap_write_4(phy_rm, PIPE_PHY_GRF_PIPE_CON(1),
+	    PIPE_PHY_GRF_PIPE_CLK_100M);
+	if (type == PHY_TYPE_PCIE)
+		rkcomphy_rk3528_pll_tune(sc);
+
+	reset_deassert_all(node);
+	
+	return 0;
 }
 
 void