Hi Shawn,
Am Dienstag, 15. Oktober 2024, 10:39:28 CEST schrieb Shawn Lin:
> 在 2024/10/15 14:15, Heiko Stübner 写道:
> > Am Dienstag, 15. Oktober 2024, 03:33:51 CEST schrieb Frank Wang:
> >> From: Kever Yang <kever.yang@xxxxxxxxxxxxxx>
> >>
>
> ...
>
> >> + 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.
> >
>
> Actually, it's more readable when we need to debug, IMO. Because the
> PHY document provided is listing the registers just like what the patch
> did(0xb/0xd/0xf). So for example, we could easily find 0xb in the
> document, which refers to su_trim[15:8].
Documentation-wise the rk3576 has gotten worse, compared to the
rk3588 and before. I guess you're refering to a document that is not
part of the trm-part1 pdf I guess.
So I guess using register numbers somewhat matching the sparse
document you have can be fine, but I guess try to use constants where
possible.
The block above though at least needs a comment about what is
happening there.
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;
> >> +}
> >
> >
> >
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
