Skip to content

Commit

Permalink
Merge branch 'net-phylink-fix-pcs-without-autoneg'
Browse files Browse the repository at this point in the history
Russell King says:

====================
net: phylink: fix PCS without autoneg

Eric Woudstra reported that a PCS attached using 2500base-X does not
see link when phylink is using in-band mode, but autoneg is disabled,
despite there being a valid 2500base-X signal being received. We have
these settings:

	act_link_an_mode = MLO_AN_INBAND
	pcs_neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED

Eric diagnosed it to phylink_decode_c37_word() setting state->link
false because the full-duplex bit isn't set in the non-existent link
partner advertisement word (which doesn't exist because in-band
autoneg is disabled!)

The test in phylink_mii_c22_pcs_decode_state() is supposed to catch
this state, but since we converted PCS to use neg_mode, testing the
Autoneg in the local advertisement is no longer sufficient - we need
to be looking at the neg_mode, which currently isn't provided.

We need to provide this via the .pcs_get_state() method, and this
will require modifying all PCS implementations to add the extra
argument to this method.

Patch 1 uses the PCS neg_mode in phylink_mac_pcs_get_state() to correct
the now obsolute usage of the Autoneg bit in the advertisement.

Patch 2 passes neg_mode into the .pcs_get_state() method, and updates
all users.

Patch 3 adds neg_mode as an argument to the various clause 22 state
decoder functions in phylink, modifying drivers to pass the neg_mode
through.

Patch 4 makes use of phylink_mii_c22_pcs_decode_state() rather than
using the Autoneg bit in the advertising field.

Patch 5 may be required for Eric's case - it ensures that we report
the correct state for interface types that we support only one set
of modes for when autoneg is disabled.
====================

Link: https://patch.msgid.link/Z4TbR93B-X8A8iHe@shell.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
kuba-moo committed Jan 15, 2025
2 parents 9eb8069 + e432ffc commit e80ed97
Show file tree
Hide file tree
Showing 22 changed files with 87 additions and 42 deletions.
4 changes: 2 additions & 2 deletions drivers/net/dsa/b53/b53_serdes.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ static void b53_serdes_an_restart(struct phylink_pcs *pcs)
SERDES_MII_BLK, reg);
}

static void b53_serdes_get_state(struct phylink_pcs *pcs,
struct phylink_link_state *state)
static void b53_serdes_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct b53_device *dev = pcs_to_b53_pcs(pcs)->dev;
u8 lane = pcs_to_b53_pcs(pcs)->lane;
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/dsa/mt7530.c
Original file line number Diff line number Diff line change
Expand Up @@ -2994,7 +2994,7 @@ static int mt753x_pcs_validate(struct phylink_pcs *pcs,
return 0;
}

static void mt7530_pcs_get_state(struct phylink_pcs *pcs,
static void mt7530_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mt7530_priv *priv = pcs_to_mt753x_pcs(pcs)->priv;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/dsa/mv88e6xxx/pcs-6185.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ static irqreturn_t mv88e6185_pcs_handle_irq(int irq, void *dev_id)
}

static void mv88e6185_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mv88e6185_pcs *mpcs = pcs_to_mv88e6185_pcs(pcs);
Expand Down
1 change: 1 addition & 0 deletions drivers/net/dsa/mv88e6xxx/pcs-6352.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ static void marvell_c22_pcs_disable(struct phylink_pcs *pcs)
}

static void marvell_c22_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct marvell_c22_pcs *mpcs = pcs_to_marvell_c22_pcs(pcs);
Expand Down
5 changes: 4 additions & 1 deletion drivers/net/dsa/mv88e6xxx/pcs-639x.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ static int mv88e639x_sgmii_pcs_post_config(struct phylink_pcs *pcs,
}

static void mv88e639x_sgmii_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mv88e639x_pcs *mpcs = sgmii_pcs_to_mv88e639x_pcs(pcs);
Expand Down Expand Up @@ -395,6 +396,7 @@ static void mv88e639x_xg_pcs_disable(struct mv88e639x_pcs *mpcs)
}

static void mv88e639x_xg_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mv88e639x_pcs *mpcs = xg_pcs_to_mv88e639x_pcs(pcs);
Expand Down Expand Up @@ -889,14 +891,15 @@ static int mv88e6393x_xg_pcs_post_config(struct phylink_pcs *pcs,
}

static void mv88e6393x_xg_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mv88e639x_pcs *mpcs = xg_pcs_to_mv88e639x_pcs(pcs);
u16 status, lp_status;
int err;

if (state->interface != PHY_INTERFACE_MODE_USXGMII)
return mv88e639x_xg_pcs_get_state(pcs, state);
return mv88e639x_xg_pcs_get_state(pcs, neg_mode, state);

state->link = false;

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/dsa/qca/qca8k-8xxx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ static struct qca8k_pcs *pcs_to_qca8k_pcs(struct phylink_pcs *pcs)
return container_of(pcs, struct qca8k_pcs, pcs);
}

static void qca8k_pcs_get_state(struct phylink_pcs *pcs,
static void qca8k_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct qca8k_priv *priv = pcs_to_qca8k_pcs(pcs)->priv;
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/cadence/macb_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ static void macb_usx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
}

static void macb_usx_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct macb *bp = container_of(pcs, struct macb, phylink_usx_pcs);
Expand Down Expand Up @@ -598,7 +599,7 @@ static int macb_usx_pcs_config(struct phylink_pcs *pcs,
return 0;
}

static void macb_pcs_get_state(struct phylink_pcs *pcs,
static void macb_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
state->link = 0;
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/freescale/fman/fman_dtsec.c
Original file line number Diff line number Diff line change
Expand Up @@ -755,12 +755,12 @@ static struct fman_mac *pcs_to_dtsec(struct phylink_pcs *pcs)
return container_of(pcs, struct fman_mac, pcs);
}

static void dtsec_pcs_get_state(struct phylink_pcs *pcs,
static void dtsec_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct fman_mac *dtsec = pcs_to_dtsec(pcs);

phylink_mii_c22_pcs_get_state(dtsec->tbidev, state);
phylink_mii_c22_pcs_get_state(dtsec->tbidev, neg_mode, state);
}

static int dtsec_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/marvell/mvneta.c
Original file line number Diff line number Diff line change
Expand Up @@ -3983,7 +3983,7 @@ static unsigned int mvneta_pcs_inband_caps(struct phylink_pcs *pcs,
return LINK_INBAND_DISABLE;
}

static void mvneta_pcs_get_state(struct phylink_pcs *pcs,
static void mvneta_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mvneta_port *pp = mvneta_pcs_to_port(pcs);
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -6188,6 +6188,7 @@ static struct mvpp2_port *mvpp2_pcs_gmac_to_port(struct phylink_pcs *pcs)
}

static void mvpp2_xlg_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mvpp2_port *port = mvpp2_pcs_xlg_to_port(pcs);
Expand Down Expand Up @@ -6247,6 +6248,7 @@ static unsigned int mvpp2_gmac_pcs_inband_caps(struct phylink_pcs *pcs,
}

static void mvpp2_gmac_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mvpp2_port *port = mvpp2_pcs_gmac_to_port(pcs);
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ethernet/marvell/prestera/prestera_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ prestera_mac_select_pcs(struct phylink_config *config,
}

static void prestera_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct prestera_port *port = container_of(pcs, struct prestera_port,
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fbnic_pcs_to_net(struct phylink_pcs *pcs)
}

static void
fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs,
fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct fbnic_net *fbn = fbnic_pcs_to_net(pcs);
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/microchip/lan966x/lan966x_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ int lan966x_stats_init(struct lan966x *lan966x);

void lan966x_port_config_down(struct lan966x_port *port);
void lan966x_port_config_up(struct lan966x_port *port);
void lan966x_port_status_get(struct lan966x_port *port,
void lan966x_port_status_get(struct lan966x_port *port, unsigned int neg_mode,
struct phylink_link_state *state);
int lan966x_port_pcs_set(struct lan966x_port *port,
struct lan966x_port_config *config);
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,12 @@ static struct lan966x_port *lan966x_pcs_to_port(struct phylink_pcs *pcs)
}

static void lan966x_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct lan966x_port *port = lan966x_pcs_to_port(pcs);

lan966x_port_status_get(port, state);
lan966x_port_status_get(port, neg_mode, state);
}

static int lan966x_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/microchip/lan966x/lan966x_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ void lan966x_port_config_up(struct lan966x_port *port)
lan966x_port_link_up(port);
}

void lan966x_port_status_get(struct lan966x_port *port,
void lan966x_port_status_get(struct lan966x_port *port, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct lan966x *lan966x = port->lan966x;
Expand Down Expand Up @@ -314,7 +314,7 @@ void lan966x_port_status_get(struct lan966x_port *port,
bmsr |= BMSR_ANEGCOMPLETE;

lp_adv = DEV_PCS1G_ANEG_STATUS_LP_ADV_GET(val);
phylink_mii_c22_pcs_decode_state(state, bmsr, lp_adv);
phylink_mii_c22_pcs_decode_state(state, neg_mode, bmsr, lp_adv);
} else {
if (!state->link)
return;
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ static struct sparx5_port *sparx5_pcs_to_port(struct phylink_pcs *pcs)
return container_of(pcs, struct sparx5_port, phylink_pcs);
}

static void sparx5_pcs_get_state(struct phylink_pcs *pcs,
static void sparx5_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct sparx5_port *port = sparx5_pcs_to_port(pcs);
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/xilinx/xilinx_axienet_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2331,11 +2331,12 @@ static struct axienet_local *pcs_to_axienet_local(struct phylink_pcs *pcs)
}

static void axienet_pcs_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mdio_device *pcs_phy = pcs_to_axienet_local(pcs)->pcs_phy;

phylink_mii_c22_pcs_get_state(pcs_phy, state);
phylink_mii_c22_pcs_get_state(pcs_phy, neg_mode, state);
}

static void axienet_pcs_an_restart(struct phylink_pcs *pcs)
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/pcs/pcs-lynx.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ static void lynx_pcs_get_state_2500basex(struct mdio_device *pcs,
state->duplex = DUPLEX_FULL;
}

static void lynx_pcs_get_state(struct phylink_pcs *pcs,
static void lynx_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
Expand All @@ -109,7 +109,7 @@ static void lynx_pcs_get_state(struct phylink_pcs *pcs,
case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_SGMII:
case PHY_INTERFACE_MODE_QSGMII:
phylink_mii_c22_pcs_get_state(lynx->mdio, state);
phylink_mii_c22_pcs_get_state(lynx->mdio, neg_mode, state);
break;
case PHY_INTERFACE_MODE_2500BASEX:
lynx_pcs_get_state_2500basex(lynx->mdio, state);
Expand Down
4 changes: 3 additions & 1 deletion drivers/net/pcs/pcs-mtk-lynxi.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ static unsigned int mtk_pcs_lynxi_inband_caps(struct phylink_pcs *pcs,
}

static void mtk_pcs_lynxi_get_state(struct phylink_pcs *pcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
struct mtk_pcs_lynxi *mpcs = pcs_to_mtk_pcs_lynxi(pcs);
Expand All @@ -114,7 +115,8 @@ static void mtk_pcs_lynxi_get_state(struct phylink_pcs *pcs,
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv);

phylink_mii_c22_pcs_decode_state(state, FIELD_GET(SGMII_BMSR, bm),
phylink_mii_c22_pcs_decode_state(state, neg_mode,
FIELD_GET(SGMII_BMSR, bm),
FIELD_GET(SGMII_LPA, adv));
}

Expand Down
7 changes: 4 additions & 3 deletions drivers/net/pcs/pcs-xpcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,7 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
}

static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
unsigned int neg_mode,
struct phylink_link_state *state)
{
int lpa, bmsr;
Expand Down Expand Up @@ -1058,7 +1059,7 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
}
}

phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
phylink_mii_c22_pcs_decode_state(state, neg_mode, bmsr, lpa);
}

return 0;
Expand Down Expand Up @@ -1086,7 +1087,7 @@ static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
return 0;
}

static void xpcs_get_state(struct phylink_pcs *pcs,
static void xpcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
struct phylink_link_state *state)
{
struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
Expand Down Expand Up @@ -1114,7 +1115,7 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
"xpcs_get_state_c37_sgmii", ERR_PTR(ret));
break;
case DW_AN_C37_1000BASEX:
ret = xpcs_get_state_c37_1000basex(xpcs, state);
ret = xpcs_get_state_c37_1000basex(xpcs, neg_mode, state);
if (ret)
dev_err(&xpcs->mdiodev->dev, "%s returned %pe\n",
"xpcs_get_state_c37_1000basex", ERR_PTR(ret));
Expand Down
Loading

0 comments on commit e80ed97

Please sign in to comment.