lkml.org 
[lkml]   [2022]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectAw: Re: [PATCH v4 3/5] phy: rockchip: Support PCIe v3
    Date
    Hi

    thanks for review.

    > Gesendet: Freitag, 15. Juli 2022 um 13:46 Uhr
    > Von: "Vinod Koul" <vkoul@kernel.org>
    > > + * 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

    \
     
     \ /
      Last update: 2022-07-16 13:58    [W:2.260 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site