| Message ID | 20230516154410.4012337-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c46e78ba9a7a09da4f192dc8df15c4e8a07fb9e0 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: pcs: xpcs: fix C73 AN not getting enabled | expand |
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Tue, 16 May 2023 18:44:10 +0300 you wrote: > The XPCS expects clause 73 (copper backplane) autoneg to follow the > ethtool autoneg bit. It actually did that until the blamed > commit inaptly replaced state->an_enabled (coming from ethtool) with > phylink_autoneg_inband() (coming from the device tree or struct > phylink_config), as part of an unrelated phylink_pcs API conversion. > > Russell King suggests that state->an_enabled from the original code was > just a proxy for the ethtool Autoneg bit, and that the correct way of > restoring the functionality is to check for this bit in the advertising > mask. > > [...] Here is the summary with links: - [net] net: pcs: xpcs: fix C73 AN not getting enabled https://git.kernel.org/netdev/net/c/c46e78ba9a7a You are awesome, thank you!
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c index 539cd43eae8d..f680d03863ff 100644 --- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c @@ -873,7 +873,7 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface, switch (compat->an_mode) { case DW_AN_C73: - if (phylink_autoneg_inband(mode)) { + if (test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising)) { ret = xpcs_config_aneg_c73(xpcs, compat); if (ret) return ret;
The XPCS expects clause 73 (copper backplane) autoneg to follow the ethtool autoneg bit. It actually did that until the blamed commit inaptly replaced state->an_enabled (coming from ethtool) with phylink_autoneg_inband() (coming from the device tree or struct phylink_config), as part of an unrelated phylink_pcs API conversion. Russell King suggests that state->an_enabled from the original code was just a proxy for the ethtool Autoneg bit, and that the correct way of restoring the functionality is to check for this bit in the advertising mask. Fixes: 11059740e616 ("net: pcs: xpcs: convert to phylink_pcs_ops") Link: https://lore.kernel.org/netdev/ZGNt2MFeRolKGFck@shell.armlinux.org.uk/ Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- The only (paranoid) test I've done is that the sja1105 driver (which also calls xpcs_do_config() outside of phylink, and provides a NULL pointer for "advertising") does not crash. Which was completely to be expected, since none of the nxp_sja1105 XPCS compatible modes uses DW_AN_C73. drivers/net/pcs/pcs-xpcs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)