Message-ID: <CAKgT0UfB-CKBsAPHRA3TMuiAdiAbBEVTHcEUgmCHL-q0zJMRJA@mail.gmail.com>
Date: Thu, 10 Jul 2025 13:44:25 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Maxime Chevallier <maxime.chevallier@...tlin.com>, 
	Heiner Kallweit <hkallweit1@...il.com>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, 
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

On Thu, Jul 10, 2025 at 11:35 AM Russell King (Oracle)
<linux@...linux.org.uk> wrote:
>
> On Thu, Jul 10, 2025 at 10:22:44AM -0700, Alexander Duyck wrote:
> > On Thu, Jul 10, 2025 at 9:11 AM Andrew Lunn <andrew@...n.ch> wrote:
> > > What is wrong, that it is reporting LP information, or that it is
> > > reporting it does not support autoneg when in fact it is actually
> > > doing autoneg?
> >
> > I have some debug code on here that is reporting the FW config as the
> > "LP Advertised". I had borrowed that approach from the phylink
> > fixedlink config as I thought it was a good way for me to know what
> > the FW was requesting without having to report it out to a log file.
>
> There are a few points to be made here.
>
> 1. Fixed link configuration is not the same as !autoneg setting with
>    the presence of a PHY. !autoneg with a PHY present means that the
>    PHY has been instructed not to perform autonegotiation, but to set
>    the specified parameters for the link and only allow the link to
>    operate at the specified speed/duplex. There are exceptions - as
>    users expect 1G to work with "autoneg" disabled, and 1G requires
>    AN in order to bring the link up. Some PHYs support disabling the
>    autoneg function at 1G speed by internally ignoring the request
>    to disable autoneg, and instead only advertising to the link
>    partner that 1G at the specified duplex is supported. We took
>    that and turned it into a software thing for all PHYs as some
>    PHYs decided to go a different route - basically not supporting
>    the AN enable bit being turned off at 1G speeds.
>
> 2. Fixed link configuration is a software concept where there is no
>    accessible PHY present. Phylink rejects fixed link configuration
>    with a PHY. There is no support to configure a PHY into fixed
>    speed/duplex if present, and has never been supported prior to
>    phylink.
>
> 3. The history. Prior to phylink (and it remains in some cases today),
>    fixed link configuration was created by providing a software
>    emulated PHY to phylib for the MAC driver to use, thus avoiding
>    MAC drivers having to add explicit code for fixed links. They
>    looked just like a normal PHY, but was limited to no faster than
>    1G speeds as the software emulation is a Clause 22 PHY.
>
>    This software emulated PHY replaces the presence of a physical
>    PHY (there is none) and the PHY it emulates looks like a PHY that
>    supports AN, has AN enabled, but only supports a single speed
>    and duplex, only advertises a single baseT(x) speed and duplex,
>    and the link partner agrees on the speed and duplex. This "fools
>    phylib into resolving the speed and duplex as per the fixed link
>    configuration.
>
>    However, in reality, there is no AN.
>
>    This has become part of the user API, because the MII registers of
>    the fixed link PHY were exported to userspace, and of course through
>    ethtool.
>
>    There has never been a MII API for reading the fixed link parameters
>    for speeds > 1G, so while phylink enables fixed link configuration
>    for those speeds, there is no MII register support for this for
>    userspace.

So the issue is in our setup we have a SerDes PHY so it isn't a real
MII based PHY and it is being managed by the firmware so we don't have
direct access.

One thought I was playing around with was emulating a 25/50/100G PMA
and AN in software and exposing that as the interface to the FW to
play the role of the PHY. The FW is playing the role of a device tree
configuration in that the EEPROM is pre-programmed with the expected
speed/lane/FEC configuration and the FW is reading that to determine
what the link configuration should be as we cannot use autoneg as the
switch port on the other side doesn't support it. For our production
use case we will always be using that speed, but for testing we will
need to support the ability to set manual speeds.

> (As an aside)
> Someone earlier today sent a reminder about a bug I'd introduced for
> 10GBASE-R, 5GBASE-R and another interface (I don't recall right now)
> and I proposed a patch that only cleared the Autoneg bit in the
> adertising mask. Having been reminded about it, and had Andrew's
> input on this thread, I'm wondering whether config.advertising
> should be entirely cleared as in !autoneg mode, the advertising mask
> makes no sense.
>
> However, I'm supposed to be on my vacation, so I'm not going to start
> testing anything... this email as a bonus that would've otherwise have
> been delayed by about two weeks... but the way things are going (family
> issues) it could turn out to be a lot longer as I may have to become a
> full time carer. So much for an opportunity to have an opportunity to
> relax, which I desperately need.

Enjoy your vacation. It will probably take me a while to try to work
out an acceptable solution for how to deal with the buried/hidden PHY
behind the firmware anyway. For now this thread has become more about
fbnic anyway then your original patch since we have more-or-less
verified it works as expected.

Powered by blists - more mailing lists