Hi
thanks for review.
> Gesendet: Freitag, 15. Juli 2022 um 13:46 Uhr
> Von: "Vinod Koul" <vkoul@xxxxxxxxxx>
> > + * Copyright (C) 2020 Rockchip Electronics Co., Ltd.
>
> 2022 now :)
ok, i change it
> > + /* Deassert PCIe PMA output clamp mode */
> > + regmap_write(priv->phy_grf, GRF_PCIE30PHY_CON9, BIT(15) | BIT(31));
>
> Can we define these bits..?
i have no naming found for it as these are not described in the TRM
maybe Peter or Liang/Shawn has these?
> > +
> > + for (int i = 0; i < priv->num_lanes; i++) {
> > + dev_info(&phy->dev, "lane number %d, val %d\n", i, priv->lanes[i]);
> > + if (priv->lanes[i] > 1)
> > + bifurcation = true;
> > + }
> > +
> > + /* Set bifurcation if needed, and it doesn't care RC/EP */
> > + if (bifurcation) {
> > + dev_info(&phy->dev, "bifurcation enabled\n");
> > + regmap_write(priv->phy_grf, GRF_PCIE30PHY_CON6,
> > + (0xf << 16) | RK3568_BIFURCATION_LANE_0_1);
>
> upper word 0xf?
afaik yes (write-enable), is there any other more readable way?
> > + regmap_write(priv->phy_grf, GRF_PCIE30PHY_CON1,
> > + BIT(15) | BIT(31));
>
> again define bits please
these are not documented too
> > + } else {
> > + dev_info(&phy->dev, "bifurcation disabled\n");
>
> debug level?
i made them same as the "bifurcation enabled" to have always an info about it in dmesg.
> > + if (ret)
> > + pr_err("%s: lock failed 0x%x, check input refclk and power supply\n",
> > + __func__, reg);
>
> Can this be made dev_err too, I still see bunch of pr_ and at least here
> you have driver context... while at it drop the __func__ from these logs
> too please
ok, i'll change it
> > diff --git a/include/linux/phy/pcie.h b/include/linux/phy/pcie.h
> > new file mode 100644
> > index 000000000000..93c997f520fe
> > --- /dev/null
> > +++ b/include/linux/phy/pcie.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
>
> Header is 2021 !
i'll change
> --
> ~Vinod
regards Frank
