When connecting RJ45 SFP modules to Linux an ethernet phy is expected -
and probed on the i2c bus when possible. Once the PHY probed, phylib
populates the supported link modes for the netdev based on bmsr
register bits set at the time (see phy_device.c: phy_probe).

Marvell phy driver probe function only allocates memory, leaving actual
configuration for config_init callback.
This means the supported link modes of the netdev depend entirely on the
power-on status of the phy bmsr register.

Certain Cisco SFP modules such as GLC-T and GLC-TE have invalid
configuration at power-on: MII_M1111_HWCFG_MODE_COPPER_1000X_AN
This means fiber with automatic negotiation to copper. As the module
exhibits a physical RJ45 connector this configuration is wrong.
As a consequence after power-on the bmsr does not set bits for 10/100
modes.

During config_init marvell phy driver identifies the correct intended
MII_M1111_HWCFG_MODE_SGMII_NO_CLK which means sgmii with automatic
negotiation to copper, and configures the phy accordingly.

At this point the bmsr register correctly indicates support for 10/100
link modes - however the netedev supported modes bitmask is never
updated.

Hence the netdev fails to negotiate or link-up at 10/100
speeds, limiting to 1000 links only.

Explicitly define features for 88e1111 phy to ensure that all supported
modes are available at runtime even when phy power-on configuration was
invalid.

[1] known functional 1Gbps RJ45 SFP module with 88E1111 PHY
[   75.117858] sfp c2-at-sfp: module LINKTEL          LX1801CNR        rev 1.0  sn 1172623934       dc 170628
[   75.127723] drivers/net/phy/sfp-bus.c:284: sfp_parse_support: 1000baseT_Half
[   75.134779] drivers/net/phy/sfp-bus.c:285: sfp_parse_support: 1000baseT_Full
[   75.141831] phylink_sfp_module_insert: sfp_may_have_phy - delaying phylink_sfp_config
[   75.204100] drivers/net/phy/phy_device.c:2942: phy_probe
[   75.212828] drivers/net/phy/phy_device.c:2961: phy_probe: phydev->drv->probe
[   75.228017] drivers/net/phy/phy_device.c:2983: phy_probe: genphy_read_abilities
[   75.246019] drivers/net/phy/phy_device.c:2502: genphy_read_abilities: MII_MARVELL_PHY_PAGE: 0x00
[   75.263045] drivers/net/phy/phy_device.c:2507: genphy_read_abilities: MII_BMSR: 0x7949
[   75.279282] sfp_add_phy
[   75.287150] phylink_sfp_connect_phy: calling phylink_sfp_config with phy settings
[   75.302778] drivers/net/phy/sfp-bus.c:445: sfp_select_interface: PHY_INTERFACE_MODE_SGMII
[   75.302778]
[   75.320600] m88e1111_config_init
[   75.334333] drivers/net/phy/marvell.c:905: m88e1111_config_init: MII_M1111_PHY_EXT_SR: 0x8084
[   75.348694] m88e1111_config_init: sgmii
[   75.364329] drivers/net/phy/marvell.c:787: m88e1111_config_init_hwcfg_mode: MII_M1111_PHY_EXT_SR: 0x8084
[   75.450737] fsl_dpaa2_eth dpni.0 eth0: PHY [i2c:c2-at-sfp:16] driver [Marvell 88E1111] (irq=POLL)
[   75.461329] sfp_sm_probe_for_phy: tried to probe clause 22 phy: 0
[   75.461333] phy detected after 0 retries
Settings for eth0:
        Supported ports: [ TP MII FIBRE ]
        Supported link modes:   10baseT/Full
                                100baseT/Full
                                1000baseT/Full
        Supports auto-negotiation: Yes
        Advertised link modes:  10baseT/Full
                                100baseT/Full
                                1000baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: Yes
[   77.445537] sfp c2-at-sfp: module removed

[2] problematic 1Gbps RJ45 SFP module with 88E1111 PHY before this patch
[   84.463372] sfp c2-at-sfp: module CISCO-AVAGO      ABCU-5710RZ-CS2  rev      sn AGM1131246C      dc 070803
[   84.473218] drivers/net/phy/sfp-bus.c:284: sfp_parse_support: 1000baseT_Half
[   84.480267] drivers/net/phy/sfp-bus.c:285: sfp_parse_support: 1000baseT_Full
[   84.487314] sfp c2-at-sfp: Unknown/unsupported extended compliance code: 0x01
[   84.487316] phylink_sfp_module_insert: sfp_may_have_phy - delaying phylink_sfp_config
[   84.548557] drivers/net/phy/phy_device.c:2942: phy_probe
[   84.557011] drivers/net/phy/phy_device.c:2961: phy_probe: phydev->drv->probe
[   84.572223] drivers/net/phy/phy_device.c:2983: phy_probe: genphy_read_abilities
[   84.589831] drivers/net/phy/phy_device.c:2502: genphy_read_abilities: MII_MARVELL_PHY_PAGE: 0x00
[   84.606107] drivers/net/phy/phy_device.c:2507: genphy_read_abilities: MII_BMSR: 0x149
[   84.622177] sfp_add_phy
[   84.631256] phylink_sfp_connect_phy: calling phylink_sfp_config with phy settings
[   84.631261] drivers/net/phy/sfp-bus.c:445: sfp_select_interface: PHY_INTERFACE_MODE_SGMII
[   84.631261]
[   84.650011] m88e1111_config_init
[   84.667424] drivers/net/phy/marvell.c:905: m88e1111_config_init: MII_M1111_PHY_EXT_SR: 0x9088
[   84.676137] m88e1111_config_init: sgmii
[   84.697088] drivers/net/phy/marvell.c:787: m88e1111_config_init_hwcfg_mode: MII_M1111_PHY_EXT_SR: 0x9084
[   84.794983] fsl_dpaa2_eth dpni.0 eth0: PHY [i2c:c2-at-sfp:16] driver [Marvell 88E1111] (irq=POLL)
[   84.805537] sfp_sm_probe_for_phy: tried to probe clause 22 phy: 0
[   84.819781] phy detected after 0 retries
Settings for eth4:
       Supported ports: [ TP MII ]
       Supported link modes:   1000baseT/Full
                               1000baseX/Full
       Supports auto-negotiation: Yes
       Advertised link modes:  1000baseT/Full
                               1000baseX/Full
[   86.149536] sfp c2-at-sfp: module removed

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/phy/marvell.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index c248c90510ae..6be2e2eb0be0 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -3752,7 +3752,7 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1111,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1111",
-		/* PHY_GBIT_FEATURES */
+		.features = PHY_GBIT_FIBRE_FEATURES,
 		.flags = PHY_POLL_CABLE_TEST,
 		.probe = marvell_probe,
 		.inband_caps = m88e1111_inband_caps,

-- 
2.43.0

On Thu, Jan 01, 2026 at 06:05:38PM +0200, Josua Mayer wrote:
> When connecting RJ45 SFP modules to Linux an ethernet phy is expected -
> and probed on the i2c bus when possible. Once the PHY probed, phylib
> populates the supported link modes for the netdev based on bmsr
> register bits set at the time (see phy_device.c: phy_probe).
> 
> Marvell phy driver probe function only allocates memory, leaving actual
> configuration for config_init callback.
> This means the supported link modes of the netdev depend entirely on the
> power-on status of the phy bmsr register.
> 
> Certain Cisco SFP modules such as GLC-T and GLC-TE have invalid
> configuration at power-on: MII_M1111_HWCFG_MODE_COPPER_1000X_AN
> This means fiber with automatic negotiation to copper. As the module
> exhibits a physical RJ45 connector this configuration is wrong.

No, it isn't wrong.

There are modules that can be bought with different configurations.
Sourcephotonics did it with their modules. Some of their modules are
configured to power up as 1000BASE-X to 1000BASE-T. Other modules power
up as SGMII to BASE-T.

The reason is device compatibility - with switches that can only talk
1000BASE-X, you want a module that powers up in 1000BASE-X to
1000BASE-T mode, and the switch can only support gigabit modes. The PHY
onboard advertises on its media side only that support.

If the switch can talk SGMII, then obviously want a module that powers
up in SGMII mode.

In the kernel, when we detect a PHY on the SFP, we *always* use SGMII
mode, and one of the things that the Marvell driver does is program the
PHY to operate in SGMII mode. This means that, even if the module was
a variant that powers up in 1000BASE-X mode, it gets switched to SGMII
mode during PHY bringup.

> As a consequence after power-on the bmsr does not set bits for 10/100
> modes.
> 
> During config_init marvell phy driver identifies the correct intended
> MII_M1111_HWCFG_MODE_SGMII_NO_CLK which means sgmii with automatic
> negotiation to copper, and configures the phy accordingly.
> 
> At this point the bmsr register correctly indicates support for 10/100
> link modes - however the netedev supported modes bitmask is never
> updated.
> 
> Hence the netdev fails to negotiate or link-up at 10/100
> speeds, limiting to 1000 links only.
> 
> Explicitly define features for 88e1111 phy to ensure that all supported
> modes are available at runtime even when phy power-on configuration was
> invalid.

This is wrong. If the PHY remains in 1000BASE-X mode on the host side
for whatever reason, when it is very reasonable not to say that 10/100
is unsupported.

If the operational mode of the PHY is reconfigured at runtime, then I
think it would be reasonable to re-read the supported linkmodes.
However, I think this will cause issues for phylink, as currently it
wants to know the link modes that are supported so it can choose an
appropriate interface mode.

We currently have phydev->possible_interfaces which indicates the
interface modes that the PHY will switch between depending on the mode
it has been configured for (so after the operating mode of the PHY has
been established.)

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.

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

Hi Russell, Josua,

On 02/01/2026 13:47, Russell King (Oracle) wrote:

> If the operational mode of the PHY is reconfigured at runtime, then I
> think it would be reasonable to re-read the supported linkmodes.
> However, I think this will cause issues for phylink, as currently it
> wants to know the link modes that are supported so it can choose an
> appropriate interface mode.

Russell, I agree that your patches for phydev->supported_interfaces
are required, but I also think we need another piece of the puzzle to
solve Josua's issue.

From what I get, it's impossible from the PHY driver's perspective only,
to know which configuration the PHY is in, i.e. is it in :

 1000X to 1000T
 SGMII to 1000T
 SGMII to something else ?

This is one of the issues I was facing with the SGMII to 100FX adapters.

Selecting the right phy_interface, is one thing, but it doesn't address
the fact that whe don't know which linkmodes to put in phydev->supported.

The approach I took to address that is in patch 3 of this series [1] :

 - The SFP's eeprom should ideally store information about the MDI of the
  module, is it outputing fiber at 1G, at 100M, is it BaseT, etc.

 - in sfp_sm_probe_phy(), we have the sfp_module_caps fully parsed, with
   fixups and quirks applied, so what I do is store a pointer to those in
   struct phy_device

 - The PHY driver can then use that in its .get_features() to report the
   proper linkmodes.

Of course, this may not be the right approach. What do we trust more, the SFP
eeprom, or the PHY's reported linkmodes through features discovery ?

IMO relying on the SFP subsystem to build a proper list of linkmodes we can
achieve on the module is a bit better, as we have the opportunity to apply
fixups and quirks.

Maxime

[1] : https://lore.kernel.org/all/20260114225731.811993-1-maxime.chevallier@bootlin.com/#t

On Mon, Jan 19, 2026 at 10:27:52AM +0100, Maxime Chevallier wrote:
> Hi Russell, Josua,
> 
> On 02/01/2026 13:47, Russell King (Oracle) wrote:
> 
> > If the operational mode of the PHY is reconfigured at runtime, then I
> > think it would be reasonable to re-read the supported linkmodes.
> > However, I think this will cause issues for phylink, as currently it
> > wants to know the link modes that are supported so it can choose an
> > appropriate interface mode.
> 
> Russell, I agree that your patches for phydev->supported_interfaces
> are required, but I also think we need another piece of the puzzle to
> solve Josua's issue.

I haven't had the time to respond until now, sorry (it's been total
chaos over the last few days.)

> From what I get, it's impossible from the PHY driver's perspective only,
> to know which configuration the PHY is in, i.e. is it in :
> 
>  1000X to 1000T
>  SGMII to 1000T
>  SGMII to something else ?

Note that phydev->supported_interfaces is only about the host side
interface.

> This is one of the issues I was facing with the SGMII to 100FX adapters.
> 
> Selecting the right phy_interface, is one thing, but it doesn't address
> the fact that whe don't know which linkmodes to put in phydev->supported.
> 
> The approach I took to address that is in patch 3 of this series [1] :
> 
>  - The SFP's eeprom should ideally store information about the MDI of the
>   module, is it outputing fiber at 1G, at 100M, is it BaseT, etc.

While they do, it's the EEPROM is horrendously unreliable. I had hoped
that Fibrestore would do better, but sadly not - it's the same rubbish
I've come to expect from SFP vendors.

From my database, for copper RJ45 SFPs, the majority specify their
connector as LC. Then, the next is RJ45 (correct) and finally "unknown".

I don't have enough 100FX SFPs to base any data upon.

Cabled SFPs seem to do well, 100% of those I have are "Copper pigtail".

Fibre SFPs seem to all state LC as well.

xPON SFPs... are basically a total mess.

For the transceiver codes, it's even worse.

Cabled SFPs again do well, 100% indicate that they're some kind of
cable, but not all of them provide all the capabilities you'd expect.

Fibre SFPs... yea, random.

Copper RJ45 SFPs... can look like fibre, fibrechannel, but may also
state a copper based type.

xPON... total trainwreck, Some gives some kind of random fibre types,
some even give copper types. Some basically state they support every
transceiver type out there.

If you want to try and work out what the media side should be, then
I think we're basically going to end up needing our own database for
every transceiver out there... which leads us towards the model that
vendors use: only accepting transceivers we know about (which leads
to claims of "vendor lock-in" for this technology.)

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

On 21/01/2026 16:06, Russell King (Oracle) wrote:

> If you want to try and work out what the media side should be, then
> I think we're basically going to end up needing our own database for
> every transceiver out there... which leads us towards the model that
> vendors use: only accepting transceivers we know about (which leads
> to claims of "vendor lock-in" for this technology.)

Does the current model of applying the fixups/quirks fits that bill of
having some internal database ?

Thing is, from my tests there are modules (e.g. Prolabs GLC-GE-100FX-C)
where neither the eeprom nor the integrated PHY's reported capabilities
are correct. The only reliable thing we have then is the SFP module's
vendor_name + vendor_pn.

From then on, we may say "We trust whatever comes out of reading the
eeprom + applying the quirks" ?

Or am I relying too much on the quirks ?

Maxime

On 19/01/2026 11:27, Maxime Chevallier wrote:
> Hi Russell, Josua,
>
> On 02/01/2026 13:47, Russell King (Oracle) wrote:
>
>> If the operational mode of the PHY is reconfigured at runtime, then I
>> think it would be reasonable to re-read the supported linkmodes.
>> However, I think this will cause issues for phylink, as currently it
>> wants to know the link modes that are supported so it can choose an
>> appropriate interface mode.
> Russell, I agree that your patches for phydev->supported_interfaces
> are required, but I also think we need another piece of the puzzle to
> solve Josua's issue.
>
>  From what I get, it's impossible from the PHY driver's perspective only,
> to know which configuration the PHY is in, i.e. is it in :
>
>   1000X to 1000T
>   SGMII to 1000T
>   SGMII to something else ?
The PHY driver cannot know what the media side is,
and it cannot trust the state after initial power-on.

In particular because hardware engineers will refuse to change PCB
just because some bootstrap signals were (un-)intentionally wrong,
when in fact the phy can be reconfigured at runtime.

But the host side can be negotiated with the MAC.
> This is one of the issues I was facing with the SGMII to 100FX adapters.
>
> Selecting the right phy_interface, is one thing, but it doesn't address
> the fact that whe don't know which linkmodes to put in phydev->supported.
>
> The approach I took to address that is in patch 3 of this series [1] :
>
>   - The SFP's eeprom should ideally store information about the MDI of the
>    module, is it outputing fiber at 1G, at 100M, is it BaseT, etc.
>
>   - in sfp_sm_probe_phy(), we have the sfp_module_caps fully parsed, with
>     fixups and quirks applied, so what I do is store a pointer to those in
>     struct phy_device
>
>   - The PHY driver can then use that in its .get_features() to report the
>     proper linkmodes.
>
> Of course, this may not be the right approach. What do we trust more, the SFP
> eeprom, or the PHY's reported linkmodes through features discovery ?
For the type of media I would trust the combination of sfp eeprom + quirks
more than the power-on-reset state of phy inside sfp.

But for the available link modes/speeds I would trust the PHY.

>
> IMO relying on the SFP subsystem to build a proper list of linkmodes we can
> achieve on the module is a bit better, as we have the opportunity to apply
> fixups and quirks.
>
> Maxime
>
> [1] : https://lore.kernel.org/all/20260114225731.811993-1-maxime.chevallier@bootlin.com/#t
>

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.

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

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.

So does this mean the host side? Or the copper side?

If it means host side, would it be okay for the 88E1111 phy to populate
possible_interfaces from its config_init for 10/100/1000 link-modes,
if host side is configured for sgmii?
Or populate possible_interfaces from BMSR at this point?

Unfortunately I do not have the affected SFPs near me for a few weeks.

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!