Re: [PATCH 2/2] phy: rockchip-naneng-combo: Support rk3576

[Frank Wang <frawang.cn@gmail.com>]

Hi Heiko,

On 2024/10/15 14:15, Heiko Stübner wrote:
> Hi Frank,
>
> Am Dienstag, 15. Oktober 2024, 03:33:51 CEST schrieb Frank Wang:
>> From: Kever Yang <kever.yang@rock-chips.com>
>>
>> phy0: pcie, sata
>> phy1: pcie, sata, usb3
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> Signed-off-by: William Wu <william.wu@rock-chips.com>
>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>> ---
>>   .../rockchip/phy-rockchip-naneng-combphy.c    | 202 ++++++++++++++++++
>>   1 file changed, 202 insertions(+)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> index 0a9989e41237..4c41317a8041 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> @@ -584,6 +585,203 @@ static const struct rockchip_combphy_cfg rk3568_combphy_cfgs = {
>>   	.combphy_cfg	= rk3568_combphy_cfg,
>>   };
>>   
>> +static int rk3576_combphy_cfg(struct rockchip_combphy_priv *priv)
>> +{
>> +	const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
>> +	unsigned long rate;
>> +
>> +	switch (priv->type) {
>> +	case PHY_TYPE_PCIE:
>> +		/* Set SSC downward spread spectrum */
>> +		rockchip_combphy_updatel(priv, GENMASK(5, 4), BIT(4), 0x7c);
> Can we get constants for those magic values please?

Ah of cause, I will amend them and send a new patch.


Best regards,
Frank


> The combophys for rk3568 and rk3588 do use actual constants to at least
> somewhat describe what happens, so it would be really nice for the rk3576
> to do this as well.
>
> Same for the rockchip_combphy_updatel and other writel calls below.
>
>
>> +		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:
>> +		/* Set SSC downward spread spectrum */
>> +		rockchip_combphy_updatel(priv, GENMASK(5, 4), BIT(4), 0x7c);
>> +
>> +		/* Enable adaptive CTLE for USB3.0 Rx */
>> +		rockchip_combphy_updatel(priv, GENMASK(0, 0), BIT(0), 0x38);
>> +
>> +		/* Set PLL KVCO fine tuning signals */
>> +		rockchip_combphy_updatel(priv, GENMASK(4, 2), BIT(3), 0x80);
>> +
>> +		/* Set PLL LPF R1 to su_trim[10:7]=1001 */
>> +		writel(0x4, priv->mmio + (0xb << 2));
>> +
>> +		/* Set PLL input clock divider 1/2 */
>> +		rockchip_combphy_updatel(priv, GENMASK(7, 6), BIT(6), 0x14);
>> +
>> +		/* Set PLL loop divider */
>> +		writel(0x32, priv->mmio + (0x11 << 2));
>> +
>> +		/* Set PLL KVCO to min and set PLL charge pump current to max */
>> +		writel(0xf0, priv->mmio + (0xa << 2));
>> +
>> +		/* Set Rx squelch input filler bandwidth */
>> +		writel(0x0d, priv->mmio + (0x14 << 2));
>> +
>> +		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;
>> +	case PHY_TYPE_SATA:
>> +		/* Enable adaptive CTLE for SATA Rx */
>> +		rockchip_combphy_updatel(priv, GENMASK(0, 0), BIT(0), 0x38);
>> +
>> +		/* Set tx_rterm = 50 ohm and rx_rterm = 43.5 ohm */
>> +		writel(0x8F, priv->mmio + (0x06 << 2));
>> +
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_sata, true);
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_sata, true);
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_sata, true);
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_sata, true);
>> +		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_con0_for_sata, true);
>> +		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_con1_for_sata, 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 || 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);
>> +
>> +			/* Set up rx_trim */
>> +			writel(0x0, priv->mmio + (0x1b << 2));
>> +
>> +			/* Set up su_trim: T0_1 */
>> +			writel(0x90, priv->mmio + (0xa << 2));
>> +			writel(0x02, priv->mmio + (0xb << 2));
>> +			writel(0x57, priv->mmio + (0xd << 2));
>> +
>> +			writel(0x5f, priv->mmio + (0xf << 2));
> This does includes both the value as well as the register addresses,
> because a hex-value with a bit shift makes that even less readable.
>
> Thanks a lot
> Heiko
>
>
>> +		}
>> +		break;
>> +	case REF_CLOCK_25MHz:
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_25m, true);
>> +		break;
>> +	case REF_CLOCK_100MHz:
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_100m, true);
>> +		if (priv->type == PHY_TYPE_PCIE) {
>> +			/* gate_tx_pck_sel length select work for L1SS */
>> +			writel(0xc0, priv->mmio + 0x74);
>> +
>> +			/* PLL KVCO tuning fine */
>> +			rockchip_combphy_updatel(priv, GENMASK(4, 2), 0x4 << 2, 0x20 << 2);
>> +
>> +			/* Set up rx_trim: PLL LPF C1 85pf R1 1.25kohm */
>> +			writel(0x4c, priv->mmio + (0x1b << 2));
>> +
>> +			/* Set up su_trim: T3_P1 650mv */
>> +			writel(0x90, priv->mmio + (0xa << 2));
>> +			writel(0x43, priv->mmio + (0xb << 2));
>> +			writel(0x88, priv->mmio + (0xc << 2));
>> +			writel(0x56, priv->mmio + (0xd << 2));
>> +		} else if (priv->type == PHY_TYPE_SATA) {
>> +			/* downward spread spectrum +500ppm */
>> +			rockchip_combphy_updatel(priv, GENMASK(7, 4), 0x50, 0x1f << 2);
>> +
>> +			/* ssc ppm adjust to 3500ppm */
>> +			rockchip_combphy_updatel(priv, GENMASK(3, 0), 0x7, 0x9 << 2);
>> +		}
>> +		break;
>> +	default:
>> +		dev_err(priv->dev, "Unsupported rate: %lu\n", rate);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (priv->ext_refclk) {
>> +		rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_clk_ext, true);
>> +		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_100MHz) {
>> +			writel(0x10, priv->mmio + (0x20 << 2));
>> +
>> +			writel(0x0c, priv->mmio + (0x1b << 2));
>> +
>> +			/* Set up su_trim: T3_P1 650mv */
>> +			writel(0x90, priv->mmio + (0xa << 2));
>> +			writel(0x43, priv->mmio + (0xb << 2));
>> +			writel(0x88, 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);
>> +
>> +		if (priv->type == PHY_TYPE_PCIE && rate == REF_CLOCK_24MHz) {
>> +			/* 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));
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>
>