[PATCH net-next 0/6] net-next: dsa: mt7530: add support for MT7531

[PATCH net-next 0/6] net-next: dsa: mt7530: add support for MT7531

[Landen Chao]

This patch series adds support for MT7531.

MT7531 is the next generation of MT7530 which could be found on Mediatek
router platforms such as MT7622 or MT7629. 

It is also a 7-ports switch with 5 giga embedded phys, 2 cpu ports, and
the same MAC logic of MT7530. Cpu port 6 only supports HSGMII interface.
Cpu port 5 supports either RGMII or HSGMII in different HW SKU. Due to
support for HSGMII interface, pll, and pad setting are different from
MT7530.

Landen Chao (6):
  net: dsa: mt7530: Refine message in Kconfig
  net: dsa: mt7530: Extend device data ready for adding a new hardware
  dt-bindings: net: dsa: add new MT7531 binding to support MT7531
  net: dsa: mt7530: Add the support of MT7531 switch
  arm64: dts: mt7622: add mt7531 dsa to mt7622-rfb1 board
  arm64: dts: mt7622: add mt7531 dsa to bananapi-bpi-r64 board

 .../devicetree/bindings/net/dsa/mt7530.txt    |  77 +-
 .../dts/mediatek/mt7622-bananapi-bpi-r64.dts  |  50 +
 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts  |  63 +-
 drivers/net/dsa/Kconfig                       |   6 +-
 drivers/net/dsa/mt7530.c                      | 874 ++++++++++++++++--
 drivers/net/dsa/mt7530.h                      | 173 +++-
 6 files changed, 1168 insertions(+), 75 deletions(-)

-- 
2.17.1

[PATCH net-next 1/6] net: dsa: mt7530: Refine message in Kconfig

[Landen Chao]

Refine message in Kconfig with fixing typo and an explicit MT7621 support.

Signed-off-by: Landen Chao <landen.chao@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/dsa/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index c7667645f04a..467f80157acf 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -33,12 +33,12 @@ config NET_DSA_LANTIQ_GSWIP
 	  the xrx200 / VR9 SoC.
 
 config NET_DSA_MT7530
-	tristate "Mediatek MT7530 Ethernet switch support"
+	tristate "MediaTek MT7530 and MT7621 Ethernet switch support"
 	depends on NET_DSA
 	select NET_DSA_TAG_MTK
 	---help---
-	  This enables support for the Mediatek MT7530 Ethernet switch
-	  chip.
+	  This enables support for the MediaTek MT7530 and MT7621 Ethernet
+	  switch chip.
 
 config NET_DSA_MV88E6060
 	tristate "Marvell 88E6060 ethernet switch chip support"
-- 
2.17.1

Re: [PATCH net-next 1/6] net: dsa: mt7530: Refine message in Kconfig

[Florian Fainelli]

On 12/10/2019 12:14 AM, Landen Chao wrote:
> Refine message in Kconfig with fixing typo and an explicit MT7621 support.
> 
> Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>

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

[PATCH net-next 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware

[Landen Chao]

Add a structure holding required operations for each device such as device
initialization, PHY port read or write, a checker whether PHY interface is
supported on a certain port, MAC port setup for either bus pad or a
specific PHY interface.

The patch is done for ready adding a new hardware MT7531.

Signed-off-by: Landen Chao <landen.chao@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/dsa/mt7530.c | 231 +++++++++++++++++++++++++++++----------
 drivers/net/dsa/mt7530.h |  29 ++++-
 2 files changed, 203 insertions(+), 57 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ed1ec10ec62b..9a648d1f5d09 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -425,7 +425,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
 }
 
 static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
+mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t mode)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 ncpo1, ssc_delta, trgint, i, xtal;
@@ -1380,13 +1380,111 @@ mt7530_setup(struct dsa_switch *ds)
 	return 0;
 }
 
-static void mt7530_phylink_mac_config(struct dsa_switch *ds, int port,
+static bool mt7530_phy_supported(struct dsa_switch *ds, int port,
+				 const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	switch (port) {
+	case 0: /* Internal phy */
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		if (state->interface != PHY_INTERFACE_MODE_GMII)
+			goto unsupported;
+		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)
+			goto unsupported;
+		break;
+	case 6: /* 1st cpu port */
+		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
+		    state->interface != PHY_INTERFACE_MODE_TRGMII)
+			goto unsupported;
+		break;
+	default:
+		dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
+			port);
+		goto unsupported;
+	}
+
+	return true;
+
+unsupported:
+	return false;
+}
+
+static bool mt753x_phy_supported(struct dsa_switch *ds, int port,
+				 const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->phy_supported(ds, port, state);
+}
+
+static int
+mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	/* Setup TX circuit incluing relevant PAD and driving */
+	mt7530_pad_clk_setup(ds, state->interface);
+
+	if (priv->id == ID_MT7530) {
+		/* Setup RX circuit, relevant PAD and driving on the
+		 * host which must be placed after the setup on the
+		 * device side is all finished.
+		 */
+		mt7623_pad_clk_setup(ds);
+	}
+
+	return 0;
+}
+
+static int
+mt753x_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->pad_setup(ds, state);
+}
+
+static int
+mt7530_mac_setup(struct dsa_switch *ds, int port, unsigned int mode,
+		 const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	/* Only need to setup port5. */
+	if (port != 5)
+		return 0;
+
+	mt7530_setup_port5(priv->ds, state->interface);
+
+	return 0;
+}
+
+static int mt753x_mac_setup(struct dsa_switch *ds, int port, unsigned int mode,
+			    const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->mac_setup(ds, port, mode, state);
+}
+
+static void mt753x_phylink_mac_config(struct dsa_switch *ds, int port,
 				      unsigned int mode,
 				      const struct phylink_link_state *state)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 mcr_cur, mcr_new;
 
+	if (!mt753x_phy_supported(ds, port, state))
+		return;
+
 	switch (port) {
 	case 0: /* Internal phy */
 	case 1:
@@ -1399,35 +1497,24 @@ static void mt7530_phylink_mac_config(struct dsa_switch *ds, int port,
 	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
 		if (priv->p5_interface == state->interface)
 			break;
-		if (!phy_interface_mode_is_rgmii(state->interface) &&
-		    state->interface != PHY_INTERFACE_MODE_MII &&
-		    state->interface != PHY_INTERFACE_MODE_GMII)
-			return;
 
-		mt7530_setup_port5(ds, state->interface);
+		if (mt753x_mac_setup(ds, port, mode, state) < 0)
+			goto unsupported;
+
 		break;
 	case 6: /* 1st cpu port */
 		if (priv->p6_interface == state->interface)
 			break;
 
-		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
-		    state->interface != PHY_INTERFACE_MODE_TRGMII)
-			return;
-
-		/* Setup TX circuit incluing relevant PAD and driving */
-		mt7530_pad_clk_setup(ds, state->interface);
+		mt753x_pad_setup(ds, state);
 
-		if (priv->id == ID_MT7530) {
-			/* Setup RX circuit, relevant PAD and driving on the
-			 * host which must be placed after the setup on the
-			 * device side is all finished.
-			 */
-			mt7623_pad_clk_setup(ds);
-		}
+		if (mt753x_mac_setup(ds, port, mode, state) < 0)
+			goto unsupported;
 
 		priv->p6_interface = state->interface;
 		break;
 	default:
+unsupported:
 		dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
 		return;
 	}
@@ -1488,38 +1575,14 @@ static void mt7530_phylink_mac_link_up(struct dsa_switch *ds, int port,
 	mt7530_port_set_status(priv, port, 1);
 }
 
-static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
+static void mt753x_phylink_validate(struct dsa_switch *ds, int port,
 				    unsigned long *supported,
 				    struct phylink_link_state *state)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
-	switch (port) {
-	case 0: /* Internal phy */
-	case 1:
-	case 2:
-	case 3:
-	case 4:
-		if (state->interface != PHY_INTERFACE_MODE_NA &&
-		    state->interface != PHY_INTERFACE_MODE_GMII)
-			goto unsupported;
-		break;
-	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
-		if (state->interface != PHY_INTERFACE_MODE_NA &&
-		    !phy_interface_mode_is_rgmii(state->interface) &&
-		    state->interface != PHY_INTERFACE_MODE_MII &&
-		    state->interface != PHY_INTERFACE_MODE_GMII)
-			goto unsupported;
-		break;
-	case 6: /* 1st cpu port */
-		if (state->interface != PHY_INTERFACE_MODE_NA &&
-		    state->interface != PHY_INTERFACE_MODE_RGMII &&
-		    state->interface != PHY_INTERFACE_MODE_TRGMII)
-			goto unsupported;
-		break;
-	default:
-		dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
-unsupported:
+	if (state->interface != PHY_INTERFACE_MODE_NA &&
+	    !mt753x_phy_supported(ds, port, state)) {
 		linkmode_zero(supported);
 		return;
 	}
@@ -1590,12 +1653,36 @@ mt7530_phylink_mac_link_state(struct dsa_switch *ds, int port,
 	return 1;
 }
 
+static int
+mt753x_setup(struct dsa_switch *ds)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->setup(ds);
+}
+
+static int
+mt753x_phy_read(struct dsa_switch *ds, int port, int regnum)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->phy_read(ds, port, regnum);
+}
+
+static int
+mt753x_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	return priv->info->phy_write(ds, port, regnum, val);
+}
+
 static const struct dsa_switch_ops mt7530_switch_ops = {
 	.get_tag_protocol	= mtk_get_tag_protocol,
-	.setup			= mt7530_setup,
+	.setup			= mt753x_setup,
 	.get_strings		= mt7530_get_strings,
-	.phy_read		= mt7530_phy_read,
-	.phy_write		= mt7530_phy_write,
+	.phy_read		= mt753x_phy_read,
+	.phy_write		= mt753x_phy_write,
 	.get_ethtool_stats	= mt7530_get_ethtool_stats,
 	.get_sset_count		= mt7530_get_sset_count,
 	.port_enable		= mt7530_port_enable,
@@ -1610,16 +1697,37 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_vlan_prepare	= mt7530_port_vlan_prepare,
 	.port_vlan_add		= mt7530_port_vlan_add,
 	.port_vlan_del		= mt7530_port_vlan_del,
-	.phylink_validate	= mt7530_phylink_validate,
+	.phylink_validate	= mt753x_phylink_validate,
 	.phylink_mac_link_state = mt7530_phylink_mac_link_state,
-	.phylink_mac_config	= mt7530_phylink_mac_config,
+	.phylink_mac_config	= mt753x_phylink_mac_config,
 	.phylink_mac_link_down	= mt7530_phylink_mac_link_down,
 	.phylink_mac_link_up	= mt7530_phylink_mac_link_up,
 };
 
+static const struct mt753x_info mt753x_table[] = {
+	[ID_MT7621] = {
+		.id = ID_MT7621,
+		.setup = mt7530_setup,
+		.phy_read = mt7530_phy_read,
+		.phy_write = mt7530_phy_write,
+		.phy_supported = mt7530_phy_supported,
+		.pad_setup = mt7530_pad_setup,
+		.mac_setup = mt7530_mac_setup,
+	},
+	[ID_MT7530] = {
+		.id = ID_MT7530,
+		.setup = mt7530_setup,
+		.phy_read = mt7530_phy_read,
+		.phy_write = mt7530_phy_write,
+		.phy_supported = mt7530_phy_supported,
+		.pad_setup = mt7530_pad_setup,
+		.mac_setup = mt7530_mac_setup,
+	},
+};
+
 static const struct of_device_id mt7530_of_match[] = {
-	{ .compatible = "mediatek,mt7621", .data = (void *)ID_MT7621, },
-	{ .compatible = "mediatek,mt7530", .data = (void *)ID_MT7530, },
+	{ .compatible = "mediatek,mt7621", .data = &mt753x_table[ID_MT7621], },
+	{ .compatible = "mediatek,mt7530", .data = &mt753x_table[ID_MT7530], },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, mt7530_of_match);
@@ -1660,8 +1768,19 @@ mt7530_probe(struct mdio_device *mdiodev)
 	/* Get the hardware identifier from the devicetree node.
 	 * We will need it for some of the clock and regulator setup.
 	 */
-	priv->id = (unsigned int)(unsigned long)
-		of_device_get_match_data(&mdiodev->dev);
+	priv->info = of_device_get_match_data(&mdiodev->dev);
+	if (!priv->info)
+		return -EINVAL;
+
+	/* Sanity check if these required device operstaions are filled
+	 * properly.
+	 */
+	if (!priv->info->setup || !priv->info->phy_read ||
+	    !priv->info->phy_write || !priv->info->phy_supported ||
+	    !priv->info->pad_setup || !priv->info->mac_setup)
+		return -EINVAL;
+
+	priv->id = priv->info->id;
 
 	if (priv->id == ID_MT7530) {
 		priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index ccb9da8cad0d..aac86e4fc148 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -11,7 +11,7 @@
 #define MT7530_NUM_FDB_RECORDS		2048
 #define MT7530_ALL_MEMBERS		0xff
 
-enum {
+enum mt753x_id {
 	ID_MT7530 = 0,
 	ID_MT7621 = 1,
 };
@@ -428,6 +428,32 @@ static const char *p5_intf_modes(unsigned int p5_interface)
 	}
 }
 
+/* struct mt753x_info -	This is the main data structure for holding the specific
+ *			part for each supported device
+ * @setup:		Holding the handler to a device initialization
+ * @phy_read:		Holding the way reading PHY port
+ * @phy_write:		Holding the way writing PHY port
+ * @phy_supported:	Check if the PHY type is being supported on a certain
+ *			port
+ * @pad_setup:		Holding the way setting up the bus pad for a certain MAC
+ *			port
+ * @mac_setup:		Holding the way setting up the PHY attribute for a
+ *			certain MAC port
+ */
+struct mt753x_info {
+	enum mt753x_id id;
+
+	int (*setup)(struct dsa_switch *ds);
+	int (*phy_read)(struct dsa_switch *ds, int port, int regnum);
+	int (*phy_write)(struct dsa_switch *ds, int port, int regnum, u16 val);
+	bool (*phy_supported)(struct dsa_switch *ds, int port,
+			      const struct phylink_link_state *state);
+	int (*pad_setup)(struct dsa_switch *ds,
+			 const struct phylink_link_state *state);
+	int (*mac_setup)(struct dsa_switch *ds, int port, unsigned int mode,
+			 const struct phylink_link_state *state);
+};
+
 /* struct mt7530_priv -	This is the main data structure for holding the state
  *			of the driver
  * @dev:		The device pointer
@@ -455,6 +481,7 @@ struct mt7530_priv {
 	struct regulator	*core_pwr;
 	struct regulator	*io_pwr;
 	struct gpio_desc	*reset;
+	const struct mt753x_info *info;
 	unsigned int		id;
 	bool			mcm;
 	phy_interface_t		p6_interface;
-- 
2.17.1

Re: [PATCH net-next 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware

[Florian Fainelli]

On 12/10/2019 12:14 AM, Landen Chao wrote:
> Add a structure holding required operations for each device such as device
> initialization, PHY port read or write, a checker whether PHY interface is
> supported on a certain port, MAC port setup for either bus pad or a
> specific PHY interface.
> 
> The patch is done for ready adding a new hardware MT7531.
> 
> Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
[snip]

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ed1ec10ec62b..9a648d1f5d09 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -425,7 +425,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
>  }
>  
>  static int
> -mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> +mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t mode)

Here you pass a phy_interface_t argument but not in mt7632_pad_clk_setup().

> +static int
> +mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +
> +	/* Setup TX circuit incluing relevant PAD and driving */
> +	mt7530_pad_clk_setup(ds, state->interface);
> +
> +	if (priv->id == ID_MT7530) {
> +		/* Setup RX circuit, relevant PAD and driving on the
> +		 * host which must be placed after the setup on the
> +		 * device side is all finished.
> +		 */
> +		mt7623_pad_clk_setup(ds);

Would not it make sense to pass it down here as well for consistency?

[snip]

> @@ -1660,8 +1768,19 @@ mt7530_probe(struct mdio_device *mdiodev)
>  	/* Get the hardware identifier from the devicetree node.
>  	 * We will need it for some of the clock and regulator setup.
>  	 */
> -	priv->id = (unsigned int)(unsigned long)
> -		of_device_get_match_data(&mdiodev->dev);
> +	priv->info = of_device_get_match_data(&mdiodev->dev);
> +	if (!priv->info)
> +		return -EINVAL;
> +
> +	/* Sanity check if these required device operstaions are filled
> +	 * properly.

Typo: operations.

Other than that, this looks okay to me.
-- 
Florian

Re: [PATCH net-next 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware

[Landen Chao]

On Thu, 2019-12-12 at 11:45 +0800, Florian Fainelli wrote:
> 
> On 12/10/2019 12:14 AM, Landen Chao wrote:
> > Add a structure holding required operations for each device such as device
> > initialization, PHY port read or write, a checker whether PHY interface is
> > supported on a certain port, MAC port setup for either bus pad or a
> > specific PHY interface.
> > 
> > The patch is done for ready adding a new hardware MT7531.
> > 
> > Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> [snip]
> 
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index ed1ec10ec62b..9a648d1f5d09 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -425,7 +425,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> >  }
> >  
> >  static int
> > -mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> > +mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t mode)
> 
> Here you pass a phy_interface_t argument but not in mt7632_pad_clk_setup().
> 
> > +static int
> > +mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
> > +{
> > +	struct mt7530_priv *priv = ds->priv;
> > +
> > +	/* Setup TX circuit incluing relevant PAD and driving */
> > +	mt7530_pad_clk_setup(ds, state->interface);
> > +
> > +	if (priv->id == ID_MT7530) {
> > +		/* Setup RX circuit, relevant PAD and driving on the
> > +		 * host which must be placed after the setup on the
> > +		 * device side is all finished.
> > +		 */
> > +		mt7623_pad_clk_setup(ds);
> 
> Would not it make sense to pass it down here as well for consistency?
mt7623_pad_clk() contains the setting for PHY_INTERFACE_MODE_TRGMII. It
is really better to pass phy_interface_t for error handling.
> 
> [snip]
> 
> > @@ -1660,8 +1768,19 @@ mt7530_probe(struct mdio_device *mdiodev)
> >  	/* Get the hardware identifier from the devicetree node.
> >  	 * We will need it for some of the clock and regulator setup.
> >  	 */
> > -	priv->id = (unsigned int)(unsigned long)
> > -		of_device_get_match_data(&mdiodev->dev);
> > +	priv->info = of_device_get_match_data(&mdiodev->dev);
> > +	if (!priv->info)
> > +		return -EINVAL;
> > +
> > +	/* Sanity check if these required device operstaions are filled
> > +	 * properly.
> 
> Typo: operations.
Oops, sorry. I'll correct it.

Landen
> 
> Other than that, this looks okay to me.

Re: [PATCH net-next 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware

[Landen Chao]

On Thu, 2019-12-12 at 11:45 +0800, Florian Fainelli wrote:
> 
> On 12/10/2019 12:14 AM, Landen Chao wrote:
> > Add a structure holding required operations for each device such as device
> > initialization, PHY port read or write, a checker whether PHY interface is
> > supported on a certain port, MAC port setup for either bus pad or a
> > specific PHY interface.
> > 
> > The patch is done for ready adding a new hardware MT7531.
> > 
> > Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> [snip]
> 
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index ed1ec10ec62b..9a648d1f5d09 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -425,7 +425,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> >  }
> >  
> >  static int
> > -mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> > +mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t mode)
> 
> Here you pass a phy_interface_t argument but not in mt7632_pad_clk_setup().
> 
> > +static int
> > +mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
> > +{
> > +	struct mt7530_priv *priv = ds->priv;
> > +
> > +	/* Setup TX circuit incluing relevant PAD and driving */
> > +	mt7530_pad_clk_setup(ds, state->interface);
> > +
> > +	if (priv->id == ID_MT7530) {
> > +		/* Setup RX circuit, relevant PAD and driving on the
> > +		 * host which must be placed after the setup on the
> > +		 * device side is all finished.
> > +		 */
> > +		mt7623_pad_clk_setup(ds);
> 
> Would not it make sense to pass it down here as well for consistency?
mt7623_pad_clk() contains the setting for PHY_INTERFACE_MODE_TRGMII. It
is really better to pass phy_interface_t for error handling.
> 
> [snip]
> 
> > @@ -1660,8 +1768,19 @@ mt7530_probe(struct mdio_device *mdiodev)
> >  	/* Get the hardware identifier from the devicetree node.
> >  	 * We will need it for some of the clock and regulator setup.
> >  	 */
> > -	priv->id = (unsigned int)(unsigned long)
> > -		of_device_get_match_data(&mdiodev->dev);
> > +	priv->info = of_device_get_match_data(&mdiodev->dev);
> > +	if (!priv->info)
> > +		return -EINVAL;
> > +
> > +	/* Sanity check if these required device operstaions are filled
> > +	 * properly.
> 
> Typo: operations.
Oops, sorry. I'll correct it.

Landen
> 
> Other than that, this looks okay to me.

[PATCH net-next 3/6] dt-bindings: net: dsa: add new MT7531 binding to support MT7531

[Landen Chao]

Add devicetree binding to support the compatible mt7531 switch as used
in the MediaTek MT7531 switch.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Landen Chao <landen.chao@mediatek.com>
---
 .../devicetree/bindings/net/dsa/mt7530.txt    | 77 ++++++++++++++++++-
 1 file changed, 74 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
index c5ed5d25f642..dc226a4e402a 100644
--- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -5,6 +5,7 @@ Required properties:
 
 - compatible: may be compatible = "mediatek,mt7530"
 	or compatible = "mediatek,mt7621"
+	or compatible = "mediatek,mt7531"
 - #address-cells: Must be 1.
 - #size-cells: Must be 0.
 - mediatek,mcm: Boolean; if defined, indicates that either MT7530 is the part
@@ -32,10 +33,13 @@ Required properties for the child nodes within ports container:
 
 - reg: Port address described must be 6 for CPU port and from 0 to 5 for
 	user ports.
-- phy-mode: String, must be either "trgmii" or "rgmii" for port labeled
-	 "cpu".
+- phy-mode: String, the follow value would be acceptable for port labeled "cpu"
+	If compatible mediatek,mt7530 or mediatek,mt7621 is set,
+	must be either "trgmii" or "rgmii"
+	If compatible mediatek,mt7531 is set,
+	must be either "sgmii", "1000base-x" or "2500base-x"
 
-Port 5 of the switch is muxed between:
+Port 5 of mt7530 and mt7621 switch is muxed between:
 1. GMAC5: GMAC5 can interface with another external MAC or PHY.
 2. PHY of port 0 or port 4: PHY interfaces with an external MAC like 2nd GMAC
    of the SOC. Used in many setups where port 0/4 becomes the WAN port.
@@ -308,3 +312,70 @@ Example 3: MT7621: Port 5 is connected to external PHY: Port 5 -> external PHY.
 		};
 	};
 };
+
+Example 4:
+
+&eth {
+	gmac0: mac@0 {
+		compatible = "mediatek,eth-mac";
+		reg = <0>;
+		phy-mode = "2500base-x";
+
+		fixed-link {
+			speed = <1000>;
+			full-duplex;
+			pause;
+		};
+	};
+
+	&mdio0 {
+		switch@0 {
+			compatible = "mediatek,mt7531";
+			reg = <0>;
+			reset-gpios = <&pio 54 0>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+
+				port@0 {
+					reg = <0>;
+					label = "lan0";
+				};
+
+				port@1 {
+					reg = <1>;
+					label = "lan1";
+				};
+
+				port@2 {
+					reg = <2>;
+					label = "lan2";
+				};
+
+				port@3 {
+					reg = <3>;
+					label = "lan3";
+				};
+
+				port@4 {
+					reg = <4>;
+					label = "wan";
+				};
+
+				port@6 {
+					reg = <6>;
+					label = "cpu";
+					ethernet = <&gmac0>;
+					phy-mode = "2500base-x";
+
+					fixed-link {
+						speed = <1000>;
+						full-duplex;
+						pause;
+					};
+				};
+			};
+		};
+	};
-- 
2.17.1

Re: [PATCH net-next 3/6] dt-bindings: net: dsa: add new MT7531 binding to support MT7531

[Andrew Lunn]

> +Example 4:
> +
> +&eth {
> +	gmac0: mac@0 {
> +		compatible = "mediatek,eth-mac";
> +		reg = <0>;
> +		phy-mode = "2500base-x";
> +
> +		fixed-link {
> +			speed = <1000>;
> +			full-duplex;
> +			pause;
> +		};
> +	};

2500Base-X, but fixed link speed 1000?

> +				port@6 {
> +					reg = <6>;
> +					label = "cpu";
> +					ethernet = <&gmac0>;
> +					phy-mode = "2500base-x";
> +
> +					fixed-link {
> +						speed = <1000>;
> +						full-duplex;
> +						pause;
> +					};

Same here!

     Andrew

Re: [PATCH net-next 3/6] dt-bindings: net: dsa: add new MT7531 binding to support MT7531

[Landen Chao]

Hi Andrew,

On Wed, 2019-12-11 at 00:20 +0800, Andrew Lunn wrote:
> > +Example 4:
> > +
> > +&eth {
> > +	gmac0: mac@0 {
> > +		compatible = "mediatek,eth-mac";
> > +		reg = <0>;
> > +		phy-mode = "2500base-x";
> > +
> > +		fixed-link {
> > +			speed = <1000>;
> > +			full-duplex;
> > +			pause;
> > +		};
> > +	};
> 
> 2500Base-X, but fixed link speed 1000?
fixed-link speed should be 2500. I will update it.
> 
> > +				port@6 {
> > +					reg = <6>;
> > +					label = "cpu";
> > +					ethernet = <&gmac0>;
> > +					phy-mode = "2500base-x";
> > +
> > +					fixed-link {
> > +						speed = <1000>;
> > +						full-duplex;
> > +						pause;
> > +					};
> 
> Same here!
I will update it or remove fixed-link block as the discussion in dts
thread.
> 
>      Andrew

regards Landen

Re: [PATCH net-next 3/6] dt-bindings: net: dsa: add new MT7531 binding to support MT7531

[Florian Fainelli]

On 12/10/2019 12:14 AM, Landen Chao wrote:
> Add devicetree binding to support the compatible mt7531 switch as used
> in the MediaTek MT7531 switch.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Landen Chao <landen.chao@mediatek.com>

With the comments from Andrew fixed, this looks good to me.
-- 
Florian

[PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

Add new support for MT7531:

MT7531 is the next generation of MT7530. It is also a 7-ports switch with
5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
or HSGMII in different HW sku. Due to HSGMII interface support, pll, and
pad setting are different from MT7530. This patch adds different initial
setting of MT7531.

Signed-off-by: Landen Chao <landen.chao@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/dsa/Kconfig  |   6 +-
 drivers/net/dsa/mt7530.c | 643 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mt7530.h | 144 +++++++++
 3 files changed, 784 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 467f80157acf..a77709469093 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -33,12 +33,12 @@ config NET_DSA_LANTIQ_GSWIP
 	  the xrx200 / VR9 SoC.
 
 config NET_DSA_MT7530
-	tristate "MediaTek MT7530 and MT7621 Ethernet switch support"
+	tristate "MediaTek MT753x and MT7621 Ethernet switch support"
 	depends on NET_DSA
 	select NET_DSA_TAG_MTK
 	---help---
-	  This enables support for the MediaTek MT7530 and MT7621 Ethernet
-	  switch chip.
+	  This enables support for the MediaTek MT7530, MT7531 and MT7621
+	  Ethernet switch chip.
 
 config NET_DSA_MV88E6060
 	tristate "Marvell 88E6060 ethernet switch chip support"
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 9a648d1f5d09..4bee04d75b49 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -285,6 +285,12 @@ mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
 	mutex_unlock(&bus->mdio_lock);
 }
 
+static u32
+_mt7530_unlocked_read(struct mt7530_dummy_poll *p)
+{
+	return mt7530_mii_read(p->priv, p->reg);
+}
+
 static u32
 _mt7530_read(struct mt7530_dummy_poll *p)
 {
@@ -338,6 +344,102 @@ mt7530_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 	mt7530_rmw(priv, reg, val, 0);
 }
 
+static int
+mt7531_ind_mmd_phy_read(struct mt7530_priv *priv, int port, int devad,
+			int regnum)
+{
+	struct mii_bus *bus = priv->bus;
+	struct mt7530_dummy_poll p;
+	u32 reg, val;
+	int ret;
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
+
+	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	reg = MDIO_CL45_ADDR | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad) |
+	      regnum;
+	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	reg = MDIO_CL45_READ | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad);
+	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	ret = val & MDIO_RW_DATA_MASK;
+out:
+	mutex_unlock(&bus->mdio_lock);
+
+	return ret;
+}
+
+static int
+mt7531_ind_mmd_phy_write(struct mt7530_priv *priv, int port, int devad,
+			 int regnum, u32 data)
+{
+	struct mii_bus *bus = priv->bus;
+	struct mt7530_dummy_poll p;
+	u32 val, reg;
+	int ret;
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
+
+	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	reg = MDIO_CL45_ADDR | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad) |
+	      regnum;
+	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	reg = MDIO_CL45_WRITE | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad) |
+	      data;
+	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+out:
+	mutex_unlock(&bus->mdio_lock);
+
+	return ret;
+}
+
 static int
 mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 {
@@ -589,6 +691,83 @@ static int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum,
 	return mdiobus_write_nested(priv->bus, port, regnum, val);
 }
 
+static int
+mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct mii_bus *bus = priv->bus;
+	struct mt7530_dummy_poll p;
+	int ret;
+	u32 val;
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
+
+	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	val = MDIO_CL22_READ | MDIO_PHY_ADDR(port) | MDIO_REG_ADDR(regnum);
+
+	mt7530_mii_write(priv, MT7531_PHY_IAC, val | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+				 !(val & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	ret = val & MDIO_RW_DATA_MASK;
+out:
+	mutex_unlock(&bus->mdio_lock);
+
+	return ret;
+}
+
+static int
+mt7531_ind_phy_write(struct dsa_switch *ds, int port, int regnum,
+		     u16 data)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct mii_bus *bus = priv->bus;
+	struct mt7530_dummy_poll p;
+	int ret;
+	u32 reg;
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
+
+	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
+				 !(reg & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+	reg = MDIO_CL22_WRITE | MDIO_PHY_ADDR(port) | MDIO_REG_ADDR(regnum) |
+	      data;
+
+	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | PHY_ACS_ST);
+
+	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
+				 !(reg & PHY_ACS_ST), 20, 100000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		goto out;
+	}
+
+out:
+	mutex_unlock(&bus->mdio_lock);
+
+	return ret;
+}
+
 static void
 mt7530_get_strings(struct dsa_switch *ds, int port, u32 stringset,
 		   uint8_t *data)
@@ -1380,6 +1559,86 @@ mt7530_setup(struct dsa_switch *ds)
 	return 0;
 }
 
+static int mt7531_setup(struct dsa_switch *ds)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct mt7530_dummy_poll p;
+	u32 val, id;
+	int ret, i;
+
+	/* Reset whole chip through gpio pin or memory-mapped registers for
+	 * different type of hardware
+	 */
+	if (priv->mcm) {
+		reset_control_assert(priv->rstc);
+		usleep_range(1000, 1100);
+		reset_control_deassert(priv->rstc);
+	} else {
+		gpiod_set_value_cansleep(priv->reset, 0);
+		usleep_range(1000, 1100);
+		gpiod_set_value_cansleep(priv->reset, 1);
+	}
+
+	/* Waiting for MT7530 got to stable */
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_HWTRAP);
+	ret = readx_poll_timeout(_mt7530_read, &p, val, val != 0,
+				 20, 1000000);
+	if (ret < 0) {
+		dev_err(priv->dev, "reset timeout\n");
+		return ret;
+	}
+
+	id = mt7530_read(priv, MT7531_CREV);
+	id >>= CHIP_NAME_SHIFT;
+
+	if (id != MT7531_ID) {
+		dev_err(priv->dev, "chip %x can't be supported\n", id);
+		return -ENODEV;
+	}
+
+	/* Reset the switch through internal reset */
+	mt7530_write(priv, MT7530_SYS_CTRL,
+		     SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
+		     SYS_CTRL_REG_RST);
+
+	priv->p6_interface = PHY_INTERFACE_MODE_NA;
+
+	/* Enable PHY power, since phy_device has not yet been created
+	 * provided for phy_[read,write]_mmd_indirect is called, we provide
+	 * our own mt7531_ind_mmd_phy_[read,write] to complete this
+	 * function.
+	 */
+	val = mt7531_ind_mmd_phy_read(priv, 0, PHY_DEV1F,
+				      MT7531_PHY_DEV1F_REG_403);
+	val |= MT7531_PHY_EN_BYPASS_MODE;
+	val &= ~MT7531_PHY_POWER_OFF;
+	mt7531_ind_mmd_phy_write(priv, 0, PHY_DEV1F,
+				 MT7531_PHY_DEV1F_REG_403, val);
+
+	/* Enable and reset MIB counters */
+	mt7530_mib_reset(ds);
+
+	mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK);
+
+	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+		/* Disable forwarding by default on all ports */
+		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
+			   PCR_MATRIX_CLR);
+
+		if (dsa_is_cpu_port(ds, i))
+			mt7530_cpu_port_enable(priv, i);
+		else
+			mt7530_port_disable(ds, i);
+	}
+
+	/* Flush the FDB table */
+	ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static bool mt7530_phy_supported(struct dsa_switch *ds, int port,
 				 const struct phylink_link_state *state)
 {
@@ -1417,6 +1676,49 @@ static bool mt7530_phy_supported(struct dsa_switch *ds, int port,
 	return false;
 }
 
+static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
+{
+	u32 val;
+
+	val = mt7530_read(priv, MT7531_TOP_SIG_SR);
+	return ((val & PAD_DUAL_SGMII_EN) != 0);
+}
+
+static bool mt7531_phy_supported(struct dsa_switch *ds, int port,
+				 const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	switch (port) {
+	case 0: /* Internal phy */
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		if (state->interface != PHY_INTERFACE_MODE_GMII)
+			goto unsupported;
+		break;
+	case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
+		if (!mt7531_dual_sgmii_supported(priv))
+			return phy_interface_mode_is_rgmii(state->interface);
+		/* fall through */
+	case 6: /* 1st cpu port supports sgmii/8023z only */
+		if (state->interface != PHY_INTERFACE_MODE_SGMII &&
+		    !phy_interface_mode_is_8023z(state->interface))
+			goto unsupported;
+		break;
+	default:
+		dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
+			port);
+		goto unsupported;
+	}
+
+	return true;
+
+unsupported:
+	return false;
+}
+
 static bool mt753x_phy_supported(struct dsa_switch *ds, int port,
 				 const struct phylink_link_state *state)
 {
@@ -1444,6 +1746,143 @@ mt7530_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
 	return 0;
 }
 
+static int
+mt7531_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+	u32 xtal, val;
+
+	if (mt7531_dual_sgmii_supported(priv))
+		return 0;
+
+	xtal = mt7530_read(priv, MT7531_HWTRAP) & HWTRAP_XTAL_FSEL_MASK;
+
+	switch (xtal) {
+	case HWTRAP_XTAL_FSEL_25MHZ:
+		/* Step 1 : Disable MT7531 COREPLL */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val &= ~EN_COREPLL;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		/* Step 2: switch to XTAL output */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= SW_CLKSW;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_EN;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Step 3: disable PLLGP and enable program PLLGP */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= SW_PLLGP;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		/* Step 4: program COREPLL output frequency to 500MHz */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_POSDIV_M;
+		val |= 2 << RG_COREPLL_POSDIV_S;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		usleep_range(25, 35);
+
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_SDM_PCW_M;
+		val |= 0x140000 << RG_COREPLL_SDM_PCW_S;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Set feedback divide ratio update signal to high */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val |= RG_COREPLL_SDM_PCW_CHG;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		/* Wait for at least 16 XTAL clocks */
+		usleep_range(10, 20);
+
+		/* Step 5: set feedback divide ratio update signal to low */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_SDM_PCW_CHG;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Enable 325M clock for SGMII */
+		mt7530_write(priv, MT7531_ANA_PLLGP_CR5, 0xad0000);
+
+		/* Enable 250SSC clock for RGMII */
+		mt7530_write(priv, MT7531_ANA_PLLGP_CR2, 0x4f40000);
+
+		/* Step 6: Enable MT7531 PLL */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val |= RG_COREPLL_EN;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= EN_COREPLL;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+		usleep_range(25, 35);
+		break;
+	case HWTRAP_XTAL_FSEL_40MHZ:
+		/* Step 1 : Disable MT7531 COREPLL */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val &= ~EN_COREPLL;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		/* Step 2: switch to XTAL output */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= SW_CLKSW;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_EN;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Step 3: disable PLLGP and enable program PLLGP */
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= SW_PLLGP;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+
+		/* Step 4: program COREPLL output frequency to 500MHz */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_POSDIV_M;
+		val |= 2 << RG_COREPLL_POSDIV_S;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		usleep_range(25, 35);
+
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_SDM_PCW_M;
+		val |= 0x190000 << RG_COREPLL_SDM_PCW_S;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Set feedback divide ratio update signal to high */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val |= RG_COREPLL_SDM_PCW_CHG;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		/* Wait for at least 16 XTAL clocks */
+		usleep_range(10, 20);
+
+		/* Step 5: set feedback divide ratio update signal to low */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val &= ~RG_COREPLL_SDM_PCW_CHG;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		/* Enable 325M clock for SGMII */
+		mt7530_write(priv, MT7531_ANA_PLLGP_CR5, 0xad0000);
+
+		/* Enable 250SSC clock for RGMII */
+		mt7530_write(priv, MT7531_ANA_PLLGP_CR2, 0x4f40000);
+
+		/* Step 6: Enable MT7531 PLL */
+		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		val |= RG_COREPLL_EN;
+		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+
+		val = mt7530_read(priv, MT7531_PLLGP_EN);
+		val |= EN_COREPLL;
+		mt7530_write(priv, MT7531_PLLGP_EN, val);
+		usleep_range(25, 35);
+		break;
+	}
+
+	return 0;
+}
+
 static int
 mt753x_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
 {
@@ -1467,6 +1906,149 @@ mt7530_mac_setup(struct dsa_switch *ds, int port, unsigned int mode,
 	return 0;
 }
 
+static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port)
+{
+	u32 val;
+
+	if (port != 5) {
+		dev_err(priv->dev, "RGMII mode is not available for port %d\n",
+			port);
+		return -EINVAL;
+	}
+
+	val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
+	val |= GP_CLK_EN;
+	val &= ~GP_MODE_MASK;
+	val |= GP_MODE(MT7531_GP_MODE_RGMII);
+	val |= TXCLK_NO_REVERSE;
+	val |= RXCLK_NO_DELAY;
+	val &= ~CLK_SKEW_IN_MASK;
+	val |= CLK_SKEW_IN(MT7531_CLK_SKEW_NO_CHG);
+	val &= ~CLK_SKEW_OUT_MASK;
+	val |= CLK_SKEW_OUT(MT7531_CLK_SKEW_NO_CHG);
+	mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
+
+	return 0;
+}
+
+static int mt7531_sgmii_setup_mode_force(struct mt7530_priv *priv, u32 port,
+					 const struct phylink_link_state *state)
+{
+	u32 val;
+
+	if (port != 5 && port != 6)
+		return -EINVAL;
+
+	val = mt7530_read(priv, MT7531_QPHY_PWR_STATE_CTRL(port));
+	val |= MT7531_SGMII_PHYA_PWD;
+	mt7530_write(priv, MT7531_QPHY_PWR_STATE_CTRL(port), val);
+
+	val = mt7530_read(priv, MT7531_PHYA_CTRL_SIGNAL3(port));
+	val &= ~MT7531_RG_TPHY_SPEED_MASK;
+	if (state->interface == PHY_INTERFACE_MODE_2500BASEX)
+		val |= MT7531_RG_TPHY_SPEED_3_125G;
+	mt7530_write(priv, MT7531_PHYA_CTRL_SIGNAL3(port), val);
+
+	val = mt7530_read(priv, MT7531_PCS_CONTROL_1(port));
+	val &= ~MT7531_SGMII_AN_ENABLE;
+	mt7530_write(priv, MT7531_PCS_CONTROL_1(port), val);
+
+	val = mt7530_read(priv, MT7531_SGMII_MODE(port));
+	val &= ~MT7531_SGMII_IF_MODE_MASK;
+
+	switch (state->speed) {
+	case SPEED_10:
+		val |= MT7531_SGMII_FORCE_SPEED_10;
+		break;
+	case SPEED_100:
+		val |= MT7531_SGMII_FORCE_SPEED_100;
+		break;
+	case SPEED_2500:
+	case SPEED_1000:
+		val |= MT7531_SGMII_FORCE_SPEED_1000;
+		break;
+	};
+
+	val &= ~MT7531_SGMII_FORCE_DUPLEX;
+	/* For sgmii force mode, 0 is full duplex and 1 is half duplex */
+	if (state->duplex == DUPLEX_HALF)
+		val |= MT7531_SGMII_FORCE_DUPLEX;
+
+	mt7530_write(priv, MT7531_SGMII_MODE(port), val);
+
+	val = mt7530_read(priv, MT7531_QPHY_PWR_STATE_CTRL(port));
+	val &= ~MT7531_SGMII_PHYA_PWD;
+	mt7530_write(priv, MT7531_QPHY_PWR_STATE_CTRL(port), val);
+
+	return 0;
+}
+
+static int mt7531_sgmii_setup_mode_an(struct mt7530_priv *priv, int port,
+				      const struct phylink_link_state *state)
+{
+	u32 val;
+
+	if (port != 5 && port != 6)
+		return -EINVAL;
+
+	val = mt7530_read(priv, MT7531_QPHY_PWR_STATE_CTRL(port));
+	val |= MT7531_SGMII_PHYA_PWD;
+	mt7530_write(priv, MT7531_QPHY_PWR_STATE_CTRL(port), val);
+
+	switch (state->speed) {
+	case SPEED_10:
+	case SPEED_100:
+	case SPEED_1000:
+		val = mt7530_read(priv, MT7531_PHYA_CTRL_SIGNAL3(port));
+		val &= ~MT7531_RG_TPHY_SPEED_MASK;
+		mt7530_write(priv, MT7531_PHYA_CTRL_SIGNAL3(port), val);
+		break;
+	default:
+		dev_info(priv->dev, "invalid SGMII speed idx %d for port %d\n",
+			 state->speed, port);
+
+		return -EINVAL;
+	}
+
+	val = mt7530_read(priv, MT7531_SGMII_MODE(port));
+	val |= MT7531_SGMII_REMOTE_FAULT_DIS;
+	mt7530_write(priv, MT7531_SGMII_MODE(port), val);
+
+	val = mt7530_read(priv, MT7531_PCS_CONTROL_1(port));
+	val |= MT7531_SGMII_AN_RESTART;
+	mt7530_write(priv, MT7531_PCS_CONTROL_1(port), val);
+
+	val = mt7530_read(priv, MT7531_QPHY_PWR_STATE_CTRL(port));
+	val &= ~MT7531_SGMII_PHYA_PWD;
+	mt7530_write(priv, MT7531_QPHY_PWR_STATE_CTRL(port), val);
+
+	return 0;
+}
+
+static int
+mt7531_mac_setup(struct dsa_switch *ds, int port, unsigned int mode,
+		 const struct phylink_link_state *state)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	if (port < 5 || port >= MT7530_NUM_PORTS) {
+		dev_err(priv->dev, "port %d is not a MAC port\n", port);
+		return -EINVAL;
+	}
+
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_RGMII:
+		return mt7531_rgmii_setup(priv, port);
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		return mt7531_sgmii_setup_mode_force(priv, port, state);
+	case PHY_INTERFACE_MODE_SGMII:
+		return mt7531_sgmii_setup_mode_an(priv, port, state);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int mt753x_mac_setup(struct dsa_switch *ds, int port, unsigned int mode,
 			    const struct phylink_link_state *state)
 {
@@ -1501,6 +2083,8 @@ static void mt753x_phylink_mac_config(struct dsa_switch *ds, int port,
 		if (mt753x_mac_setup(ds, port, mode, state) < 0)
 			goto unsupported;
 
+		priv->p5_interface = state->interface;
+
 		break;
 	case 6: /* 1st cpu port */
 		if (priv->p6_interface == state->interface)
@@ -1515,11 +2099,11 @@ static void mt753x_phylink_mac_config(struct dsa_switch *ds, int port,
 		break;
 	default:
 unsupported:
-		dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
 		return;
 	}
 
-	if (phylink_autoneg_inband(mode)) {
+	if (phylink_autoneg_inband(mode) &&
+	    state->interface != PHY_INTERFACE_MODE_SGMII) {
 		dev_err(ds->dev, "%s: in-band negotiation unsupported\n",
 			__func__);
 		return;
@@ -1530,13 +2114,15 @@ static void mt753x_phylink_mac_config(struct dsa_switch *ds, int port,
 	mcr_new &= ~(PMCR_FORCE_SPEED_1000 | PMCR_FORCE_SPEED_100 |
 		     PMCR_FORCE_FDX | PMCR_TX_FC_EN | PMCR_RX_FC_EN);
 	mcr_new |= PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | PMCR_BACKOFF_EN |
-		   PMCR_BACKPR_EN | PMCR_FORCE_MODE | PMCR_FORCE_LNK;
+		   PMCR_BACKPR_EN | PMCR_FORCE_MODE_ID(priv->id) |
+		   PMCR_FORCE_LNK;
 
 	/* Are we connected to external phy */
 	if (port == 5 && dsa_is_user_port(ds, 5))
 		mcr_new |= PMCR_EXT_PHY;
 
 	switch (state->speed) {
+	case SPEED_2500:
 	case SPEED_1000:
 		mcr_new |= PMCR_FORCE_SPEED_1000;
 		break;
@@ -1556,6 +2142,27 @@ static void mt753x_phylink_mac_config(struct dsa_switch *ds, int port,
 		mt7530_write(priv, MT7530_PMCR_P(port), mcr_new);
 }
 
+void mt7531_sgmii_restart_an(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+	u32 val;
+
+	val = mt7530_read(priv, MT7531_PCS_CONTROL_1(port));
+	val |= MT7531_SGMII_AN_RESTART;
+	mt7530_write(priv, MT7531_PCS_CONTROL_1(port), val);
+}
+
+static void
+mt753x_phylink_mac_an_restart(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	if (!priv->info->port_an_restart)
+		return;
+
+	priv->info->port_an_restart(ds, port);
+}
+
 static void mt7530_phylink_mac_link_down(struct dsa_switch *ds, int port,
 					 unsigned int mode,
 					 phy_interface_t interface)
@@ -1590,9 +2197,20 @@ static void mt753x_phylink_validate(struct dsa_switch *ds, int port,
 	phylink_set_port_modes(mask);
 	phylink_set(mask, Autoneg);
 
-	if (state->interface == PHY_INTERFACE_MODE_TRGMII) {
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_TRGMII:
 		phylink_set(mask, 1000baseT_Full);
-	} else {
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		phylink_set(mask, 1000baseX_Full);
+		phylink_set(mask, 2500baseX_Full);
+		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		phylink_set(mask, 1000baseT_Full);
+		phylink_set(mask, 1000baseX_Full);
+		/* fall through */
+	default:
 		phylink_set(mask, 10baseT_Half);
 		phylink_set(mask, 10baseT_Full);
 		phylink_set(mask, 100baseT_Half);
@@ -1604,6 +2222,7 @@ static void mt753x_phylink_validate(struct dsa_switch *ds, int port,
 			if (port == 5)
 				phylink_set(mask, 1000baseX_Full);
 		}
+		break;
 	}
 
 	phylink_set(mask, Pause);
@@ -1698,8 +2317,9 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_vlan_add		= mt7530_port_vlan_add,
 	.port_vlan_del		= mt7530_port_vlan_del,
 	.phylink_validate	= mt753x_phylink_validate,
-	.phylink_mac_link_state = mt7530_phylink_mac_link_state,
+	.phylink_mac_link_state	= mt7530_phylink_mac_link_state,
 	.phylink_mac_config	= mt753x_phylink_mac_config,
+	.phylink_mac_an_restart	= mt753x_phylink_mac_an_restart,
 	.phylink_mac_link_down	= mt7530_phylink_mac_link_down,
 	.phylink_mac_link_up	= mt7530_phylink_mac_link_up,
 };
@@ -1723,11 +2343,22 @@ static const struct mt753x_info mt753x_table[] = {
 		.pad_setup = mt7530_pad_setup,
 		.mac_setup = mt7530_mac_setup,
 	},
+	[ID_MT7531] = {
+		.id = ID_MT7531,
+		.setup = mt7531_setup,
+		.phy_read = mt7531_ind_phy_read,
+		.phy_write = mt7531_ind_phy_write,
+		.phy_supported = mt7531_phy_supported,
+		.pad_setup = mt7531_pad_setup,
+		.mac_setup = mt7531_mac_setup,
+		.port_an_restart = mt7531_sgmii_restart_an,
+	},
 };
 
 static const struct of_device_id mt7530_of_match[] = {
 	{ .compatible = "mediatek,mt7621", .data = &mt753x_table[ID_MT7621], },
 	{ .compatible = "mediatek,mt7530", .data = &mt753x_table[ID_MT7530], },
+	{ .compatible = "mediatek,mt7531", .data = &mt753x_table[ID_MT7531], },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, mt7530_of_match);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index aac86e4fc148..557328c6c032 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -14,6 +14,7 @@
 enum mt753x_id {
 	ID_MT7530 = 0,
 	ID_MT7621 = 1,
+	ID_MT7531 = 2,
 };
 
 #define	NUM_TRGMII_CTRL			5
@@ -201,6 +202,19 @@ enum mt7530_vlan_port_attr {
 #define  PMCR_FORCE_LNK			BIT(0)
 #define  PMCR_SPEED_MASK		(PMCR_FORCE_SPEED_100 | \
 					 PMCR_FORCE_SPEED_1000)
+#define  MT7531_FORCE_LNK		BIT(31)
+#define  MT7531_FORCE_SPD		BIT(30)
+#define  MT7531_FORCE_DPX		BIT(29)
+#define  MT7531_FORCE_RX_FC		BIT(28)
+#define  MT7531_FORCE_TX_FC		BIT(27)
+#define  MT7531_FORCE_MODE		(MT7531_FORCE_LNK | \
+					 MT7531_FORCE_SPD | \
+					 MT7531_FORCE_DPX | \
+					 MT7531_FORCE_RX_FC | \
+					 MT7531_FORCE_TX_FC)
+#define  PMCR_FORCE_MODE_ID(id)		(((id) == ID_MT7531) ? \
+					 MT7531_FORCE_MODE : \
+					 PMCR_FORCE_MODE)
 
 #define MT7530_PMSR_P(x)		(0x3008 + (x) * 0x100)
 #define  PMSR_EEE1G			BIT(7)
@@ -231,12 +245,111 @@ enum mt7530_vlan_port_attr {
 					 CCR_RX_OCT_CNT_BAD | \
 					 CCR_TX_OCT_CNT_GOOD | \
 					 CCR_TX_OCT_CNT_BAD)
+
+/* SGMII registers */
+#define MT7531_SGMII_REG_BASE		0x5000
+#define MT7531_SGMII_REG(p, r)		(MT7531_SGMII_REG_BASE + \
+					((p) - 5) * 0x1000 + (r))
+
+/* SGMII PCS_CONTROL_1 */
+#define MT7531_PCS_CONTROL_1(p)		MT7531_SGMII_REG(p, 0x00)
+#define  MT7531_SGMII_LINK_STATUS	BIT(18)
+#define  MT7531_SGMII_AN_ENABLE		BIT(12)
+#define  MT7531_SGMII_AN_RESTART	BIT(9)
+
+/* Fields of SGMII_MODE */
+#define MT7531_SGMII_MODE(p)		MT7531_SGMII_REG(p, 0x20)
+#define  MT7531_SGMII_REMOTE_FAULT_DIS	BIT(8)
+#define  MT7531_SGMII_FORCE_DUPLEX	BIT(4)
+#define  MT7531_SGMII_IF_MODE_MASK	GENMASK(5, 1)
+#define  MT7531_SGMII_FORCE_SPEED_10	0x0
+#define  MT7531_SGMII_FORCE_SPEED_100	BIT(2)
+#define  MT7531_SGMII_FORCE_SPEED_1000	BIT(3)
+
+/* Fields of QPHY_PWR_STATE_CTRL */
+#define MT7531_QPHY_PWR_STATE_CTRL(p)	MT7531_SGMII_REG(p, 0xe8)
+#define  MT7531_SGMII_PHYA_PWD		BIT(4)
+
+/* Values of SGMII SPEED */
+#define MT7531_PHYA_CTRL_SIGNAL3(p)	MT7531_SGMII_REG(p, 0x128)
+#define  MT7531_RG_TPHY_SPEED_MASK	(BIT(2) | BIT(3))
+#define  MT7531_RG_TPHY_SPEED_1_25G	0x0
+#define  MT7531_RG_TPHY_SPEED_3_125G	BIT(2)
+
 /* Register for system reset */
 #define MT7530_SYS_CTRL			0x7000
 #define  SYS_CTRL_PHY_RST		BIT(2)
 #define  SYS_CTRL_SW_RST		BIT(1)
 #define  SYS_CTRL_REG_RST		BIT(0)
 
+/* Register for PHY Indirect Access Control */
+#define MT7531_PHY_IAC			0x701C
+#define  PHY_ACS_ST			BIT(31)
+#define  MDIO_REG_ADDR_MASK		(0x1f << 25)
+#define  MDIO_PHY_ADDR_MASK		(0x1f << 20)
+#define  MDIO_CMD_MASK			(0x3 << 18)
+#define  MDIO_ST_MASK			(0x3 << 16)
+#define  MDIO_RW_DATA_MASK		(0xffff)
+#define  MDIO_REG_ADDR(x)		(((x) & 0x1f) << 25)
+#define  MDIO_DEV_ADDR(x)		(((x) & 0x1f) << 25)
+#define  MDIO_PHY_ADDR(x)		(((x) & 0x1f) << 20)
+#define  MDIO_CMD(x)			(((x) & 0x3) << 18)
+#define  MDIO_ST(x)			(((x) & 0x3) << 16)
+
+enum mt7531_phy_iac_cmd {
+	MT7531_MDIO_ADDR = 0,
+	MT7531_MDIO_WRITE = 1,
+	MT7531_MDIO_READ = 2,
+	MT7531_MDIO_READ_CL45 = 3,
+};
+
+/* MDIO_ST: MDIO start field */
+enum mt7531_mdio_st {
+	MT7531_MDIO_ST_CL45 = 0,
+	MT7531_MDIO_ST_CL22 = 1,
+};
+
+#define  MDIO_CL22_READ			(MDIO_ST(MT7531_MDIO_ST_CL22) | \
+					 MDIO_CMD(MT7531_MDIO_READ))
+#define  MDIO_CL22_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL22) | \
+					 MDIO_CMD(MT7531_MDIO_WRITE))
+#define  MDIO_CL45_ADDR			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
+					 MDIO_CMD(MT7531_MDIO_ADDR))
+#define  MDIO_CL45_READ			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
+					 MDIO_CMD(MT7531_MDIO_READ))
+#define  MDIO_CL45_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL45) | \
+					 MDIO_CMD(MT7531_MDIO_WRITE))
+
+#define MT7531_CLKGEN_CTRL		0x7500
+#define  CLK_SKEW_OUT(x)		(((x) & 0x3) << 8)
+#define  CLK_SKEW_OUT_MASK		(0x3 << 8)
+#define  CLK_SKEW_IN(x)			(((x) & 0x3) << 6)
+#define  CLK_SKEW_IN_MASK		(0x3 << 6)
+#define  RXCLK_NO_DELAY			BIT(5)
+#define  TXCLK_NO_REVERSE		BIT(4)
+#define  GP_MODE(x)			(((x) & 0x3) << 1)
+#define  GP_MODE_MASK			(0x3 << 1)
+#define  GP_CLK_EN			BIT(0)
+
+#define PHY_DEV1F			0x1f
+#define MT7531_PHY_DEV1F_REG_403	0x403
+
+#define MT7531_PHY_EN_BYPASS_MODE	BIT(4)
+#define MT7531_PHY_POWER_OFF		BIT(5)
+
+enum mt7531_gp_mode {
+	MT7531_GP_MODE_RGMII = 0,
+	MT7531_GP_MODE_MII = 1,
+	MT7531_GP_MODE_REV_MII = 2
+};
+
+enum mt7531_clk_skew {
+	MT7531_CLK_SKEW_NO_CHG = 0,
+	MT7531_CLK_SKEW_DLY_100PPS = 1,
+	MT7531_CLK_SKEW_DLY_200PPS = 2,
+	MT7531_CLK_SKEW_REVERSE = 3,
+};
+
 /* Register for hw trap status */
 #define MT7530_HWTRAP			0x7800
 #define  HWTRAP_XTAL_MASK		(BIT(10) | BIT(9))
@@ -244,6 +357,11 @@ enum mt7530_vlan_port_attr {
 #define  HWTRAP_XTAL_40MHZ		(BIT(10))
 #define  HWTRAP_XTAL_20MHZ		(BIT(9))
 
+#define MT7531_HWTRAP			0x7800
+#define  HWTRAP_XTAL_FSEL_MASK		BIT(7)
+#define  HWTRAP_XTAL_FSEL_25MHZ		BIT(7)
+#define  HWTRAP_XTAL_FSEL_40MHZ		0
+
 /* Register for hw trap modification */
 #define MT7530_MHWTRAP			0x7804
 #define  MHWTRAP_PHY0_SEL		BIT(20)
@@ -258,14 +376,34 @@ enum mt7530_vlan_port_attr {
 #define MT7530_TOP_SIG_CTRL		0x7808
 #define  TOP_SIG_CTRL_NORMAL		(BIT(17) | BIT(16))
 
+#define MT7531_TOP_SIG_SR		0x780c
+#define  PAD_DUAL_SGMII_EN		BIT(1)
+
 #define MT7530_IO_DRV_CR		0x7810
 #define  P5_IO_CLK_DRV(x)		((x) & 0x3)
 #define  P5_IO_DATA_DRV(x)		(((x) & 0x3) << 4)
 
+#define MT7531_PLLGP_EN			0x7820
+#define  EN_COREPLL			BIT(2)
+#define  SW_CLKSW			BIT(1)
+#define  SW_PLLGP			BIT(0)
+
+#define MT7531_PLLGP_CR0		0x78a8
+#define  RG_COREPLL_EN			BIT(22)
+#define  RG_COREPLL_POSDIV_S		23
+#define  RG_COREPLL_POSDIV_M		0x3800000
+#define  RG_COREPLL_SDM_PCW_S		1
+#define  RG_COREPLL_SDM_PCW_M		0x3ffffe
+#define  RG_COREPLL_SDM_PCW_CHG		BIT(0)
+
 #define MT7530_P6ECR			0x7830
 #define  P6_INTF_MODE_MASK		0x3
 #define  P6_INTF_MODE(x)		((x) & 0x3)
 
+/* RGMII and SGMII PLL clock */
+#define MT7531_ANA_PLLGP_CR2		0x78b0
+#define MT7531_ANA_PLLGP_CR5		0x78bc
+
 /* Registers for TRGMII on the both side */
 #define MT7530_TRGMII_RCK_CTRL		0x7a00
 #define GSW_TRGMII_RCK_CTRL		0x300
@@ -316,6 +454,9 @@ enum mt7530_vlan_port_attr {
 #define  CHIP_NAME_SHIFT		16
 #define  MT7530_ID			0x7530
 
+#define MT7531_CREV			0x781C
+#define  MT7531_ID			0x7531
+
 /* Registers for core PLL access through mmd indirect */
 #define CORE_PLL_GROUP2			0x401
 #define  RG_SYSPLL_EN_NORMAL		BIT(15)
@@ -439,6 +580,8 @@ static const char *p5_intf_modes(unsigned int p5_interface)
  *			port
  * @mac_setup:		Holding the way setting up the PHY attribute for a
  *			certain MAC port
+ * @port_an_restart	Holding the way restarting 802.3z BaseX autonegotiation
+ *			for a certain MAC port
  */
 struct mt753x_info {
 	enum mt753x_id id;
@@ -452,6 +595,7 @@ struct mt753x_info {
 			 const struct phylink_link_state *state);
 	int (*mac_setup)(struct dsa_switch *ds, int port, unsigned int mode,
 			 const struct phylink_link_state *state);
+	void (*port_an_restart)(struct dsa_switch *ds, int port);
 };
 
 /* struct mt7530_priv -	This is the main data structure for holding the state
-- 
2.17.1

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Andrew Lunn]

On Tue, Dec 10, 2019 at 04:14:40PM +0800, Landen Chao wrote:
> Add new support for MT7531:
> 
> MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> or HSGMII in different HW sku.

Hi Landen

Looking at the code, you seem to treat HSGMII as 2500Base-X. Is this
correct? Or is it SGMII over clocked to 2.5Gbps?

	 Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Vladimir Oltean]

Hi Landen, Andrew,

On Tue, 10 Dec 2019 at 18:36, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Dec 10, 2019 at 04:14:40PM +0800, Landen Chao wrote:
> > Add new support for MT7531:
> >
> > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> > or HSGMII in different HW sku.
>
> Hi Landen
>
> Looking at the code, you seem to treat HSGMII as 2500Base-X. Is this
> correct? Or is it SGMII over clocked to 2.5Gbps?
>
>          Andrew

I think the better question is: what is being understood by HSGMII?
What does the AN base page look like? Do you support AN with the 2500
baud speed? Do you support AN with lower speeds? Do you support lower
speeds at all? What PHYs do you interoperate with in this mode? If you
don't support AN, then what's so SGMII about it? And for that matter,
if you don't support AN, what's so 2500Base-X (802.3z) about it? I see
you unconditionally force the speed and disable AN in this mode. Do
you have any reference for what SerDes protocol your hardware
implements in this mode?

Thanks,
-Vladimir

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Marek Behun]

On Tue, 10 Dec 2019 17:35:57 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Tue, Dec 10, 2019 at 04:14:40PM +0800, Landen Chao wrote:
> > Add new support for MT7531:
> > 
> > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> > or HSGMII in different HW sku.  
> 
> Hi Landen
> 
> Looking at the code, you seem to treat HSGMII as 2500Base-X. Is this
> correct? Or is it SGMII over clocked to 2.5Gbps?
> 
> 	 Andrew

How would that work? Would 10 and 100 be overclocked to 25 and 250?

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Andrew Lunn]

On Tue, Dec 10, 2019 at 09:33:51PM +0100, Marek Behun wrote:
> On Tue, 10 Dec 2019 17:35:57 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > On Tue, Dec 10, 2019 at 04:14:40PM +0800, Landen Chao wrote:
> > > Add new support for MT7531:
> > > 
> > > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > > port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> > > or HSGMII in different HW sku.  
> > 
> > Hi Landen
> > 
> > Looking at the code, you seem to treat HSGMII as 2500Base-X. Is this
> > correct? Or is it SGMII over clocked to 2.5Gbps?
> > 
> > 	 Andrew
> 
> How would that work? Would 10 and 100 be overclocked to 25 and 250?

No. SGMII clocked up to 2.5G does not support any of the lower
speeds. And inband signalling does not make much sense, the control
word is all wrong.

     Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

Hi Andrew,

On Wed, 2019-12-11 at 00:35 +0800, Andrew Lunn wrote:
> On Tue, Dec 10, 2019 at 04:14:40PM +0800, Landen Chao wrote:
> > Add new support for MT7531:
> > 
> > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> > or HSGMII in different HW sku.
> 
> Hi Landen
> 
> Looking at the code, you seem to treat HSGMII as 2500Base-X. Is this
> correct? Or is it SGMII over clocked to 2.5Gbps?
After re-read MT7622 tread[0] again, and according to the configurable
part of this IP, it is closer to 2500Base-X definition:
``PHY_INTERFACE_MODE_2500BASEX``
    This defines a variant of 1000BASE-X which is clocked 2.5 times
faster, than the 802.3 standard giving a fixed bit rate of 3.125Gbaud.

If HSGMII means SGMII over clocked to 2.5Gbps, the introduction needs to
be changed to "support SGMII/1000Base-X/2500Base-x".

[0]:
https://patchwork.kernel.org/patch/11057527/
> 
> 	 Andrew

Regards Landen

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Andrew Lunn]

> +static int
> +mt7531_ind_mmd_phy_read(struct mt7530_priv *priv, int port, int devad,
> +			int regnum)
> +{
> +	struct mii_bus *bus = priv->bus;
> +	struct mt7530_dummy_poll p;
> +	u32 reg, val;
> +	int ret;
> +
> +	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
> +
> +	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> +
> +	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
> +				 !(val & PHY_ACS_ST), 20, 100000);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "poll timeout\n");
> +		goto out;
> +	}
> +
> +	reg = MDIO_CL45_ADDR | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad) |
> +	      regnum;

It might be better to call this mt7531_ind_c45_phy_read()

> +static int
> +mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +	struct mii_bus *bus = priv->bus;
> +	struct mt7530_dummy_poll p;
> +	int ret;
> +	u32 val;
> +
> +	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
> +
> +	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> +
> +	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
> +				 !(val & PHY_ACS_ST), 20, 100000);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "poll timeout\n");
> +		goto out;
> +	}
> +
> +	val = MDIO_CL22_READ | MDIO_PHY_ADDR(port) | MDIO_REG_ADDR(regnum);

This is then mt7531_ind_c22_phy_read().

And then you can add a wrapper around this to provide

mt7531_phy_read() which can do both C22 and C45.

> +	[ID_MT7531] = {
> +		.id = ID_MT7531,
> +		.setup = mt7531_setup,
> +		.phy_read = mt7531_ind_phy_read,

and use it here.

  Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

On Wed, 2019-12-11 at 00:44 +0800, Andrew Lunn wrote:
> > +static int
> > +mt7531_ind_mmd_phy_read(struct mt7530_priv *priv, int port, int devad,
> > +			int regnum)
> > +{
> > +	struct mii_bus *bus = priv->bus;
> > +	struct mt7530_dummy_poll p;
> > +	u32 reg, val;
> > +	int ret;
> > +
> > +	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
> > +
> > +	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> > +
> > +	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
> > +				 !(val & PHY_ACS_ST), 20, 100000);
> > +	if (ret < 0) {
> > +		dev_err(priv->dev, "poll timeout\n");
> > +		goto out;
> > +	}
> > +
> > +	reg = MDIO_CL45_ADDR | MDIO_PHY_ADDR(port) | MDIO_DEV_ADDR(devad) |
> > +	      regnum;
> 
> It might be better to call this mt7531_ind_c45_phy_read()
> 
> > +static int
> > +mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
> > +{
> > +	struct mt7530_priv *priv = ds->priv;
> > +	struct mii_bus *bus = priv->bus;
> > +	struct mt7530_dummy_poll p;
> > +	int ret;
> > +	u32 val;
> > +
> > +	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
> > +
> > +	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> > +
> > +	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
> > +				 !(val & PHY_ACS_ST), 20, 100000);
> > +	if (ret < 0) {
> > +		dev_err(priv->dev, "poll timeout\n");
> > +		goto out;
> > +	}
> > +
> > +	val = MDIO_CL22_READ | MDIO_PHY_ADDR(port) | MDIO_REG_ADDR(regnum);
> 
> This is then mt7531_ind_c22_phy_read().
> 
> And then you can add a wrapper around this to provide
> 
> mt7531_phy_read() which can do both C22 and C45.
I'll update the code base on above suggestion.
> 
> > +	[ID_MT7531] = {
> > +		.id = ID_MT7531,
> > +		.setup = mt7531_setup,
> > +		.phy_read = mt7531_ind_phy_read,
> 
> and use it here.
Do you also hint at using the same number of parameters for
mt7531_ind_c22_phy_read() and mt7531_ind_c45_phy_read()?

Landen
> 
>   Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Andrew Lunn]

> Do you also hint at using the same number of parameters for
> mt7531_ind_c22_phy_read() and mt7531_ind_c45_phy_read()?

That is up to you. It just seems like your MDIO bus can do both C22
and C45. And somebody might connect an external C45 PHY, so you might
as well support it.

   Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

On Thu, 2019-12-12 at 03:27 +0800, Andrew Lunn wrote:
> > Do you also hint at using the same number of parameters for
> > mt7531_ind_c22_phy_read() and mt7531_ind_c45_phy_read()?
> 
> That is up to you. It just seems like your MDIO bus can do both C22
> and C45. And somebody might connect an external C45 PHY, so you might
> as well support it.
Yes, it is better to support external C45 PHY. Thanks for your advice.

Landen
> 
>    Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Andrew Lunn]

> +static int mt7531_setup(struct dsa_switch *ds)
> +{
> +	/* Enable PHY power, since phy_device has not yet been created
> +	 * provided for phy_[read,write]_mmd_indirect is called, we provide
> +	 * our own mt7531_ind_mmd_phy_[read,write] to complete this
> +	 * function.
> +	 */
> +	val = mt7531_ind_mmd_phy_read(priv, 0, PHY_DEV1F,
> +				      MT7531_PHY_DEV1F_REG_403);
> +	val |= MT7531_PHY_EN_BYPASS_MODE;
> +	val &= ~MT7531_PHY_POWER_OFF;
> +	mt7531_ind_mmd_phy_write(priv, 0, PHY_DEV1F,
> +				 MT7531_PHY_DEV1F_REG_403, val);
> +

Is this power to all the PHYs? Or just one?

   Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

On Wed, 2019-12-11 at 00:48 +0800, Andrew Lunn wrote:
> > +static int mt7531_setup(struct dsa_switch *ds)
> > +{
> > +	/* Enable PHY power, since phy_device has not yet been created
> > +	 * provided for phy_[read,write]_mmd_indirect is called, we provide
> > +	 * our own mt7531_ind_mmd_phy_[read,write] to complete this
> > +	 * function.
> > +	 */
> > +	val = mt7531_ind_mmd_phy_read(priv, 0, PHY_DEV1F,
> > +				      MT7531_PHY_DEV1F_REG_403);
> > +	val |= MT7531_PHY_EN_BYPASS_MODE;
> > +	val &= ~MT7531_PHY_POWER_OFF;
> > +	mt7531_ind_mmd_phy_write(priv, 0, PHY_DEV1F,
> > +				 MT7531_PHY_DEV1F_REG_403, val);
> > +
> 
> Is this power to all the PHYs? Or just one?
This is an internal circuit that controls the power to all PHYs before
Generic MII registers are available.

Landen
> 
>    Andrew

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Florian Fainelli]

On 12/10/2019 12:14 AM, Landen Chao wrote:
> Add new support for MT7531:
> 
> MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> or HSGMII in different HW sku. Due to HSGMII interface support, pll, and
> pad setting are different from MT7530. This patch adds different initial
> setting of MT7531.
> 
> Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---

[snip]

> +	/* Enable PHY power, since phy_device has not yet been created
> +	 * provided for phy_[read,write]_mmd_indirect is called, we provide
> +	 * our own mt7531_ind_mmd_phy_[read,write] to complete this
> +	 * function.
> +	 */
> +	val = mt7531_ind_mmd_phy_read(priv, 0, PHY_DEV1F,
> +				      MT7531_PHY_DEV1F_REG_403);
> +	val |= MT7531_PHY_EN_BYPASS_MODE;
> +	val &= ~MT7531_PHY_POWER_OFF;
> +	mt7531_ind_mmd_phy_write(priv, 0, PHY_DEV1F,
> +				 MT7531_PHY_DEV1F_REG_403, val);

You are doing this for port 0 only, is that because this broadcasts to
all internal PHYs as well, or is it enough to somehow do it just for
port 0? It sounds like you might want to make this operation a bit more
scoped, if you have an external PHY that also responds to broadcast MDIO
writes this could possibly cause some unattended effects, no?

[snip]

> +static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port)
> +{
> +	u32 val;
> +
> +	if (port != 5) {
> +		dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> +			port);
> +		return -EINVAL;
> +	}
> +
> +	val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
> +	val |= GP_CLK_EN;
> +	val &= ~GP_MODE_MASK;
> +	val |= GP_MODE(MT7531_GP_MODE_RGMII);
> +	val |= TXCLK_NO_REVERSE;
> +	val |= RXCLK_NO_DELAY;

You actually need to look at the port's phy_interface_t value to
determine whether the delays should be set/clear in either RX or TX
directions.

[snip]

> -	if (phylink_autoneg_inband(mode)) {
> +	if (phylink_autoneg_inband(mode) &&
> +	    state->interface != PHY_INTERFACE_MODE_SGMII) {

So you don't support in-band auto-negotiation for 1000BaseX either?

[snip]

> @@ -1590,9 +2197,20 @@ static void mt753x_phylink_validate(struct dsa_switch *ds, int port,
>  	phylink_set_port_modes(mask);
>  	phylink_set(mask, Autoneg);
>  
> -	if (state->interface == PHY_INTERFACE_MODE_TRGMII) {
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_TRGMII:
>  		phylink_set(mask, 1000baseT_Full);
> -	} else {
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		phylink_set(mask, 1000baseX_Full);
> +		phylink_set(mask, 2500baseX_Full);

Did you intend this to be:

	case PHY_INTERFACE_MODE_2500BASEX:
		phylink_set(mask, 2500baseX_Full);
		/* fall through */
	case PHY_INTERFACE_MODE_1000BASEX:
		phylink_set(mask, 1000baseX_Full);
		break;

?
[snip]

> +/* Register for PHY Indirect Access Control */
> +#define MT7531_PHY_IAC			0x701C
> +#define  PHY_ACS_ST			BIT(31)
> +#define  MDIO_REG_ADDR_MASK		(0x1f << 25)
> +#define  MDIO_PHY_ADDR_MASK		(0x1f << 20)
> +#define  MDIO_CMD_MASK			(0x3 << 18)
> +#define  MDIO_ST_MASK			(0x3 << 16)
> +#define  MDIO_RW_DATA_MASK		(0xffff)
> +#define  MDIO_REG_ADDR(x)		(((x) & 0x1f) << 25)
> +#define  MDIO_DEV_ADDR(x)		(((x) & 0x1f) << 25)
> +#define  MDIO_PHY_ADDR(x)		(((x) & 0x1f) << 20)
> +#define  MDIO_CMD(x)			(((x) & 0x3) << 18)
> +#define  MDIO_ST(x)			(((x) & 0x3) << 16)

I would suggest names that are more scoped because these could easily
collide with existing of future definitions from include/linux/mdio.h.

> +
> +enum mt7531_phy_iac_cmd {
> +	MT7531_MDIO_ADDR = 0,
> +	MT7531_MDIO_WRITE = 1,
> +	MT7531_MDIO_READ = 2,
> +	MT7531_MDIO_READ_CL45 = 3,
> +};
> +
> +/* MDIO_ST: MDIO start field */
> +enum mt7531_mdio_st {
> +	MT7531_MDIO_ST_CL45 = 0,
> +	MT7531_MDIO_ST_CL22 = 1,
> +};
> +
> +#define  MDIO_CL22_READ			(MDIO_ST(MT7531_MDIO_ST_CL22) | \
> +					 MDIO_CMD(MT7531_MDIO_READ))
> +#define  MDIO_CL22_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL22) | \
> +					 MDIO_CMD(MT7531_MDIO_WRITE))
> +#define  MDIO_CL45_ADDR			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> +					 MDIO_CMD(MT7531_MDIO_ADDR))
> +#define  MDIO_CL45_READ			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> +					 MDIO_CMD(MT7531_MDIO_READ))
> +#define  MDIO_CL45_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> +					 MDIO_CMD(MT7531_MDIO_WRITE))

Likewise.
-- 
Florian

Re: [PATCH net-next 4/6] net: dsa: mt7530: Add the support of MT7531 switch

[Landen Chao]

On Thu, 2019-12-12 at 11:57 +0800, Florian Fainelli wrote:
> 
> On 12/10/2019 12:14 AM, Landen Chao wrote:
> > Add new support for MT7531:
> > 
> > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > port 6 only supports HSGMII interface. Cpu port 5 supports either RGMII
> > or HSGMII in different HW sku. Due to HSGMII interface support, pll, and
> > pad setting are different from MT7530. This patch adds different initial
> > setting of MT7531.
> > 
> > Signed-off-by: Landen Chao <landen.chao@mediatek.com>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> 
> [snip]
> 
> > +	/* Enable PHY power, since phy_device has not yet been created
> > +	 * provided for phy_[read,write]_mmd_indirect is called, we provide
> > +	 * our own mt7531_ind_mmd_phy_[read,write] to complete this
> > +	 * function.
> > +	 */
> > +	val = mt7531_ind_mmd_phy_read(priv, 0, PHY_DEV1F,
> > +				      MT7531_PHY_DEV1F_REG_403);
> > +	val |= MT7531_PHY_EN_BYPASS_MODE;
> > +	val &= ~MT7531_PHY_POWER_OFF;
> > +	mt7531_ind_mmd_phy_write(priv, 0, PHY_DEV1F,
> > +				 MT7531_PHY_DEV1F_REG_403, val);
> 
> You are doing this for port 0 only, is that because this broadcasts to
> all internal PHYs as well, or is it enough to somehow do it just for
> port 0? It sounds like you might want to make this operation a bit more
> scoped, if you have an external PHY that also responds to broadcast MDIO
> writes this could possibly cause some unattended effects, no?
All internal PHY addresses can be used to access the same PHY_DEV1F
group of registers.

I think the port "0" here must be changed to reference the "first
internal phy address" to prevent the situation you mentioned.
> 
> [snip]
> 
> > +static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port)
> > +{
> > +	u32 val;
> > +
> > +	if (port != 5) {
> > +		dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> > +			port);
> > +		return -EINVAL;
> > +	}
> > +
> > +	val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
> > +	val |= GP_CLK_EN;
> > +	val &= ~GP_MODE_MASK;
> > +	val |= GP_MODE(MT7531_GP_MODE_RGMII);
> > +	val |= TXCLK_NO_REVERSE;
> > +	val |= RXCLK_NO_DELAY;
> 
> You actually need to look at the port's phy_interface_t value to
> determine whether the delays should be set/clear in either RX or TX
> directions.
Sure. Thanks for your advice.
> 
> [snip]
> 
> > -	if (phylink_autoneg_inband(mode)) {
> > +	if (phylink_autoneg_inband(mode) &&
> > +	    state->interface != PHY_INTERFACE_MODE_SGMII) {
> 
> So you don't support in-band auto-negotiation for 1000BaseX either?
According to the user manual I have, it only provides the configure 10M
+100M+1000M AN mode/1000M force mode/2500M force mode, so I mapping them
to SGMII/1000BaseX/2500BaseX. The user friendly part of this IP wraps
too much detail to map to the spec. I'll try to dig it out.
> 
> [snip]
> 
> > @@ -1590,9 +2197,20 @@ static void mt753x_phylink_validate(struct dsa_switch *ds, int port,
> >  	phylink_set_port_modes(mask);
> >  	phylink_set(mask, Autoneg);
> >  
> > -	if (state->interface == PHY_INTERFACE_MODE_TRGMII) {
> > +	switch (state->interface) {
> > +	case PHY_INTERFACE_MODE_TRGMII:
> >  		phylink_set(mask, 1000baseT_Full);
> > -	} else {
> > +		break;
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> > +		phylink_set(mask, 1000baseX_Full);
> > +		phylink_set(mask, 2500baseX_Full);
> 
> Did you intend this to be:
> 
> 	case PHY_INTERFACE_MODE_2500BASEX:
> 		phylink_set(mask, 2500baseX_Full);
> 		/* fall through */
> 	case PHY_INTERFACE_MODE_1000BASEX:
> 		phylink_set(mask, 1000baseX_Full);
> 		break;
> 
> ?
As the user manual mentioned, it is more likely to be:
 	case PHY_INTERFACE_MODE_2500BASEX:
 		phylink_set(mask, 2500baseX_Full);
 		break;
 	case PHY_INTERFACE_MODE_1000BASEX:
 		phylink_set(mask, 1000baseX_Full);
 		break;
> [snip]
> 
> > +/* Register for PHY Indirect Access Control */
> > +#define MT7531_PHY_IAC			0x701C
> > +#define  PHY_ACS_ST			BIT(31)
> > +#define  MDIO_REG_ADDR_MASK		(0x1f << 25)
> > +#define  MDIO_PHY_ADDR_MASK		(0x1f << 20)
> > +#define  MDIO_CMD_MASK			(0x3 << 18)
> > +#define  MDIO_ST_MASK			(0x3 << 16)
> > +#define  MDIO_RW_DATA_MASK		(0xffff)
> > +#define  MDIO_REG_ADDR(x)		(((x) & 0x1f) << 25)
> > +#define  MDIO_DEV_ADDR(x)		(((x) & 0x1f) << 25)
> > +#define  MDIO_PHY_ADDR(x)		(((x) & 0x1f) << 20)
> > +#define  MDIO_CMD(x)			(((x) & 0x3) << 18)
> > +#define  MDIO_ST(x)			(((x) & 0x3) << 16)
> 
> I would suggest names that are more scoped because these could easily
> collide with existing of future definitions from include/linux/mdio.h.
Sure, I'll add "MT7531_" as the prefix.
> 
> > +
> > +enum mt7531_phy_iac_cmd {
> > +	MT7531_MDIO_ADDR = 0,
> > +	MT7531_MDIO_WRITE = 1,
> > +	MT7531_MDIO_READ = 2,
> > +	MT7531_MDIO_READ_CL45 = 3,
> > +};
> > +
> > +/* MDIO_ST: MDIO start field */
> > +enum mt7531_mdio_st {
> > +	MT7531_MDIO_ST_CL45 = 0,
> > +	MT7531_MDIO_ST_CL22 = 1,
> > +};
> > +
> > +#define  MDIO_CL22_READ			(MDIO_ST(MT7531_MDIO_ST_CL22) | \
> > +					 MDIO_CMD(MT7531_MDIO_READ))
> > +#define  MDIO_CL22_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL22) | \
> > +					 MDIO_CMD(MT7531_MDIO_WRITE))
> > +#define  MDIO_CL45_ADDR			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> > +					 MDIO_CMD(MT7531_MDIO_ADDR))
> > +#define  MDIO_CL45_READ			(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> > +					 MDIO_CMD(MT7531_MDIO_READ))
> > +#define  MDIO_CL45_WRITE		(MDIO_ST(MT7531_MDIO_ST_CL45) | \
> > +					 MDIO_CMD(MT7531_MDIO_WRITE))
> 
> Likewise.
I'll add "MT7531_" as the prefix.

Landen

[PATCH net-next 5/6] arm64: dts: mt7622: add mt7531 dsa to mt7622-rfb1 board

[Landen Chao]

Add mt7531 dsa to mt7622-rfb1 board for 5 giga Ethernet ports support.
mt7622 only supports 1 sgmii interface, so either gmac0 or gmac1 can be
configured as sgmii interface. In this patch, change to connet mt7622
gmac0 and mt7531 port6 through sgmii interface.

Signed-off-by: Landen Chao <landen.chao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 63 ++++++++++++++++++--
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 3f783348c66a..e509b0c2fe79 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -105,20 +105,71 @@
 	pinctrl-0 = <&eth_pins>;
 	status = "okay";
 
-	gmac1: mac@1 {
+	gmac0: mac@0 {
 		compatible = "mediatek,eth-mac";
-		reg = <1>;
-		phy-handle = <&phy5>;
+		reg = <0>;
+		phy-mode = "2500base-x";
+
+		fixed-link {
+			speed = <2500>;
+			full-duplex;
+			pause;
+		};
 	};
 
 	mdio-bus {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		phy5: ethernet-phy@5 {
-			reg = <5>;
-			phy-mode = "sgmii";
+		switch@0 {
+			compatible = "mediatek,mt7531";
+			reg = <0>;
+			reset-gpios = <&pio 54 0>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					label = "lan0";
+				};
+
+				port@1 {
+					reg = <1>;
+					label = "lan1";
+				};
+
+				port@2 {
+					reg = <2>;
+					label = "lan2";
+				};
+
+				port@3 {
+					reg = <3>;
+					label = "lan3";
+				};
+
+				port@4 {
+					reg = <4>;
+					label = "wan";
+				};
+
+				port@6 {
+					reg = <6>;
+					label = "cpu";
+					ethernet = <&gmac0>;
+					phy-mode = "2500base-x";
+
+					fixed-link {
+						speed = <2500>;
+						full-duplex;
+						pause;
+					};
+				};
+			};
 		};
+
 	};
 };
 
-- 
2.17.1

Re: [PATCH net-next 5/6] arm64: dts: mt7622: add mt7531 dsa to mt7622-rfb1 board

[Andrew Lunn]

> +				port@6 {
> +					reg = <6>;
> +					label = "cpu";
> +					ethernet = <&gmac0>;
> +					phy-mode = "2500base-x";
> +
> +					fixed-link {
> +						speed = <2500>;
> +						full-duplex;
> +						pause;
> +					};

This fixed-link should not be needed. The DSA driver is supposed to
configure the CPU port to its fastest speed by default. 2500 is
the fastest speed a 2500Base-X link can do...

    Andrew

Re: [PATCH net-next 5/6] arm64: dts: mt7622: add mt7531 dsa to mt7622-rfb1 board

[Landen Chao]

On Wed, 2019-12-11 at 00:51 +0800, Andrew Lunn wrote:
> > +				port@6 {
> > +					reg = <6>;
> > +					label = "cpu";
> > +					ethernet = <&gmac0>;
> > +					phy-mode = "2500base-x";
> > +
> > +					fixed-link {
> > +						speed = <2500>;
> > +						full-duplex;
> > +						pause;
> > +					};
> 
> This fixed-link should not be needed. The DSA driver is supposed to
> configure the CPU port to its fastest speed by default. 2500 is
> the fastest speed a 2500Base-X link can do...
I'll add the cpu port logic to use the fastest speed by default.
It also needs to modify the mt753x_phylink_mac_config() logic.

Landen
> 
>     Andrew

[PATCH net-next 6/6] arm64: dts: mt7622: add mt7531 dsa to bananapi-bpi-r64 board

[Landen Chao]

Add mt7531 dsa to bananapi-bpi-r64 board for 5 giga Ethernet ports support.

Signed-off-by: Landen Chao <landen.chao@mediatek.com>
---
 .../dts/mediatek/mt7622-bananapi-bpi-r64.dts  | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 83e10591e0e5..ffacefee8e2a 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -143,6 +143,56 @@
 	mdio: mdio-bus {
 		#address-cells = <1>;
 		#size-cells = <0>;
+
+		switch@0 {
+			compatible = "mediatek,mt7531";
+			reg = <0>;
+			reset-gpios = <&pio 54 0>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					label = "wan";
+				};
+
+				port@1 {
+					reg = <1>;
+					label = "lan0";
+				};
+
+				port@2 {
+					reg = <2>;
+					label = "lan1";
+				};
+
+				port@3 {
+					reg = <3>;
+					label = "lan2";
+				};
+
+				port@4 {
+					reg = <4>;
+					label = "lan3";
+				};
+
+				port@6 {
+					reg = <6>;
+					label = "cpu";
+					ethernet = <&gmac0>;
+					phy-mode = "2500base-x";
+
+					fixed-link {
+						speed = <2500>;
+						full-duplex;
+						pause;
+					};
+				};
+			};
+		};
+
 	};
 };
 
-- 
2.17.1

Aw: [PATCH net-next 0/6] net-next: dsa: mt7530: add support for MT7531

[Frank Wunderlich]

Hi

thank you for the DSA-driver, works so far, but a bit to improve:

i got some retransmitts on RX (TX looks good)...i guess a clk/pll problem

Iperf3-Client (BPI-R64,192.168.0.19):

root@bpi-r64:~# iperf3 -c 192.168.0.21
Connecting to host 192.168.0.21, port 5201
[  5] local 192.168.0.19 port 56412 connected to 192.168.0.21 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec   114 MBytes   957 Mbits/sec    0   1003 KBytes
[  5]   1.00-2.00   sec   113 MBytes   944 Mbits/sec    0   1.08 MBytes
[  5]   2.00-3.00   sec   112 MBytes   941 Mbits/sec    0   1.08 MBytes
[  5]   3.00-4.00   sec   112 MBytes   941 Mbits/sec    0   1.08 MBytes
[  5]   4.00-5.00   sec   112 MBytes   942 Mbits/sec    0   1.08 MBytes
[  5]   5.00-6.00   sec   112 MBytes   944 Mbits/sec    0   1.21 MBytes
[  5]   6.00-7.00   sec   112 MBytes   944 Mbits/sec    0   1.27 MBytes
[  5]   7.00-8.00   sec   112 MBytes   943 Mbits/sec    0   1.27 MBytes
[  5]   8.00-9.00   sec   111 MBytes   934 Mbits/sec    0   1.27 MBytes
[  5]   9.00-10.00  sec   112 MBytes   944 Mbits/sec    0   1.27 MBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  1.10 GBytes   943 Mbits/sec    0             sender
[  5]   0.00-10.00  sec  1.10 GBytes   941 Mbits/sec                  receiver

iperf Done.

root@bpi-r64:~# iperf3 -c 192.168.0.21 -R
Connecting to host 192.168.0.21, port 5201
Reverse mode, remote host 192.168.0.21 is sending
[  5] local 192.168.0.19 port 56420 connected to 192.168.0.21 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec   112 MBytes   941 Mbits/sec
[  5]   1.00-2.00   sec   112 MBytes   941 Mbits/sec
[  5]   2.00-3.00   sec   111 MBytes   933 Mbits/sec
[  5]   3.00-4.00   sec   112 MBytes   941 Mbits/sec
[  5]   4.00-5.00   sec   112 MBytes   937 Mbits/sec
[  5]   5.00-6.00   sec   112 MBytes   941 Mbits/sec
[  5]   6.00-7.00   sec   111 MBytes   933 Mbits/sec
[  5]   7.00-8.00   sec   112 MBytes   939 Mbits/sec
[  5]   8.00-9.00   sec   112 MBytes   936 Mbits/sec
[  5]   9.00-10.00  sec   112 MBytes   941 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  1.10 GBytes   942 Mbits/sec  605             sender
[  5]   0.00-10.00  sec  1.09 GBytes   939 Mbits/sec                  receiver

iperf Done.

Iperf3-Server (my Laptop,192.168.0.21, reverse mode only):

Accepted connection from 192.168.0.19, port 56418
[  5] local 192.168.0.21 port 5201 connected to 192.168.0.19 port 56420
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  5]   0.00-1.00   sec   115 MBytes   965 Mbits/sec    0    772 KBytes
[  5]   1.00-2.00   sec   112 MBytes   944 Mbits/sec    0    772 KBytes
[  5]   2.00-3.00   sec   111 MBytes   933 Mbits/sec  157    643 KBytes
[  5]   3.00-4.00   sec   112 MBytes   944 Mbits/sec    0    755 KBytes
[  5]   4.00-5.00   sec   111 MBytes   933 Mbits/sec  141    625 KBytes
[  5]   5.00-6.00   sec   112 MBytes   944 Mbits/sec    0    740 KBytes
[  5]   6.00-7.00   sec   111 MBytes   933 Mbits/sec  307    438 KBytes
[  5]   7.00-8.00   sec   111 MBytes   933 Mbits/sec    0    585 KBytes
[  5]   8.00-9.00   sec   112 MBytes   944 Mbits/sec    0    700 KBytes
[  5]   9.00-10.00  sec   112 MBytes   944 Mbits/sec    0    803 KBytes
[  5]  10.00-10.00  sec  0.00 Bytes  0.00 bits/sec    0    803 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  5]   0.00-10.00  sec  1.10 GBytes   942 Mbits/sec  605             sender
[  5]   0.00-10.00  sec  0.00 Bytes  0.00 bits/sec                  receiver

regards Frank

[1] https://github.com/frank-w/BPI-R2-4.14/tree/5.5-r64-netnext