net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

RFC PATCH In review Russell King (Oracle) 3 participants 17 messages Feb 3, 2022 – Feb 17, 2022

Subsystems (4)

Networking Network drivers Networking DSA Broadcom B53/SF2 Ethernet switch driver

lore

Share & tools

lore

Russell King (Oracle)

author

2022-02-03 14:48 UTC #0 lore raw

cover net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy

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(-)

Signature · 2 lines

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

Russell King (Oracle)

2022-02-03 14:48 UTC #1 lore raw

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

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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/net/dsa/b53/b53_common.c +7 -4

index a3b98992f180..7d62b0aeaae9 100644
@@ -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);
 	}

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 14:48 UTC #5 lore raw

RFC PATCH 2/5 net: dsa: b53: populate supported_interfaces and mac_capabilities

In reply to 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) <[email protected]>

---
 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(+)

drivers/net/dsa/b53/b53_common.c +28 -0

index 7d62b0aeaae9..211c0e499370 100644
@@ -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,

drivers/net/dsa/b53/b53_priv.h +2 -0

index b41dc8ac2ca8..b9d1b4819c5f 100644
@@ -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);

drivers/net/dsa/b53/b53_serdes.c +27 -0

index 5ae3d9783b68..7e1ec51ab4c9 100644
@@ -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);

drivers/net/dsa/b53/b53_serdes.h +2 -0

index 55d280fe38e4..8fa24f7001aa 100644
@@ -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);

drivers/net/dsa/b53/b53_srab.c +34 -0

index 4591bb1c05d2..7d72f3b293d3 100644
@@ -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,

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 14:48 UTC #9 lore raw

RFC PATCH 3/5 net: dsa: b53: drop use of phylink_helper_basex_speed()

In reply to 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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 2 --
 1 file changed, 2 deletions(-)

drivers/net/dsa/b53/b53_common.c +0 -2

index 211c0e499370..a637e44bce0b 100644
@@ -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);

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 14:48 UTC #11 lore raw

RFC PATCH 4/5 net: dsa: b53: switch to using phylink_generic_validate()

In reply to 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) <[email protected]>

---
 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(-)

drivers/net/dsa/b53/b53_common.c +13 ~~-44~~

index a637e44bce0b..50a372dc32ae 100644
@@ -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,

drivers/net/dsa/b53/b53_priv.h +0 -6

index b9d1b4819c5f..a6b339fcb17e 100644
@@ -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,

drivers/net/dsa/b53/b53_serdes.c +0 ~~-22~~

index 7e1ec51ab4c9..c39c315afa8e 100644
@@ -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)
 {

drivers/net/dsa/b53/b53_serdes.h +0 -3

index 8fa24f7001aa..f47d5caa7557 100644
@@ -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

drivers/net/dsa/b53/b53_srab.c +0 -1

index 7d72f3b293d3..c51b716657db 100644
@@ -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
 };

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 14:48 UTC #13 lore raw

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

In reply to 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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/net/dsa/b53/b53_common.c +6 -0

index 50a372dc32ae..83bf30349c26 100644
@@ -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,

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 17:31 UTC #3 lore raw

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

In reply to Russell King (Oracle)

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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/net/dsa/b53/b53_common.c +7 -4

index a3b98992f180..7d62b0aeaae9 100644
@@ -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);
 	}

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 17:31 UTC #8 lore raw

RFC PATCH 2/5 net: dsa: b53: populate supported_interfaces and mac_capabilities

In reply to 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) <[email protected]>

---
 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(+)

drivers/net/dsa/b53/b53_common.c +28 -0

index 7d62b0aeaae9..211c0e499370 100644
@@ -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,

drivers/net/dsa/b53/b53_priv.h +2 -0

index b41dc8ac2ca8..b9d1b4819c5f 100644
@@ -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);

drivers/net/dsa/b53/b53_serdes.c +27 -0

index 5ae3d9783b68..7e1ec51ab4c9 100644
@@ -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);

drivers/net/dsa/b53/b53_serdes.h +2 -0

index 55d280fe38e4..8fa24f7001aa 100644
@@ -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);

drivers/net/dsa/b53/b53_srab.c +34 -0

index 4591bb1c05d2..7d72f3b293d3 100644
@@ -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,

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 17:31 UTC #10 lore raw

RFC PATCH 3/5 net: dsa: b53: drop use of phylink_helper_basex_speed()

In reply to 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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 2 --
 1 file changed, 2 deletions(-)

drivers/net/dsa/b53/b53_common.c +0 -2

index 211c0e499370..a637e44bce0b 100644
@@ -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);

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 17:31 UTC #12 lore raw

RFC PATCH 4/5 net: dsa: b53: switch to using phylink_generic_validate()

In reply to 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) <[email protected]>

---
 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(-)

drivers/net/dsa/b53/b53_common.c +13 ~~-44~~

index a637e44bce0b..50a372dc32ae 100644
@@ -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,

drivers/net/dsa/b53/b53_priv.h +0 -6

index b9d1b4819c5f..a6b339fcb17e 100644
@@ -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,

drivers/net/dsa/b53/b53_serdes.c +0 ~~-22~~

index 7e1ec51ab4c9..c39c315afa8e 100644
@@ -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)
 {

drivers/net/dsa/b53/b53_serdes.h +0 -3

index 8fa24f7001aa..f47d5caa7557 100644
@@ -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

drivers/net/dsa/b53/b53_srab.c +0 -1

index 7d72f3b293d3..c51b716657db 100644
@@ -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
 };

Signature · 1 line

2.30.2

Russell King (Oracle)

2022-02-03 17:31 UTC #14 lore raw

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

In reply to 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) <[email protected]>

---
 drivers/net/dsa/b53/b53_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/net/dsa/b53/b53_common.c +6 -0

index 50a372dc32ae..83bf30349c26 100644
@@ -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,

Signature · 1 line

2.30.2

Russell King (Oracle)

author

2022-02-03 17:33 UTC #4 lore raw

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

In reply to Russell King (Oracle)

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

Quoted 41 lines · 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) <[email protected]>
---
 drivers/net/dsa/b53/b53_common.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/net/dsa/b53/b53_common.c +7 -4
index a3b98992f180..7d62b0aeaae9 100644
@@ -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);
 	}
Signature · 1 line
2.30.2

Signature · 2 lines

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

Florian Fainelli

2022-02-03 18:28 UTC #15 lore raw

In reply to Russell King (Oracle)

Quoted 11 lines · 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.

Quoted 6 lines

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

Russell King (Oracle)

author

2022-02-03 18:30 UTC #16 lore raw

Quoted 15 lines · Florian Fainelli

Quoted 14 lines · 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.

Quoted 7 lines

Quoted 3 lines
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!

Signature · 2 lines

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

Florian Fainelli

Reviewed-by on v1 1/5

2022-02-16 17:52 UTC #2 lore raw

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

In reply to Russell King (Oracle)

Quoted 10 lines · 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) <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

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

Signature · 1 line

Florian

Florian Fainelli

2022-02-16 19:51 UTC #6 lore raw

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

In reply to Russell King (Oracle)

Quoted 8 lines · 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

Quoted 5 lines

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) <[email protected]>

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

Russell King (Oracle)

author

2022-02-17 10:17 UTC #7 lore raw

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

Quoted 37 lines · Florian Fainelli

Quoted 8 lines · 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
Quoted 5 lines
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) <[email protected]>
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:

Review void b53_serdes_phylink_get_caps(struct b53_device · line 189

Quoted 10 lines

(quoted diff) +2 -0

@@ -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.

Signature · 2 lines

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