| Message ID | E1tVuG1-000BXo-S7@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: phylink: fix PCS without autoneg | expand |
On 1/9/25 4:15 PM, Russell King (Oracle) wrote: > When decoding clause 22 state, if in-band is disabled and using either > 1000base-X or 2500base-X, rather than reporting link-down, we know the > speed, and we only support full duplex. Pause modes taken from XPCS. > > This fixes a problem reported by Eric Woudstra. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/phy/phylink.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c After changing 'if (pcs->neg_mode)' to 'if (pcs && pcs->neg_mode)' in patch 1/5, I have tested this patch-set and I get link up. Tested-by: Eric Woudstra <ericwouds@gmail.com>
On Fri, Jan 10, 2025 at 09:04:56AM +0100, Eric Woudstra wrote: > > On 1/9/25 4:15 PM, Russell King (Oracle) wrote: > > When decoding clause 22 state, if in-band is disabled and using either > > 1000base-X or 2500base-X, rather than reporting link-down, we know the > > speed, and we only support full duplex. Pause modes taken from XPCS. > > > > This fixes a problem reported by Eric Woudstra. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/phy/phylink.c | 29 +++++++++++++++++++---------- > > 1 file changed, 19 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > After changing 'if (pcs->neg_mode)' to 'if (pcs && pcs->neg_mode)' in > patch 1/5, I have tested this patch-set and I get link up. > > Tested-by: Eric Woudstra <ericwouds@gmail.com> Thanks Eric. Much appreciate your patience with this tangent to the issue you have - your report highlighted that there was this other bug that needed fixing in addition to the problem you were experiencing. I've fixed that slightly differently (as below) and I'll post a v2 shortly. + if (!pcs || pcs->neg_mode) + autoneg = pl->pcs_neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED; + else + autoneg = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + state->advertising); there, since the "else" clause is the legacy case. I doubt that makes any difference to your testing scenario, but please let me know if you want to re-test with that before I add your t-b. Next, we need to address your problem properly... I'll be looking at that today.
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 5174c22e2091..0ae96d1376b4 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -3882,27 +3882,36 @@ void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state, if (!state->link) return; - /* If in-band is disabled, then the advertisement data is not - * meaningful. - */ - if (neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) - return; - switch (state->interface) { case PHY_INTERFACE_MODE_1000BASEX: - phylink_decode_c37_word(state, lpa, SPEED_1000); + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) { + phylink_decode_c37_word(state, lpa, SPEED_1000); + } else { + state->speed = SPEED_1000; + state->duplex = DUPLEX_FULL; + state->pause |= MLO_PAUSE_TX | MLO_PAUSE_RX; + } break; case PHY_INTERFACE_MODE_2500BASEX: - phylink_decode_c37_word(state, lpa, SPEED_2500); + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) { + phylink_decode_c37_word(state, lpa, SPEED_2500); + } else { + state->speed = SPEED_2500; + state->duplex = DUPLEX_FULL; + state->pause |= MLO_PAUSE_TX | MLO_PAUSE_RX; + } break; case PHY_INTERFACE_MODE_SGMII: case PHY_INTERFACE_MODE_QSGMII: - phylink_decode_sgmii_word(state, lpa); + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) + phylink_decode_sgmii_word(state, lpa); break; + case PHY_INTERFACE_MODE_QUSGMII: - phylink_decode_usgmii_word(state, lpa); + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) + phylink_decode_usgmii_word(state, lpa); break; default:
When decoding clause 22 state, if in-band is disabled and using either 1000base-X or 2500base-X, rather than reporting link-down, we know the speed, and we only support full duplex. Pause modes taken from XPCS. This fixes a problem reported by Eric Woudstra. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/phylink.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)