Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: rkcomphy: add rk3528 support
To:
Jonathan Matthew <jonathan@d14n.org>
Cc:
tech@openbsd.org, kettenis@openbsd.org
Date:
Sun, 22 Mar 2026 19:17:48 +0100

Download raw body.

Thread
> 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?

Also,

> Index: rkclock.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/rkclock.c,v
> diff -u -p -u -p -r1.96 rkclock.c
> --- rkclock.c	12 Mar 2026 20:44:38 -0000	1.96
> +++ rkclock.c	22 Mar 2026 10:33:52 -0000
> @@ -3291,6 +3291,8 @@ rk3528_get_frequency(void *cookie, uint3
>  		return rk3328_get_pll(sc, RK3528_PCIE_CRU_PLL_CON(32));
>  	case RK3528_XIN24M:
>  		return 24000000;
> +	case RK3528_CLK_REF_PCIE_INNER_PHY:
> +		return 100000000;
>  	default:
>  		break;
>  	}
> @@ -3303,6 +3305,13 @@ rk3528_set_frequency(void *cookie, uint3
>  {
>  	struct rkclock_softc *sc = cookie;
>  	uint32_t idx = cells[0];
> +
> +	switch (idx) {
> +	case RK3528_CLK_REF_PCIE_INNER_PHY:
> +		return 0;
> +	default:
> +		break;
> +	}
>  
>  	return rkclock_set_frequency(sc, idx, freq);
>  }
> Index: rkclock_clocks.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/rkclock_clocks.h,v
> diff -u -p -u -p -r1.68 rkclock_clocks.h
> --- rkclock_clocks.h	12 Mar 2026 20:44:38 -0000	1.68
> +++ rkclock_clocks.h	22 Mar 2026 10:33:52 -0000
> @@ -297,6 +297,7 @@
>  #define RK3528_CLK_MATRIX_200M_SRC	10
>  #define RK3528_CLK_PWM0			111
>  #define RK3528_CLK_PWM1			114
> +#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: rkcomphy.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/rkcomphy.c,v
> diff -u -p -u -p -r1.2 rkcomphy.c
> --- rkcomphy.c	27 Apr 2023 08:56:39 -0000	1.2
> +++ rkcomphy.c	22 Mar 2026 10:33:52 -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,86 @@ 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(4));
> +	reg |= COMBO_PIPE_PHY_GATE_TX_PCK_DLY_PLL_OFF;
> +	HWRITE4(sc, COMBO_PIPE_PHY_REG(4), reg);
> +
> +	reg = HREAD4(sc, COMBO_PIPE_PHY_REG(5));
> +	reg &= ~COMBO_PIPE_PHY_PLL_KVCO_MASK_RK3528;
> +	reg |= COMBO_PIPE_PHY_PLL_KVCO_VALUE_RK3528;
> +	HWRITE4(sc, COMBO_PIPE_PHY_REG(5), reg);
> +
> +	/* su_trim[6:4]=111, [10:7]=1001, [2:0]=000, swing 650mv */
> +	HWRITE4(sc, COMBO_PIPE_PHY_REG(41), 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(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).

> +
> +	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
> 
>