Re: [PATCH net-next v6 4/4] net: dsa: initial support for MT7628 embedded switch
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-07-20 23:52:21
Also in:
linux-devicetree
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- net: dsa: initial support for MT7628 embedded switch Add support for the MT7628 embedded switch. The switch has 5 built-in 100Mbps user ports and one 1Gbps CPU port.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig index 4ab567c5bbafc..daa1d3d4ba609 100644 --- a/drivers/net/dsa/Kconfig +++ b/drivers/net/dsa/Kconfig@@ -63,6 +63,14 @@ config NET_DSA_MT7530_MMIO are directly mapped into the SoCs register space rather than being accessible via MDIO. +config NET_DSA_MT7628 + tristate "MediaTek MT7628 Embedded Ethernet switch support" + select NET_DSA_TAG_MT7628 + select MEDIATEK_FE_SOC_PHY + help
[Severity: Low] Can this lead to a build or link failure on randconfig? It looks like this is missing a select REGMAP_MMIO and depends on HAS_IOMEM, which might cause devm_regmap_init_mmio() to be unresolved when CONFIG_REGMAP_MMIO is not set.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/mt7628.c b/drivers/net/dsa/mt7628.c new file mode 100644 index 0000000000000..eccdb05fce9b7 --- /dev/null +++ b/drivers/net/dsa/mt7628.c@@ -0,0 +1,649 @@[ ... ] +static int mt7628_setup_internal_mdio(struct dsa_switch *ds) +{ + struct mt7628_esw *esw = ds->priv; + struct device *dev = ds->dev; + struct mii_bus *bus; + + bus = devm_mdiobus_alloc(dev); + if (!bus) + return -ENOMEM; + + bus->name = "MT7628 internal MDIO bus"; + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(dev)); + bus->priv = esw; + bus->read = mt7628_mii_read; + bus->write = mt7628_mii_write; + bus->parent = dev; + + ds->user_mii_bus = bus; + bus->phy_mask = ~ds->phys_mii_mask; + + return devm_mdiobus_register(dev, bus); +}
[Severity: High] Does this cause a resource leak and sysfs name collision if the switch setup callback runs multiple times? If the DSA master device unbinds and rebinds, dsa_tree_teardown() and dsa_tree_setup() can be triggered again. Because devm_mdiobus_alloc() and devm_mdiobus_register() tie the resource to the platform device's lifetime rather than the DSA tree setup/teardown cycle, the first registration will leak, and the second might fail with -EEXIST.
[ ... ]
+static void mt7628_switch_init(struct dsa_switch *ds)
+{
+ struct mt7628_esw *esw = ds->priv;
+
+ regmap_write(esw->regmap, MT7628_ESW_REG_FCT0,
+ FIELD_PREP(MT7628_ESW_FCT0_DROP_SET_TH, 0x50) |
+ FIELD_PREP(MT7628_ESW_FCT0_DROP_RLS_TH, 0x78) |
+ FIELD_PREP(MT7628_ESW_FCT0_FC_SET_TH, 0xa0) |
+ FIELD_PREP(MT7628_ESW_FCT0_FC_RLS_TH, 0xc8));[Severity: High] Can traffic leak across isolated ports (like WAN and LAN) during boot? It looks like mt7628_switch_init() resets the switch but fails to explicitly set the MT7628_ESW_POC0_PORT_DISABLE bits for the user ports. Since the DSA core relies on port_disable during ndo_close and doesn't automatically disable ports upon initialization, does the hardware default to acting as an unmanaged switch, bridging all networks until the interfaces are brought up administratively?