Message-ID: <a29b1106-87a6-4ea2-bb1d-9858f9ab425b@lunn.ch>
Date:   Tue, 28 Nov 2023 01:20:11 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Christian Marangi <ansuelsmth@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Robert Marko <robert.marko@...tura.hr>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [net-next PATCH RFC v3 0/8] net: phy: Support DT PHY package

On Sun, Nov 26, 2023 at 02:53:38AM +0100, Christian Marangi wrote:
> This depends on another series for PHY package API change. [1]
> 
> Idea of this big series is to introduce the concept of PHY package in DT
> and generalize the support of it by PHY driver.
> 
> The concept of PHY package is nothing new and is already a thing in the
> kernel with the API phy_package_join/leave/read/write.
> 
> The main idea of those API is to permit the PHY to have a shared global
> data and to run probe/config only once for the PHY package. There are
> various example of this already in the kernel with the mscc, bcm54140
> mediatek ge and micrle driver and they all follow the same pattern.
> 
> What is currently lacking is describing this in DT and better reference
> the PHY in charge of global configuration of the PHY package. For the
> already present PHY, the implementation is simple enough with only one
> PHY having the required regs to apply global configuration.
> 
> This can be ok for simple PHY package but some Qcom PHY package on
> ""modern"" SoC have more complex implementation. One example is the PHY
> for qca807x where some global regs are present in the so-called "combo"
> port and everything about psgmii calibration is placed in a 5th port in
> the PHY package.
> 
> Given these additional thing, the original phy_package API are extended
> with support for multiple global PHY for configuration. Each PHY driver
> will have an enum of the ID for the global PHY to reference and is
> required to pass to the read/write function.

Please update the text. As far as i see, a lot of this is not relevant
for this patch set. phy_package_join() etc has no relation to DT,
since the driver knows how many devices are in its package, it knows
its base address, etc.

The DT is only about properties which are shared by all PHYs within
the package, e.g reset, regulators, maybe the MODE_CFG register for
this particular PHY package.

> 
> On top of this, it's added correct DT support for describing PHY
> package.
> 
> One example is this:
> 
>         ethernet-phy-package@0 {
>             compatible = "ethernet-phy-package";

This needs a compatible for this particular PHY package.

>             #address-cells = <1>;
>             #size-cells = <0>;
> 
>             reg = <0>;
>             qcom,package-mode = "qsgmii";

This property it not useful. Why PCA does it apply to, when there are
two? It makes much more sense to describe the overall configuration
mode, from which you can derive what interface mode each port should
be using, and thus validate the phy-mode in DT.

   Andrew

Powered by blists - more mailing lists