Commit 018c00dd authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'add-and-use-helper-for-pcs-negotiation-modes'

Russell King says:

====================
Add and use helper for PCS negotiation modes

Earlier this month, I proposed a helper for deciding whether a PCS
should use inband negotiation modes or not. There was some discussion
around this topic, and I believe there was no disagreement about
providing the helper.

The initial discussion can be found at:

https://lore.kernel.org/r/ZGIkGmyL8yL1q1zp@shell.armlinux.org.uk

Subsequently, I posted a RFC series back in May:

https://lore.kernel.org/r/ZGzhvePzPjJ0v2En@shell.armlinux.org.uk

that added a helper, phylink_pcs_neg_mode() which PCS drivers could use
to parse the state, and updated a bunch of drivers to use it. I got
a couple of bits of feedback to it, including some ACKs.

However, I've decided to take this slightly further and change the
"mode" parameter to both the pcs_config() and pcs_link_up() methods
when a PCS driver opts in to this (by setting "neg_mode" in the
phylink_pcs structure.) If this is not set, we default to the old
behaviour. That said, this series converts all the PCS implementations
I can find currently in net-next.

Doing this has the added benefit that the negotiation mode parameter
is also available to the pcs_link_up() function, which can now know
whether inband negotiation was in fact enabled or not at pcs_config()
time.

It has been posted as RFC at:

https://lore.kernel.org/r/ZIh/CLQ3z89g0Ua0@shell.armlinux.org.uk

and received one reply, thanks Elad, which is a similar amount of
interest to previous postings. Let's post it as non-RFC and see
whether we get more reaction.
====================

Link: https://lore.kernel.org/r/ZIxQIBfO9dH5xFlg@shell.armlinux.org.uk


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 84ef94d9 f40df95d
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -65,7 +65,7 @@ static u16 b53_serdes_read(struct b53_device *dev, u8 lane,
	return b53_serdes_read_blk(dev, offset, block);
}

static int b53_serdes_config(struct phylink_pcs *pcs, unsigned int mode,
static int b53_serdes_config(struct phylink_pcs *pcs, unsigned int neg_mode,
			     phy_interface_t interface,
			     const unsigned long *advertising,
			     bool permit_pause_to_mac)
@@ -239,6 +239,7 @@ int b53_serdes_init(struct b53_device *dev, int port)
	pcs->dev = dev;
	pcs->lane = lane;
	pcs->pcs.ops = &b53_pcs_ops;
	pcs->pcs.neg_mode = true;

	return 0;
}
+2 −1
Original line number Diff line number Diff line
@@ -3005,7 +3005,7 @@ static void mt7530_pcs_get_state(struct phylink_pcs *pcs,
		state->pause |= MLO_PAUSE_TX;
}

static int mt753x_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
static int mt753x_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
			     phy_interface_t interface,
			     const unsigned long *advertising,
			     bool permit_pause_to_mac)
@@ -3033,6 +3033,7 @@ mt753x_setup(struct dsa_switch *ds)
	/* Initialise the PCS devices */
	for (i = 0; i < priv->ds->num_ports; i++) {
		priv->pcs[i].pcs.ops = priv->info->pcs_ops;
		priv->pcs[i].pcs.neg_mode = true;
		priv->pcs[i].priv = priv;
		priv->pcs[i].port = i;
	}
+6 −7
Original line number Diff line number Diff line
@@ -1493,7 +1493,7 @@ static void qca8k_pcs_get_state(struct phylink_pcs *pcs,
		state->pause |= MLO_PAUSE_TX;
}

static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
			    phy_interface_t interface,
			    const unsigned long *advertising,
			    bool permit_pause_to_mac)
@@ -1520,14 +1520,12 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
	}

	/* Enable/disable SerDes auto-negotiation as necessary */
	ret = qca8k_read(priv, QCA8K_REG_PWS, &val);
	val = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED ?
		0 : QCA8K_PWS_SERDES_AEN_DIS;

	ret = qca8k_rmw(priv, QCA8K_REG_PWS, QCA8K_PWS_SERDES_AEN_DIS, val);
	if (ret)
		return ret;
	if (phylink_autoneg_inband(mode))
		val &= ~QCA8K_PWS_SERDES_AEN_DIS;
	else
		val |= QCA8K_PWS_SERDES_AEN_DIS;
	qca8k_write(priv, QCA8K_REG_PWS, val);

	/* Configure the SGMII parameters */
	ret = qca8k_read(priv, QCA8K_REG_SGMII_CTRL, &val);
@@ -1598,6 +1596,7 @@ static void qca8k_setup_pcs(struct qca8k_priv *priv, struct qca8k_pcs *qpcs,
			    int port)
{
	qpcs->pcs.ops = &qca8k_pcs_ops;
	qpcs->pcs.neg_mode = true;

	/* We don't have interrupts for link changes, so we need to poll */
	qpcs->pcs.poll = true;
+6 −8
Original line number Diff line number Diff line
@@ -2314,7 +2314,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,

	for (i = 0; i < ds->num_ports; i++) {
		struct dw_xpcs *xpcs = priv->xpcs[i];
		unsigned int mode;
		unsigned int neg_mode;

		rc = sja1105_adjust_port_config(priv, i, speed_mbps[i]);
		if (rc < 0)
@@ -2324,17 +2324,15 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
			continue;

		if (bmcr[i] & BMCR_ANENABLE)
			mode = MLO_AN_INBAND;
		else if (priv->fixed_link[i])
			mode = MLO_AN_FIXED;
			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
		else
			mode = MLO_AN_PHY;
			neg_mode = PHYLINK_PCS_NEG_OUTBAND;

		rc = xpcs_do_config(xpcs, priv->phy_mode[i], mode, NULL);
		rc = xpcs_do_config(xpcs, priv->phy_mode[i], NULL, neg_mode);
		if (rc < 0)
			goto out;

		if (!phylink_autoneg_inband(mode)) {
		if (neg_mode == PHYLINK_PCS_NEG_OUTBAND) {
			int speed = SPEED_UNKNOWN;

			if (priv->phy_mode[i] == PHY_INTERFACE_MODE_2500BASEX)
@@ -2346,7 +2344,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
			else
				speed = SPEED_10;

			xpcs_link_up(&xpcs->pcs, mode, priv->phy_mode[i],
			xpcs_link_up(&xpcs->pcs, neg_mode, priv->phy_mode[i],
				     speed, DUPLEX_FULL);
		}
	}
+5 −3
Original line number Diff line number Diff line
@@ -563,7 +563,7 @@ static void macb_set_tx_clk(struct macb *bp, int speed)
		netdev_err(bp->dev, "adjusting tx_clk failed.\n");
}

static void macb_usx_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
static void macb_usx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
				 phy_interface_t interface, int speed,
				 int duplex)
{
@@ -596,7 +596,7 @@ static void macb_usx_pcs_get_state(struct phylink_pcs *pcs,
}

static int macb_usx_pcs_config(struct phylink_pcs *pcs,
			       unsigned int mode,
			       unsigned int neg_mode,
			       phy_interface_t interface,
			       const unsigned long *advertising,
			       bool permit_pause_to_mac)
@@ -621,7 +621,7 @@ static void macb_pcs_an_restart(struct phylink_pcs *pcs)
}

static int macb_pcs_config(struct phylink_pcs *pcs,
			   unsigned int mode,
			   unsigned int neg_mode,
			   phy_interface_t interface,
			   const unsigned long *advertising,
			   bool permit_pause_to_mac)
@@ -862,7 +862,9 @@ static int macb_mii_probe(struct net_device *dev)
	struct macb *bp = netdev_priv(dev);

	bp->phylink_sgmii_pcs.ops = &macb_phylink_pcs_ops;
	bp->phylink_sgmii_pcs.neg_mode = true;
	bp->phylink_usx_pcs.ops = &macb_phylink_usx_pcs_ops;
	bp->phylink_usx_pcs.neg_mode = true;

	bp->phylink_config.dev = &dev->dev;
	bp->phylink_config.type = PHYLINK_NETDEV;
Loading