Re: [RFC v3 4/4] net: dsa: initial support for MT7628 embedded switch

[Daniel Golle <daniel@makrotopia.org>]

On Sat, Mar 21, 2026 at 09:43:40PM +0200, Joris Vaisvila wrote:
> Add support for the MT7628 embedded switch.
> 
> The switch has 5 built-in 100Mbps user ports (ports 0-4) and one 1Gbps
> port that is internally attached to the SoCs CPU MAC and serves as the
> CPU port.
> 
> The switch hardware has a very limited 16 entry VLAN table. Configuring
> VLANs is the only way to control switch forwarding. Currently 6 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>
> [...]
> +static int mt7628_setup_internal_mdio(struct dsa_switch *ds,
> +				      struct device_node *node)
> +{
> +	struct mt7628_esw *esw = ds->priv;
> +	struct mii_bus *bus;
> +	int ret;
> +
> +	bus = devm_mdiobus_alloc(esw->dev);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	bus->name = "MT7628 internal MDIO bus";
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(ds->dev));
> +	bus->priv = esw;
> +	bus->read = mt7628_mii_read;
> +	bus->write = mt7628_mii_write;
> +	bus->parent = esw->dev;
> +
> +	ret = devm_of_mdiobus_register(esw->dev, bus, node);

As mentioned in the binding comment:
- The MDIO bus is exclusively used to speak with the built-in PHYs.
- The PHY addresses match the port IDs.
- Only Clause-22 read/write operations are supported.
- There is no way to connect an external PHY (no MII interface
  exposed on external pins)

Imho it would hence be much easier to just use DSA's simple .phy_read
and .phy_write operations instead of registering a bus manually in the
driver, and even requiring a description of the MDIO bus in device
tree.

If you want to reserve the option of adding PHY-specific DT properties
in future (eg. for PHY-controlled LEDs? but afair the LEDs are
controlled by the switch itself and SoC-level pinctrl), at least set
`ds->user_mii_bus = bus;` to make the device tree description
optional. Completely omitting the whole bus definition and changing
mt7628_mii_read and mt7628_mii_write to be useful as .phy_read and
.phy_write ops in struct dsa_switch is the better option for simple
legacy hardware like that imho. See b53 driver, for example.

@DSA maintainers: correct me if I'm wrong and, for which ever reason,
using the .phy_read/.phy_write ops is discouraged in new drivers, even
for dead-simple hardware like that one.