net: fix optical SFP failures
Subsystems (4)
Russell King (Oracle)
maintainercover net: fix optical SFP failures
Hi,
A regression was reported back in April concerning pcs-lynx and 10G optical SFPs. This patch series addresses that regression, and likely similar unreported regressions.
These patches:
- Add phy_interface_weight() which will be used in the solution.
- Split out the code that determines the inband "type" for an interface mode.
- Clear the Autoneg bit in the advertising mask, or the Autoneg bit in the support mask and the entire advertising mask if the selected interface mode has no inband capabilties.
Tested with the mvpp2 patch posted earlier today.
drivers/net/phy/phylink.c | 97 ++++++++++++++++++++++++++++++----------------- include/linux/phy.h | 5 +++ 2 files changed, 67 insertions(+), 35 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
PATCH 1/3 net: phy: add phy_interface_weight()
--- include/linux/phy.h | 5 +++++ 1 file changed, 5 insertions(+)
include/linux/phy.h +5 -0
@@ -169,6 +169,11 @@ static inline bool phy_interface_empty(const unsigned long *intf)return bitmap_empty (intf,PHY_INTERFACE_MODE_MAX ); } +static inline unsigned int phy_interface_weight (const unsigned long *intf) +{ +return bitmap_weight (intf,PHY_INTERFACE_MODE_MAX ); +} +static inline void phy_interface_and (unsigned long *dst,const unsigned long *a,const unsigned long *b) {
Signature · 1 line
2.47.2
Re: [PATCH net 1/3] net: phy: add phy_interface_weight()
Quoted 1 line · Russell King (Oracle)
Signed-off-by Russell King (Oracle) <[email protected]>
Andrew
PATCH 2/3 net: phylink: provide phylink_get_inband_type()
In reply to Russell King (Oracle)Provide a function to get the type of the inband signalling used for a PHY interface type. This will be used in the subsequent patch to address problems with 10G optical modules.
--- drivers/net/phy/phylink.c | 79 ++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 35 deletions(-)
drivers/net/phy/phylink.c +44 -35
@@ -1016,6 +1016,42 @@ static void phylink_pcs_an_restart(struct phylink *pl) pl->pcs->ops->pcs_an_restart (pl->pcs); } +enum inband_type { +INBAND_NONE , +INBAND_CISCO_SGMII , +INBAND_BASEX , +}; + +static enum inband_type phylink_get_inband_type (phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +case PHY_INTERFACE_MODE_QUSGMII : +case PHY_INTERFACE_MODE_USXGMII : +case PHY_INTERFACE_MODE_10G_QXGMII : +/* These protocols are designed for use with a PHY which +* communicates its negotiation result back to the MAC via +* inband communication. Note: there exist PHYs that run +* with SGMII but do not send the inband data. +*/ +return INBAND_CISCO_SGMII ; + +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +/* 1000base-X is designed for use media-side for Fibre +* connections, and thus the Autoneg bit needs to be +* taken into account. We also do this for 2500base-X +* as well, but drivers may not support this, so may +* need to override this. +*/ +return INBAND_BASEX ; + +default : +return INBAND_NONE ; + } +} +/** * phylink_pcs_neg_mode() - helper to determine PCS inband mode * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -1043,46 +1079,19 @@ static void phylink_pcs_neg_mode(struct phylink *pl, struct phylink_pcs *pcs,unsigned int pcs_ib_caps =0 ;unsigned int phy_ib_caps =0 ;unsigned int neg_mode, mode; -enum { -INBAND_CISCO_SGMII , -INBAND_BASEX , - } type; - - mode = pl->req_link_an_mode; +enum inband_type type; - pl->phy_ib_mode =0 ; - -switch (interface) { -case PHY_INTERFACE_MODE_SGMII : -case PHY_INTERFACE_MODE_QSGMII : -case PHY_INTERFACE_MODE_QUSGMII : -case PHY_INTERFACE_MODE_USXGMII : -case PHY_INTERFACE_MODE_10G_QXGMII : -/* These protocols are designed for use with a PHY which -* communicates its negotiation result back to the MAC via -* inband communication. Note: there exist PHYs that run -* with SGMII but do not send the inband data. -*/ - type =INBAND_CISCO_SGMII ; -break ; - -case PHY_INTERFACE_MODE_1000BASEX : -case PHY_INTERFACE_MODE_2500BASEX : -/* 1000base-X is designed for use media-side for Fibre -* connections, and thus the Autoneg bit needs to be -* taken into account. We also do this for 2500base-X -* as well, but drivers may not support this, so may -* need to override this. -*/ - type =INBAND_BASEX ; -break ; - -default : + type =phylink_get_inband_type (interface); +if (type ==INBAND_NONE ) { pl->pcs_neg_mode =PHYLINK_PCS_NEG_NONE ; - pl->act_link_an_mode = mode; + pl->act_link_an_mode = pl->req_link_an_mode;return ; } + mode = pl->req_link_an_mode; + + pl->phy_ib_mode =0 ; +if (pcs) pcs_ib_caps =phylink_pcs_inband_caps (pcs, interface);
Signature · 1 line
2.47.2
Re: [PATCH net 2/3] net: phylink: provide phylink_get_inband_type()
Quoted 5 lines · Russell King (Oracle)
Provide a function to get the type of the inband signalling used for a PHY interface type. This will be used in the subsequent patch to address problems with 10G optical modules.
Signed-off-by Russell King (Oracle) <[email protected]>
Andrew
PATCH 3/3 net: phylink: disable autoneg for interfaces that have no inband
In reply to Russell King (Oracle)Mathew reports that as a result of commit 6561f0e547be ("net: pcs:
pcs-lynx: implement pcs_inband_caps() method"), 10G SFP modules no longer work with the Lynx PCS.
This problem is not specific to the Lynx PCS, but is caused by commit df874f9e52c3 ("net: phylink: add pcs_inband_caps() method") which added validation of the autoneg state to the optical SFP configuration path.
Fix this by handling interface modes that fundamentally have no inband negotiation more correctly - if we only have a single interface mode, clear the Autoneg support bit and the advertising mask. If the module can operate with several different interface modes, autoneg may be supported for other modes, so leave the support mask alone and just clear the Autoneg bit in the advertising mask.
This restores 10G optical module functionality with PCS that supply their inband support, and makes ethtool output look sane.
--- drivers/net/phy/phylink.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/net/phy/phylink.c +18 -0
@@ -3634,6 +3634,7 @@ static int phylink_sfp_config_optical(struct phylink *pl) {__ETHTOOL_DECLARE_LINK_MODE_MASK (support);struct phylink_link_state config; +enum inband_type inband_type;phy_interface_t interface;int ret;
@@ -3680,6 +3681,23 @@ static int phylink_sfp_config_optical(struct phylink *pl)phylink_dbg (pl,"optical SFP: chosen %s interface\n" ,phy_modes (interface)); + inband_type =phylink_get_inband_type (interface); +if (inband_type ==INBAND_NONE ) { +/* If this is the sole interface, and there is no inband +* support, clear the advertising mask and Autoneg bit in +* the support mask. Otherwise, just clear the Autoneg bit +* in the advertising mask. +*/ +if (phy_interface_weight (pl->sfp_interfaces) ==1 ) { +linkmode_clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, + pl->sfp_support); +linkmode_zero (config.advertising); + }else { +linkmode_clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, + config.advertising); + } + } +if (!phylink_validate_pcs_inband_autoneg (pl, interface, config.advertising)) {phylink_err (pl,"autoneg setting not compatible with PCS" );
Signature · 1 line
2.47.2
Re: [PATCH net 3/3] net: phylink: disable autoneg for interfaces that have no inband
Quoted 15 lines · Russell King (Oracle)
Mathew reports that as a result of commit
6561f0e547be("net: pcs:
pcs-lynx: implementpcs_inband_caps()method"), 10G SFP modules no longer work with the Lynx PCS.This problem is not specific to the Lynx PCS, but is caused by commit
df874f9e52c3("net: phylink: addpcs_inband_caps()method") which added validation of the autoneg state to the optical SFP configuration path.Fix this by handling interface modes that fundamentally have no inband negotiation more correctly - if we only have a single interface mode, clear the Autoneg support bit and the advertising mask. If the module can operate with several different interface modes, autoneg may be supported for other modes, so leave the support mask alone and just clear the Autoneg bit in the advertising mask.
Quoted 8 lines
This restores 10G optical module functionality with PCS that supply their inband support, and makes ethtool output look sane.
Reported-by Mathew McBride <[email protected]>Fixes df874f9e52c3 ("net: phylink: add pcs_inband_caps() method")Signed-off-by Russell King (Oracle) <[email protected]>---
I had missed this discussion and was about to report the same problem as Mathew again.
Re: [PATCH net 3/3] net: phylink: disable autoneg for interfaces that have no inband
In reply to Russell King (Oracle)Hey folks, this seems to have regressed a different setup: I have a Banana Pi BPi-R4 connected to a dumb unmanaged switch with a 10GbaseCR cable, and it fails to negotiate a link with a21202743f9c applied. Reverting it makes things work again. It seems like phylink_get_inband_type doesn't handle base-R modes at all? I'll maybe have some more time to poke this myself later this week, but throwing it out there in case anyone more experienced has ideas.
Russell King (Oracle)
maintainerRe: [PATCH net 3/3] net: phylink: disable autoneg for interfaces that have no inband
Quoted 1 line · Ilya K
Hey folks, this seems to have regressed a different setup: I have a Banana Pi BPi-R4 connected to a dumb unmanaged switch with a 10GbaseCR cable, and it fails to negotiate a link with
a21202743f9capplied. Reverting it makes things work again. It seems likephylink_get_inband_typedoesn't handle base-R modes at all? I'll maybe have some more time to poke this myself later this week, but throwing it out there in case anyone more experienced has ideas.
10GBASE-CR (based upon BASE-R) has no inband signalling for autonegotiation.
Please enable debugging in phylink and include this is any bug report.
Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net 3/3] net: phylink: disable autoneg for interfaces that have no inband
Quoted 4 lines
10GBASE-CR (based upon BASE-R) has no inband signalling for autonegotiation.
Ugh. Sorry, that got me to the right idea, WIP driver was misreporting capabilities. The fix turned out to be trivial: https://github.com/K900/linux/commit/15406149c6e45de3884f1ccdafa1920da37d1bf
This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 14 lines · On Sun, 31 Aug 2025 18:34:17 +0100 you wrote:
Hi,
A regression was reported back in April concerning pcs-lynx and 10G optical SFPs. This patch series addresses that regression, and likely similar unreported regressions.
These patches:
- Addphy_interface_weight()which will be used in the solution.
- Split out the code that determines the inband "type" for an interface mode.
- Clear the Autoneg bit in the advertising mask, or the Autoneg bit in the support mask and the entire advertising mask if the selected interface mode has no inband capabilties.
Quoted 1 line
[...]
Here is the summary with links:
- [net,1/3] net: phy: add phy_interface_weight()
https://git.kernel.org/netdev/net/c/4beb44a2d62d - [net,2/3] net: phylink: provide phylink_get_inband_type() https://git.kernel.org/netdev/net/c/1bd905dfea98 - [net,3/3] net: phylink: disable autoneg for interfaces that have no inband https://git.kernel.org/netdev/net/c/a21202743f9c
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html