Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: write the ATU FID register on 88E6141/88E6341

[Luke Howard]

> On 4 Jul 2026, at 1:31 am, Andrew Lunn <andrew@xxxxxxx> wrote:
> 
> On Fri, Jul 03, 2026 at 04:42:55PM +1000, Luke Howard wrote:
>> The existing code assumed the 88E6141/88E6341 did not have a dedicated
>> ATU FID register because of its database count (256), instead taking
>> the legacy path which resulted in the FID register never being set.
>> 
>> This resulted in every FDB entry being loaded into FID 0, breaking
>> VLAN aware bridging.
>> 
>> Fixes: a75961d0ebfd ("net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341")
> 
> Please submit fixes to the net tree.
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Will do.

>> @@ -131,6 +131,7 @@ struct mv88e6xxx_info {
>> u16 prod_num;
>> const char *name;
>> unsigned int num_databases;
>> + bool atu_fid_reg;
>> unsigned int num_macs;
>> unsigned int num_ports;
>> unsigned int num_internal_phys;
> 
> Please think about padding. The current structure layout is not great,
> invalid_port_mask should be somewhere else, but please don't make it
> worse. Also, this structure has some reasonable comments. You have the
> chance to comment that the number of FIBs does not imply if there is a
> dedicated register, there are examples with 255 and a dedicated
> register.

Noted.

> 
>> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> index c47f068f56b32..aa5adc78607ca 100644
>> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> @@ -135,7 +135,7 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
>> int err;
>> 
>> /* FID bits are dispatched all around gradually as more are supported */
>> - if (mv88e6xxx_num_databases(chip) > 256) {
>> + if (mv88e6xxx_num_databases(chip) > 256 || chip->info->atu_fid_reg) {
> 
> So currently, > 256 implies a dedicated register.  But do we need both
>> 255 and chip->info->atu_fid_reg? I would probably set atu_fid_reg
> true for all devices which have a dedicated register.

Noted.

Luke