Re: [PATCH net-next v5 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

On Mon, 31 Mar 2025 15:54:20 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Mar 31, 2025 at 04:17:02PM +0200, Andrew Lunn wrote:
> > On Fri, Mar 28, 2025 at 04:26:04PM -0700, Alexander Duyck wrote:  
> > > A serdes PHY is part of it, but not a traditional twisted pair PHY as
> > > we are talking about 25R, 50R(50GAUI & LAUI), and 100P interfaces. I
> > > agree it is a different beast, but are we saying that the fixed-link
> > > is supposed to be a twisted pair PHY only?  
> > 
> > With phylink, the PCS enumerates its capabilities, the PHY enumerates
> > its capabilities, and the MAC enumerates it capabilities. phylink then
> > finds the subset which all support.
> > 
> > As i said, historically, fixed_link was used in place of a PHY, since
> > it emulated a PHY. phylinks implementation of fixed_link is however
> > different. Can it be used in place of both a PCS and a PHY? I don't
> > know.  
> 
> In fixed-link mode, phylink will use a PCS if the MAC driver says there
> is one, but it will not look for a PHY.
> 
> > You are pushing the envelope here, and maybe we need to take a step
> > back and consider what is a fixed link, how does it fit into the MAC,
> > PCS, PHY model of enumeration? Maybe fixed link should only represent
> > the PHY and we need a second sort of fixed_link object to represent
> > the PCS? I don't know?  
> 
> As I previously wrote today in response to an earlier email, the
> link modes that phylink used were the first-match from the old
> settings[] array in phylib which is now gone. This would only ever
> return _one_ link mode, which invariably was a baseT link mode for
> the slower speeds.
> 
> Maxime's first approach at adapting this to his new system was to
> set every single link mode that corresponded with the speed. I
> objected to that, because it quickly gets rediculous when we end
> up with lots of link modes being indicated for e.g. 10, 100M, 1G
> but the emulated PHY for these speeds only indicates baseT. That's
> just back-compatibility but... in principle changing the link modes
> that are reported to userspace for a fixed link is something we
> should not be doing - we don't know if userspace tooling has come
> to rely on that.
> 
> Yes, it's a bit weird to be reporting 1000baseT for a 1000BASE-X
> interface mode, but that's what we've always done in the past and
> phylink was coded to maintain that (following the principle that
> we shouldn't do gratuitous changes to the information exposed to
> userspace.)
> 
> Maxime's replacement approach is to just expose baseT, which
> means that for the speeds which do not have a baseT mode, we go
> from supporting it but with a weird link mode (mostly baseCR*)
> based on first-match in the settings[] table, to not supporting the
> speed.

I very wrongfully considered that there was no >10G fixed-link users, I
plan to fix that with something like the proposed patch in the
discussion, that reports all linkmodes for speeds above 10G (looks less
like a randomly selected mode, you can kind-of see what's going on as
you get all the linkmodes) but is a change in what we expose to
userspace.

Or maybe simpler, I could extend the list of compat fixed-link linkmodes
to all speeds with the previous arbitrary values that Russell listed in
the other mail (that way, no user-visible changes :) )

I was hoping Alexander could give option 1 a try, but let me know if
you think we should instead adopt option 2, which is probably the safer
on.

Maxime