Re: [RFC v2 3/3] net: dsa: initial support for MT7628 embedded switch

[Daniel Golle <daniel@makrotopia.org>]

On Sat, Mar 14, 2026 at 05:08:45PM +0200, Joris Vaisvila wrote:
> Add support for the MT7628 embedded switch.

Thank you for working on this, MT7628 (especially in "IoT" mode with 3
UARTs and MMC bus exposed instead of the additional switch ports)
still has it's special niche somewhere in between ESP32 and "proper"
Linux-running SoCs ;)

> 
> The switch has 5 built-in 100Mbps ports (ports 0-4) and 2x 1Gbps ports.
> One of the 1Gbps ports (port 6) is internally attached to the SoCs CPU
> MAC and serves as the CPU port. The other 1Gbps port (port 5) requires
> an external MAC to function.

I thought port 5 is a dead-end on MT76x8. Afaik it was only used on
Ralink Rt3052 for RGMII/xMII with this switch IP, and not wired to
any external pins nor to an internal MAC on all other SoCs which came
after.

> 
> The switch hardware has a very limited 16 entry VLAN table. Configuring
> VLANs is the only way to control switch forwarding. Currently 7 entries
> are used by tag_8021q to isolate the ports. Double tag feature is
> enabled to force the switch to append the VLAN tag even if the incoming
> packet is already tagged, this simulates VLAN-unaware functionality and
> simplifies the tagger implementation.
> 
> Signed-off-by: Joris Vaisvila <joey@tinyisr.com>

> +config NET_DSA_MT7628
> +	tristate "MT7628 Embedded ethernet switch support"
> +	select NET_DSA_TAG_MT7628
> +	select MEDIATEK_FE_SOC_PHY
> +	help
> +	  This enables support for the switch in the MT7628 SoC.

And potentially many others (Rt3052, Rt3050, Rt3352, Rt5350).
As support for the Rt5350 is present in mtk_eth_soc at least that
sounds like it would be worth mentioning (or even naming the driver
and tagger interely after rt3050 which is the origin of that IP
block).

> +static int mt7628_mii_read(struct mii_bus *bus, int port, int regnum)
> +{
> +	struct mt7628_esw *esw = bus->priv;
> +	int ret;
> +	u32 val;
> +
> +	ret =
> +	    regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,

Unnecessary linebreak.

> +				     !(val & MT7628_ESW_PCR1_RD_DONE), 10,
> +				     5000);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(esw->regmap, MT7628_ESW_REG_PCR0,
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_REG,
> +				      regnum) |
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_ADDR,
> +				      port) | MT7628_ESW_PCR0_RD_PHY_CMD);
> +	if (ret)
> +		goto out;
> +
> +	ret =
> +	    regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,

Here as well.

> +				     (val & MT7628_ESW_PCR1_RD_DONE), 10,
> +				     5000);
> +out:
> +	if (ret) {
> +		dev_err(&bus->dev, "read failed. MDIO timeout?\n");
> +		return -ETIMEDOUT;
> +	}
> +	return FIELD_GET(MT7628_ESW_PCR1_RD_DATA, val);
> +}
> +
> +static int mt7628_mii_write(struct mii_bus *bus, int port, int regnum,
> +			    u16 dat)
> +{
> +	struct mt7628_esw *esw = bus->priv;
> +	u32 val;
> +	int ret;
> +
> +	ret =
> +	    regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,

Here as well.

> +				     !(val & MT7628_ESW_PCR1_WT_DONE), 10,
> +				     5000);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(esw->regmap, MT7628_ESW_REG_PCR0,
> +			   FIELD_PREP(MT7628_ESW_PCR0_WT_NWAY_DATA, dat) |
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_REG,
> +				      regnum) |
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_ADDR,
> +				      port) | MT7628_ESW_PCR0_WT_PHY_CMD);
> +	if (ret)
> +		goto out;
> +
> +	ret =
> +	    regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,

And here as well.

> [...]
> +static void esw_set_vlan_id(struct mt7628_esw *esw, unsigned int vlan,
> +			    unsigned int vid)
> +{
> +	unsigned int s = MT7628_ESW_VLANI_VID_S * (vlan % 2);
> +
> +	regmap_update_bits(esw->regmap, MT7628_ESW_REG_VLANI(vlan / 2),
> +			   MT7628_ESW_VLANI_VID_M << s,
> +			   (vid & MT7628_ESW_VLANI_VID_M) << s);

Please add helper macros for those registers and field in them

> +}
> +
> +static void esw_set_pvid(struct mt7628_esw *esw, unsigned int port,
> +			 unsigned int pvid)
> +{
> +	unsigned int s = MT7628_ESW_PVIDC_PVID_S * (port % 2);
> +
> +	regmap_update_bits(esw->regmap, MT7628_ESW_REG_PVIDC(port / 2),
> +			   MT7628_ESW_PVIDC_PVID_M << s,
> +			   (pvid & MT7628_ESW_PVIDC_PVID_M) << s);

and those as well

> +}
> +
> +static void esw_set_vmsc(struct mt7628_esw *esw, unsigned int vlan,
> +			 unsigned int msc)
> +{
> +	unsigned int s = MT7628_ESW_VMSC_MSC_S * (vlan % 4);
> +
> +	regmap_update_bits(esw->regmap, MT7628_ESW_REG_VMSC(vlan / 4),
> +			   MT7628_ESW_VMSC_MSC_M << s,
> +			   (msc & MT7628_ESW_VMSC_MSC_M) << s);

those too

> +}
> +
> +static void esw_set_vub(struct mt7628_esw *esw, unsigned int vlan,
> +			unsigned int msc)
> +{
> +	unsigned int s = MT7628_ESW_VUB_S * (vlan % 4);
> +
> +	regmap_update_bits(esw->regmap, MT7628_ESW_REG_VUB(vlan / 4),
> +			   MT7628_ESW_VUB_M << s,
> +			   (msc & MT7628_ESW_VUB_M) << s);

and those.

> [...]
> +static void mt7628_phylink_get_caps(struct dsa_switch *ds, int port,
> +				    struct phylink_config *config)
> +{
> +	config->mac_capabilities = MAC_100 | MAC_10;
> +	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
> +	__set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);

This looks a bit wrong.
Aren't all 5 PHYs internal? PHY_INTERFACE_INTERNAL would be best to
describe them then. And doesn't the CPU port (6) connect with 1000M,
if so it should have MAC_1000 set as well (as neither the Ethernet
driver nor the DSA driver do anything to configure the interface that
is purely cosmetic, but still)

Iff you want to support port 5 on Rt3052 (which will also require
setting the correct interface mode in .mac_config) I'd expect something
like this:

config->mac_capabilities = MAC_100 | MAC_10;

switch (port) {
case 0...4:
	__set_bit(PHY_INTERFACE_INTERNAL, config->supported_interfaces);
	break;
case 5:
	__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
	__set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces);
	__set_bit(PHY_INTERFACE_MODE_RMII, config->supported_interfaces);
	__set_bit(PHY_INTERFACE_MODE_RGMII, config->supported_interfaces);
	config->mac_capabilities |= MAC_1000;
	break;
case 6:
	__set_bit(PHY_INTERFACE_INTERNAL, config->supported_interfaces);
	config->mac_capabilities |= MAC_1000;
	break;
default:
	return;
}

If you don't want to deal with port 5 you should skip it here as well.


Cheers


Daniel