[PATCH RFC 0/9] Add and use helper for PCS negotiation modes

[PATCH RFC 0/9] Add and use helper for PCS negotiation modes

[Russell King (Oracle)]

Hi,

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 discussion can be found at:

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

This series adds that helper, and modifies most code to use it. I have
a couple of further patches that hoist this function out of every PCS
driver and into phylink's new phylink_pcs_config() function that I've
posted separately, and drop the "mode" argument to the pcs_config()
method, instead passing the result of phylink_pcs_neg_mode().

I haven't included those because this series doesn't update everything
in net-next, but for RFC purposes, I think this is good enough to get
a few whether people are generally happy or not.

Note that this helper is only about modes that affect the PCS such as
the SGMII family and 802.3z types, not amount negotiation that happens
in order to select a PCS (e.g. for backplanes.)

 drivers/net/dsa/qca/qca8k-8xxx.c                   | 13 ++--
 drivers/net/ethernet/freescale/fman/fman_dtsec.c   |  7 +-
 drivers/net/ethernet/marvell/mvneta.c              |  5 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  4 +-
 .../net/ethernet/marvell/prestera/prestera_main.c  | 10 ++-
 .../ethernet/microchip/lan966x/lan966x_phylink.c   |  8 ++-
 .../net/ethernet/microchip/sparx5/sparx5_phylink.c |  8 ++-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c  |  5 +-
 drivers/net/pcs/pcs-lynx.c                         | 18 +++--
 drivers/net/phy/phylink.c                          | 14 ++--
 include/linux/phylink.h                            | 81 +++++++++++++++++++++-
 11 files changed, 136 insertions(+), 37 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH RFC net-next 1/9] net: phylink: add phylink_pcs_neg_mode()

[Russell King (Oracle)]

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 include/linux/phylink.h | 76 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 0cf07d7d11b8..e3cb0b4eed1d 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -21,6 +21,18 @@ enum {
 	MLO_AN_FIXED,	/* Fixed-link mode */
 	MLO_AN_INBAND,	/* In-band protocol */
 
+	/* PCS "negotiation" mode.
+	 *  PHYLINK_PCS_NEG_NONE - protocol has no inband capability
+	 *  PHYLINK_PCS_NEG_OUTBAND - some out of band or fixed link setting
+	 *  PHYLINK_PCS_NEG_INBAND_DISABLED - inband mode disabled, e.g.
+	 *				      1000base-X with autoneg off
+	 *  PHYLINK_PCS_NEG_INBAND_ENABLED - inband mode enabled
+	 */
+	PHYLINK_PCS_NEG_NONE = 0,
+	PHYLINK_PCS_NEG_OUTBAND,
+	PHYLINK_PCS_NEG_INBAND_DISABLED,
+	PHYLINK_PCS_NEG_INBAND_ENABLED,
+
 	/* MAC_SYM_PAUSE and MAC_ASYM_PAUSE are used when configuring our
 	 * autonegotiation advertisement. They correspond to the PAUSE and
 	 * ASM_DIR bits defined by 802.3, respectively.
@@ -79,6 +91,70 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
 	return mode == MLO_AN_INBAND;
 }
 
+/**
+ * phylink_pcs_neg_mode() - helper to determine PCS inband mode
+ * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
+ * @interface: interface mode to be used
+ * @advertising: adertisement ethtool link mode mask
+ *
+ * Determines the negotiation mode to be used by the PCS, and returns
+ * one of:
+ * %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
+ * %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
+ *   will be used.
+ * %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg disabled
+ * %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
+ *
+ * Note: this is for cases where the PCS itself is involved in negotiation
+ * (e.g. Clause 37, SGMII and similar) not Clause 73.
+ */
+static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
+						phy_interface_t interface,
+						const unsigned long *advertising)
+{
+	unsigned int neg_mode;
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_QSGMII:
+	case PHY_INTERFACE_MODE_QUSGMII:
+	case PHY_INTERFACE_MODE_USXGMII:
+		/* These protocols are designed for use with a PHY which
+		 * communicates its negotiation result back to the MAC via
+		 * inband communication. Note: there exist PHYs that run
+		 * with SGMII but do not send the inband data.
+		 */
+		if (!phylink_autoneg_inband(mode))
+			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
+		else
+			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
+		break;
+
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		/* 1000base-X is designed for use media-side for Fibre
+		 * connections, and thus the Autoneg bit needs to be
+		 * taken into account. We also do this for 2500base-X
+		 * as well, but drivers may not support this, so may
+		 * need to override this.
+		 */
+		if (!phylink_autoneg_inband(mode))
+			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
+		else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+					   advertising))
+			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
+		else
+			neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
+		break;
+
+	default:
+		neg_mode = PHYLINK_PCS_NEG_NONE;
+		break;
+	}
+
+	return neg_mode;
+}
+
 /**
  * struct phylink_link_state - link state structure
  * @advertising: ethtool bitmask containing advertised link modes
-- 
2.30.2

[PATCH RFC net-next 2/9] net: phylink: use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use phylink_pcs_neg_mode() for phylink_mii_c22_pcs_config(). This
results in no functional change.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 530bfafd4f81..c800bdcddeb8 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3522,6 +3522,7 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
 			       phy_interface_t interface,
 			       const unsigned long *advertising)
 {
+	unsigned int neg_mode;
 	bool changed = 0;
 	u16 bmcr;
 	int ret, adv;
@@ -3535,15 +3536,13 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
 		changed = ret;
 	}
 
-	/* Ensure ISOLATE bit is disabled */
-	if (mode == MLO_AN_INBAND &&
-	    (interface == PHY_INTERFACE_MODE_SGMII ||
-	     interface == PHY_INTERFACE_MODE_QSGMII ||
-	     linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising)))
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 		bmcr = BMCR_ANENABLE;
 	else
 		bmcr = 0;
 
+	/* Configure the inband state. Ensure ISOLATE bit is disabled */
 	ret = mdiodev_modify(pcs, MII_BMCR, BMCR_ANENABLE | BMCR_ISOLATE, bmcr);
 	if (ret < 0)
 		return ret;
-- 
2.30.2

[PATCH RFC net-next 3/9] net: phylink: pass aneg_mode into phylink_mii_c22_pcs_config()

[Russell King (Oracle)]

Pass the aneg_mode into phylink_mii_c22_pcs_config() in preparation
to moving it up as a parameter passed to the pcs_config() method.
In doing so, we no longer need to pass the MLO_AN_* mode to this
function.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/freescale/fman/fman_dtsec.c   |  7 +++++--
 .../net/ethernet/xilinx/xilinx_axienet_main.c  |  5 ++++-
 drivers/net/pcs/pcs-lynx.c                     | 18 ++++++++++++------
 drivers/net/phy/phylink.c                      |  9 ++++-----
 include/linux/phylink.h                        |  5 +++--
 5 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index d528ca681b6f..e6a5a7f03dbe 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
@@ -769,9 +769,12 @@ static int dtsec_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 			    bool permit_pause_to_mac)
 {
 	struct fman_mac *dtsec = pcs_to_dtsec(pcs);
+	unsigned int neg_mode;
 
-	return phylink_mii_c22_pcs_config(dtsec->tbidev, mode, interface,
-					  advertising);
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
+	return phylink_mii_c22_pcs_config(dtsec->tbidev, interface,
+					  advertising, neg_mode);
 }
 
 static void dtsec_pcs_an_restart(struct phylink_pcs *pcs)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 3e310b55bce2..380a4db686b7 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1639,6 +1639,7 @@ static int axienet_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 	struct mdio_device *pcs_phy = pcs_to_axienet_local(pcs)->pcs_phy;
 	struct net_device *ndev = pcs_to_axienet_local(pcs)->ndev;
 	struct axienet_local *lp = netdev_priv(ndev);
+	unsigned int neg_mode;
 	int ret;
 
 	if (lp->switch_x_sgmii) {
@@ -1653,7 +1654,9 @@ static int axienet_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 		}
 	}
 
-	ret = phylink_mii_c22_pcs_config(pcs_phy, mode, interface, advertising);
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+	ret = phylink_mii_c22_pcs_config(pcs_phy, interface, advertising,
+					 neg_mode);
 	if (ret < 0)
 		netdev_warn(ndev, "Failed to configure PCS: %d\n", ret);
 
diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index 622c3de3f3a8..5a0de1fcb833 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -119,9 +119,10 @@ static void lynx_pcs_get_state(struct phylink_pcs *pcs,
 		state->link, state->an_complete);
 }
 
-static int lynx_pcs_config_giga(struct mdio_device *pcs, unsigned int mode,
+static int lynx_pcs_config_giga(struct mdio_device *pcs,
 				phy_interface_t interface,
-				const unsigned long *advertising)
+				const unsigned long *advertising,
+				unsigned int neg_mode)
 {
 	int link_timer_ns;
 	u32 link_timer;
@@ -139,8 +140,9 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs, unsigned int mode,
 	if (interface == PHY_INTERFACE_MODE_1000BASEX) {
 		if_mode = 0;
 	} else {
+		/* SGMII and QSGMII */
 		if_mode = IF_MODE_SGMII_EN;
-		if (mode == MLO_AN_INBAND)
+		if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 			if_mode |= IF_MODE_USE_SGMII_AN;
 	}
 
@@ -150,7 +152,8 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs, unsigned int mode,
 	if (err)
 		return err;
 
-	return phylink_mii_c22_pcs_config(pcs, mode, interface, advertising);
+	return phylink_mii_c22_pcs_config(pcs, interface, advertising,
+					  neg_mode);
 }
 
 static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, unsigned int mode,
@@ -177,13 +180,16 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 			   bool permit)
 {
 	struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+	unsigned int neg_mode;
+
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
 
 	switch (ifmode) {
 	case PHY_INTERFACE_MODE_1000BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
-		return lynx_pcs_config_giga(lynx->mdio, mode, ifmode,
-					    advertising);
+		return lynx_pcs_config_giga(lynx->mdio, ifmode, advertising,
+					    neg_mode);
 	case PHY_INTERFACE_MODE_2500BASEX:
 		if (phylink_autoneg_inband(mode)) {
 			dev_err(&lynx->mdio->dev,
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index c800bdcddeb8..03e4ff8bf289 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3509,20 +3509,20 @@ EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_encode_advertisement);
 /**
  * phylink_mii_c22_pcs_config() - configure clause 22 PCS
  * @pcs: a pointer to a &struct mdio_device.
- * @mode: link autonegotiation mode
  * @interface: the PHY interface mode being configured
  * @advertising: the ethtool advertisement mask
+ * @neg_mode: PCS negotiation mode
  *
  * Configure a Clause 22 PCS PHY with the appropriate negotiation
  * parameters for the @mode, @interface and @advertising parameters.
  * Returns negative error number on failure, zero if the advertisement
  * has not changed, or positive if there is a change.
  */
-int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
+int phylink_mii_c22_pcs_config(struct mdio_device *pcs,
 			       phy_interface_t interface,
-			       const unsigned long *advertising)
+			       const unsigned long *advertising,
+			       unsigned int neg_mode)
 {
-	unsigned int neg_mode;
 	bool changed = 0;
 	u16 bmcr;
 	int ret, adv;
@@ -3536,7 +3536,6 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
 		changed = ret;
 	}
 
-	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
 	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 		bmcr = BMCR_ANENABLE;
 	else
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index e3cb0b4eed1d..8510c922b3f4 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -727,9 +727,10 @@ void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
 				   struct phylink_link_state *state);
 int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
 					     const unsigned long *advertising);
-int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
+int phylink_mii_c22_pcs_config(struct mdio_device *pcs,
 			       phy_interface_t interface,
-			       const unsigned long *advertising);
+			       const unsigned long *advertising,
+			       unsigned int neg_mode);
 void phylink_mii_c22_pcs_an_restart(struct mdio_device *pcs);
 
 void phylink_resolve_c73(struct phylink_link_state *state);
-- 
2.30.2

[PATCH RFC net-next 4/9] net: qca8k: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 6d5ac7588a69..acede9f3c253 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1500,6 +1500,7 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 {
 	struct qca8k_priv *priv = pcs_to_qca8k_pcs(pcs)->priv;
 	int cpu_port_index, ret, port;
+	unsigned int neg_mode;
 	u32 reg, val;
 
 	port = pcs_to_qca8k_pcs(pcs)->port;
@@ -1519,15 +1520,15 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 		return -EINVAL;
 	}
 
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
 	/* 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);
-- 
2.30.2

[PATCH RFC net-next 5/9] net: mvneta: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e2abc00d0472..d669276639b1 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4009,6 +4009,7 @@ static int mvneta_pcs_config(struct phylink_pcs *pcs,
 {
 	struct mvneta_port *pp = mvneta_pcs_to_port(pcs);
 	u32 mask, val, an, old_an, changed;
+	unsigned int neg_mode;
 
 	mask = MVNETA_GMAC_INBAND_AN_ENABLE |
 	       MVNETA_GMAC_INBAND_RESTART_AN |
@@ -4016,7 +4017,9 @@ static int mvneta_pcs_config(struct phylink_pcs *pcs,
 	       MVNETA_GMAC_AN_FLOW_CTRL_EN |
 	       MVNETA_GMAC_AN_DUPLEX_EN;
 
-	if (phylink_autoneg_inband(mode)) {
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
+	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
 		mask |= MVNETA_GMAC_CONFIG_MII_SPEED |
 			MVNETA_GMAC_CONFIG_GMII_SPEED |
 			MVNETA_GMAC_CONFIG_FULL_DUPLEX;
-- 
2.30.2

[PATCH RFC net-next 6/9] net: mvpp2: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index adc953611913..9ec806d6cd20 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6239,6 +6239,7 @@ static int mvpp2_gmac_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 {
 	struct mvpp2_port *port = mvpp2_pcs_gmac_to_port(pcs);
 	u32 mask, val, an, old_an, changed;
+	unsigned int neg_mode;
 
 	mask = MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS |
 	       MVPP2_GMAC_IN_BAND_AUTONEG |
@@ -6246,7 +6247,8 @@ static int mvpp2_gmac_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
 	       MVPP2_GMAC_FLOW_CTRL_AUTONEG |
 	       MVPP2_GMAC_AN_DUPLEX_EN;
 
-	if (phylink_autoneg_inband(mode)) {
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
 		mask |= MVPP2_GMAC_CONFIG_MII_SPEED |
 			MVPP2_GMAC_CONFIG_GMII_SPEED |
 			MVPP2_GMAC_CONFIG_FULL_DUPLEX;
-- 
2.30.2

[PATCH RFC net-next 7/9] net: prestera: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/prestera/prestera_main.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 9d504142e51a..2a26f96fbed2 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -308,38 +308,36 @@ static int prestera_pcs_config(struct phylink_pcs *pcs,
 {
 	struct prestera_port *port = prestera_pcs_to_port(pcs);
 	struct prestera_port_mac_config cfg_mac;
+	unsigned int neg_mode;
 	int err;
 
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
 	err = prestera_port_cfg_mac_read(port, &cfg_mac);
 	if (err)
 		return err;
 
 	cfg_mac.admin = true;
 	cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
+	cfg_mac.inband = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
 
 	switch (interface) {
 	case PHY_INTERFACE_MODE_10GBASER:
 		cfg_mac.speed = SPEED_10000;
-		cfg_mac.inband = 0;
 		cfg_mac.mode = PRESTERA_MAC_MODE_SR_LR;
 		break;
 	case PHY_INTERFACE_MODE_2500BASEX:
 		cfg_mac.speed = SPEED_2500;
 		cfg_mac.duplex = DUPLEX_FULL;
-		cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
-					  advertising);
 		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
-		cfg_mac.inband = 1;
 		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
 		break;
 	case PHY_INTERFACE_MODE_1000BASEX:
 	default:
 		cfg_mac.speed = SPEED_1000;
 		cfg_mac.duplex = DUPLEX_FULL;
-		cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
-					  advertising);
 		cfg_mac.mode = PRESTERA_MAC_MODE_1000BASE_X;
 		break;
 	}
-- 
2.30.2

RE: [EXT] [PATCH RFC net-next 7/9] net: prestera: switch PCS driver to use phylink_pcs_neg_mode()

[Elad Nachman]

> -----Original Message-----
> From: Russell King <rmk@armlinux.org.uk> On Behalf Of Russell King (Oracle)
> Sent: Tuesday, May 23, 2023 6:56 PM
> To: Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit
> <hkallweit1@gmail.com>
> Cc: Daniel Machon <daniel.machon@microchip.com>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Florian
> Fainelli <f.fainelli@gmail.com>; Horatiu Vultur
> <horatiu.vultur@microchip.com>; Ioana Ciornei <ioana.ciornei@nxp.com>;
> Jakub Kicinski <kuba@kernel.org>; Lars Povlsen
> <lars.povlsen@microchip.com>; linux-arm-kernel@lists.infradead.org;
> Madalin Bucur <madalin.bucur@nxp.com>; Marcin Wojtas
> <mw@semihalf.com>; Michal Simek <michal.simek@amd.com>;
> netdev@vger.kernel.org; Paolo Abeni <pabeni@redhat.com>; Radhey Shyam
> Pandey <radhey.shyam.pandey@xilinx.com>; Sean Anderson
> <sean.anderson@seco.com>; Steen Hegelund
> <Steen.Hegelund@microchip.com>; Taras Chornyi
> <taras.chornyi@plvision.eu>; Thomas Petazzoni
> <thomas.petazzoni@bootlin.com>; UNGLinuxDriver@microchip.com;
> Vladimir Oltean <olteanv@gmail.com>
> Subject: [EXT] [PATCH RFC net-next 7/9] net: prestera: switch PCS driver to
> use phylink_pcs_neg_mode()
> 
> External Email
> 
> ----------------------------------------------------------------------
> Use the newly introduced phylink_pcs_neg_mode() to configure whether
> inband-AN should be used.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/prestera/prestera_main.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index 9d504142e51a..2a26f96fbed2 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -308,38 +308,36 @@ static int prestera_pcs_config(struct phylink_pcs
> *pcs,  {
>  	struct prestera_port *port = prestera_pcs_to_port(pcs);
>  	struct prestera_port_mac_config cfg_mac;
> +	unsigned int neg_mode;
>  	int err;
> 
> +	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
> +
>  	err = prestera_port_cfg_mac_read(port, &cfg_mac);
>  	if (err)
>  		return err;
> 
>  	cfg_mac.admin = true;
>  	cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
> +	cfg_mac.inband = neg_mode ==
> PHYLINK_PCS_NEG_INBAND_ENABLED;
> 
>  	switch (interface) {
>  	case PHY_INTERFACE_MODE_10GBASER:
>  		cfg_mac.speed = SPEED_10000;
> -		cfg_mac.inband = 0;
>  		cfg_mac.mode = PRESTERA_MAC_MODE_SR_LR;
>  		break;
>  	case PHY_INTERFACE_MODE_2500BASEX:
>  		cfg_mac.speed = SPEED_2500;
>  		cfg_mac.duplex = DUPLEX_FULL;
> -		cfg_mac.inband =
> test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> -					  advertising);
>  		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
>  		break;
>  	case PHY_INTERFACE_MODE_SGMII:
> -		cfg_mac.inband = 1;
>  		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
>  		break;
>  	case PHY_INTERFACE_MODE_1000BASEX:
>  	default:
>  		cfg_mac.speed = SPEED_1000;
>  		cfg_mac.duplex = DUPLEX_FULL;
> -		cfg_mac.inband =
> test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> -					  advertising);
>  		cfg_mac.mode = PRESTERA_MAC_MODE_1000BASE_X;
>  		break;
>  	}
> --
> 2.30.2
> 

Acked-by: Elad Nachman <enachman@marvell.com>

[PATCH RFC net-next 8/9] net: lan966x: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
index c5f9803e6e63..29bbd642cec8 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
@@ -103,12 +103,16 @@ static int lan966x_pcs_config(struct phylink_pcs *pcs,
 {
 	struct lan966x_port *port = lan966x_pcs_to_port(pcs);
 	struct lan966x_port_config config;
+	unsigned int neg_mode;
 	int ret;
 
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
 	config = port->config;
 	config.portmode = interface;
-	config.inband = phylink_autoneg_inband(mode);
-	config.autoneg = phylink_test(advertising, Autoneg);
+	config.inband = neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED ||
+			neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+	config.autoneg = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
 	config.advertising = advertising;
 
 	ret = lan966x_port_pcs_set(port, &config);
-- 
2.30.2

[PATCH RFC net-next 9/9] net: sparx5: switch PCS driver to use phylink_pcs_neg_mode()

[Russell King (Oracle)]

Use the newly introduced phylink_pcs_neg_mode() to configure whether
inband-AN should be used.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
index bb97d27a1da4..87bdec185383 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
@@ -99,13 +99,17 @@ static int sparx5_pcs_config(struct phylink_pcs *pcs,
 {
 	struct sparx5_port *port = sparx5_pcs_to_port(pcs);
 	struct sparx5_port_config conf;
+	unsigned int neg_mode;
 	int ret = 0;
 
+	neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
+
 	conf = port->conf;
 	conf.power_down = false;
 	conf.portmode = interface;
-	conf.inband = phylink_autoneg_inband(mode);
-	conf.autoneg = phylink_test(advertising, Autoneg);
+	conf.inband = neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED ||
+		      neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+	conf.autoneg = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
 	conf.pause_adv = 0;
 	if (phylink_test(advertising, Pause))
 		conf.pause_adv |= ADVERTISE_1000XPAUSE;
-- 
2.30.2

Re: [PATCH RFC net-next 9/9] net: sparx5: switch PCS driver to use phylink_pcs_neg_mode()

[Daniel Machon]

> Use the newly introduced phylink_pcs_neg_mode() to configure whether
> inband-AN should be used.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> index bb97d27a1da4..87bdec185383 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
> @@ -99,13 +99,17 @@ static int sparx5_pcs_config(struct phylink_pcs *pcs,
>  {
>         struct sparx5_port *port = sparx5_pcs_to_port(pcs);
>         struct sparx5_port_config conf;
> +       unsigned int neg_mode;
>         int ret = 0;
> 
> +       neg_mode = phylink_pcs_neg_mode(mode, interface, advertising);
> +
>         conf = port->conf;
>         conf.power_down = false;
>         conf.portmode = interface;
> -       conf.inband = phylink_autoneg_inband(mode);
> -       conf.autoneg = phylink_test(advertising, Autoneg);
> +       conf.inband = neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED ||
> +                     neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
> +       conf.autoneg = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
>         conf.pause_adv = 0;
>         if (phylink_test(advertising, Pause))
>                 conf.pause_adv |= ADVERTISE_1000XPAUSE;
> --
> 2.30.2
> 
>

Hi Russel,

This looks good to me. Tested on sparx5 pcb134 and pcb135.

/Daniel

Re: [PATCH RFC 0/9] Add and use helper for PCS negotiation modes

[Horatiu Vultur]

The 05/23/2023 16:54, Russell King (Oracle) wrote:

Hi Russell,

I have tried this series on lan966x and it seems to be working fine.
There was a small issue applying the patch 3, as the function
'phylink_resolve_c73' doesn't exist yet.

> 
> Hi,
> 
> 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 discussion can be found at:
> 
> https://lore.kernel.org/r/ZGIkGmyL8yL1q1zp@shell.armlinux.org.uk
> 
> This series adds that helper, and modifies most code to use it. I have
> a couple of further patches that hoist this function out of every PCS
> driver and into phylink's new phylink_pcs_config() function that I've
> posted separately, and drop the "mode" argument to the pcs_config()
> method, instead passing the result of phylink_pcs_neg_mode().
> 
> I haven't included those because this series doesn't update everything
> in net-next, but for RFC purposes, I think this is good enough to get
> a few whether people are generally happy or not.
> 
> Note that this helper is only about modes that affect the PCS such as
> the SGMII family and 802.3z types, not amount negotiation that happens
> in order to select a PCS (e.g. for backplanes.)
> 
>  drivers/net/dsa/qca/qca8k-8xxx.c                   | 13 ++--
>  drivers/net/ethernet/freescale/fman/fman_dtsec.c   |  7 +-
>  drivers/net/ethernet/marvell/mvneta.c              |  5 +-
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  4 +-
>  .../net/ethernet/marvell/prestera/prestera_main.c  | 10 ++-
>  .../ethernet/microchip/lan966x/lan966x_phylink.c   |  8 ++-
>  .../net/ethernet/microchip/sparx5/sparx5_phylink.c |  8 ++-
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c  |  5 +-
>  drivers/net/pcs/pcs-lynx.c                         | 18 +++--
>  drivers/net/phy/phylink.c                          | 14 ++--
>  include/linux/phylink.h                            | 81 +++++++++++++++++++++-
>  11 files changed, 136 insertions(+), 37 deletions(-)
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu

Re: [PATCH RFC 0/9] Add and use helper for PCS negotiation modes

[Russell King (Oracle)]

On Wed, May 24, 2023 at 09:26:19AM +0200, Horatiu Vultur wrote:
> The 05/23/2023 16:54, Russell King (Oracle) wrote:
> 
> Hi Russell,
> 
> I have tried this series on lan966x and it seems to be working fine.

Thanks for testing.

> There was a small issue applying the patch 3, as the function
> 'phylink_resolve_c73' doesn't exist yet.

It's for applying after my XPCS cleanup series that has been sent as RFC
twice and now been sent for merging. Sorry for not stating that in the
cover message.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC 0/9] Add and use helper for PCS negotiation modes

[Russell King (Oracle)]

On Wed, May 24, 2023 at 09:10:15AM +0100, Russell King (Oracle) wrote:
> On Wed, May 24, 2023 at 09:26:19AM +0200, Horatiu Vultur wrote:
> > The 05/23/2023 16:54, Russell King (Oracle) wrote:
> > 
> > Hi Russell,
> > 
> > I have tried this series on lan966x and it seems to be working fine.
> 
> Thanks for testing.
> 
> > There was a small issue applying the patch 3, as the function
> > 'phylink_resolve_c73' doesn't exist yet.
> 
> It's for applying after my XPCS cleanup series that has been sent as RFC
> twice and now been sent for merging. Sorry for not stating that in the
> cover message.

... which is now in net-next.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!