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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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.

> > In addition one advantage is that it makes it possible to support
> > speeds that don't yet have a type in the phy_interface_t, so as I was
> > enabling things it allowed some backwards compatibility with older
> > kernels.
> 
> I don't like the sound of that. I would simply not support the older
> kernels, rather than do some hacks.

I think Alexander is referring to having a PHY interface modes that
supports the speed - for example, if we didn't have a PHY interface
mode that supported 100G speeds, then 100G speed would not be
supported.

Phylink has already restricted this and has done for quite some time.
We only allow speeds that the selected interface mode can support,
with the exception of PHY_INTERFACE_MODE_INTERNAL which supports
everything.

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