Re: [PATCH net-next v6 4/4] net: dsa: initial support for MT7628 embedded switch

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

This patch adds a DSA driver for the MT7628 embedded switch, which has
five 100Mbps user ports and a 1Gbps CPU port. tag_8021q is used to
isolate ports via the switch's 16-entry VLAN table, with the double tag
feature enabled to simulate VLAN-unaware behaviour.

[ ... ]

+static const struct dsa_switch_ops mt7628_switch_ops = {
+	.get_tag_protocol = mt7628_get_tag_proto,
+	.setup = mt7628_setup,
+	.teardown = mt7628_teardown,
+	.port_enable = mt7628_port_enable,
+	.port_disable = mt7628_port_disable,
+	.phylink_get_caps = mt7628_phylink_get_caps,
+	.tag_8021q_vlan_add = mt7628_dsa_8021q_vlan_add,
+	.tag_8021q_vlan_del = mt7628_dsa_8021q_vlan_del,
+};

[Medium]
How is a user expected to place two of the user ports into a Linux bridge?

The ops table does not implement port_bridge_join or port_bridge_leave,
so dsa_switch_bridge_join() in net/dsa/switch.c returns -EOPNOTSUPP:

    if (!ds->ops->port_bridge_join)
        return -EOPNOTSUPP;

That makes ip link set swpN master brX fail for every user port, which
seems to contradict the commit message:

    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.

Without a port_bridge_join hook, dsa_tag_8021q_bridge_join() is never
invoked, so there is no path to migrate a port's tag_8021q VID from its
standalone VID to a bridge VID.

Would wiring the tag_8021q helpers directly, similar to what
vitesse-vsc73xx-core.c does, be sufficient here:

    .port_bridge_join = dsa_tag_8021q_bridge_join,
    .port_bridge_leave = dsa_tag_8021q_bridge_leave,

Related bridge-lifecycle callbacks such as port_stp_state_set,
port_pre_bridge_flags, port_bridge_flags and port_fast_age also appear
to be absent, is that intentional for this initial submission?