Re: DSA using cpsw and lan9303

["Måns Rullgård" <mans@mansr.com>]

Vladimir Oltean <olteanv@gmail.com> writes:

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

What would be the proper way to do it?

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

Yes, I just didn't include that bit in the initial query.

-- 
Måns Rullgård