[PATCH RFC net-next 0/7] net: dsa: mt7530: updates for phylink changes

[PATCH RFC net-next 0/7] net: dsa: mt7530: updates for phylink changes

[Russell King (Oracle)]

Hi,

This series is a partial conversion of the mt7530 DSA driver to the
modern phylink infrastructure. This driver has some exceptional cases
which prevent - at the moment - its full conversion (particularly with
the Autoneg bit) to using phylink_generic_validate().

What stands in the way is this if() condition in
mt753x_phylink_validate():

	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
	    !phy_interface_mode_is_8023z(state->interface)) {

reduces to being always true. I highlight this here for the attention
of the driver maintainers.

Patch 1 populates the supported_interfaces for each port

Patch 2 removes the interface checks that become unnecessary as a result
of patch 1.

Patch 3 removes use of phylink_helper_basex_speed() which is no longer
required by phylink.

Patch 4 becomes possible after patch 3, only indicating the ethtool
modes that can be supported with a particular interface mode - this
involves removing some modes and adding others as per phylink
documentation.

Patch 5 continues patch 4, as RGMII can support 1000base-X ethtool link
mode with an appropriate external PHY.

Patch 6 switches the driver to use phylink_get_linkmodes(), which moves
the driver as close as we can to phylink_generic_validate() due to the
Autoneg bit issue mentioned above.

Patch 7 marks the driver as non-legacy.

 drivers/net/dsa/mt7530.c | 166 ++++++++++++++++-------------------------------
 drivers/net/dsa/mt7530.h |   5 +-
 2 files changed, 58 insertions(+), 113 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/7] net: dsa: mt7530: populate supported_interfaces

[Russell King (Oracle)]

Populate the supported interfaces for mt7530, mt7531 and mt7621 DSA
switches. Filling this in will enable phylink to pre-check the PHY
interface mode against the the supported interfaces bitmap prior to
calling the validate function.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mt7530.c | 68 ++++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mt7530.h |  2 ++
 2 files changed, 70 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index bc77a26c825a..1d01738cacea 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2384,6 +2384,32 @@ mt7531_setup(struct dsa_switch *ds)
 	return 0;
 }
 
+static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
+				     struct phylink_config *config)
+{
+	switch (port) {
+	case 0 ... 4: /* Internal phy */
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  config->supported_interfaces);
+		break;
+
+	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
+		phy_interface_set_rgmii(config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_MII,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  config->supported_interfaces);
+		break;
+
+	case 6: /* 1st cpu port */
+		__set_bit(PHY_INTERFACE_MODE_RGMII,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_TRGMII,
+			  config->supported_interfaces);
+		break;
+	}
+}
+
 static bool
 mt7530_phy_mode_supported(struct dsa_switch *ds, int port,
 			  const struct phylink_link_state *state)
@@ -2420,6 +2446,35 @@ static bool mt7531_is_rgmii_port(struct mt7530_priv *priv, u32 port)
 	return (port == 5) && (priv->p5_intf_sel != P5_INTF_SEL_GMAC5_SGMII);
 }
 
+static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
+				     struct phylink_config *config)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	switch (port) {
+	case 0 ... 4: /* Internal phy */
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  config->supported_interfaces);
+		break;
+
+	case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
+		if (mt7531_is_rgmii_port(priv, port)) {
+			phy_interface_set_rgmii(config->supported_interfaces);
+			break;
+		}
+		fallthrough;
+
+	case 6: /* 1st cpu port supports sgmii/8023z only */
+		__set_bit(PHY_INTERFACE_MODE_SGMII,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  config->supported_interfaces);
+		break;
+	}
+}
+
 static bool
 mt7531_phy_mode_supported(struct dsa_switch *ds, int port,
 			  const struct phylink_link_state *state)
@@ -2904,6 +2959,14 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
 	return 0;
 }
 
+static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
+				    struct phylink_config *config)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	priv->info->mac_port_get_caps(ds, port, config);
+}
+
 static void
 mt7530_mac_port_validate(struct dsa_switch *ds, int port,
 			 unsigned long *supported)
@@ -3139,6 +3202,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_vlan_del		= mt7530_port_vlan_del,
 	.port_mirror_add	= mt753x_port_mirror_add,
 	.port_mirror_del	= mt753x_port_mirror_del,
+	.phylink_get_caps	= mt753x_phylink_get_caps,
 	.phylink_validate	= mt753x_phylink_validate,
 	.phylink_mac_link_state	= mt753x_phylink_mac_link_state,
 	.phylink_mac_config	= mt753x_phylink_mac_config,
@@ -3156,6 +3220,7 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_read = mt7530_phy_read,
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
+		.mac_port_get_caps = mt7530_mac_port_get_caps,
 		.phy_mode_supported = mt7530_phy_mode_supported,
 		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
@@ -3167,6 +3232,7 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_read = mt7530_phy_read,
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
+		.mac_port_get_caps = mt7530_mac_port_get_caps,
 		.phy_mode_supported = mt7530_phy_mode_supported,
 		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
@@ -3179,6 +3245,7 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_write = mt7531_ind_phy_write,
 		.pad_setup = mt7531_pad_setup,
 		.cpu_port_config = mt7531_cpu_port_config,
+		.mac_port_get_caps = mt7531_mac_port_get_caps,
 		.phy_mode_supported = mt7531_phy_mode_supported,
 		.mac_port_validate = mt7531_mac_port_validate,
 		.mac_port_get_state = mt7531_phylink_mac_link_state,
@@ -3241,6 +3308,7 @@ mt7530_probe(struct mdio_device *mdiodev)
 	 */
 	if (!priv->info->sw_setup || !priv->info->pad_setup ||
 	    !priv->info->phy_read || !priv->info->phy_write ||
+	    !priv->info->mac_port_get_caps ||
 	    !priv->info->phy_mode_supported ||
 	    !priv->info->mac_port_validate ||
 	    !priv->info->mac_port_get_state || !priv->info->mac_port_config)
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 91508e2feef9..e285b68ba354 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -769,6 +769,8 @@ struct mt753x_info {
 	int (*phy_write)(struct mt7530_priv *priv, int port, int regnum, u16 val);
 	int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
 	int (*cpu_port_config)(struct dsa_switch *ds, int port);
+	void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
+				  struct phylink_config *config);
 	bool (*phy_mode_supported)(struct dsa_switch *ds, int port,
 				   const struct phylink_link_state *state);
 	void (*mac_port_validate)(struct dsa_switch *ds, int port,
-- 
2.30.2

[PATCH RFC net-next 2/7] net: dsa: mt7530: remove interface checks

[Russell King (Oracle)]

As phylink checks the interface mode against the supported_interfaces
bitmap, we no longer need to validate the interface mode, nor handle
PHY_INTERFACE_MODE_NA in the validation function. Remove these to
simplify the implementation.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mt7530.c | 82 ----------------------------------------
 drivers/net/dsa/mt7530.h |  2 -
 2 files changed, 84 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 1d01738cacea..ae54c6a49676 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2410,37 +2410,6 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
 	}
 }
 
-static bool
-mt7530_phy_mode_supported(struct dsa_switch *ds, int port,
-			  const struct phylink_link_state *state)
-{
-	struct mt7530_priv *priv = ds->priv;
-
-	switch (port) {
-	case 0 ... 4: /* Internal phy */
-		if (state->interface != PHY_INTERFACE_MODE_GMII)
-			return false;
-		break;
-	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
-		if (!phy_interface_mode_is_rgmii(state->interface) &&
-		    state->interface != PHY_INTERFACE_MODE_MII &&
-		    state->interface != PHY_INTERFACE_MODE_GMII)
-			return false;
-		break;
-	case 6: /* 1st cpu port */
-		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
-		    state->interface != PHY_INTERFACE_MODE_TRGMII)
-			return false;
-		break;
-	default:
-		dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
-			port);
-		return false;
-	}
-
-	return true;
-}
-
 static bool mt7531_is_rgmii_port(struct mt7530_priv *priv, u32 port)
 {
 	return (port == 5) && (priv->p5_intf_sel != P5_INTF_SEL_GMAC5_SGMII);
@@ -2475,44 +2444,6 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
 	}
 }
 
-static bool
-mt7531_phy_mode_supported(struct dsa_switch *ds, int port,
-			  const struct phylink_link_state *state)
-{
-	struct mt7530_priv *priv = ds->priv;
-
-	switch (port) {
-	case 0 ... 4: /* Internal phy */
-		if (state->interface != PHY_INTERFACE_MODE_GMII)
-			return false;
-		break;
-	case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
-		if (mt7531_is_rgmii_port(priv, port))
-			return phy_interface_mode_is_rgmii(state->interface);
-		fallthrough;
-	case 6: /* 1st cpu port supports sgmii/8023z only */
-		if (state->interface != PHY_INTERFACE_MODE_SGMII &&
-		    !phy_interface_mode_is_8023z(state->interface))
-			return false;
-		break;
-	default:
-		dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
-			port);
-		return false;
-	}
-
-	return true;
-}
-
-static bool
-mt753x_phy_mode_supported(struct dsa_switch *ds, int port,
-			  const struct phylink_link_state *state)
-{
-	struct mt7530_priv *priv = ds->priv;
-
-	return priv->info->phy_mode_supported(ds, port, state);
-}
-
 static int
 mt753x_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
 {
@@ -2773,9 +2704,6 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 	struct mt7530_priv *priv = ds->priv;
 	u32 mcr_cur, mcr_new;
 
-	if (!mt753x_phy_mode_supported(ds, port, state))
-		goto unsupported;
-
 	switch (port) {
 	case 0 ... 4: /* Internal phy */
 		if (state->interface != PHY_INTERFACE_MODE_GMII)
@@ -2991,12 +2919,6 @@ mt753x_phylink_validate(struct dsa_switch *ds, int port,
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 	struct mt7530_priv *priv = ds->priv;
 
-	if (state->interface != PHY_INTERFACE_MODE_NA &&
-	    !mt753x_phy_mode_supported(ds, port, state)) {
-		linkmode_zero(supported);
-		return;
-	}
-
 	phylink_set_port_modes(mask);
 
 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
@@ -3221,7 +3143,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
-		.phy_mode_supported = mt7530_phy_mode_supported,
 		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
 		.mac_port_config = mt7530_mac_config,
@@ -3233,7 +3154,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
-		.phy_mode_supported = mt7530_phy_mode_supported,
 		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
 		.mac_port_config = mt7530_mac_config,
@@ -3246,7 +3166,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.pad_setup = mt7531_pad_setup,
 		.cpu_port_config = mt7531_cpu_port_config,
 		.mac_port_get_caps = mt7531_mac_port_get_caps,
-		.phy_mode_supported = mt7531_phy_mode_supported,
 		.mac_port_validate = mt7531_mac_port_validate,
 		.mac_port_get_state = mt7531_phylink_mac_link_state,
 		.mac_port_config = mt7531_mac_config,
@@ -3309,7 +3228,6 @@ mt7530_probe(struct mdio_device *mdiodev)
 	if (!priv->info->sw_setup || !priv->info->pad_setup ||
 	    !priv->info->phy_read || !priv->info->phy_write ||
 	    !priv->info->mac_port_get_caps ||
-	    !priv->info->phy_mode_supported ||
 	    !priv->info->mac_port_validate ||
 	    !priv->info->mac_port_get_state || !priv->info->mac_port_config)
 		return -EINVAL;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index e285b68ba354..cbebbcc76509 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -771,8 +771,6 @@ struct mt753x_info {
 	int (*cpu_port_config)(struct dsa_switch *ds, int port);
 	void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
 				  struct phylink_config *config);
-	bool (*phy_mode_supported)(struct dsa_switch *ds, int port,
-				   const struct phylink_link_state *state);
 	void (*mac_port_validate)(struct dsa_switch *ds, int port,
 				  unsigned long *supported);
 	int (*mac_port_get_state)(struct dsa_switch *ds, int port,
-- 
2.30.2

[PATCH RFC net-next 3/7] net: dsa: mt7530: 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/mt7530.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ae54c6a49676..edfc2c6bae37 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2941,11 +2941,6 @@ mt753x_phylink_validate(struct dsa_switch *ds, int port,
 
 	linkmode_and(supported, supported, mask);
 	linkmode_and(state->advertising, state->advertising, mask);
-
-	/* We can only operate at 2500BaseX or 1000BaseX.  If requested
-	 * to advertise both, only report advertising at 2500BaseX.
-	 */
-	phylink_helper_basex_speed(state);
 }
 
 static int
-- 
2.30.2

[PATCH RFC net-next 4/7] net: dsa: mt7530: only indicate linkmodes that can be supported

[Russell King (Oracle)]

Now that mt7530 is not using the basex helper, it becomes unnecessary to
indicate support for both 1000baseX and 2500baseX when one of the 803.3z
PHY interface modes is being selected. Ensure that the driver indicates
only those linkmodes that can actually be supported by the PHY interface
mode.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mt7530.c | 19 +++++++++++++------
 drivers/net/dsa/mt7530.h |  1 +
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index edfc2c6bae37..cbdf4a58e507 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2515,6 +2515,7 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
 }
 
 static void mt7531_sgmii_validate(struct mt7530_priv *priv, int port,
+				  phy_interface_t interface,
 				  unsigned long *supported)
 {
 	/* Port5 supports ethier RGMII or SGMII.
@@ -2526,9 +2527,12 @@ static void mt7531_sgmii_validate(struct mt7530_priv *priv, int port,
 			break;
 		fallthrough;
 	case 6:
-		phylink_set(supported, 1000baseX_Full);
-		phylink_set(supported, 2500baseX_Full);
-		phylink_set(supported, 2500baseT_Full);
+		if (interface == PHY_INTERFACE_MODE_2500BASEX) {
+			phylink_set(supported, 2500baseX_Full);
+			phylink_set(supported, 2500baseT_Full);
+		} else {
+			phylink_set(supported, 1000baseX_Full);
+		}
 	}
 }
 
@@ -2897,6 +2901,7 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
 
 static void
 mt7530_mac_port_validate(struct dsa_switch *ds, int port,
+			 phy_interface_t interface,
 			 unsigned long *supported)
 {
 	if (port == 5)
@@ -2904,11 +2909,12 @@ mt7530_mac_port_validate(struct dsa_switch *ds, int port,
 }
 
 static void mt7531_mac_port_validate(struct dsa_switch *ds, int port,
+				     phy_interface_t interface,
 				     unsigned long *supported)
 {
 	struct mt7530_priv *priv = ds->priv;
 
-	mt7531_sgmii_validate(priv, port, supported);
+	mt7531_sgmii_validate(priv, port, interface, supported);
 }
 
 static void
@@ -2931,10 +2937,11 @@ mt753x_phylink_validate(struct dsa_switch *ds, int port,
 	}
 
 	/* This switch only supports 1G full-duplex. */
-	if (state->interface != PHY_INTERFACE_MODE_MII)
+	if (state->interface != PHY_INTERFACE_MODE_MII &&
+	    state->interface != PHY_INTERFACE_MODE_2500BASEX)
 		phylink_set(mask, 1000baseT_Full);
 
-	priv->info->mac_port_validate(ds, port, mask);
+	priv->info->mac_port_validate(ds, port, state->interface, mask);
 
 	phylink_set(mask, Pause);
 	phylink_set(mask, Asym_Pause);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index cbebbcc76509..73cfd29fbb17 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -772,6 +772,7 @@ struct mt753x_info {
 	void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
 				  struct phylink_config *config);
 	void (*mac_port_validate)(struct dsa_switch *ds, int port,
+				  phy_interface_t interface,
 				  unsigned long *supported);
 	int (*mac_port_get_state)(struct dsa_switch *ds, int port,
 				  struct phylink_link_state *state);
-- 
2.30.2

[PATCH RFC net-next 5/7] net: dsa: mt7530: RGMII can support 1000base-X

[Russell King (Oracle)]

When using an external PHY connected using RGMII to mt7531 port 5, the
PHY can be used to used support 1000baseX connections. Therefore, it
makes no sense to exclude this from the linkmodes permitted for mt7531
port 5, so allow 1000baseX.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index cbdf4a58e507..a2fa629cccc0 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2523,9 +2523,6 @@ static void mt7531_sgmii_validate(struct mt7530_priv *priv, int port,
 	 */
 	switch (port) {
 	case 5:
-		if (mt7531_is_rgmii_port(priv, port))
-			break;
-		fallthrough;
 	case 6:
 		if (interface == PHY_INTERFACE_MODE_2500BASEX) {
 			phylink_set(supported, 2500baseX_Full);
-- 
2.30.2

[PATCH RFC net-next 6/7] net: dsa: mt7530: switch to use phylink_get_linkmodes()

[Russell King (Oracle)]

Switch mt7530 to use phylink_get_linkmodes() to generate the ethtool
linkmodes that can be supported. We are unable to use the generic
helper for this as pause modes are dependent on the interface as
the Autoneg bit depends on the interface mode.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a2fa629cccc0..2a829fc86c01 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2440,6 +2440,8 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
 			  config->supported_interfaces);
 		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
 			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_2500FD;
 		break;
 	}
 }
@@ -2514,25 +2516,6 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
 	return 0;
 }
 
-static void mt7531_sgmii_validate(struct mt7530_priv *priv, int port,
-				  phy_interface_t interface,
-				  unsigned long *supported)
-{
-	/* Port5 supports ethier RGMII or SGMII.
-	 * Port6 supports SGMII only.
-	 */
-	switch (port) {
-	case 5:
-	case 6:
-		if (interface == PHY_INTERFACE_MODE_2500BASEX) {
-			phylink_set(supported, 2500baseX_Full);
-			phylink_set(supported, 2500baseT_Full);
-		} else {
-			phylink_set(supported, 1000baseX_Full);
-		}
-	}
-}
-
 static void
 mt7531_sgmii_link_up_force(struct dsa_switch *ds, int port,
 			   unsigned int mode, phy_interface_t interface,
@@ -2893,25 +2876,11 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
 {
 	struct mt7530_priv *priv = ds->priv;
 
-	priv->info->mac_port_get_caps(ds, port, config);
-}
-
-static void
-mt7530_mac_port_validate(struct dsa_switch *ds, int port,
-			 phy_interface_t interface,
-			 unsigned long *supported)
-{
-	if (port == 5)
-		phylink_set(supported, 1000baseX_Full);
-}
-
-static void mt7531_mac_port_validate(struct dsa_switch *ds, int port,
-				     phy_interface_t interface,
-				     unsigned long *supported)
-{
-	struct mt7530_priv *priv = ds->priv;
+	/* This switch only supports 1G full-duplex. */
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+		MAC_10 | MAC_100 | MAC_1000FD;
 
-	mt7531_sgmii_validate(priv, port, interface, supported);
+	priv->info->mac_port_get_caps(ds, port, config);
 }
 
 static void
@@ -2920,28 +2889,16 @@ mt753x_phylink_validate(struct dsa_switch *ds, int port,
 			struct phylink_link_state *state)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
-	struct mt7530_priv *priv = ds->priv;
+	u32 caps;
+
+	caps = dsa_to_port(ds, port)->pl_config.mac_capabilities;
 
 	phylink_set_port_modes(mask);
+	phylink_get_linkmodes(mask, state->interface, caps);
 
 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
-	    !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);
+	    !phy_interface_mode_is_8023z(state->interface))
 		phylink_set(mask, Autoneg);
-	}
-
-	/* This switch only supports 1G full-duplex. */
-	if (state->interface != PHY_INTERFACE_MODE_MII &&
-	    state->interface != PHY_INTERFACE_MODE_2500BASEX)
-		phylink_set(mask, 1000baseT_Full);
-
-	priv->info->mac_port_validate(ds, port, state->interface, mask);
-
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
 
 	linkmode_and(supported, supported, mask);
 	linkmode_and(state->advertising, state->advertising, mask);
@@ -3142,7 +3099,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
-		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
 		.mac_port_config = mt7530_mac_config,
 	},
@@ -3153,7 +3109,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.phy_write = mt7530_phy_write,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
-		.mac_port_validate = mt7530_mac_port_validate,
 		.mac_port_get_state = mt7530_phylink_mac_link_state,
 		.mac_port_config = mt7530_mac_config,
 	},
@@ -3165,7 +3120,6 @@ static const struct mt753x_info mt753x_table[] = {
 		.pad_setup = mt7531_pad_setup,
 		.cpu_port_config = mt7531_cpu_port_config,
 		.mac_port_get_caps = mt7531_mac_port_get_caps,
-		.mac_port_validate = mt7531_mac_port_validate,
 		.mac_port_get_state = mt7531_phylink_mac_link_state,
 		.mac_port_config = mt7531_mac_config,
 		.mac_pcs_an_restart = mt7531_sgmii_restart_an,
@@ -3227,7 +3181,6 @@ mt7530_probe(struct mdio_device *mdiodev)
 	if (!priv->info->sw_setup || !priv->info->pad_setup ||
 	    !priv->info->phy_read || !priv->info->phy_write ||
 	    !priv->info->mac_port_get_caps ||
-	    !priv->info->mac_port_validate ||
 	    !priv->info->mac_port_get_state || !priv->info->mac_port_config)
 		return -EINVAL;
 
-- 
2.30.2

[PATCH RFC net-next 7/7] net: dsa: mt7530: mark as non-legacy

[Russell King (Oracle)]

The mt7530 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/mt7530.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2a829fc86c01..ad5355e94a4f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2880,6 +2880,12 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
 	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100 | MAC_1000FD;
 
+	/* 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;
+
 	priv->info->mac_port_get_caps(ds, port, config);
 }
 
-- 
2.30.2

Re: [PATCH RFC net-next 0/7] net: dsa: mt7530: updates for phylink changes

[Russell King (Oracle)]

On Thu, Feb 03, 2022 at 05:30:31PM +0000, Russell King (Oracle) wrote:
> Hi,
> 
> This series is a partial conversion of the mt7530 DSA driver to the
> modern phylink infrastructure. This driver has some exceptional cases
> which prevent - at the moment - its full conversion (particularly with
> the Autoneg bit) to using phylink_generic_validate().
> 
> What stands in the way is this if() condition in
> mt753x_phylink_validate():
> 
> 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
> 	    !phy_interface_mode_is_8023z(state->interface)) {
> 
> reduces to being always true. I highlight this here for the attention
> of the driver maintainers.

I'm intending to submit this series later today, preserving the above
behaviour, as I like to keep drivers bug-for-bug compatible, with the
assumption that they've been tested as working, even if the code looks
wrong. However, it would be good if this point could be addressed.
Thanks.

-- 
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/7] net: dsa: mt7530: updates for phylink changes

[Landen Chao]

On Wed, 2022-02-09 at 21:06 +0800, Russell King (Oracle) wrote:
> On Thu, Feb 03, 2022 at 05:30:31PM +0000, Russell King (Oracle)
> wrote:
> > Hi,
> > 
> > This series is a partial conversion of the mt7530 DSA driver to the
> > modern phylink infrastructure. This driver has some exceptional
> > cases
> > which prevent - at the moment - its full conversion (particularly
> > with
> > the Autoneg bit) to using phylink_generic_validate().
> > 
> > What stands in the way is this if() condition in
> > mt753x_phylink_validate():
> > 
> > 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
> > 	    !phy_interface_mode_is_8023z(state->interface)) {
> > 
> > reduces to being always true. I highlight this here for the
> > attention
> > of the driver maintainers.
Hi Russel,

The above behaviour is really a bug. "&&" should be used to prevent
setting MAC_10, MAC_100 and Antoneg capability in particular interface
mode in original code. However, these capability depend on the link
partner of the MAC, such as Ethernet phy. It's okay to keep setting
them.

Thanks for this series.

> 
> I'm intending to submit this series later today, preserving the above
> behaviour, as I like to keep drivers bug-for-bug compatible, with the
> assumption that they've been tested as working, even if the code
> looks
> wrong. However, it would be good if this point could be addressed.
> Thanks.
> 

Re: [PATCH RFC net-next 0/7] net: dsa: mt7530: updates for phylink changes

[Russell King (Oracle)]

On Thu, Feb 10, 2022 at 01:33:34AM +0800, Landen Chao wrote:
> On Wed, 2022-02-09 at 21:06 +0800, Russell King (Oracle) wrote:
> > On Thu, Feb 03, 2022 at 05:30:31PM +0000, Russell King (Oracle)
> > wrote:
> > > Hi,
> > > 
> > > This series is a partial conversion of the mt7530 DSA driver to the
> > > modern phylink infrastructure. This driver has some exceptional
> > > cases
> > > which prevent - at the moment - its full conversion (particularly
> > > with
> > > the Autoneg bit) to using phylink_generic_validate().
> > > 
> > > What stands in the way is this if() condition in
> > > mt753x_phylink_validate():
> > > 
> > > 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
> > > 	    !phy_interface_mode_is_8023z(state->interface)) {
> > > 
> > > reduces to being always true. I highlight this here for the
> > > attention
> > > of the driver maintainers.
> Hi Russel,
> 
> The above behaviour is really a bug. "&&" should be used to prevent
> setting MAC_10, MAC_100 and Antoneg capability in particular interface
> mode in original code. However, these capability depend on the link
> partner of the MAC, such as Ethernet phy. It's okay to keep setting
> them.

Hi Landen,

Thanks for the response. I think you have a slight misunderstanding
about these capabilities, both in the old code and the new code.

You shouldn't care about e.g. the ethernet PHY's capabilities in the
validate() callback at all - phylink will look at the capabilities
reported by phylib, and mask out anything that the MAC says shouldn't
be supported, which has the effect of restricting what the ethernet
PHY will advertise.

In the old code, the validate() callback should only be concerned with
what the MAC and PCS can support - e.g. if the MAC isn't capable of
supportig 1G half-duplex, then the 1G HD capabilities should be masked
out.

With the new code, PCS gain their own validation function, which means
that the validate() callback then becomes very much just about the MAC,
and with phylink_generic_validate(), we can get away with just
specifying a bitmap of the supported interface types for the MAC/PCS
end of the system, and the MAC speeds that are supported.

Given your feedback, I will re-jig the series to take account of your
comments - thanks.

-- 
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/7] net: dsa: mt7530: updates for phylink changes

[Landen Chao]

On Thu, 2022-02-10 at 01:47 +0800, Russell King (Oracle) wrote:
> On Thu, Feb 10, 2022 at 01:33:34AM +0800, Landen Chao wrote:
> > On Wed, 2022-02-09 at 21:06 +0800, Russell King (Oracle) wrote:
> > > On Thu, Feb 03, 2022 at 05:30:31PM +0000, Russell King (Oracle)
> > > wrote:
> > > > Hi,
> > > > 
> > > > This series is a partial conversion of the mt7530 DSA driver to
> > > > the
> > > > modern phylink infrastructure. This driver has some exceptional
> > > > cases
> > > > which prevent - at the moment - its full conversion
> > > > (particularly
> > > > with
> > > > the Autoneg bit) to using phylink_generic_validate().
> > > > 
> > > > What stands in the way is this if() condition in
> > > > mt753x_phylink_validate():
> > > > 
> > > > 	if (state->interface != PHY_INTERFACE_MODE_TRGMII ||
> > > > 	    !phy_interface_mode_is_8023z(state->interface)) {
> > > > 
> > > > reduces to being always true. I highlight this here for the
> > > > attention
> > > > of the driver maintainers.
> > 
> > Hi Russel,
> > 
> > The above behaviour is really a bug. "&&" should be used to prevent
> > setting MAC_10, MAC_100 and Antoneg capability in particular
> > interface
> > mode in original code. However, these capability depend on the link
> > partner of the MAC, such as Ethernet phy. It's okay to keep setting
> > them.
> 
> Hi Landen,
> 
> Thanks for the response. I think you have a slight misunderstanding
> about these capabilities, both in the old code and the new code.
> 
> You shouldn't care about e.g. the ethernet PHY's capabilities in the
> validate() callback at all - phylink will look at the capabilities
> reported by phylib, and mask out anything that the MAC says shouldn't
> be supported, which has the effect of restricting what the ethernet
> PHY will advertise.
> 
> In the old code, the validate() callback should only be concerned
> with
> what the MAC and PCS can support - e.g. if the MAC isn't capable of
> supportig 1G half-duplex, then the 1G HD capabilities should be
> masked
> out.
> 
> With the new code, PCS gain their own validation function, which
> means
> that the validate() callback then becomes very much just about the
> MAC,
> and with phylink_generic_validate(), we can get away with just
> specifying a bitmap of the supported interface types for the MAC/PCS
> end of the system, and the MAC speeds that are supported.
> 
> Given your feedback, I will re-jig the series to take account of your
> comments - thanks.
> 
Hi Russell,

Thanks for your guidance.

I've been stuck with an unnecessary problem, "should I export
MAC_1000/MAC_100/MAC_10 capability of MAC when PCS is connected with an
Ethernet PHY supports both 2500base-X and SGMII mode?" Now I know the
answer, just export all capability that MAC and PCS can support in the
validate(). phylink will help to find out the final configuration by
coworking with phylib.