[PATCH 22/27] phy: rockchip-naneng-combphy: Add RK3572 support

[Manivannan Sadhasivam]

On 2026-10-22 16:13 +0000, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] phy: rockchip-naneng-combphy: writel() clobbers SSC bit in rk3572_combphy_cfg()
> --
> 
> commit 9ca36a68e2ebd41ee7e126faeae68aae4fa05eae
> Author: Cunhao Lu <1579567540 at qq.com>
> 
> phy: rockchip-naneng-combphy: Add RK3572 support
> 
> This commit adds configuration tables, register programming, and device tree
> matching support for the RK3572 combo PHY operating in PCIe, USB3, and SATA
> modes.
> 
> > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > index 7843356a4dd47..e13a78ab57779 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > @@ -1000,6 +1000,204 @@ static const struct rockchip_combphy_cfg rk3568_combphy_cfgs = {
> >  	.combphy_cfg	= rk3568_combphy_cfg,
> >  };
> >  
> > +static int rk3572_combphy_cfg(struct rockchip_combphy_priv *priv)
> > +{
> 
> [ ... ]
> 
> > +	switch (rate) {
> > +	case 24000000:
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_24m, true);
> > +		if (priv->type == PHY_TYPE_USB3 || priv->type == PHY_TYPE_SATA) {
> > +			/* Set ssc_cnt[9:0]=0101111101 & 31.5KHz */
> > +			rockchip_combphy_updatel(priv, GENMASK(7, 6), BIT(6), 0xe << 2);
> > +
> > +			rockchip_combphy_updatel(priv, GENMASK(7, 0), 0x5f, 0xf << 2);
> > +		} else if (priv->type == PHY_TYPE_PCIE) {
> > +			/* PLL KVCO tuning fine */
> > +			rockchip_combphy_updatel(priv, GENMASK(4, 2), 0x4 << 2, 0x20 << 2);
> 
> [ ... ]
> 
> > +	if (priv->ext_refclk) {
> > +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_ext, true);
> > +		if (priv->type == PHY_TYPE_PCIE && rate == 100000000) {
> > +			writel(0x10, priv->mmio + (0x20 << 2));
> > +
> > +			writel(0x0c, priv->mmio + (0x1b << 2));
> > +
> > +			/*
> > +			 * Set up su_trim: T3_P1 650mv
> > +			 * CKRCV termination resistor adjust 11: Hi-Z
> > +			 */
> > +			writel(0x90, priv->mmio + (0xa << 2));
> > +			writel(0x43, priv->mmio + (0xb << 2));
> > +			writel(0xb8, priv->mmio + (0xc << 2));
> > +			writel(0x56, priv->mmio + (0xd << 2));
> > +		}
> > +	}
> > +
> > +	if (priv->enable_ssc) {
> > +		rockchip_combphy_updatel(priv, GENMASK(4, 4), BIT(4), 0x7 << 2);
> > +		/* PLL control SSC module period should be set */
> > +		rockchip_combphy_updatel(priv, GENMASK(5, 5), BIT(5), 0x20 << 2);
> > +
> > +		if (priv->type == PHY_TYPE_PCIE && rate == 24000000) {
> > +			/* xin24m T0_1 650mV */
> > +			writel(0x00, priv->mmio + (0x10 << 2));
> > +			writel(0x32, priv->mmio + (0x11 << 2));
> > +			writel(0x00, priv->mmio + (0x1b << 2));
> > +			writel(0x90, priv->mmio + (0x0a << 2));
> > +			writel(0x02, priv->mmio + (0x0b << 2));
> > +			writel(0x08, priv->mmio + (0x0c << 2));
> > +			writel(0x57, priv->mmio + (0x0d << 2));
> > +			writel(0x40, priv->mmio + (0x0e << 2));
> > +			writel(0x5f, priv->mmio + (0x0f << 2));
> > +			writel(0x10, priv->mmio + (0x20 << 2));
> 
> [Severity: Medium]
> Does this writel(0x10, priv->mmio + (0x20 << 2)) in rk3572_combphy_cfg()
> clobber the SSC period control bit that was just set above?
> 
> When priv->enable_ssc is true, rk3572_combphy_cfg() sets BIT(5) in register
> offset 0x20 << 2 via rockchip_combphy_updatel(). Immediately afterwards, when
> priv->type == PHY_TYPE_PCIE and rate == 24000000, this writel() overwrites the
> entire register at 0x20 << 2 with 0x10, clearing BIT(5) back to 0.
> 
> Should both this write and the writel(0x10, priv->mmio + (0x20 << 2)) in the
> priv->ext_refclk block above use rockchip_combphy_updatel() with GENMASK(4, 2)
> instead, matching how the 24 MHz and 100 MHz PHY_TYPE_PCIE cases update the
> PLL KVCO tuning field earlier in rk3572_combphy_cfg()?
> 

Looks valid.

- Mani