Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Message-ID: <trinity-7c08bee0-07dc-4e45-8157-50aa7fa7c3cc-1657972653195@3c-app-gmx-bap26>
From:   Frank Wunderlich <frank-w@public-files.de>
To:     Vinod Koul <vkoul@kernel.org>,
        Shawn Lin <shawn.lin@rock-chips.com>,
        Liang Chen <cl@rock-chips.com>,
        Peter Geis <pgwipeout@gmail.com>
Cc:     Frank Wunderlich <linux@fw-web.de>,
        linux-rockchip@lists.infradead.org,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Heiko Stuebner <heiko@sntech.de>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Johan Jonker <jbx6244@gmail.com>,
        Yifeng Zhao <yifeng.zhao@rock-chips.com>,
        Michael Riesch <michael.riesch@wolfvision.net>,
        Simon Xue <xxm@rock-chips.com>, linux-phy@lists.infradead.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, Shawn Lin <shawn.lin@rock-chips.com>,
        Liang Chen <cl@rock-chips.com>
Subject: Aw: Re: [PATCH v4 3/5] phy: rockchip: Support PCIe v3
Content-Type: text/plain; charset=UTF-8
Date:   Sat, 16 Jul 2022 13:57:33 +0200
Importance: normal
Sensitivity: Normal
In-Reply-To: <YtFTrEzefxIYswnj@matsya>
References: <20220619082605.7935-1-linux@fw-web.de>
 <20220619082605.7935-4-linux@fw-web.de> <YtFTrEzefxIYswnj@matsya>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 =3D 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 =3D 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