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

["Joris Vaišvila" <joey@tinyisr.com>]

On Sat, Mar 14, 2026 at 11:41:28PM +0000, Daniel Golle wrote:
> > 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.

I assumed this was true as that's what the datasheet says, but looking
at the pinout it really doesn't have any pins to attach an external mac.

> > +	  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).

The RT3050 does not support all the same features the RT5350 does (which
I believe the MT7628 is based on). The DSA tag is now set up in a way
that only the MT7628 and potentially RT5350 can support. The DSA tag
does not appear to be working as described in the datasheet on these
switches and I do not have any of the ralink switches to test this on.
I don't think it's a good idea to assert support for hardware that has
not been tested with this.

I thought it would be fitting for the driver to be called after the
MT7628 as that would be the first supported SoC and add support for the
ralink switches later.

> > [...]
> > +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)

Will fix this and coding style issues in v3.

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

Will skip port 5 for now, due to this driver likely not working on
RT3052 as it is anyway for the aforementioned reasons.

Thanks for the review.