[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-qPs7y8C09xh5_b@shell.armlinux.org.uk>
Date: Mon, 31 Mar 2025 13:50:59 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Alexander H Duyck <alexander.duyck@...il.com>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net,
Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
linux-arm-kernel@...ts.infradead.org,
Christophe Leroy <christophe.leroy@...roup.eu>,
Herve Codina <herve.codina@...tlin.com>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Simon Horman <horms@...nel.org>,
Romain Gantois <romain.gantois@...tlin.com>
Subject: Re: [PATCH net-next v5 09/13] net: phylink: Use phy_caps_lookup for
fixed-link configuration
On Thu, Mar 27, 2025 at 06:16:00PM -0700, Alexander H Duyck wrote:
> On Fri, 2025-03-07 at 18:36 +0100, Maxime Chevallier wrote:
> > When phylink creates a fixed-link configuration, it finds a matching
> > linkmode to set as the advertised, lp_advertising and supported modes
> > based on the speed and duplex of the fixed link.
> >
> > Use the newly introduced phy_caps_lookup to get these modes instead of
> > phy_lookup_settings(). This has the side effect that the matched
> > settings and configured linkmodes may now contain several linkmodes (the
> > intersection of supported linkmodes from the phylink settings and the
> > linkmodes that match speed/duplex) instead of the one from
> > phy_lookup_settings().
> >
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> > ---
> > drivers/net/phy/phylink.c | 44 +++++++++++++++++++++++++++------------
> > 1 file changed, 31 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index cf9f019382ad..8e2b7d647a92 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -802,12 +802,26 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
> > return phylink_validate_mac_and_pcs(pl, supported, state);
> > }
> >
> > +static void phylink_fill_fixedlink_supported(unsigned long *supported)
> > +{
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
> > +}
> > +
>
> Any chance we can go with a different route here than just locking
> fixed mode to being only for BaseT configurations?
>
> I am currently working on getting the fbnic driver up and running and I
> am using fixed-link mode as a crutch until I can finish up enabling
> QSFP module support for phylink and this just knocked out the supported
> link modes as I was using 25CR, 50CR, 50CR2, and 100CR2.
>
> Seems like this should really be something handled by some sort of
> validation function rather than just forcing all devices using fixed
> link to assume that they are BaseT. I know in our direct attached
> copper case we aren't running autoneg so that plan was to use fixed
> link until we can add more flexibility by getting QSFP support going.
Please look back at phylink's historical behaviour. Phylink used to
use phy_lookup_setting() to locate the linkmode for the speed and
duplex. That is the behaviour that we should be aiming to preserve.
We were getting:
speed duplex linkmode
10M Half 10baseT_Half
10M Full 10baseT_Full
100M Half 100baseT_Half
100M Full 100baseT_Full
1G Half 1000baseT_Half
1G Full 1000baseT_Full (this changed over time)
2.5G Full 2500baseT_Full
5G Full 5000baseT_Full
At this point, things get weird because of the way linkmodes were
added, as we return the _first_ match. Before commit 3c6b59d6f07c
("net: phy: Add more link modes to the settings table"):
10G Full 10000baseKR_Full
Faster speeds not supported
After the commit:
10G Full 10000baseCR_Full
20G Full 20000baseKR2_Full
25G Full 25000baseCR_Full
40G Full 40000baseCR4_Full
50G Full 50000baseCR2_Full
56G Full 56000baseCR4_Full
100G Full 100000baseCR4_Full
It's all a bit random. :(
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists