netdev - Re: binding for scanning a MDIO bus

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: <20191122174229.GG6602@lunn.ch>
Date:   Fri, 22 Nov 2019 18:42:29 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Alexandru Marginean <alexandru.marginean@....com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        network dev <netdev@...r.kernel.org>,
        Grygorii Strashko <grygorii.strashko@...com>
Subject: Re: binding for scanning a MDIO bus

> > Hi Alexandru
> > 
> > You often see the bus registered using mdiobus_register(). That then
> > means a scan is performed and all phys on the bus found. The MAC
> > driver then uses phy_find_first() to find the first PHY on the bus.
> > The danger here is that the hardware design changes, somebody adds a
> > second PHY, and it all stops working in interesting and confusing
> > ways.
> > 
> > Would this work for you?
> > 
> >        Andrew
> > 
> 
> How does the MAC get a reference to the mdio bus though, is there some 
> way to describe this relationship in the DT?  I did say that Eth and 
> mdio are associated and they are, but not in the way Eth would just know 
> without looking in the DT what mdio that is.

What i described is generally used for PCIe card, USB dongles,
etc. The MAC driver is the one registering the MDIO bus, so it has
what it needs. Such hardware is also pretty much guaranteed to only
have one PHY on the bus, so phy_find_first() is less dangerous.

> Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux 
> itself is accessed over I2C and its parent-mdio is a stand-alone device 
> that is not associated with a specific Ethernet device.  And on top of 
> that, based on serdes configuration, some Eth interfaces may end up on a 
> different slot and for that I want to apply a DT overlay to set the 
> proper Eth/mdio association.
> 
> Current code allows me to do something like this, as seen by Linux on boot:
> / {
> ....
> 	mdio-mux {
> 		/* slot 1 */
> 		mdio@4 {
> 			slot1_phy0: phy {
> 				/* 'reg' missing on purpose */
> 			};
> 		};
> 	};
> ....
> };
> 
> &enetc_port0 {
> 	phy-handle = <&slot1_phy0>;
> 	phy-mode = "sgmii";
> };
> 
> But the binding does not allow this, 'reg' is a required property of 
> phys.  Is this kind of DT structure acceptable even if it's not 
> compliant to the binding?  Assuming it's fine, any thoughts on making 
> this official in the binding?  If it's not, are there alternative 
> options for such a set-up?

In principle, this is O.K. The code seems to support it, even if the
binding does not give it as an option. It get messy when you have
multiple PHYs on the bus though. And if you are using DT, you are
supposed to know what the hardware is. Since you don't seems to know
what your hardware is, you are going to spam your kernel logs with

                        /* be noisy to encourage people to set reg property */
                        dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
                                 child, addr);

which i agree with.

      Andrew