When phylink handles an SFP module that contains a PHY, it selects a
phy_interface to use to communicate with it. This selection ensures that
the highest speed gets achieved, based on the linkmodes we want to
support in the module.

This approach doesn't take into account the supported interfaces
reported by the module, but also doesn't handle the fact that the same
linkmode may be achieved using different phy_interface modes.

It becomes an issue when trying to support SGMII to 100FX modules. We
can't feed it 100BaseX, and its MDI isn't 1000BaseT (the modes we expect
for SGMII to be selected).

Let's therefore use a different approach :

 - Get the common interfaces between what the module and the SFP Bus
   support
 - From that common interface list, select the one that can allow us to
   achieve the highest speed

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy-caps.h |  5 ++++
 drivers/net/phy/phy_caps.c | 47 ++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phylink.c  | 24 ++++++-------------
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 5f3f757e0b2f..4a07ac74ef13 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -10,6 +10,7 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 
+/* this must be sorted by speed */
 enum {
 	LINK_CAPA_10HD = 0,
 	LINK_CAPA_10FD,
@@ -66,4 +67,8 @@ void phy_caps_medium_get_supported(unsigned long *supported,
 				   int lanes);
 u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes);
 
+phy_interface_t
+phy_caps_select_fastest_interface(const unsigned long *interfaces,
+				  const unsigned long *linkmodes);
+
 #endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 17a63c931335..11e7a1efcf30 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -443,3 +443,50 @@ u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes)
 	return mediums;
 }
 EXPORT_SYMBOL_GPL(phy_caps_mediums_from_linkmodes);
+
+/**
+ * phy_caps_select_fastest_interface - Select the fastest interface that can
+ *				       support the fastest of the given
+ *				       linkmodes
+ * @interfaces: The interface list to lookup from
+ * @linkmodes: Linkmodes we want to support
+ *
+ * Returns: The fastest matching interface, PHY_INTERFACE_MODE_NA otherwise.
+ */
+phy_interface_t
+phy_caps_select_fastest_interface(const unsigned long *interfaces,
+				  const unsigned long *linkmodes)
+{
+	phy_interface_t interface = PHY_INTERFACE_MODE_NA;
+	u32 target_link_caps = 0;
+	int i, max_capa = 0;
+
+	/* Link caps from the linkmodes */
+	for_each_set_bit(i, linkmodes, __ETHTOOL_LINK_MODE_MASK_NBITS) {
+		const struct link_mode_info *linkmode;
+
+		linkmode = &link_mode_params[i];
+		target_link_caps |= speed_duplex_to_capa(linkmode->speed,
+							 linkmode->duplex);
+	}
+
+	for_each_set_bit(i, interfaces, PHY_INTERFACE_MODE_MAX) {
+		u32 interface_caps = phy_caps_from_interface(i);
+		u32 interface_max_capa;
+
+		/* Can we achieve at least one mode with this interface ? */
+		if (!(interface_caps & target_link_caps))
+			continue;
+
+		/* Biggest link_capa we can achieve with this interface */
+		interface_max_capa = fls(interface_caps & target_link_caps);
+
+		if (interface_max_capa > max_capa) {
+			max_capa = interface_max_capa;
+			interface = i;
+		}
+	}
+
+	return interface;
+}
+EXPORT_SYMBOL_GPL(phy_caps_select_fastest_interface);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 43d8380aaefb..18fa417b87dd 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2808,27 +2808,19 @@ EXPORT_SYMBOL_GPL(phylink_ethtool_set_wol);
 static phy_interface_t phylink_sfp_select_interface(struct phylink *pl,
 						const unsigned long *link_modes)
 {
-	phy_interface_t interface;
+	DECLARE_PHY_INTERFACE_MASK(common_interfaces);
 
-	interface = sfp_select_interface(pl->sfp_bus, link_modes);
-	if (interface == PHY_INTERFACE_MODE_NA) {
-		phylink_err(pl,
-			    "selection of interface failed, advertisement %*pb\n",
-			    __ETHTOOL_LINK_MODE_MASK_NBITS,
-			    link_modes);
-		return interface;
-	}
+	/* Interfaces supported both by the module and the bus */
+	phy_interface_and(common_interfaces, pl->sfp_interfaces,
+			  pl->config->supported_interfaces);
 
-	if (!test_bit(interface, pl->config->supported_interfaces)) {
+	if (phy_interface_empty(common_interfaces)) {
 		phylink_err(pl,
-			    "selection of interface failed, SFP selected %s (%u) but MAC supports %*pbl\n",
-			    phy_modes(interface), interface,
-			    (int)PHY_INTERFACE_MODE_MAX,
-			    pl->config->supported_interfaces);
+			    "selection of interface failed, no common interface between MAC and SFP\n");
 		return PHY_INTERFACE_MODE_NA;
 	}
 
-	return interface;
+	return phy_caps_select_fastest_interface(common_interfaces, link_modes);
 }
 
 static phy_interface_t phylink_sfp_select_interface_speed(struct phylink *pl,
@@ -3697,8 +3689,6 @@ static int phylink_sfp_config_phy(struct phylink *pl, struct phy_device *phy)
 	struct phylink_link_state config;
 	int ret;
 
-	/* We're not using pl->sfp_interfaces, so clear it. */
-	phy_interface_zero(pl->sfp_interfaces);
 	linkmode_copy(support, phy->supported);
 
 	memset(&config, 0, sizeof(config));
-- 
2.49.0

On Wed, Jan 14, 2026 at 11:57:24PM +0100, Maxime Chevallier wrote:
> When phylink handles an SFP module that contains a PHY, it selects a
> phy_interface to use to communicate with it. This selection ensures that
> the highest speed gets achieved, based on the linkmodes we want to
> support in the module.
> 
> This approach doesn't take into account the supported interfaces
> reported by the module

This is intentional by design, because the capabilities of the PHY
override in this case. Unfortunately, as I've said previously, the
rush to throw in a regurgitated version of my obsoleted
"host_interfaces" rather messed up my replacement patch set which
had the PHY driver advertising the interface capabilities of the
PHY, which were then going to be used to make the PHY interface
selection when attaching the PHY.

I've still got the code, but I can't now push it into mainline
because, with the obsolete host_interfaces stuff merged, we will end
up with two competing solutions.

In any case, I really would appreciate people looking through
http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=net-queue

before doing development on SFP and phylink to see whether I've
already something that solves their issue. Quite simply, I don't have
the time to push every patch out that I have, especially as I'm up to
my eyeballs with the crappy stmmac driver now, but also because I
have work items from Oracle that reduce the time I can work on
mainline. BTW, the "age" stated in cgit is based on the commit time
(which gets reset when rebased) not the initial merge time. You will
see that the "supported_interfaces" stuff dates from 2019, not 2025.

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

Hi Russell,

On 15/01/2026 00:30, Russell King (Oracle) wrote:
> On Wed, Jan 14, 2026 at 11:57:24PM +0100, Maxime Chevallier wrote:
>> When phylink handles an SFP module that contains a PHY, it selects a
>> phy_interface to use to communicate with it. This selection ensures that
>> the highest speed gets achieved, based on the linkmodes we want to
>> support in the module.
>>
>> This approach doesn't take into account the supported interfaces
>> reported by the module
> 
> This is intentional by design, because the capabilities of the PHY
> override in this case.

OK makes sense. Just to summarize my understanding, let me know if
I'm wrong there :

 - The interfaces list we have in sfp_module_caps is to be used when we 
   don't have a PHY in the module (there may be one, but we don't
   know/care about it).

 - When we do have a PHY, we _should_ select the interface based on what
   the MAC (+ PCS + Serdes etc.) can output on this sfp-bus and what
   the SFP PHY can take as an input. We ignore the sfp_module_caps
   interfaces list.

> Unfortunately, as I've said previously, the> rush to throw in a regurgitated version of my obsoleted
> "host_interfaces" rather messed up my replacement patch set which
> had the PHY driver advertising the interface capabilities of the
> PHY, which were then going to be used to make the PHY interface
> selection when attaching the PHY.
>
> I've still got the code, but I can't now push it into mainline
> because, with the obsolete host_interfaces stuff merged, we will end
> up with two competing solutions.
> 
> In any case, I really would appreciate people looking through
> http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=net-queue
> 
> before doing development on SFP and phylink to see whether I've
> already something that solves their issue.

So what's the plan there ? This work here is kinda low priority
for me, I wanted to get this out there before continuing with
phy_port followup. Without this patch though, this whole series
is blocked as SGMII will never be selected for 100FX modules.

With your permission, can I pick up your patchs for supported_interfaces
and see what I can do from there ? I also found host_interfaces to be
not enough there.

Knowing that for me, phy_port is the priorty, this is going to be
something I'll do on my free time so don't expect fast progress :(

> Quite simply, I don't have> the time to push every patch out that I have, especially as I'm up to
> my eyeballs with the crappy stmmac driver now, but also because I
> have work items from Oracle that reduce the time I can work on
> mainline. BTW, the "age" stated in cgit is based on the commit time
> (which gets reset when rebased) not the initial merge time. You will
> see that the "supported_interfaces" stuff dates from 2019, not 2025.

Besides that part, will you take a look at the rest of the series ? I'm
not saying that to rush you, but this whole SGMII to 100Fx journey seemed
to me that a lot of hacky stuff, I'd like to get your opinion on the rest
before iterating and facing anther blocking problem down the line on
another part of that series.

I know you have a lot on your plate, but as I said, this series is probably
going to move slowly anyways :)

Maxime