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: <ZNpEaMJjmDqhK1dW@shell.armlinux.org.uk>
Date: Mon, 14 Aug 2023 16:12:40 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Andrew Lunn <andrew@...n.ch>,
	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

On Mon, Aug 14, 2023 at 05:59:48PM +0300, Vladimir Oltean wrote:
> On Sat, Aug 12, 2023 at 01:16:00PM +0100, Russell King (Oracle) wrote:
> > So for realtek, I propose (completely untested):
> > 
> > 8<====
> > From: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
> > Subject: [PATCH net-next] net: dsa: realtek: add phylink_get_caps
> >  implementation
> > 
> > The user ports use RSGMII, but we don't have that, and DT doesn't
> > specify a phy interface mode, so phylib defaults to GMII. These support
> > 1G, 100M and 10M with flow control. It is unknown whether asymetric
> > pause is supported at all speeds.
> > 
> > The CPU port uses MII/GMII/RGMII/REVMII by hardware pin strapping,
> > and support speeds specific to each, with full duplex only supported
> > in some modes. Flow control may be supported again by hardware pin
> > strapping, and theoretically is readable through a register but no
> > information is given in the datasheet for that.
> > 
> > So, we do a best efforts - and be lenient.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---
> >  drivers/net/dsa/realtek/rtl8366rb.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
> > index 25f88022b9e4..76b5c43e1430 100644
> > --- a/drivers/net/dsa/realtek/rtl8366rb.c
> > +++ b/drivers/net/dsa/realtek/rtl8366rb.c
> > @@ -1049,6 +1049,32 @@ static enum dsa_tag_protocol rtl8366_get_tag_protocol(struct dsa_switch *ds,
> >  	return DSA_TAG_PROTO_RTL4_A;
> >  }
> >  
> > +static void rtl8366rb_phylink_get_caps(struct dsa_switch *ds, int port,
> > +				       struct phylink_config *config)
> > +{
> > +	unsigned long *interfaces = config->supported_interfaces;
> > +	struct realtek_priv *priv = ds->priv;
> > +
> > +	if (port == priv->cpu_port) {
> > +		__set_bit(PHY_INTERFACE_MODE_MII, interfaces);
> > +		__set_bit(PHY_INTERFACE_MODE_GMII, interfaces);
> > +		/* Only supports 100M FD */
> > +		__set_bit(PHY_INTERFACE_MODE_REVMII, interfaces);
> > +		/* Only supports 1G FD */
> > +		__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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!