public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Josua Mayer <josua@solid-run.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC net-next v2 1/2] net: phy: marvell: 88e1111: define gigabit features
Date: Mon, 19 Jan 2026 10:52:17 +0000	[thread overview]
Message-ID: <aW4M4TqdHtEH10U1@shell.armlinux.org.uk> (raw)
In-Reply-To: <c2b6af56-57f7-4658-9e6f-e671be19e300@solid-run.com>

On Mon, Jan 19, 2026 at 08:30:12AM +0000, Josua Mayer wrote:
> Hi Russell,
> 
> Thank you for the extensive feedback!
> 
> On 02/01/2026 15:55, Russell King (Oracle) wrote:
> > On Fri, Jan 02, 2026 at 12:47:06PM +0000, Russell King (Oracle) wrote:
> >> I do have patches that add phydev->supported_interfaces which are
> >> populated at probe time to inform phylink which host interface modes
> >> that the PHY can be reconfigured between - and this overrides the
> >> linkmode-derivation of that information - it basically becomes:
> >>
> >>          phy_interface_and(interfaces, phy->supported_interfaces,
> >>                            pl->config->supported_interfaces);
> >>          interface = phylink_choose_sfp_interface(pl, interfaces);
> >>          if (interface == PHY_INTERFACE_MODE_NA) {
> >>                  phylink_err(pl, "selection of interface for PHY failed\n");
> >>                  return -EINVAL;
> >>          }
> >>
> >>          phylink_dbg(pl, "copper SFP: chosen %s interface\n",
> >>                      phy_modes(interface));
> >>
> >>          ret = phylink_attach_phy(pl, phy, interface);
> >>
> >> and phylink_attach_phy() will result in the PHY driver's config_init
> >> being called, configuring the appropriate operating mode for the
> >> PHY, which can then be used to update phydev->supported as appropriate.
> >>
> >> phylink will then look at phydev->supported once the above has
> >> completed when it will do so in phylink_bringup_phy().
> >>
> >> Deriving the host side PHY interface mode from the link modes has
> >> always been rather sketchy.
> > These patches can be found at:
> >
> > http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=net-queue
> >
> > See:
> >
> > net: phylink: use phy interface mode bitmaps for SFP PHYs
> > net: phy: add supported_interfaces to Aquantia AQR113C
> > net: phy: add supported_interfaces to marvell10g PHYs
> > net: phy: add supported_interfaces to marvell PHYs
> > net: phy: add supported_interfaces to bcm84881
> > net: phy: add supported_interfaces to phylib
> >
> > The reason I didn't end up pushing them (they're almost six years old)
> > is because I decided that the host_interfaces approach wasn't a good
> > idea, and dropped those patches. Marek Behún took my patches for
> > host_interfaces and they were merged in 2022. I had already junked
> > the host_interfaces approach.
> >
> > The problem is that we now have two ways that PHY drivers configure
> > their interface mode - one where config_init() decides on its own
> > based on the host_interfaces supplied to it, and this approach above
> > where phylink attempts to choose the interface based on what the
> > PHY and host (and datapath) can support. These two approaches are
> > mutually incompatible if we get both phylink _and_ the PHY driver
> > attempting to do the same thing.
> 
> All this left me puzzled.
> 
> I understand that .features / get_features happens too early
> and doesn't account for different host interfaces.
> 
> Further populating supported bitmask before config_init is wrong
> at least for this module where supported link-modes are different
> per host interface.
> 
> The marvell10g.c driver which now uses the host_interfaces bitmask
> is now extending the supported_interfaces bitmask in config_init.
> Yet I didn't understand what it means.
> 
>  From phy.h:
> 
>   * @possible_interfaces: bitmap if interface modes that the attached PHY
>   *             will switch between depending on media speed.

Note, I said *supported* not *possible*. They're two different things.

The supported mask is the list of interfaces that the PHY has support
for e.g. 88x3310 can support 10GBASE-R, 5GBASE-R, 2500BASE-X, SGMII,
USXGMII, and a few others that I can't recall off the top of my head.
This needs to be known before attaching to the PHY to allow the
operating mode to be chosen.

The possible mask is "we've configured it for this operating mode,
now which of those interfaces will we switch between depending on the
media"

Both are the host side.

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

  reply	other threads:[~2026-01-19 10:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-01 16:05 [PATCH RFC net-next v2 0/2] net: phy: marvell: 88e1111: define gigabit features Josua Mayer
2026-01-01 16:05 ` [PATCH RFC net-next v2 1/2] " Josua Mayer
2026-01-02 12:47   ` Russell King (Oracle)
2026-01-02 13:55     ` Russell King (Oracle)
2026-01-19  8:30       ` Josua Mayer
2026-01-19 10:52         ` Russell King (Oracle) [this message]
2026-01-19  9:27     ` Maxime Chevallier
2026-01-21  9:52       ` Josua Mayer
2026-01-21 15:06       ` Russell King (Oracle)
2026-01-22 12:31         ` Maxime Chevallier
2026-01-01 16:05 ` [PATCH RFC net-next v2 2/2] net: sfp: support 25G long-range modules (extended compliance code 0x3) Josua Mayer
2026-01-01 17:43   ` Andrew Lunn
2026-01-02 12:48   ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aW4M4TqdHtEH10U1@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=josua@solid-run.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox