netdev - Re: [PATCH net-next v2 4/8] net: phylink: update supported

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211124125044.10c08485@thinkpad>
Date:   Wed, 24 Nov 2021 12:50:44 +0100
From:   Marek Behún <kabel@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        Russell King <rmk+kernel@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        Sean Anderson <sean.anderson@...o.com>, davem@...emloft.net
Subject: Re: [PATCH net-next v2 4/8] net: phylink: update
 supported_interfaces with modes from fwnode

On Wed, 24 Nov 2021 00:54:18 +0200
Vladimir Oltean <olteanv@...il.com> wrote:

> > As I said above, the best example is with SerDeses which use multiple
> > lanes where not all may be wired. But this backward compatibility code
> > concerns only one-lane SerDeses: usxgmii, 10gbaser-r, 5gbase-r,
> > 2500base-x, 1000base-x, sgmii.  
> 
> Right, why so? From phy-mode = "xaui", you could also infer support for
> single-lane SERDES protocols up to 3.125 GHz, aren't you interested in
> doing that too?

OK, this would seem to make sense if I dropped the DT binding change
for now, and just updated the code to change the supported_interfaces
member so that marvell10g driver could select appropriate mode.

I will think about it. Thanks.

> > 2) the mode is not supported by the board because the generic PHY
> >    driver uses SMC calls to the firmware to change the SerDes mode, but
> >    the board may have old version of firmware which does not support
> >    SMC call correctly (or at all). This was a real issue with TF-A
> >    firmware on Macchiatobin, where the 5gbase-r mode was not supported
> >    in older versions  
> 
> Ok, so in your proposal, U-Boot would have to fix up the device tree
> based on a certain version of ATF, and remove "5gbase-r" from the
> phy-mode array. Whereas in my proposal, the mvpp2 driver would need to
> find out about the ATF version in use, and remove "5gbase-r" from the
> supported interfaces.
> 
> As a user, I'd certainly prefer if Linux could figure this one out.

You are right here, it would really be better for the mvpp2 driver to
do this discovering.

> This implies that when you bring up a board and write the device tree
> for it, you know that PHY mode X works without ever testing it. What if
> it doesn't work when you finally add support for it? Now you already
> have one DT blob in circulation. That's why I'm saying that maybe it
> could be better if we could think in terms that are a bit more physical
> and easy to characterize.

The thing is that this same could happen with your proposal of
max-data-rate + number-of-lanes, as Russell explained in his reply.

> > The whole idea of this code is to guarantee backward compatibility with
> > older device-trees. In my opinion (and I think Russell agrees with me),
> > this should be at one place, instead of putting various weird
> > exceptions into various MAC drivers.  
> 
> Yes, but they're more flexible in the driver... What if the check is not
> as simple as a machine compatible (think about the ATF firmware example
> you gave).

You persuaded me, it makes more sense in the driver.

Marek