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

[Joris Vaisvila <joey@tinyisr.com>]

On Mon, Jul 20, 2026 at 04:52:01PM -0700, Jakub Kicinski wrote:
> 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.
> 
> > diff --git a/drivers/net/dsa/mt7628.c b/drivers/net/dsa/mt7628.c
> > new file mode 100644
> > index 000000000000..eccdb05fce9b
> > --- /dev/null
> > +++ b/drivers/net/dsa/mt7628.c
> 
> [ ... ]
> 
> > +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.
> 

It does not contradict the commit message. The driver intentionally does
not implement bridge offloading.

The ports can still be bridged in software without propagating it to
the hardware. This is a deliberate simplification to keep the initial
driver easier to review. It is also a practical initial solution for
this switch, as all of the user ports are 100mbps, while the CPU to
switch link is 1gbps, making it possible to bridge multiple ports at
line rate in software.

> 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?

For the initial submission tag_8021q is only used for the standalone VID
feature and software bridging is used. I intend to work on hardware
bridging after initial support is complete.