Re: [PATCH RFC net-next v2 1/2] net: phy: marvell: 88e1111: define gigabit features

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

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!