netdev - Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy switch drivers

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: <055be6c4-3c28-459d-bb52-5ac2ee24f1f1@lunn.ch>
Date: Mon, 14 Aug 2023 17:46:21 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Vladimir Oltean <olteanv@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy
 switch drivers

> > > +		__set_bit(PHY_INTERFACE_MODE_RGMII, interfaces);
> > 
> > also, I guess that this should allow all 4 variants of RGMII.
> 
> I'm not sure - looking at what's available, the RTL8366 datasheet (not
> RB) says that there's pinstrapping for the RGMII delays. It also suggests
> that there may be a register that can be modified for this, but the driver
> doesn't appear to touch it - in fact, it does nothing with the interface
> mode. Moreover, the only in-kernel DT for this has:
> 
>                         rtl8366rb_cpu_port: port@5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                                 ethernet = <&gmac0>;
>                                 phy-mode = "rgmii";
>                                 fixed-link {
>                                         speed = <1000>;
>                                         full-duplex;
>                                         pause;
>                                 };
>                         };
> 
> Whether that can be changed in the RB version of the device or not, I
> don't know, so whether it makes sense to allow the other RGMII modes,
> again, I don't know.
> 
> Annoyingly, gmac0 doesn't exist in this file, it's defined in
> gemini.dtsi, which this file references through a heirarchy of nodes
> (makes it very much less readable), but it points at:
> 
> / {
> ...
>         soc {
> ...
>                 ethernet@...00000 {
> ...
>                         ethernet-port@0 {
>                                 phy-mode = "rgmii";
>                                 fixed-link {
>                                         speed = <1000>;
>                                         full-duplex;
>                                         pause;
>                                 };
>                         };
> 
> So that also uses "rgmii".
> 
> I'm tempted not to allow the others as the driver doesn't make any
> adjustments, and we only apparently have the one user.

RGMII on both ends is unlikely to work, so probably one is
wrong. Probably the switch has strapping to set it to rgmii-id, but we
don't actually know that. And i guess we have no ability to find out
the truth?

So a narrow definition seems reasonable at the moment, to raise a red
warning flag if somebody does try to use rgmii-id which is not
actually implemented in the driver. And that user then gets to sort
out the problem.

	 Andrew