Download raw body.
rkcomphy: add rk3528 support
> Date: Mon, 23 Mar 2026 15:22:46 +1000
> From: Jonathan Matthew <jonathan@d14n.org>
Hey,
>
> 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 }
> + },
Hayk already mentioned the mistake in DIV().
> + {
> + 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);
Fun, so now the Linux driver has rgeister names that are off-by-one,
register names that are not of-by-one and register names that are
hexadecimal. All in one driver. It is a bit of a tossup I guess, but
I'd probably consistently use decimal. So COMBO_PIPE_PHY_REG(66).
Either way, ok kettenis@ (with the DIV thing fixed)
> +}
> +
> +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
>
rkcomphy: add rk3528 support