Message-ID: <20241119115136.74297db7@fedora.home>
Date: Tue, 19 Nov 2024 11:51:36 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: netdev@...r.kernel.org, pabeni@...hat.com, kuba@...nel.org,
 edumazet@...gle.com, davem@...emloft.net, Andrew Lunn <andrew@...n.ch>,
 Russell King <linux@...linux.org.uk>, Romain Gantois
 <romain.gantois@...tlin.com>, Florian Fainelli <f.fainelli@...il.com>,
 Vladimir Oltean <olteanv@...il.com>, Heiner Kallweit
 <hkallweit1@...il.com>, Oleksij Rempel <o.rempel@...gutronix.de>, Kory
 Maincent <kory.maincent@...tlin.com>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Moving forward with stacked PHYs

Hi Netdev,

With Romain, we're currently trying to work on the stacked PHY problem,
where we'd like to get a proper support for Copper SFPs that are driven
by a media converter :

     RGMII       SGMII  +sfp----+
MAC ------- PHY ------- | PHY   |
                        +-------+

This is one of the cases where we have multiple PHYs on the link, on my
side I've been working on PHY muxes with parallel PHYs on the link :


       +-- PHY
MAC ---|
       +-- PHY
       
Both of these use-cases share one common issue, which is that some parts
of the kernel will try to directly access netdev->phydev, assuming there's
one and only PHY device that sits behind a MAC.

In the past months, I've worked on introducing the phy_link_topology, that
keeps track of all the PHYs that are sitting behind a netdev, and I've
used that infrastructure for some netlink commands that would access
the netdev->phydev field and replace that with :
  - A way to list the PHYs behind a netdev
  - Allowing netlink commands to target a specific PHY, and not "just"
    the netdev->phydev PHY.
    
On that front, there's more work coming to improve on that :
  - It would be good if the stats reported through ethtool would account
    for all the PHYs on the link, so that we would get a full overview
    of all stats from all components of the link
  - A netlink notification to indicate that a new PHY was found on the
    link is also in the plans.
    
There's a lot more work to do however, as these user-facing commands aren't
the only place where netdev->phydev is used.

The first place are the MAC drivers. For this, there seems to be 2 options :

 - move to phylink. The phylink API is pretty well designed as it makes
   the phydev totally irrelevant for the MAC driver. The MAC driver doesn't
   even need to know if it's connected to a PHY or something else (and SFP
   cage for example). That's nice, but there are still plenty of drivers
   that don't use phylink.
   
 - Introduce a new set of helpers that would abstact away the phydev from
   the netdev, but in a more straightforward way. MAC drivers that directly
   access netdev->phydev to configure the PHY's internal state or get some
   PHY info would instead be converted to using a set of helpers that will
   take the netdev as a parameter :
   
 static void ftgmac100_adjust_link(struct net_device *netdev)
 {
+	int phy_link, phy_speed, phy_duplex;
 	struct ftgmac100 *priv = netdev_priv(netdev);
 	struct phy_device *phydev = netdev->phydev;
 	bool tx_pause, rx_pause;
 	int new_speed;
 
+	netdev_phy_link_settings(netdev, &phy_link, &phy_speed, &phy_duplex);
+
 	/* We store "no link" as speed 0 */
-	if (!phydev->link)
+	if (!phy_link)
 		new_speed = 0;
 	else
-		new_speed = phydev->speed;
+		new_speed = phy_speed;
[...]

   The above is just an example of such helpers, Romain is currently
   investigating this and going over all the MAC drivers out there to
   assess to what extent they are directly accessing the netdev->phydev,
   and wrapping that with phydev-independent helpers.

   The idea here is to avoid accessing phydev directly from the MAC
   driver, and have a helper query the parameters for us. This helper
   could make use of netdev->phydev, but also use phy_link_topology
   to get an aggregated version of these parameters, if there are chained
   PHYs for example.
   
There are other parts of the kernel that accesses the netdev->phydev. There
are a few places in DSA where this is done, but the same helpers as the ones
introduced for MAC drivers could be used. The other remaining part would
be the Timestamping code, but Köry Maincent is currently working on an
series to help deal with the various timestamping sources, that will also
help on removing this strong netdev->phydev dependency.
   
Finally, there's the whole work of actually configuring the PHY themselves
correctly when they are chained to one another, and getting the logic right
for the link-up/down detection, getting the ksettings values aggregated
correctly, etc.

We have some local patches as well for that, to handle the stacked PHY
state-machine synchronisation, link-reporting and negociation but
it's still WIP to cover all the corner-cases and hard-to-test features, for
example how to deal with WoL or EEE in these situations.

Please don't hesitate to give any comments, remarks or point some shortcomings
if you see any. We've tried to keep this mail short but we'll be happy to
discuss any details. We're really looking for some feedback on that overall
approach before sending any RFC, but of course if you prefer to discuss over
some actual code we can move forward and send RFC code when it's a bit more
polished.

Thanks a lot,

Maxime Chevallier & Romain Gantois

Powered by blists - more mailing lists