[PATCH RFC net-next 0/5] net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

[PATCH RFC net-next 0/5] net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

[Russell King (Oracle)]

Hi,

This series converts b53 to use phylink_generic_validate() and also
marks this driver as non-legacy.

Patch 1 cleans up an if() condition to be more readable before we
proceed with the conversion.

Patch 2 populates the supported_interfaces and mac_capabilities members
of phylink_config.

Patch 3 drops the use of phylink_helper_basex_speed() which is now not
necessary.

Patch 4 switches the driver to use phylink_generic_validate()

Patch 5 marks the driver as non-legacy.

 drivers/net/dsa/b53/b53_common.c | 68 +++++++++++++++++++++-------------------
 drivers/net/dsa/b53/b53_priv.h   |  8 ++---
 drivers/net/dsa/b53/b53_serdes.c | 17 ++++++----
 drivers/net/dsa/b53/b53_serdes.h |  5 ++-
 drivers/net/dsa/b53/b53_srab.c   | 35 ++++++++++++++++++++-
 5 files changed, 85 insertions(+), 48 deletions(-)

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

[PATCH RFC net-next 1/5] net: dsa: b53: clean up if() condition to be more readable

[Russell King (Oracle)]

I've stared at this if() statement for a while trying to work out if
it really does correspond with the comment above, and it does seem to.
However, let's make it more readable and phrase it in the same way as
the comment.

Also add a FIXME into the comment - we appear to deny Gigabit modes for
802.3z interface modes, but 802.3z interface modes only operate at
gigabit and above.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a3b98992f180..7d62b0aeaae9 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1327,11 +1327,14 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 
 	/* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we
 	 * support Gigabit, including Half duplex.
+	 *
+	 * FIXME: this is weird - 802.3z is always Gigabit, but we exclude
+	 * it here. Why? This makes no sense.
 	 */
-	if (state->interface != PHY_INTERFACE_MODE_MII &&
-	    state->interface != PHY_INTERFACE_MODE_REVMII &&
-	    !phy_interface_mode_is_8023z(state->interface) &&
-	    !(is5325(dev) || is5365(dev))) {
+	if (!(state->interface == PHY_INTERFACE_MODE_MII ||
+	      state->interface == PHY_INTERFACE_MODE_REVMII ||
+	      phy_interface_mode_is_8023z(state->interface) ||
+	      is5325(dev) || is5365(dev))) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseT_Half);
 	}
-- 
2.30.2

[PATCH RFC net-next 2/5] net: dsa: b53: populate supported_interfaces and mac_capabilities

[Russell King (Oracle)]

Populate the supported interfaces and MAC capabilities for the Broadcom
B53 DSA switches in preparation to using these for the generic
validation functionality.

The interface modes are derived from:
- b53_serdes_phylink_validate()
- SRAB mux configuration

NOTE: much of this conversion is a guess as the driver doesn't contain
sufficient information. I would appreciate a thorough review and
testing of this change before it is merged.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 28 ++++++++++++++++++++++++++
 drivers/net/dsa/b53/b53_priv.h   |  2 ++
 drivers/net/dsa/b53/b53_serdes.c | 27 +++++++++++++++++++++++++
 drivers/net/dsa/b53/b53_serdes.h |  2 ++
 drivers/net/dsa/b53/b53_srab.c   | 34 ++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 7d62b0aeaae9..211c0e499370 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1353,6 +1353,33 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_phylink_validate);
 
+static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
+				 struct phylink_config *config)
+{
+	struct b53_device *dev = ds->priv;
+
+	/* Internal ports need GMII for PHYLIB */
+	__set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
+
+	/* These switches appear to support MII and RevMII too, but beyond
+	 * this, the code gives very few clues. FIXME: We probably need more
+	 * interface modes here.
+	 */
+	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces);
+
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+		MAC_10 | MAC_100;
+
+	/* 5325/5365 are not capable of gigabit speeds, everything else is */
+	if (!(is5325(dev) || is5365(dev)))
+		config->mac_capabilities |= MAC_1000;
+
+	/* Get the implementation specific capabilities */
+	if (dev->ops->phylink_get_caps)
+		dev->ops->phylink_get_caps(dev, port, config);
+}
+
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
 			       struct phylink_link_state *state)
 {
@@ -2262,6 +2289,7 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.phy_read		= b53_phy_read16,
 	.phy_write		= b53_phy_write16,
 	.adjust_link		= b53_adjust_link,
+	.phylink_get_caps	= b53_phylink_get_caps,
 	.phylink_validate	= b53_phylink_validate,
 	.phylink_mac_link_state	= b53_phylink_mac_link_state,
 	.phylink_mac_config	= b53_phylink_mac_config,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index b41dc8ac2ca8..b9d1b4819c5f 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -46,6 +46,8 @@ struct b53_io_ops {
 	int (*phy_write16)(struct b53_device *dev, int addr, int reg, u16 value);
 	int (*irq_enable)(struct b53_device *dev, int port);
 	void (*irq_disable)(struct b53_device *dev, int port);
+	void (*phylink_get_caps)(struct b53_device *dev, int port,
+				 struct phylink_config *config);
 	u8 (*serdes_map_lane)(struct b53_device *dev, int port);
 	int (*serdes_link_state)(struct b53_device *dev, int port,
 				 struct phylink_link_state *state);
diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c
index 5ae3d9783b68..7e1ec51ab4c9 100644
--- a/drivers/net/dsa/b53/b53_serdes.c
+++ b/drivers/net/dsa/b53/b53_serdes.c
@@ -180,6 +180,33 @@ void b53_serdes_phylink_validate(struct b53_device *dev, int port,
 }
 EXPORT_SYMBOL(b53_serdes_phylink_validate);
 
+void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
+				 struct phylink_config *config)
+{
+	u8 lane = b53_serdes_map_lane(dev, port);
+
+	if (lane == B53_INVALID_LANE)
+		return;
+
+	switch (lane) {
+	case 0:
+		/* It appears lane 0 supports 2500base-X and 1000base-X */
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_2500FD;
+		fallthrough;
+	case 1:
+		/* It appears lane 1 only supports 1000base-X */
+		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_1000FD;
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL(b53_serdes_phylink_get_caps);
+
 int b53_serdes_init(struct b53_device *dev, int port)
 {
 	u8 lane = b53_serdes_map_lane(dev, port);
diff --git a/drivers/net/dsa/b53/b53_serdes.h b/drivers/net/dsa/b53/b53_serdes.h
index 55d280fe38e4..8fa24f7001aa 100644
--- a/drivers/net/dsa/b53/b53_serdes.h
+++ b/drivers/net/dsa/b53/b53_serdes.h
@@ -115,6 +115,8 @@ void b53_serdes_config(struct b53_device *dev, int port, unsigned int mode,
 void b53_serdes_an_restart(struct b53_device *dev, int port);
 void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 			 phy_interface_t interface, bool link_up);
+void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
+				 struct phylink_config *config);
 void b53_serdes_phylink_validate(struct b53_device *dev, int port,
 				unsigned long *supported,
 				struct phylink_link_state *state);
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 4591bb1c05d2..7d72f3b293d3 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -443,6 +443,39 @@ static void b53_srab_irq_disable(struct b53_device *dev, int port)
 	}
 }
 
+static void b53_srab_phylink_get_caps(struct b53_device *dev, int port,
+				      struct phylink_config *config)
+{
+	struct b53_srab_priv *priv = dev->priv;
+	struct b53_srab_port_priv *p = &priv->port_intrs[port];
+
+	switch (p->mode) {
+	case PHY_INTERFACE_MODE_SGMII:
+#if IS_ENABLED(CONFIG_B53_SERDES)
+		/* If p->mode indicates SGMII mode, that essentially means we
+		 * are using a serdes. As the serdes for the capabilities.
+		 */
+		b53_serdes_phylink_get_caps(dev, port, config);
+#endif
+		break;
+
+	case PHY_INTERFACE_MODE_NA:
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		/* If we support RGMII, support all RGMII modes, since
+		 * that dictates the PHY delay settings.
+		 */
+		phy_interface_set_rgmii(config->supported_interfaces);
+		break;
+
+	default:
+		/* Some other mode (e.g. MII, GMII etc) */
+		__set_bit(p->mode, config->supported_interfaces);
+		break;
+	}
+}
+
 static const struct b53_io_ops b53_srab_ops = {
 	.read8 = b53_srab_read8,
 	.read16 = b53_srab_read16,
@@ -456,6 +489,7 @@ static const struct b53_io_ops b53_srab_ops = {
 	.write64 = b53_srab_write64,
 	.irq_enable = b53_srab_irq_enable,
 	.irq_disable = b53_srab_irq_disable,
+	.phylink_get_caps = b53_srab_phylink_get_caps,
 #if IS_ENABLED(CONFIG_B53_SERDES)
 	.serdes_map_lane = b53_srab_serdes_map_lane,
 	.serdes_link_state = b53_serdes_link_state,
-- 
2.30.2

[PATCH RFC net-next 3/5] net: dsa: b53: drop use of phylink_helper_basex_speed()

[Russell King (Oracle)]

Now that we have a better method to select SFP interface modes, we
no longer need to use phylink_helper_basex_speed() in a driver's
validation function.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 211c0e499370..a637e44bce0b 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1348,8 +1348,6 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 
 	linkmode_and(supported, supported, mask);
 	linkmode_and(state->advertising, state->advertising, mask);
-
-	phylink_helper_basex_speed(state);
 }
 EXPORT_SYMBOL(b53_phylink_validate);
 
-- 
2.30.2

[PATCH RFC net-next 4/5] net: dsa: b53: switch to using phylink_generic_validate()

[Russell King (Oracle)]

Switch the Broadcom b53 driver to using the phylink_generic_validate()
implementation by removing its own .phylink_validate method and
associated code.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 57 ++++++++------------------------
 drivers/net/dsa/b53/b53_priv.h   |  6 ----
 drivers/net/dsa/b53/b53_serdes.c | 22 ------------
 drivers/net/dsa/b53/b53_serdes.h |  3 --
 drivers/net/dsa/b53/b53_srab.c   |  1 -
 5 files changed, 13 insertions(+), 76 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a637e44bce0b..50a372dc32ae 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1309,48 +1309,6 @@ void b53_port_event(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_port_event);
 
-void b53_phylink_validate(struct dsa_switch *ds, int port,
-			  unsigned long *supported,
-			  struct phylink_link_state *state)
-{
-	struct b53_device *dev = ds->priv;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
-
-	if (dev->ops->serdes_phylink_validate)
-		dev->ops->serdes_phylink_validate(dev, port, mask, state);
-
-	/* Allow all the expected bits */
-	phylink_set(mask, Autoneg);
-	phylink_set_port_modes(mask);
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
-
-	/* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we
-	 * support Gigabit, including Half duplex.
-	 *
-	 * FIXME: this is weird - 802.3z is always Gigabit, but we exclude
-	 * it here. Why? This makes no sense.
-	 */
-	if (!(state->interface == PHY_INTERFACE_MODE_MII ||
-	      state->interface == PHY_INTERFACE_MODE_REVMII ||
-	      phy_interface_mode_is_8023z(state->interface) ||
-	      is5325(dev) || is5365(dev))) {
-		phylink_set(mask, 1000baseT_Full);
-		phylink_set(mask, 1000baseT_Half);
-	}
-
-	if (!phy_interface_mode_is_8023z(state->interface)) {
-		phylink_set(mask, 10baseT_Half);
-		phylink_set(mask, 10baseT_Full);
-		phylink_set(mask, 100baseT_Half);
-		phylink_set(mask, 100baseT_Full);
-	}
-
-	linkmode_and(supported, supported, mask);
-	linkmode_and(state->advertising, state->advertising, mask);
-}
-EXPORT_SYMBOL(b53_phylink_validate);
-
 static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 				 struct phylink_config *config)
 {
@@ -1362,6 +1320,13 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	/* These switches appear to support MII and RevMII too, but beyond
 	 * this, the code gives very few clues. FIXME: We probably need more
 	 * interface modes here.
+	 *
+	 * According to b53_srab_mux_init(), ports 3..5 can support:
+	 *  SGMII, MII, GMII, RGMII or INTERNAL depending on the MUX setting.
+	 * However, the interface mode read from the MUX configuration is
+	 * not passed back to DSA, so phylink uses NA.
+	 * DT can specify RGMII for ports 0, 1.
+	 * For MDIO, port 8 can be RGMII_TXID.
 	 */
 	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
 	__set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces);
@@ -1369,7 +1334,12 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100;
 
-	/* 5325/5365 are not capable of gigabit speeds, everything else is */
+	/* 5325/5365 are not capable of gigabit speeds, everything else is.
+	 * Note: the original code also exclulded Gigagbit for MII, RevMII
+	 * and 802.3z modes. MII and RevMII are not able to work above 100M,
+	 * so will be excluded by the generic validator implementation.
+	 * However, the exclusion of Gigabit for 802.3z just seems wrong.
+	 */
 	if (!(is5325(dev) || is5365(dev)))
 		config->mac_capabilities |= MAC_1000;
 
@@ -2288,7 +2258,6 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.phy_write		= b53_phy_write16,
 	.adjust_link		= b53_adjust_link,
 	.phylink_get_caps	= b53_phylink_get_caps,
-	.phylink_validate	= b53_phylink_validate,
 	.phylink_mac_link_state	= b53_phylink_mac_link_state,
 	.phylink_mac_config	= b53_phylink_mac_config,
 	.phylink_mac_an_restart	= b53_phylink_mac_an_restart,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index b9d1b4819c5f..a6b339fcb17e 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -58,9 +58,6 @@ struct b53_io_ops {
 	void (*serdes_link_set)(struct b53_device *dev, int port,
 				unsigned int mode, phy_interface_t interface,
 				bool link_up);
-	void (*serdes_phylink_validate)(struct b53_device *dev, int port,
-					unsigned long *supported,
-					struct phylink_link_state *state);
 };
 
 #define B53_INVALID_LANE	0xff
@@ -339,9 +336,6 @@ int b53_br_flags(struct dsa_switch *ds, int port,
 		 struct netlink_ext_ack *extack);
 int b53_setup_devlink_resources(struct dsa_switch *ds);
 void b53_port_event(struct dsa_switch *ds, int port);
-void b53_phylink_validate(struct dsa_switch *ds, int port,
-			  unsigned long *supported,
-			  struct phylink_link_state *state);
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
 			       struct phylink_link_state *state);
 void b53_phylink_mac_config(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c
index 7e1ec51ab4c9..c39c315afa8e 100644
--- a/drivers/net/dsa/b53/b53_serdes.c
+++ b/drivers/net/dsa/b53/b53_serdes.c
@@ -158,28 +158,6 @@ void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 }
 EXPORT_SYMBOL(b53_serdes_link_set);
 
-void b53_serdes_phylink_validate(struct b53_device *dev, int port,
-				 unsigned long *supported,
-				 struct phylink_link_state *state)
-{
-	u8 lane = b53_serdes_map_lane(dev, port);
-
-	if (lane == B53_INVALID_LANE)
-		return;
-
-	switch (lane) {
-	case 0:
-		phylink_set(supported, 2500baseX_Full);
-		fallthrough;
-	case 1:
-		phylink_set(supported, 1000baseX_Full);
-		break;
-	default:
-		break;
-	}
-}
-EXPORT_SYMBOL(b53_serdes_phylink_validate);
-
 void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
 				 struct phylink_config *config)
 {
diff --git a/drivers/net/dsa/b53/b53_serdes.h b/drivers/net/dsa/b53/b53_serdes.h
index 8fa24f7001aa..f47d5caa7557 100644
--- a/drivers/net/dsa/b53/b53_serdes.h
+++ b/drivers/net/dsa/b53/b53_serdes.h
@@ -117,9 +117,6 @@ void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 			 phy_interface_t interface, bool link_up);
 void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
 				 struct phylink_config *config);
-void b53_serdes_phylink_validate(struct b53_device *dev, int port,
-				unsigned long *supported,
-				struct phylink_link_state *state);
 #if IS_ENABLED(CONFIG_B53_SERDES)
 int b53_serdes_init(struct b53_device *dev, int port);
 #else
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 7d72f3b293d3..c51b716657db 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -496,7 +496,6 @@ static const struct b53_io_ops b53_srab_ops = {
 	.serdes_config = b53_serdes_config,
 	.serdes_an_restart = b53_serdes_an_restart,
 	.serdes_link_set = b53_serdes_link_set,
-	.serdes_phylink_validate = b53_serdes_phylink_validate,
 #endif
 };
 
-- 
2.30.2

[PATCH RFC net-next 5/5] net: dsa: b53: mark as non-legacy

[Russell King (Oracle)]

The B53 driver does not make use of the speed, duplex, pause or
advertisement in its phylink_mac_config() implementation, so it can be
marked as a non-legacy driver.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 50a372dc32ae..83bf30349c26 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1346,6 +1346,12 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	/* Get the implementation specific capabilities */
 	if (dev->ops->phylink_get_caps)
 		dev->ops->phylink_get_caps(dev, port, config);
+
+	/* This driver does not make use of the speed, duplex, pause or the
+	 * advertisement in its mac_config, so it is safe to mark this driver
+	 * as non-legacy.
+	 */
+	config->legacy_pre_march2020 = false;
 }
 
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
-- 
2.30.2

[PATCH RFC net-next 1/5] net: dsa: b53: clean up if() condition to be more readable

[Russell King (Oracle)]

I've stared at this if() statement for a while trying to work out if
it really does correspond with the comment above, and it does seem to.
However, let's make it more readable and phrase it in the same way as
the comment.

Also add a FIXME into the comment - we appear to deny Gigabit modes for
802.3z interface modes, but 802.3z interface modes only operate at
gigabit and above.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a3b98992f180..7d62b0aeaae9 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1327,11 +1327,14 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 
 	/* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we
 	 * support Gigabit, including Half duplex.
+	 *
+	 * FIXME: this is weird - 802.3z is always Gigabit, but we exclude
+	 * it here. Why? This makes no sense.
 	 */
-	if (state->interface != PHY_INTERFACE_MODE_MII &&
-	    state->interface != PHY_INTERFACE_MODE_REVMII &&
-	    !phy_interface_mode_is_8023z(state->interface) &&
-	    !(is5325(dev) || is5365(dev))) {
+	if (!(state->interface == PHY_INTERFACE_MODE_MII ||
+	      state->interface == PHY_INTERFACE_MODE_REVMII ||
+	      phy_interface_mode_is_8023z(state->interface) ||
+	      is5325(dev) || is5365(dev))) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseT_Half);
 	}
-- 
2.30.2

[PATCH RFC net-next 2/5] net: dsa: b53: populate supported_interfaces and mac_capabilities

[Russell King (Oracle)]

Populate the supported interfaces and MAC capabilities for the Broadcom
B53 DSA switches in preparation to using these for the generic
validation functionality.

The interface modes are derived from:
- b53_serdes_phylink_validate()
- SRAB mux configuration

NOTE: much of this conversion is a guess as the driver doesn't contain
sufficient information. I would appreciate a thorough review and
testing of this change before it is merged.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 28 ++++++++++++++++++++++++++
 drivers/net/dsa/b53/b53_priv.h   |  2 ++
 drivers/net/dsa/b53/b53_serdes.c | 27 +++++++++++++++++++++++++
 drivers/net/dsa/b53/b53_serdes.h |  2 ++
 drivers/net/dsa/b53/b53_srab.c   | 34 ++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 7d62b0aeaae9..211c0e499370 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1353,6 +1353,33 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_phylink_validate);
 
+static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
+				 struct phylink_config *config)
+{
+	struct b53_device *dev = ds->priv;
+
+	/* Internal ports need GMII for PHYLIB */
+	__set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);
+
+	/* These switches appear to support MII and RevMII too, but beyond
+	 * this, the code gives very few clues. FIXME: We probably need more
+	 * interface modes here.
+	 */
+	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces);
+
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+		MAC_10 | MAC_100;
+
+	/* 5325/5365 are not capable of gigabit speeds, everything else is */
+	if (!(is5325(dev) || is5365(dev)))
+		config->mac_capabilities |= MAC_1000;
+
+	/* Get the implementation specific capabilities */
+	if (dev->ops->phylink_get_caps)
+		dev->ops->phylink_get_caps(dev, port, config);
+}
+
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
 			       struct phylink_link_state *state)
 {
@@ -2262,6 +2289,7 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.phy_read		= b53_phy_read16,
 	.phy_write		= b53_phy_write16,
 	.adjust_link		= b53_adjust_link,
+	.phylink_get_caps	= b53_phylink_get_caps,
 	.phylink_validate	= b53_phylink_validate,
 	.phylink_mac_link_state	= b53_phylink_mac_link_state,
 	.phylink_mac_config	= b53_phylink_mac_config,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index b41dc8ac2ca8..b9d1b4819c5f 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -46,6 +46,8 @@ struct b53_io_ops {
 	int (*phy_write16)(struct b53_device *dev, int addr, int reg, u16 value);
 	int (*irq_enable)(struct b53_device *dev, int port);
 	void (*irq_disable)(struct b53_device *dev, int port);
+	void (*phylink_get_caps)(struct b53_device *dev, int port,
+				 struct phylink_config *config);
 	u8 (*serdes_map_lane)(struct b53_device *dev, int port);
 	int (*serdes_link_state)(struct b53_device *dev, int port,
 				 struct phylink_link_state *state);
diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c
index 5ae3d9783b68..7e1ec51ab4c9 100644
--- a/drivers/net/dsa/b53/b53_serdes.c
+++ b/drivers/net/dsa/b53/b53_serdes.c
@@ -180,6 +180,33 @@ void b53_serdes_phylink_validate(struct b53_device *dev, int port,
 }
 EXPORT_SYMBOL(b53_serdes_phylink_validate);
 
+void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
+				 struct phylink_config *config)
+{
+	u8 lane = b53_serdes_map_lane(dev, port);
+
+	if (lane == B53_INVALID_LANE)
+		return;
+
+	switch (lane) {
+	case 0:
+		/* It appears lane 0 supports 2500base-X and 1000base-X */
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_2500FD;
+		fallthrough;
+	case 1:
+		/* It appears lane 1 only supports 1000base-X */
+		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_1000FD;
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL(b53_serdes_phylink_get_caps);
+
 int b53_serdes_init(struct b53_device *dev, int port)
 {
 	u8 lane = b53_serdes_map_lane(dev, port);
diff --git a/drivers/net/dsa/b53/b53_serdes.h b/drivers/net/dsa/b53/b53_serdes.h
index 55d280fe38e4..8fa24f7001aa 100644
--- a/drivers/net/dsa/b53/b53_serdes.h
+++ b/drivers/net/dsa/b53/b53_serdes.h
@@ -115,6 +115,8 @@ void b53_serdes_config(struct b53_device *dev, int port, unsigned int mode,
 void b53_serdes_an_restart(struct b53_device *dev, int port);
 void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 			 phy_interface_t interface, bool link_up);
+void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
+				 struct phylink_config *config);
 void b53_serdes_phylink_validate(struct b53_device *dev, int port,
 				unsigned long *supported,
 				struct phylink_link_state *state);
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 4591bb1c05d2..7d72f3b293d3 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -443,6 +443,39 @@ static void b53_srab_irq_disable(struct b53_device *dev, int port)
 	}
 }
 
+static void b53_srab_phylink_get_caps(struct b53_device *dev, int port,
+				      struct phylink_config *config)
+{
+	struct b53_srab_priv *priv = dev->priv;
+	struct b53_srab_port_priv *p = &priv->port_intrs[port];
+
+	switch (p->mode) {
+	case PHY_INTERFACE_MODE_SGMII:
+#if IS_ENABLED(CONFIG_B53_SERDES)
+		/* If p->mode indicates SGMII mode, that essentially means we
+		 * are using a serdes. As the serdes for the capabilities.
+		 */
+		b53_serdes_phylink_get_caps(dev, port, config);
+#endif
+		break;
+
+	case PHY_INTERFACE_MODE_NA:
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		/* If we support RGMII, support all RGMII modes, since
+		 * that dictates the PHY delay settings.
+		 */
+		phy_interface_set_rgmii(config->supported_interfaces);
+		break;
+
+	default:
+		/* Some other mode (e.g. MII, GMII etc) */
+		__set_bit(p->mode, config->supported_interfaces);
+		break;
+	}
+}
+
 static const struct b53_io_ops b53_srab_ops = {
 	.read8 = b53_srab_read8,
 	.read16 = b53_srab_read16,
@@ -456,6 +489,7 @@ static const struct b53_io_ops b53_srab_ops = {
 	.write64 = b53_srab_write64,
 	.irq_enable = b53_srab_irq_enable,
 	.irq_disable = b53_srab_irq_disable,
+	.phylink_get_caps = b53_srab_phylink_get_caps,
 #if IS_ENABLED(CONFIG_B53_SERDES)
 	.serdes_map_lane = b53_srab_serdes_map_lane,
 	.serdes_link_state = b53_serdes_link_state,
-- 
2.30.2

[PATCH RFC net-next 3/5] net: dsa: b53: drop use of phylink_helper_basex_speed()

[Russell King (Oracle)]

Now that we have a better method to select SFP interface modes, we
no longer need to use phylink_helper_basex_speed() in a driver's
validation function.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 211c0e499370..a637e44bce0b 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1348,8 +1348,6 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
 
 	linkmode_and(supported, supported, mask);
 	linkmode_and(state->advertising, state->advertising, mask);
-
-	phylink_helper_basex_speed(state);
 }
 EXPORT_SYMBOL(b53_phylink_validate);
 
-- 
2.30.2

[PATCH RFC net-next 4/5] net: dsa: b53: switch to using phylink_generic_validate()

[Russell King (Oracle)]

Switch the Broadcom b53 driver to using the phylink_generic_validate()
implementation by removing its own .phylink_validate method and
associated code.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 57 ++++++++------------------------
 drivers/net/dsa/b53/b53_priv.h   |  6 ----
 drivers/net/dsa/b53/b53_serdes.c | 22 ------------
 drivers/net/dsa/b53/b53_serdes.h |  3 --
 drivers/net/dsa/b53/b53_srab.c   |  1 -
 5 files changed, 13 insertions(+), 76 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a637e44bce0b..50a372dc32ae 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1309,48 +1309,6 @@ void b53_port_event(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_port_event);
 
-void b53_phylink_validate(struct dsa_switch *ds, int port,
-			  unsigned long *supported,
-			  struct phylink_link_state *state)
-{
-	struct b53_device *dev = ds->priv;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
-
-	if (dev->ops->serdes_phylink_validate)
-		dev->ops->serdes_phylink_validate(dev, port, mask, state);
-
-	/* Allow all the expected bits */
-	phylink_set(mask, Autoneg);
-	phylink_set_port_modes(mask);
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
-
-	/* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we
-	 * support Gigabit, including Half duplex.
-	 *
-	 * FIXME: this is weird - 802.3z is always Gigabit, but we exclude
-	 * it here. Why? This makes no sense.
-	 */
-	if (!(state->interface == PHY_INTERFACE_MODE_MII ||
-	      state->interface == PHY_INTERFACE_MODE_REVMII ||
-	      phy_interface_mode_is_8023z(state->interface) ||
-	      is5325(dev) || is5365(dev))) {
-		phylink_set(mask, 1000baseT_Full);
-		phylink_set(mask, 1000baseT_Half);
-	}
-
-	if (!phy_interface_mode_is_8023z(state->interface)) {
-		phylink_set(mask, 10baseT_Half);
-		phylink_set(mask, 10baseT_Full);
-		phylink_set(mask, 100baseT_Half);
-		phylink_set(mask, 100baseT_Full);
-	}
-
-	linkmode_and(supported, supported, mask);
-	linkmode_and(state->advertising, state->advertising, mask);
-}
-EXPORT_SYMBOL(b53_phylink_validate);
-
 static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 				 struct phylink_config *config)
 {
@@ -1362,6 +1320,13 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	/* These switches appear to support MII and RevMII too, but beyond
 	 * this, the code gives very few clues. FIXME: We probably need more
 	 * interface modes here.
+	 *
+	 * According to b53_srab_mux_init(), ports 3..5 can support:
+	 *  SGMII, MII, GMII, RGMII or INTERNAL depending on the MUX setting.
+	 * However, the interface mode read from the MUX configuration is
+	 * not passed back to DSA, so phylink uses NA.
+	 * DT can specify RGMII for ports 0, 1.
+	 * For MDIO, port 8 can be RGMII_TXID.
 	 */
 	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
 	__set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces);
@@ -1369,7 +1334,12 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100;
 
-	/* 5325/5365 are not capable of gigabit speeds, everything else is */
+	/* 5325/5365 are not capable of gigabit speeds, everything else is.
+	 * Note: the original code also exclulded Gigagbit for MII, RevMII
+	 * and 802.3z modes. MII and RevMII are not able to work above 100M,
+	 * so will be excluded by the generic validator implementation.
+	 * However, the exclusion of Gigabit for 802.3z just seems wrong.
+	 */
 	if (!(is5325(dev) || is5365(dev)))
 		config->mac_capabilities |= MAC_1000;
 
@@ -2288,7 +2258,6 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.phy_write		= b53_phy_write16,
 	.adjust_link		= b53_adjust_link,
 	.phylink_get_caps	= b53_phylink_get_caps,
-	.phylink_validate	= b53_phylink_validate,
 	.phylink_mac_link_state	= b53_phylink_mac_link_state,
 	.phylink_mac_config	= b53_phylink_mac_config,
 	.phylink_mac_an_restart	= b53_phylink_mac_an_restart,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index b9d1b4819c5f..a6b339fcb17e 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -58,9 +58,6 @@ struct b53_io_ops {
 	void (*serdes_link_set)(struct b53_device *dev, int port,
 				unsigned int mode, phy_interface_t interface,
 				bool link_up);
-	void (*serdes_phylink_validate)(struct b53_device *dev, int port,
-					unsigned long *supported,
-					struct phylink_link_state *state);
 };
 
 #define B53_INVALID_LANE	0xff
@@ -339,9 +336,6 @@ int b53_br_flags(struct dsa_switch *ds, int port,
 		 struct netlink_ext_ack *extack);
 int b53_setup_devlink_resources(struct dsa_switch *ds);
 void b53_port_event(struct dsa_switch *ds, int port);
-void b53_phylink_validate(struct dsa_switch *ds, int port,
-			  unsigned long *supported,
-			  struct phylink_link_state *state);
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
 			       struct phylink_link_state *state);
 void b53_phylink_mac_config(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c
index 7e1ec51ab4c9..c39c315afa8e 100644
--- a/drivers/net/dsa/b53/b53_serdes.c
+++ b/drivers/net/dsa/b53/b53_serdes.c
@@ -158,28 +158,6 @@ void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 }
 EXPORT_SYMBOL(b53_serdes_link_set);
 
-void b53_serdes_phylink_validate(struct b53_device *dev, int port,
-				 unsigned long *supported,
-				 struct phylink_link_state *state)
-{
-	u8 lane = b53_serdes_map_lane(dev, port);
-
-	if (lane == B53_INVALID_LANE)
-		return;
-
-	switch (lane) {
-	case 0:
-		phylink_set(supported, 2500baseX_Full);
-		fallthrough;
-	case 1:
-		phylink_set(supported, 1000baseX_Full);
-		break;
-	default:
-		break;
-	}
-}
-EXPORT_SYMBOL(b53_serdes_phylink_validate);
-
 void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
 				 struct phylink_config *config)
 {
diff --git a/drivers/net/dsa/b53/b53_serdes.h b/drivers/net/dsa/b53/b53_serdes.h
index 8fa24f7001aa..f47d5caa7557 100644
--- a/drivers/net/dsa/b53/b53_serdes.h
+++ b/drivers/net/dsa/b53/b53_serdes.h
@@ -117,9 +117,6 @@ void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 			 phy_interface_t interface, bool link_up);
 void b53_serdes_phylink_get_caps(struct b53_device *dev, int port,
 				 struct phylink_config *config);
-void b53_serdes_phylink_validate(struct b53_device *dev, int port,
-				unsigned long *supported,
-				struct phylink_link_state *state);
 #if IS_ENABLED(CONFIG_B53_SERDES)
 int b53_serdes_init(struct b53_device *dev, int port);
 #else
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 7d72f3b293d3..c51b716657db 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -496,7 +496,6 @@ static const struct b53_io_ops b53_srab_ops = {
 	.serdes_config = b53_serdes_config,
 	.serdes_an_restart = b53_serdes_an_restart,
 	.serdes_link_set = b53_serdes_link_set,
-	.serdes_phylink_validate = b53_serdes_phylink_validate,
 #endif
 };
 
-- 
2.30.2

[PATCH RFC net-next 5/5] net: dsa: b53: mark as non-legacy

[Russell King (Oracle)]

The B53 driver does not make use of the speed, duplex, pause or
advertisement in its phylink_mac_config() implementation, so it can be
marked as a non-legacy driver.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 50a372dc32ae..83bf30349c26 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1346,6 +1346,12 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
 	/* Get the implementation specific capabilities */
 	if (dev->ops->phylink_get_caps)
 		dev->ops->phylink_get_caps(dev, port, config);
+
+	/* This driver does not make use of the speed, duplex, pause or the
+	 * advertisement in its mac_config, so it is safe to mark this driver
+	 * as non-legacy.
+	 */
+	config->legacy_pre_march2020 = false;
 }
 
 int b53_phylink_mac_link_state(struct dsa_switch *ds, int port,
-- 
2.30.2

Re: [PATCH RFC net-next 1/5] net: dsa: b53: clean up if() condition to be more readable

[Russell King (Oracle)]

Sorry for the unintentional re-send, please ignore the second set.

On Thu, Feb 03, 2022 at 05:30:52PM +0000, Russell King (Oracle) wrote:
> I've stared at this if() statement for a while trying to work out if
> it really does correspond with the comment above, and it does seem to.
> However, let's make it more readable and phrase it in the same way as
> the comment.
> 
> Also add a FIXME into the comment - we appear to deny Gigabit modes for
> 802.3z interface modes, but 802.3z interface modes only operate at
> gigabit and above.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/dsa/b53/b53_common.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index a3b98992f180..7d62b0aeaae9 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1327,11 +1327,14 @@ void b53_phylink_validate(struct dsa_switch *ds, int port,
>  
>  	/* With the exclusion of 5325/5365, MII, Reverse MII and 802.3z, we
>  	 * support Gigabit, including Half duplex.
> +	 *
> +	 * FIXME: this is weird - 802.3z is always Gigabit, but we exclude
> +	 * it here. Why? This makes no sense.
>  	 */
> -	if (state->interface != PHY_INTERFACE_MODE_MII &&
> -	    state->interface != PHY_INTERFACE_MODE_REVMII &&
> -	    !phy_interface_mode_is_8023z(state->interface) &&
> -	    !(is5325(dev) || is5365(dev))) {
> +	if (!(state->interface == PHY_INTERFACE_MODE_MII ||
> +	      state->interface == PHY_INTERFACE_MODE_REVMII ||
> +	      phy_interface_mode_is_8023z(state->interface) ||
> +	      is5325(dev) || is5365(dev))) {
>  		phylink_set(mask, 1000baseT_Full);
>  		phylink_set(mask, 1000baseT_Half);
>  	}
> -- 
> 2.30.2
> 
> 

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

Re: [PATCH RFC net-next 0/5] net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

[Florian Fainelli]

On 2/3/2022 6:48 AM, Russell King (Oracle) wrote:
> Hi,
> 
> This series converts b53 to use phylink_generic_validate() and also
> marks this driver as non-legacy.
> 
> Patch 1 cleans up an if() condition to be more readable before we
> proceed with the conversion.
> 
> Patch 2 populates the supported_interfaces and mac_capabilities members
> of phylink_config.
> 
> Patch 3 drops the use of phylink_helper_basex_speed() which is now not
> necessary.
> 
> Patch 4 switches the driver to use phylink_generic_validate()
> 
> Patch 5 marks the driver as non-legacy.

I won't be able to test these patches on the platform that does use the 
SRAB code until the beginning of next week since I don't actually have 
access to the hardware right now, but I will respond once tested.
--
Florian

Re: [PATCH RFC net-next 0/5] net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

[Russell King (Oracle)]

On Thu, Feb 03, 2022 at 10:27:56AM -0800, Florian Fainelli wrote:
> On 2/3/2022 6:48 AM, Russell King (Oracle) wrote:
> > Hi,
> > 
> > This series converts b53 to use phylink_generic_validate() and also
> > marks this driver as non-legacy.
> > 
> > Patch 1 cleans up an if() condition to be more readable before we
> > proceed with the conversion.
> > 
> > Patch 2 populates the supported_interfaces and mac_capabilities members
> > of phylink_config.
> > 
> > Patch 3 drops the use of phylink_helper_basex_speed() which is now not
> > necessary.
> > 
> > Patch 4 switches the driver to use phylink_generic_validate()
> > 
> > Patch 5 marks the driver as non-legacy.
> 
> I won't be able to test these patches on the platform that does use the SRAB
> code until the beginning of next week since I don't actually have access to
> the hardware right now, but I will respond once tested.

That's fine, thanks for letting me know!

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

Re: [PATCH RFC net-next 1/5] net: dsa: b53: clean up if() condition to be more readable

[Florian Fainelli]

On 2/3/22 6:48 AM, Russell King (Oracle) wrote:
> I've stared at this if() statement for a while trying to work out if
> it really does correspond with the comment above, and it does seem to.
> However, let's make it more readable and phrase it in the same way as
> the comment.
> 
> Also add a FIXME into the comment - we appear to deny Gigabit modes for
> 802.3z interface modes, but 802.3z interface modes only operate at
> gigabit and above.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Did I mention that I always struggled and still do with double negations
and not just while programming?
-- 
Florian

Re: [PATCH RFC net-next 2/5] net: dsa: b53: populate supported_interfaces and mac_capabilities

[Florian Fainelli]

On 2/3/22 6:48 AM, Russell King (Oracle) wrote:
> Populate the supported interfaces and MAC capabilities for the Broadcom
> B53 DSA switches in preparation to using these for the generic
> validation functionality.
> 
> The interface modes are derived from:
> - b53_serdes_phylink_validate()
> - SRAB mux configuration
> 
> NOTE: much of this conversion is a guess as the driver doesn't contain
> sufficient information. I would appreciate a thorough review and
> testing of this change before it is merged.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

This patch breaks with the following:

[    2.680318] b53-srab-switch 18036000.ethernet-switch sfp
(uninitialized): failed to validate link configuration for in-band status
[    2.692470] error creating PHYLINK: -22
[    2.696441] b53-srab-switch 18036000.ethernet-switch sfp
(uninitialized): error -22 setting up PHY for tree 0, switch 0, port 5

Adding more debug shows us the following:

[    2.804854] phylink_validate: unable to find a mode for 4
0000000,000001ff,0060004c
[    2.812807] phylink_validate: unable to find a mode for 4
0000000,000001ff,0060004c
[    2.820733] b53-srab-switch 18036000.ethernet-switch sfp
(uninitialized): failed to validate link configuration for in-band status
[    2.832868] error creating PHYLINK: -22

4 = PHY_INTERFACE_MODE_SGMII and the config->supported_interfaces bitmap
is printed. If we add this hunk, you entire patch set works again:

@@ -178,10 +180,14 @@ void b53_serdes_phylink_get_caps(struct b53_device
*dev, int port,
                __set_bit(PHY_INTERFACE_MODE_1000BASEX,
                          config->supported_interfaces);
                config->mac_capabilities |= MAC_1000FD;
+               __set_bit(PHY_INTERFACE_MODE_SGMII,
+                         config->supported_interfaces);
                break;
        default:
                break;
        }
--
Florian

Re: [PATCH RFC net-next 2/5] net: dsa: b53: populate supported_interfaces and mac_capabilities

[Russell King (Oracle)]

On Wed, Feb 16, 2022 at 11:51:49AM -0800, Florian Fainelli wrote:
> On 2/3/22 6:48 AM, Russell King (Oracle) wrote:
> > Populate the supported interfaces and MAC capabilities for the Broadcom
> > B53 DSA switches in preparation to using these for the generic
> > validation functionality.
> > 
> > The interface modes are derived from:
> > - b53_serdes_phylink_validate()
> > - SRAB mux configuration
> > 
> > NOTE: much of this conversion is a guess as the driver doesn't contain
> > sufficient information. I would appreciate a thorough review and
> > testing of this change before it is merged.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> 
> This patch breaks with the following:
> 
> [    2.680318] b53-srab-switch 18036000.ethernet-switch sfp
> (uninitialized): failed to validate link configuration for in-band status
> [    2.692470] error creating PHYLINK: -22
> [    2.696441] b53-srab-switch 18036000.ethernet-switch sfp
> (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 5
> 
> Adding more debug shows us the following:
> 
> [    2.804854] phylink_validate: unable to find a mode for 4
> 0000000,000001ff,0060004c
> [    2.812807] phylink_validate: unable to find a mode for 4
> 0000000,000001ff,0060004c
> [    2.820733] b53-srab-switch 18036000.ethernet-switch sfp
> (uninitialized): failed to validate link configuration for in-band status
> [    2.832868] error creating PHYLINK: -22
> 
> 4 = PHY_INTERFACE_MODE_SGMII and the config->supported_interfaces bitmap
> is printed. If we add this hunk, you entire patch set works again:
> 
> @@ -178,10 +180,14 @@ void b53_serdes_phylink_get_caps(struct b53_device
> *dev, int port,
>                 __set_bit(PHY_INTERFACE_MODE_1000BASEX,
>                           config->supported_interfaces);
>                 config->mac_capabilities |= MAC_1000FD;
> +               __set_bit(PHY_INTERFACE_MODE_SGMII,
> +                         config->supported_interfaces);
>                 break;
>         default:
>                 break;
>         }

Thanks, added, and I've updated the comment as well just above the first
line of the above hunk to be consistent with the code.

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