Re: DSA using cpsw and lan9303

[Vladimir Oltean <olteanv@gmail.com>]

Hi Måns,

On Mon, Feb 14, 2022 at 09:16:10AM -0800, Florian Fainelli wrote:
> +others,
> 
> netdev is a high volume list, you should probably copy directly the
> people involved with the code you are working with.
> 
> On 2/14/22 8:44 AM, Måns Rullgård wrote:
> > The hardware I'm working on has a LAN9303 switch connected to the
> > Ethernet port of an AM335x (ZCE package).  In trying to make DSA work
> > with this combination, I have encountered two problems.
> > 
> > Firstly, the cpsw driver configures the hardware to filter out frames
> > with unknown VLAN tags.  To make it accept the tagged frames coming from
> > the LAN9303, I had to modify the latter driver like this:
> > 
> > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> > index 2de67708bbd2..460c998c0c33 100644
> > --- a/drivers/net/dsa/lan9303-core.c
> > +++ b/drivers/net/dsa/lan9303-core.c
> > @@ -1078,20 +1079,28 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
> >                                struct phy_device *phy)
> >  {
> >         struct lan9303 *chip = ds->priv;
> > +       struct net_device *master;
> >  
> >         if (!dsa_is_user_port(ds, port))
> >                 return 0;
> >  
> > +       master = dsa_to_port(chip->ds, 0)->master;
> > +       vlan_vid_add(master, htons(ETH_P_8021Q), port);
> 
> That looks about right given that net/dsa/tag_lan9303.c appears to be a
> quasi DSA_TAG_PROTO_8021Q implementation AFAICT.

In case it was not clear, I agree with Florian that this looks "about right",
I just thought that mentioning it wouldn't bring much to the table.
But I noticed you submitted a patch for the other issue and not for this.

Some complaints about accessing the CPU port as dsa_to_port(chip->ds, 0),
but it's not the first place in this driver where that is done.

dsa_tag_8021q_port_setup() also has:

	/* Add @rx_vid to the master's RX filter. */
	vlan_vid_add(master, ctx->proto, rx_vid);

which is an indication that other switches with VLAN-based tagging
protocols should handle this somehow, somewhere.

Note, though, that vlan_vid_add() allocates memory, so it would be good
to have a vlan_vid_del() too at some point.