[PATCH net] net: pcs: xpcs: fix C73 AN not getting enabled

[PATCH net] net: pcs: xpcs: fix C73 AN not getting enabled

[Vladimir Oltean]

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(-)

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;
-- 
2.34.1

Re: [PATCH net] net: pcs: xpcs: fix C73 AN not getting enabled

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html