1. Patchew
  2. linux
  3. [PATCH v4 0/6] Support RK3528 variant of Rockchip naneng-combphy
  4. View patch

[PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support

Yao Zi posted 6 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
Posted by Yao Zi 3 months, 2 weeks ago 
Rockchip RK3528 integrates one naneng-combphy that is able to operate in
PCIe and USB3 mode. The control logic is similar to previous variants of
naneng-combphy but the register layout is apperantly different from the
RK3568 one.

Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../rockchip/phy-rockchip-naneng-combphy.c    | 186 +++++++++++++++++-
 1 file changed, 185 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 1d1c7723584b..bf00a85a113b 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -20,7 +20,46 @@
 #define REF_CLOCK_25MHz			(25 * HZ_PER_MHZ)
 #define REF_CLOCK_100MHz		(100 * HZ_PER_MHZ)
 
-/* COMBO PHY REG */
+/* RK3528 COMBO PHY REG */
+#define RK3528_PHYREG6				0x18
+#define RK3528_PHYREG6_PLL_KVCO			GENMASK(12, 10)
+#define RK3528_PHYREG6_PLL_KVCO_VALUE		0x2
+#define RK3528_PHYREG6_SSC_DIR			GENMASK(5, 4)
+#define RK3528_PHYREG6_SSC_UPWARD		0
+#define RK3528_PHYREG6_SSC_DOWNWARD		1
+
+#define RK3528_PHYREG40				0x100
+#define RK3528_PHYREG40_SSC_EN			BIT(20)
+#define RK3528_PHYREG40_SSC_CNT			GENMASK(10, 0)
+#define RK3528_PHYREG40_SSC_CNT_VALUE		0x17d
+
+#define RK3528_PHYREG42				0x108
+#define RK3528_PHYREG42_CKDRV_CLK_SEL		BIT(29)
+#define RK3528_PHYREG42_CKDRV_CLK_PLL		0
+#define RK3528_PHYREG42_CKDRV_CLK_CKRCV		1
+#define RK3528_PHYREG42_PLL_LPF_R1_ADJ		GENMASK(10, 7)
+#define RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE	0x9
+#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	GENMASK(6, 4)
+#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE 0x7
+#define RK3528_PHYREG42_PLL_KVCO_ADJ		GENMASK(2, 0)
+#define RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE	0x0
+
+#define RK3528_PHYREG80				0x200
+#define RK3528_PHYREG80_CTLE_EN			BIT(17)
+
+#define RK3528_PHYREG81				0x204
+#define RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X	BIT(5)
+#define RK3528_PHYREG81_SLEW_RATE_CTRL		GENMASK(2, 0)
+#define RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW	0x7
+
+#define RK3528_PHYREG83				0x20c
+#define RK3528_PHYREG83_RX_SQUELCH		GENMASK(2, 0)
+#define RK3528_PHYREG83_RX_SQUELCH_VALUE	0x6
+
+#define RK3528_PHYREG86				0x218
+#define RK3528_PHYREG86_RTERM_DET_CLK_EN	BIT(14)
+
+/* RK3568 COMBO PHY REG */
 #define RK3568_PHYREG6				0x14
 #define RK3568_PHYREG6_PLL_DIV_MASK		GENMASK(7, 6)
 #define RK3568_PHYREG6_PLL_DIV_SHIFT		6
@@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(phy_provider);
 }
 
+static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
+{
+	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
+	unsigned long rate;
+	u32 val;
+
+	/* Set SSC downward spread spectrum */
+	val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
+	rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
+
+	switch (priv->type) {
+	case PHY_TYPE_PCIE:
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
+		break;
+	case PHY_TYPE_USB3:
+		/* Enable adaptive CTLE for USB3.0 Rx */
+		rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
+					 RK3528_PHYREG80);
+
+		/* Set slow slew rate control for PI */
+		val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
+				 RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
+		rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
+					 RK3528_PHYREG81);
+
+		/* Set CDR phase path with 2x gain */
+		rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
+					 RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
+
+		/* Set Rx squelch input filler bandwidth */
+		val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
+		rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
+
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);
+		break;
+	default:
+		dev_err(priv->dev, "incompatible PHY type\n");
+		return -EINVAL;
+	}
+
+	rate = clk_get_rate(priv->refclk);
+
+	switch (rate) {
+	case REF_CLOCK_24MHz:
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_24m, true);
+		if (priv->type == PHY_TYPE_USB3) {
+			/* Set ssc_cnt[10:0]=00101111101 & 31.5KHz */
+			val = FIELD_PREP(RK3528_PHYREG40_SSC_CNT, RK3528_PHYREG40_SSC_CNT_VALUE);
+			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_CNT, val,
+						 RK3528_PHYREG40);
+		} else if (priv->type == PHY_TYPE_PCIE) {
+			/* tx_trim[14]=1, Enable the counting clock of the rterm detect */
+			rockchip_combphy_updatel(priv, RK3528_PHYREG86_RTERM_DET_CLK_EN,
+						 RK3528_PHYREG86_RTERM_DET_CLK_EN, RK3528_PHYREG86);
+		}
+		break;
+	case REF_CLOCK_100MHz:
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_100m, true);
+		if (priv->type == PHY_TYPE_PCIE) {
+			/* PLL KVCO tuning fine */
+			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
+			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
+						 RK3528_PHYREG6);
+
+			/* su_trim[6:4]=111, [10:7]=1001, [2:0]=000, swing 650mv */
+			writel(0x570804f0, priv->mmio + RK3528_PHYREG42);
+		}
+		break;
+	default:
+		dev_err(priv->dev, "Unsupported rate: %lu\n", rate);
+		return -EINVAL;
+	}
+
+	if (device_property_read_bool(priv->dev, "rockchip,ext-refclk")) {
+		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_ext, true);
+
+		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_100MHz) {
+			val = FIELD_PREP(RK3528_PHYREG42_CKDRV_CLK_SEL,
+					 RK3528_PHYREG42_CKDRV_CLK_CKRCV);
+			val |= FIELD_PREP(RK3528_PHYREG42_PLL_LPF_R1_ADJ,
+					  RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE);
+			val |= FIELD_PREP(RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ,
+					  RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE);
+			val |= FIELD_PREP(RK3528_PHYREG42_PLL_KVCO_ADJ,
+					  RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE);
+			rockchip_combphy_updatel(priv,
+						 RK3528_PHYREG42_CKDRV_CLK_SEL		|
+						 RK3528_PHYREG42_PLL_LPF_R1_ADJ		|
+						 RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	|
+						 RK3528_PHYREG42_PLL_KVCO_ADJ,
+						 val, RK3528_PHYREG42);
+
+			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
+			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
+						 RK3528_PHYREG6);
+		}
+	}
+
+	if (priv->type == PHY_TYPE_PCIE) {
+		if (device_property_read_bool(priv->dev, "rockchip,enable-ssc"))
+			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_EN,
+						 RK3528_PHYREG40_SSC_EN, RK3528_PHYREG40);
+	}
+
+	return 0;
+}
+
+static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
+	/* pipe-phy-grf */
+	.pcie_mode_set		= { 0x0000, 5, 0, 0x00, 0x11 },
+	.usb_mode_set		= { 0x0000, 5, 0, 0x00, 0x04 },
+	.pipe_rxterm_set	= { 0x0000, 12, 12, 0x00, 0x01 },
+	.pipe_txelec_set	= { 0x0004, 1, 1, 0x00, 0x01 },
+	.pipe_txcomp_set	= { 0x0004, 4, 4, 0x00, 0x01 },
+	.pipe_clk_24m		= { 0x0004, 14, 13, 0x00, 0x00 },
+	.pipe_clk_100m		= { 0x0004, 14, 13, 0x00, 0x02 },
+	.pipe_rxterm_sel	= { 0x0008, 8, 8, 0x00, 0x01 },
+	.pipe_txelec_sel	= { 0x0008, 12, 12, 0x00, 0x01 },
+	.pipe_txcomp_sel	= { 0x0008, 15, 15, 0x00, 0x01 },
+	.pipe_clk_ext		= { 0x000c, 9, 8, 0x02, 0x01 },
+	.pipe_phy_status	= { 0x0034, 6, 6, 0x01, 0x00 },
+	.con0_for_pcie		= { 0x0000, 15, 0, 0x00, 0x110 },
+	.con1_for_pcie		= { 0x0004, 15, 0, 0x00, 0x00 },
+	.con2_for_pcie		= { 0x0008, 15, 0, 0x00, 0x101 },
+	.con3_for_pcie		= { 0x000c, 15, 0, 0x00, 0x0200 },
+};
+
+static const struct rockchip_combphy_cfg rk3528_combphy_cfgs = {
+	.num_phys	= 1,
+	.phy_ids	= {
+		0xffdc0000,
+	},
+	.grfcfg		= &rk3528_combphy_grfcfgs,
+	.combphy_cfg	= rk3528_combphy_cfg,
+};
+
 static int rk3562_combphy_cfg(struct rockchip_combphy_priv *priv)
 {
 	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
@@ -1213,6 +1393,10 @@ static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
 };
 
 static const struct of_device_id rockchip_combphy_of_match[] = {
+	{
+		.compatible = "rockchip,rk3528-naneng-combphy",
+		.data = &rk3528_combphy_cfgs,
+	},
 	{
 		.compatible = "rockchip,rk3562-naneng-combphy",
 		.data = &rk3562_combphy_cfgs,
-- 
2.49.0
Re: [PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
Posted by Jonas Karlman 2 months, 2 weeks ago 
Hi Yao Zi,

On 6/24/2025 5:37 AM, Yao Zi wrote:
> Rockchip RK3528 integrates one naneng-combphy that is able to operate in
> PCIe and USB3 mode. The control logic is similar to previous variants of
> naneng-combphy but the register layout is apperantly different from the
> RK3568 one.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../rockchip/phy-rockchip-naneng-combphy.c    | 186 +++++++++++++++++-
>  1 file changed, 185 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 1d1c7723584b..bf00a85a113b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -20,7 +20,46 @@
>  #define REF_CLOCK_25MHz			(25 * HZ_PER_MHZ)
>  #define REF_CLOCK_100MHz		(100 * HZ_PER_MHZ)
>  
> -/* COMBO PHY REG */
> +/* RK3528 COMBO PHY REG */
> +#define RK3528_PHYREG6				0x18
> +#define RK3528_PHYREG6_PLL_KVCO			GENMASK(12, 10)
> +#define RK3528_PHYREG6_PLL_KVCO_VALUE		0x2
> +#define RK3528_PHYREG6_SSC_DIR			GENMASK(5, 4)
> +#define RK3528_PHYREG6_SSC_UPWARD		0
> +#define RK3528_PHYREG6_SSC_DOWNWARD		1
> +
> +#define RK3528_PHYREG40				0x100
> +#define RK3528_PHYREG40_SSC_EN			BIT(20)
> +#define RK3528_PHYREG40_SSC_CNT			GENMASK(10, 0)
> +#define RK3528_PHYREG40_SSC_CNT_VALUE		0x17d
> +
> +#define RK3528_PHYREG42				0x108
> +#define RK3528_PHYREG42_CKDRV_CLK_SEL		BIT(29)
> +#define RK3528_PHYREG42_CKDRV_CLK_PLL		0
> +#define RK3528_PHYREG42_CKDRV_CLK_CKRCV		1
> +#define RK3528_PHYREG42_PLL_LPF_R1_ADJ		GENMASK(10, 7)
> +#define RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE	0x9
> +#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	GENMASK(6, 4)
> +#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE 0x7
> +#define RK3528_PHYREG42_PLL_KVCO_ADJ		GENMASK(2, 0)
> +#define RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE	0x0
> +
> +#define RK3528_PHYREG80				0x200
> +#define RK3528_PHYREG80_CTLE_EN			BIT(17)
> +
> +#define RK3528_PHYREG81				0x204
> +#define RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X	BIT(5)
> +#define RK3528_PHYREG81_SLEW_RATE_CTRL		GENMASK(2, 0)
> +#define RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW	0x7
> +
> +#define RK3528_PHYREG83				0x20c
> +#define RK3528_PHYREG83_RX_SQUELCH		GENMASK(2, 0)
> +#define RK3528_PHYREG83_RX_SQUELCH_VALUE	0x6
> +
> +#define RK3528_PHYREG86				0x218
> +#define RK3528_PHYREG86_RTERM_DET_CLK_EN	BIT(14)
> +
> +/* RK3568 COMBO PHY REG */
>  #define RK3568_PHYREG6				0x14
>  #define RK3568_PHYREG6_PLL_DIV_MASK		GENMASK(7, 6)
>  #define RK3568_PHYREG6_PLL_DIV_SHIFT		6
> @@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  }
>  
> +static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
> +{
> +	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> +	unsigned long rate;
> +	u32 val;
> +
> +	/* Set SSC downward spread spectrum */
> +	val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
> +	rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
> +
> +	switch (priv->type) {
> +	case PHY_TYPE_PCIE:
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> +		break;
> +	case PHY_TYPE_USB3:
> +		/* Enable adaptive CTLE for USB3.0 Rx */
> +		rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
> +					 RK3528_PHYREG80);
> +
> +		/* Set slow slew rate control for PI */
> +		val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
> +				 RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
> +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
> +					 RK3528_PHYREG81);
> +
> +		/* Set CDR phase path with 2x gain */
> +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
> +					 RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
> +
> +		/* Set Rx squelch input filler bandwidth */
> +		val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
> +		rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
> +
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);

I suggest we add something like following here:

		rockchip_combphy_param_write(priv->pipe_grf, &cfg->u3otg0_port_en, true);

to ensure that U3 is enabled in case boot firmware disable the U3 port.

> +		break;
> +	default:
> +		dev_err(priv->dev, "incompatible PHY type\n");
> +		return -EINVAL;
> +	}
> +
> +	rate = clk_get_rate(priv->refclk);
> +
> +	switch (rate) {
> +	case REF_CLOCK_24MHz:
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_24m, true);
> +		if (priv->type == PHY_TYPE_USB3) {
> +			/* Set ssc_cnt[10:0]=00101111101 & 31.5KHz */
> +			val = FIELD_PREP(RK3528_PHYREG40_SSC_CNT, RK3528_PHYREG40_SSC_CNT_VALUE);
> +			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_CNT, val,
> +						 RK3528_PHYREG40);
> +		} else if (priv->type == PHY_TYPE_PCIE) {
> +			/* tx_trim[14]=1, Enable the counting clock of the rterm detect */
> +			rockchip_combphy_updatel(priv, RK3528_PHYREG86_RTERM_DET_CLK_EN,
> +						 RK3528_PHYREG86_RTERM_DET_CLK_EN, RK3528_PHYREG86);
> +		}
> +		break;
> +	case REF_CLOCK_100MHz:
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_100m, true);
> +		if (priv->type == PHY_TYPE_PCIE) {
> +			/* PLL KVCO tuning fine */
> +			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
> +			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
> +						 RK3528_PHYREG6);
> +
> +			/* su_trim[6:4]=111, [10:7]=1001, [2:0]=000, swing 650mv */
> +			writel(0x570804f0, priv->mmio + RK3528_PHYREG42);
> +		}
> +		break;
> +	default:
> +		dev_err(priv->dev, "Unsupported rate: %lu\n", rate);
> +		return -EINVAL;
> +	}
> +
> +	if (device_property_read_bool(priv->dev, "rockchip,ext-refclk")) {
> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_ext, true);
> +
> +		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_100MHz) {
> +			val = FIELD_PREP(RK3528_PHYREG42_CKDRV_CLK_SEL,
> +					 RK3528_PHYREG42_CKDRV_CLK_CKRCV);
> +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_LPF_R1_ADJ,
> +					  RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE);
> +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ,
> +					  RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE);
> +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_KVCO_ADJ,
> +					  RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE);
> +			rockchip_combphy_updatel(priv,
> +						 RK3528_PHYREG42_CKDRV_CLK_SEL		|
> +						 RK3528_PHYREG42_PLL_LPF_R1_ADJ		|
> +						 RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	|
> +						 RK3528_PHYREG42_PLL_KVCO_ADJ,
> +						 val, RK3528_PHYREG42);
> +
> +			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
> +			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
> +						 RK3528_PHYREG6);
> +		}
> +	}
> +
> +	if (priv->type == PHY_TYPE_PCIE) {
> +		if (device_property_read_bool(priv->dev, "rockchip,enable-ssc"))
> +			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_EN,
> +						 RK3528_PHYREG40_SSC_EN, RK3528_PHYREG40);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
> +	/* pipe-phy-grf */
> +	.pcie_mode_set		= { 0x0000, 5, 0, 0x00, 0x11 },
> +	.usb_mode_set		= { 0x0000, 5, 0, 0x00, 0x04 },
> +	.pipe_rxterm_set	= { 0x0000, 12, 12, 0x00, 0x01 },
> +	.pipe_txelec_set	= { 0x0004, 1, 1, 0x00, 0x01 },
> +	.pipe_txcomp_set	= { 0x0004, 4, 4, 0x00, 0x01 },
> +	.pipe_clk_24m		= { 0x0004, 14, 13, 0x00, 0x00 },
> +	.pipe_clk_100m		= { 0x0004, 14, 13, 0x00, 0x02 },
> +	.pipe_rxterm_sel	= { 0x0008, 8, 8, 0x00, 0x01 },
> +	.pipe_txelec_sel	= { 0x0008, 12, 12, 0x00, 0x01 },
> +	.pipe_txcomp_sel	= { 0x0008, 15, 15, 0x00, 0x01 },
> +	.pipe_clk_ext		= { 0x000c, 9, 8, 0x02, 0x01 },
> +	.pipe_phy_status	= { 0x0034, 6, 6, 0x01, 0x00 },
> +	.con0_for_pcie		= { 0x0000, 15, 0, 0x00, 0x110 },
> +	.con1_for_pcie		= { 0x0004, 15, 0, 0x00, 0x00 },
> +	.con2_for_pcie		= { 0x0008, 15, 0, 0x00, 0x101 },
> +	.con3_for_pcie		= { 0x000c, 15, 0, 0x00, 0x0200 },

And adding something like this:

	/* pipe-grf */
	.u3otg0_port_en		= { 0x0044, 15, 0, 0x0181, 0x1100 },

Should be possible with ("phy: rockchip: naneng-combphy: Enable U3 OTG
port for RK3568") [1].

Most RK3528 boards I have come across this far seem to use PCIe instead
of USB3, so having boot firmware disable U3 early (to help support USB
gadget in boot firmware) and instead having this PHY driver re-enable U3
when needed seem most logical to me.

I will push an updated U-Boot rk3528 branch [2] where I include such
early U3 port disable once source.denx.de is back online again.

[1] https://lore.kernel.org/r/20250723072324.2246498-1-jonas@kwiboo.se
[2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528

Regards,
Jonas

> +};
> +
> +static const struct rockchip_combphy_cfg rk3528_combphy_cfgs = {
> +	.num_phys	= 1,
> +	.phy_ids	= {
> +		0xffdc0000,
> +	},
> +	.grfcfg		= &rk3528_combphy_grfcfgs,
> +	.combphy_cfg	= rk3528_combphy_cfg,
> +};
> +
>  static int rk3562_combphy_cfg(struct rockchip_combphy_priv *priv)
>  {
>  	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> @@ -1213,6 +1393,10 @@ static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
>  };
>  
>  static const struct of_device_id rockchip_combphy_of_match[] = {
> +	{
> +		.compatible = "rockchip,rk3528-naneng-combphy",
> +		.data = &rk3528_combphy_cfgs,
> +	},
>  	{
>  		.compatible = "rockchip,rk3562-naneng-combphy",
>  		.data = &rk3562_combphy_cfgs,
Re: [PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
Posted by Yao Zi 2 months, 2 weeks ago 
On Wed, Jul 23, 2025 at 04:51:15PM +0200, Jonas Karlman wrote:
> Hi Yao Zi,
> 
> On 6/24/2025 5:37 AM, Yao Zi wrote:
> > Rockchip RK3528 integrates one naneng-combphy that is able to operate in
> > PCIe and USB3 mode. The control logic is similar to previous variants of
> > naneng-combphy but the register layout is apperantly different from the
> > RK3568 one.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> > ---
> >  .../rockchip/phy-rockchip-naneng-combphy.c    | 186 +++++++++++++++++-
> >  1 file changed, 185 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > index 1d1c7723584b..bf00a85a113b 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > @@ -20,7 +20,46 @@
> >  #define REF_CLOCK_25MHz			(25 * HZ_PER_MHZ)
> >  #define REF_CLOCK_100MHz		(100 * HZ_PER_MHZ)
> >  
> > -/* COMBO PHY REG */
> > +/* RK3528 COMBO PHY REG */
> > +#define RK3528_PHYREG6				0x18
> > +#define RK3528_PHYREG6_PLL_KVCO			GENMASK(12, 10)
> > +#define RK3528_PHYREG6_PLL_KVCO_VALUE		0x2
> > +#define RK3528_PHYREG6_SSC_DIR			GENMASK(5, 4)
> > +#define RK3528_PHYREG6_SSC_UPWARD		0
> > +#define RK3528_PHYREG6_SSC_DOWNWARD		1
> > +
> > +#define RK3528_PHYREG40				0x100
> > +#define RK3528_PHYREG40_SSC_EN			BIT(20)
> > +#define RK3528_PHYREG40_SSC_CNT			GENMASK(10, 0)
> > +#define RK3528_PHYREG40_SSC_CNT_VALUE		0x17d
> > +
> > +#define RK3528_PHYREG42				0x108
> > +#define RK3528_PHYREG42_CKDRV_CLK_SEL		BIT(29)
> > +#define RK3528_PHYREG42_CKDRV_CLK_PLL		0
> > +#define RK3528_PHYREG42_CKDRV_CLK_CKRCV		1
> > +#define RK3528_PHYREG42_PLL_LPF_R1_ADJ		GENMASK(10, 7)
> > +#define RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE	0x9
> > +#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	GENMASK(6, 4)
> > +#define RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE 0x7
> > +#define RK3528_PHYREG42_PLL_KVCO_ADJ		GENMASK(2, 0)
> > +#define RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE	0x0
> > +
> > +#define RK3528_PHYREG80				0x200
> > +#define RK3528_PHYREG80_CTLE_EN			BIT(17)
> > +
> > +#define RK3528_PHYREG81				0x204
> > +#define RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X	BIT(5)
> > +#define RK3528_PHYREG81_SLEW_RATE_CTRL		GENMASK(2, 0)
> > +#define RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW	0x7
> > +
> > +#define RK3528_PHYREG83				0x20c
> > +#define RK3528_PHYREG83_RX_SQUELCH		GENMASK(2, 0)
> > +#define RK3528_PHYREG83_RX_SQUELCH_VALUE	0x6
> > +
> > +#define RK3528_PHYREG86				0x218
> > +#define RK3528_PHYREG86_RTERM_DET_CLK_EN	BIT(14)
> > +
> > +/* RK3568 COMBO PHY REG */
> >  #define RK3568_PHYREG6				0x14
> >  #define RK3568_PHYREG6_PLL_DIV_MASK		GENMASK(7, 6)
> >  #define RK3568_PHYREG6_PLL_DIV_SHIFT		6
> > @@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
> >  	return PTR_ERR_OR_ZERO(phy_provider);
> >  }
> >  
> > +static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
> > +{
> > +	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> > +	unsigned long rate;
> > +	u32 val;
> > +
> > +	/* Set SSC downward spread spectrum */
> > +	val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
> > +	rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
> > +
> > +	switch (priv->type) {
> > +	case PHY_TYPE_PCIE:
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> > +		break;
> > +	case PHY_TYPE_USB3:
> > +		/* Enable adaptive CTLE for USB3.0 Rx */
> > +		rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
> > +					 RK3528_PHYREG80);
> > +
> > +		/* Set slow slew rate control for PI */
> > +		val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
> > +				 RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
> > +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
> > +					 RK3528_PHYREG81);
> > +
> > +		/* Set CDR phase path with 2x gain */
> > +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
> > +					 RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
> > +
> > +		/* Set Rx squelch input filler bandwidth */
> > +		val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
> > +		rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
> > +
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);
> 
> I suggest we add something like following here:
> 
> 		rockchip_combphy_param_write(priv->pipe_grf, &cfg->u3otg0_port_en, true);
> 
> to ensure that U3 is enabled in case boot firmware disable the U3 port.

Thanks for the hint, I'm willing to adapt it. Should we handle the case
that USB is enabled by firmware but PCIe is going to be used in kernel,
too? It's desirable to make fewer assumptions about the state set by
firmware.

P.S., I'm assuming the register should be written as "disabled" value
whenever PCIe is used, and "enabled" whenever USB is used, as
the LSB of USB_GRF_USB3OTG0_CON1 is said to be "USB 3.0 SS Port Disable
control" according to RK3588's TRM, which doesn't look like something
compatible with PCIe mode when setting to 1'b0 (port enabled). Please
correct me if I'm wrong.

Regards,
Yao Zi

> > +		break;
> > +	default:
> > +		dev_err(priv->dev, "incompatible PHY type\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	rate = clk_get_rate(priv->refclk);
> > +
> > +	switch (rate) {
> > +	case REF_CLOCK_24MHz:
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_24m, true);
> > +		if (priv->type == PHY_TYPE_USB3) {
> > +			/* Set ssc_cnt[10:0]=00101111101 & 31.5KHz */
> > +			val = FIELD_PREP(RK3528_PHYREG40_SSC_CNT, RK3528_PHYREG40_SSC_CNT_VALUE);
> > +			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_CNT, val,
> > +						 RK3528_PHYREG40);
> > +		} else if (priv->type == PHY_TYPE_PCIE) {
> > +			/* tx_trim[14]=1, Enable the counting clock of the rterm detect */
> > +			rockchip_combphy_updatel(priv, RK3528_PHYREG86_RTERM_DET_CLK_EN,
> > +						 RK3528_PHYREG86_RTERM_DET_CLK_EN, RK3528_PHYREG86);
> > +		}
> > +		break;
> > +	case REF_CLOCK_100MHz:
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_100m, true);
> > +		if (priv->type == PHY_TYPE_PCIE) {
> > +			/* PLL KVCO tuning fine */
> > +			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
> > +			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
> > +						 RK3528_PHYREG6);
> > +
> > +			/* su_trim[6:4]=111, [10:7]=1001, [2:0]=000, swing 650mv */
> > +			writel(0x570804f0, priv->mmio + RK3528_PHYREG42);
> > +		}
> > +		break;
> > +	default:
> > +		dev_err(priv->dev, "Unsupported rate: %lu\n", rate);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (device_property_read_bool(priv->dev, "rockchip,ext-refclk")) {
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_ext, true);
> > +
> > +		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_100MHz) {
> > +			val = FIELD_PREP(RK3528_PHYREG42_CKDRV_CLK_SEL,
> > +					 RK3528_PHYREG42_CKDRV_CLK_CKRCV);
> > +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_LPF_R1_ADJ,
> > +					  RK3528_PHYREG42_PLL_LPF_R1_ADJ_VALUE);
> > +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ,
> > +					  RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ_VALUE);
> > +			val |= FIELD_PREP(RK3528_PHYREG42_PLL_KVCO_ADJ,
> > +					  RK3528_PHYREG42_PLL_KVCO_ADJ_VALUE);
> > +			rockchip_combphy_updatel(priv,
> > +						 RK3528_PHYREG42_CKDRV_CLK_SEL		|
> > +						 RK3528_PHYREG42_PLL_LPF_R1_ADJ		|
> > +						 RK3528_PHYREG42_PLL_CHGPUMP_CUR_ADJ	|
> > +						 RK3528_PHYREG42_PLL_KVCO_ADJ,
> > +						 val, RK3528_PHYREG42);
> > +
> > +			val = FIELD_PREP(RK3528_PHYREG6_PLL_KVCO, RK3528_PHYREG6_PLL_KVCO_VALUE);
> > +			rockchip_combphy_updatel(priv, RK3528_PHYREG6_PLL_KVCO, val,
> > +						 RK3528_PHYREG6);
> > +		}
> > +	}
> > +
> > +	if (priv->type == PHY_TYPE_PCIE) {
> > +		if (device_property_read_bool(priv->dev, "rockchip,enable-ssc"))
> > +			rockchip_combphy_updatel(priv, RK3528_PHYREG40_SSC_EN,
> > +						 RK3528_PHYREG40_SSC_EN, RK3528_PHYREG40);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
> > +	/* pipe-phy-grf */
> > +	.pcie_mode_set		= { 0x0000, 5, 0, 0x00, 0x11 },
> > +	.usb_mode_set		= { 0x0000, 5, 0, 0x00, 0x04 },
> > +	.pipe_rxterm_set	= { 0x0000, 12, 12, 0x00, 0x01 },
> > +	.pipe_txelec_set	= { 0x0004, 1, 1, 0x00, 0x01 },
> > +	.pipe_txcomp_set	= { 0x0004, 4, 4, 0x00, 0x01 },
> > +	.pipe_clk_24m		= { 0x0004, 14, 13, 0x00, 0x00 },
> > +	.pipe_clk_100m		= { 0x0004, 14, 13, 0x00, 0x02 },
> > +	.pipe_rxterm_sel	= { 0x0008, 8, 8, 0x00, 0x01 },
> > +	.pipe_txelec_sel	= { 0x0008, 12, 12, 0x00, 0x01 },
> > +	.pipe_txcomp_sel	= { 0x0008, 15, 15, 0x00, 0x01 },
> > +	.pipe_clk_ext		= { 0x000c, 9, 8, 0x02, 0x01 },
> > +	.pipe_phy_status	= { 0x0034, 6, 6, 0x01, 0x00 },
> > +	.con0_for_pcie		= { 0x0000, 15, 0, 0x00, 0x110 },
> > +	.con1_for_pcie		= { 0x0004, 15, 0, 0x00, 0x00 },
> > +	.con2_for_pcie		= { 0x0008, 15, 0, 0x00, 0x101 },
> > +	.con3_for_pcie		= { 0x000c, 15, 0, 0x00, 0x0200 },
> 
> And adding something like this:
> 
> 	/* pipe-grf */
> 	.u3otg0_port_en		= { 0x0044, 15, 0, 0x0181, 0x1100 },
> 
> Should be possible with ("phy: rockchip: naneng-combphy: Enable U3 OTG
> port for RK3568") [1].
> 
> Most RK3528 boards I have come across this far seem to use PCIe instead
> of USB3, so having boot firmware disable U3 early (to help support USB
> gadget in boot firmware) and instead having this PHY driver re-enable U3
> when needed seem most logical to me.
> 
> I will push an updated U-Boot rk3528 branch [2] where I include such
> early U3 port disable once source.denx.de is back online again.
> 
> [1] https://lore.kernel.org/r/20250723072324.2246498-1-jonas@kwiboo.se
> [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528
> 
> Regards,
> Jonas
> 
> > +};
> > +
> > +static const struct rockchip_combphy_cfg rk3528_combphy_cfgs = {
> > +	.num_phys	= 1,
> > +	.phy_ids	= {
> > +		0xffdc0000,
> > +	},
> > +	.grfcfg		= &rk3528_combphy_grfcfgs,
> > +	.combphy_cfg	= rk3528_combphy_cfg,
> > +};
> > +
> >  static int rk3562_combphy_cfg(struct rockchip_combphy_priv *priv)
> >  {
> >  	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> > @@ -1213,6 +1393,10 @@ static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
> >  };
> >  
> >  static const struct of_device_id rockchip_combphy_of_match[] = {
> > +	{
> > +		.compatible = "rockchip,rk3528-naneng-combphy",
> > +		.data = &rk3528_combphy_cfgs,
> > +	},
> >  	{
> >  		.compatible = "rockchip,rk3562-naneng-combphy",
> >  		.data = &rk3562_combphy_cfgs,
>
Re: [PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
Posted by Yao Zi 2 months, 1 week ago 
On Thu, Jul 24, 2025 at 05:23:55AM +0000, Yao Zi wrote:
> On Wed, Jul 23, 2025 at 04:51:15PM +0200, Jonas Karlman wrote:
> > Hi Yao Zi,
> > 
> > On 6/24/2025 5:37 AM, Yao Zi wrote:
> > > Rockchip RK3528 integrates one naneng-combphy that is able to operate in
> > > PCIe and USB3 mode. The control logic is similar to previous variants of
> > > naneng-combphy but the register layout is apperantly different from the
> > > RK3568 one.
> > > 
> > > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > > Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > ---
> > >  .../rockchip/phy-rockchip-naneng-combphy.c    | 186 +++++++++++++++++-
> > >  1 file changed, 185 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index 1d1c7723584b..bf00a85a113b 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c

...

> > > @@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
> > >  	return PTR_ERR_OR_ZERO(phy_provider);
> > >  }
> > >  
> > > +static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
> > > +{
> > > +	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> > > +	unsigned long rate;
> > > +	u32 val;
> > > +
> > > +	/* Set SSC downward spread spectrum */
> > > +	val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
> > > +	rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
> > > +
> > > +	switch (priv->type) {
> > > +	case PHY_TYPE_PCIE:
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> > > +		break;
> > > +	case PHY_TYPE_USB3:
> > > +		/* Enable adaptive CTLE for USB3.0 Rx */
> > > +		rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
> > > +					 RK3528_PHYREG80);
> > > +
> > > +		/* Set slow slew rate control for PI */
> > > +		val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
> > > +				 RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
> > > +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
> > > +					 RK3528_PHYREG81);
> > > +
> > > +		/* Set CDR phase path with 2x gain */
> > > +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
> > > +					 RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
> > > +
> > > +		/* Set Rx squelch input filler bandwidth */
> > > +		val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
> > > +		rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
> > > +
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
> > > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);
> > 
> > I suggest we add something like following here:
> > 
> > 		rockchip_combphy_param_write(priv->pipe_grf, &cfg->u3otg0_port_en, true);
> > 
> > to ensure that U3 is enabled in case boot firmware disable the U3 port.
> 
> Thanks for the hint, I'm willing to adapt it. Should we handle the case
> that USB is enabled by firmware but PCIe is going to be used in kernel,
> too? It's desirable to make fewer assumptions about the state set by
> firmware.
> 
> P.S., I'm assuming the register should be written as "disabled" value
> whenever PCIe is used, and "enabled" whenever USB is used, as
> the LSB of USB_GRF_USB3OTG0_CON1 is said to be "USB 3.0 SS Port Disable
> control" according to RK3588's TRM, which doesn't look like something
> compatible with PCIe mode when setting to 1'b0 (port enabled). Please
> correct me if I'm wrong.

I've read through the manual and done some tests today, and it seems I
misunderstood the purpose of USB3OTG0_CON1. This register has only
something to do with USB3, but not PCIe. Writing either "disabled" or
"enabled" value to it doesn't affect PCIe functionality. Thus for the
naneng-combphy driver, it should be enough to only write the "enabled"
value to u3otg0_port_en if USB-3 mode is used.

Anyway, thanks for your remind on this register :) Its reset value
allows USB-3 to function thus I just forgot about it during clean-up.
I'll send v5 soon.

...

> > > +static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
> > > +	/* pipe-phy-grf */
> > > +	.pcie_mode_set		= { 0x0000, 5, 0, 0x00, 0x11 },
> > > +	.usb_mode_set		= { 0x0000, 5, 0, 0x00, 0x04 },
> > > +	.pipe_rxterm_set	= { 0x0000, 12, 12, 0x00, 0x01 },
> > > +	.pipe_txelec_set	= { 0x0004, 1, 1, 0x00, 0x01 },
> > > +	.pipe_txcomp_set	= { 0x0004, 4, 4, 0x00, 0x01 },
> > > +	.pipe_clk_24m		= { 0x0004, 14, 13, 0x00, 0x00 },
> > > +	.pipe_clk_100m		= { 0x0004, 14, 13, 0x00, 0x02 },
> > > +	.pipe_rxterm_sel	= { 0x0008, 8, 8, 0x00, 0x01 },
> > > +	.pipe_txelec_sel	= { 0x0008, 12, 12, 0x00, 0x01 },
> > > +	.pipe_txcomp_sel	= { 0x0008, 15, 15, 0x00, 0x01 },
> > > +	.pipe_clk_ext		= { 0x000c, 9, 8, 0x02, 0x01 },
> > > +	.pipe_phy_status	= { 0x0034, 6, 6, 0x01, 0x00 },
> > > +	.con0_for_pcie		= { 0x0000, 15, 0, 0x00, 0x110 },
> > > +	.con1_for_pcie		= { 0x0004, 15, 0, 0x00, 0x00 },
> > > +	.con2_for_pcie		= { 0x0008, 15, 0, 0x00, 0x101 },
> > > +	.con3_for_pcie		= { 0x000c, 15, 0, 0x00, 0x0200 },
> > 
> > And adding something like this:
> > 
> > 	/* pipe-grf */
> > 	.u3otg0_port_en		= { 0x0044, 15, 0, 0x0181, 0x1100 },
> > 
> > Should be possible with ("phy: rockchip: naneng-combphy: Enable U3 OTG
> > port for RK3568") [1].
> > 
> > Most RK3528 boards I have come across this far seem to use PCIe instead
> > of USB3, so having boot firmware disable U3 early (to help support USB
> > gadget in boot firmware) and instead having this PHY driver re-enable U3
> > when needed seem most logical to me.

Thank you for the U-Boot patch! While reading through, I saw commit
"rockchip: rk3528: Disable USB3OTG U3 port early" states,

> Some board designs may not use the COMBPHY for USB3 purpose. For these
> board to use USB OTG the input clock source must change to use UTMI
> clk instead of PIPE clk.

Does this mean we should ideally add similar handling for USB3OTG in the
kernel's usb2phy driver, too? Otherwise if the firmware doesn't handle
clock stuff well, the kernel'll fail to operate in OTG mode, either.

> > 
> > I will push an updated U-Boot rk3528 branch [2] where I include such
> > early U3 port disable once source.denx.de is back online again.
> > 
> > [1] https://lore.kernel.org/r/20250723072324.2246498-1-jonas@kwiboo.se
> > [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528
> > 
> > Regards,
> > Jonas

Thanks,
Yao Zi
Re: [PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
Posted by Jonas Karlman 2 months, 1 week ago 
Hi Yao Zi,

On 7/28/2025 7:07 AM, Yao Zi wrote:
> On Thu, Jul 24, 2025 at 05:23:55AM +0000, Yao Zi wrote:
>> On Wed, Jul 23, 2025 at 04:51:15PM +0200, Jonas Karlman wrote:
>>> Hi Yao Zi,
>>>
>>> On 6/24/2025 5:37 AM, Yao Zi wrote:
>>>> Rockchip RK3528 integrates one naneng-combphy that is able to operate in
>>>> PCIe and USB3 mode. The control logic is similar to previous variants of
>>>> naneng-combphy but the register layout is apperantly different from the
>>>> RK3568 one.
>>>>
>>>> Signed-off-by: Yao Zi <ziyao@disroot.org>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>  .../rockchip/phy-rockchip-naneng-combphy.c    | 186 +++++++++++++++++-
>>>>  1 file changed, 185 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>>>> index 1d1c7723584b..bf00a85a113b 100644
>>>> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>>>> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> 
> ...
> 
>>>> @@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>>>>  	return PTR_ERR_OR_ZERO(phy_provider);
>>>>  }
>>>>  
>>>> +static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
>>>> +{
>>>> +	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
>>>> +	unsigned long rate;
>>>> +	u32 val;
>>>> +
>>>> +	/* Set SSC downward spread spectrum */
>>>> +	val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
>>>> +	rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
>>>> +
>>>> +	switch (priv->type) {
>>>> +	case PHY_TYPE_PCIE:
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
>>>> +		break;
>>>> +	case PHY_TYPE_USB3:
>>>> +		/* Enable adaptive CTLE for USB3.0 Rx */
>>>> +		rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
>>>> +					 RK3528_PHYREG80);
>>>> +
>>>> +		/* Set slow slew rate control for PI */
>>>> +		val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
>>>> +				 RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
>>>> +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
>>>> +					 RK3528_PHYREG81);
>>>> +
>>>> +		/* Set CDR phase path with 2x gain */
>>>> +		rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
>>>> +					 RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
>>>> +
>>>> +		/* Set Rx squelch input filler bandwidth */
>>>> +		val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
>>>> +		rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
>>>> +
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
>>>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);
>>>
>>> I suggest we add something like following here:
>>>
>>> 		rockchip_combphy_param_write(priv->pipe_grf, &cfg->u3otg0_port_en, true);
>>>
>>> to ensure that U3 is enabled in case boot firmware disable the U3 port.
>>
>> Thanks for the hint, I'm willing to adapt it. Should we handle the case
>> that USB is enabled by firmware but PCIe is going to be used in kernel,
>> too? It's desirable to make fewer assumptions about the state set by
>> firmware.
>>
>> P.S., I'm assuming the register should be written as "disabled" value
>> whenever PCIe is used, and "enabled" whenever USB is used, as
>> the LSB of USB_GRF_USB3OTG0_CON1 is said to be "USB 3.0 SS Port Disable
>> control" according to RK3588's TRM, which doesn't look like something
>> compatible with PCIe mode when setting to 1'b0 (port enabled). Please
>> correct me if I'm wrong.
> 
> I've read through the manual and done some tests today, and it seems I
> misunderstood the purpose of USB3OTG0_CON1. This register has only
> something to do with USB3, but not PCIe. Writing either "disabled" or
> "enabled" value to it doesn't affect PCIe functionality. Thus for the
> naneng-combphy driver, it should be enough to only write the "enabled"
> value to u3otg0_port_en if USB-3 mode is used.
> 
> Anyway, thanks for your remind on this register :) Its reset value
> allows USB-3 to function thus I just forgot about it during clean-up.
> I'll send v5 soon.

Sorry for not getting back to you sooner, I fully agree with your
assessment here, the USB3OTG0_CON1 reg only affects USB and has nothing
to do with PCIe. Trying to adjust it when probing PCIe drivers could
open up for strange bahavior in case USB3 controller driver has already
been loaded or similar.

> 
> ...
> 
>>>> +static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
>>>> +	/* pipe-phy-grf */
>>>> +	.pcie_mode_set		= { 0x0000, 5, 0, 0x00, 0x11 },
>>>> +	.usb_mode_set		= { 0x0000, 5, 0, 0x00, 0x04 },
>>>> +	.pipe_rxterm_set	= { 0x0000, 12, 12, 0x00, 0x01 },
>>>> +	.pipe_txelec_set	= { 0x0004, 1, 1, 0x00, 0x01 },
>>>> +	.pipe_txcomp_set	= { 0x0004, 4, 4, 0x00, 0x01 },
>>>> +	.pipe_clk_24m		= { 0x0004, 14, 13, 0x00, 0x00 },
>>>> +	.pipe_clk_100m		= { 0x0004, 14, 13, 0x00, 0x02 },
>>>> +	.pipe_rxterm_sel	= { 0x0008, 8, 8, 0x00, 0x01 },
>>>> +	.pipe_txelec_sel	= { 0x0008, 12, 12, 0x00, 0x01 },
>>>> +	.pipe_txcomp_sel	= { 0x0008, 15, 15, 0x00, 0x01 },
>>>> +	.pipe_clk_ext		= { 0x000c, 9, 8, 0x02, 0x01 },
>>>> +	.pipe_phy_status	= { 0x0034, 6, 6, 0x01, 0x00 },
>>>> +	.con0_for_pcie		= { 0x0000, 15, 0, 0x00, 0x110 },
>>>> +	.con1_for_pcie		= { 0x0004, 15, 0, 0x00, 0x00 },
>>>> +	.con2_for_pcie		= { 0x0008, 15, 0, 0x00, 0x101 },
>>>> +	.con3_for_pcie		= { 0x000c, 15, 0, 0x00, 0x0200 },
>>>
>>> And adding something like this:
>>>
>>> 	/* pipe-grf */
>>> 	.u3otg0_port_en		= { 0x0044, 15, 0, 0x0181, 0x1100 },
>>>
>>> Should be possible with ("phy: rockchip: naneng-combphy: Enable U3 OTG
>>> port for RK3568") [1].
>>>
>>> Most RK3528 boards I have come across this far seem to use PCIe instead
>>> of USB3, so having boot firmware disable U3 early (to help support USB
>>> gadget in boot firmware) and instead having this PHY driver re-enable U3
>>> when needed seem most logical to me.
> 
> Thank you for the U-Boot patch! While reading through, I saw commit
> "rockchip: rk3528: Disable USB3OTG U3 port early" states,

I will try to send out a small RK3528 U-Boot series including that patch
later today, with v6.16 just released I am hoping U-Boot shortly merges
the v6.16 DTs for its next release, v2025.10, to reduce the size of such
U-Boot series.

> 
>> Some board designs may not use the COMBPHY for USB3 purpose. For these
>> board to use USB OTG the input clock source must change to use UTMI
>> clk instead of PIPE clk.
> 
> Does this mean we should ideally add similar handling for USB3OTG in the
> kernel's usb2phy driver, too? Otherwise if the firmware doesn't handle
> clock stuff well, the kernel'll fail to operate in OTG mode, either.

Checking for use of a usb3-phy from the usb2phy driver feel like the
wrong place to do that, if anything possible from the DWC3 driver. But
that also feel a bit strange to me, and could possibly be complex.

What we know for sure is that when the combo phy, or usbdp phy for e.g.
rk3576 and rk3588, is used for usb3, we should be sure that we can
"enable" U3 in USB3OTGx_CON1 regs without affecting anything.

Based on my testing it only seem to be the utmi/pipe clk source bit that
has any effect on USB2 only function. So could possibly be something for
the clock tree to handle as an alternative approach.

To be able to use usb2 in U-Boot, and with the usbdp phy driver already
having support in both Linux and U-Boot to enable U3 port, doing an
early disable in U-Boot and later re-enable when we know for sure is
easy and make some sense to me ;-)

Regards,
Jonas

> 
>>>
>>> I will push an updated U-Boot rk3528 branch [2] where I include such
>>> early U3 port disable once source.denx.de is back online again.
>>>
>>> [1] https://lore.kernel.org/r/20250723072324.2246498-1-jonas@kwiboo.se
>>> [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528
>>>
>>> Regards,
>>> Jonas
> 
> Thanks,
> Yao Zi

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.