[PATCH net-next 0/2] net: dsa: mv88e6xxx: 6141/6341 workarounds

[PATCH net-next 0/2] net: dsa: mv88e6xxx: 6141/6341 workarounds

[Luke Howard]

This patch series addresses two issues I found with the 88E6341
switch, which likely also apply to the 88E6141 (which is the same
chip without AVB/TSN support).

The first was a genuine bug, which assumed the chip did not have
a dedicated ATU FID register (it does).

The other is an issue I noticed with hash collisions in the ATU.
I would appreciate some external testing before this commit is
reviewed, but I am including it as it was necessary for reliable
operation with multiple FIDs.

Signed-off-by: Luke Howard <lukeh@padl.com>
---
Luke Howard (2):
      net: dsa: mv88e6xxx: write the ATU FID register on 88E6141/88E6341
      net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

 drivers/net/dsa/mv88e6xxx/chip.c        | 14 ++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h        |  1 +
 drivers/net/dsa/mv88e6xxx/global1.h     |  6 +++++-
 drivers/net/dsa/mv88e6xxx/global1_atu.c |  4 ++--
 4 files changed, 22 insertions(+), 3 deletions(-)
---
base-commit: 1c664ec4b9ea827b609d296921ed5bad8a40a158
change-id: 20260624-mv88e6x41-fixes-d4c84f955ebc

Best regards,
--  
Luke Howard <lukeh@padl.com>

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

[Luke Howard]

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")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Luke Howard <lukeh@padl.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c        | 2 ++
 drivers/net/dsa/mv88e6xxx/chip.h        | 1 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 80b877c74513d..60d46680fb50e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5863,6 +5863,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6341,
 		.name = "Marvell 88E6141",
 		.num_databases = 256,
+		.atu_fid_reg = true,
 		.num_macs = 2048,
 		.num_ports = 6,
 		.num_internal_phys = 5,
@@ -6350,6 +6351,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6341,
 		.name = "Marvell 88E6341",
 		.num_databases = 256,
+		.atu_fid_reg = true,
 		.num_macs = 2048,
 		.num_internal_phys = 5,
 		.num_ports = 6,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e966e7c4cc5de..9f23d2a7165f0 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -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;
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) {
 		err = mv88e6xxx_g1_atu_fid_write(chip, fid);
 		if (err)
 			return err;
@@ -179,7 +179,7 @@ static int mv88e6xxx_g1_atu_fid_read(struct mv88e6xxx_chip *chip, u16 *fid)
 	u16 val = 0, upper = 0, op = 0;
 	int err = -EOPNOTSUPP;
 
-	if (mv88e6xxx_num_databases(chip) > 256) {
+	if (mv88e6xxx_num_databases(chip) > 256 || chip->info->atu_fid_reg) {
 		err = mv88e6xxx_g1_read(chip, MV88E6352_G1_ATU_FID, &val);
 		val &= 0xfff;
 		if (err)

-- 
2.43.0

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

[Andrew Lunn]

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

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

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

    Andrew

---
pw-bot: cr

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@lunn.ch> 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

[PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

[Luke Howard]

The default 88E6341/88E6141 ATU hash algorithm appears to result
in frequent collisions, evicting permanent registrations (including
the broadcast address) out of the ATU.

This workaround disables hasing in the ATU control register. It may
have a performance impact (the data sheet notes this is for testing
only), but it also enables correctness, at least in local testing.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Luke Howard <lukeh@padl.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c    | 12 ++++++++++++
 drivers/net/dsa/mv88e6xxx/global1.h |  6 +++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 60d46680fb50e..53ed2b9e30979 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1699,6 +1699,18 @@ static int mv88e6xxx_atu_setup(struct mv88e6xxx_chip *chip)
 {
 	int err;
 
+	/* Avoid collisions on the 6341 family by disabling hashing. */
+	if (chip->info->family == MV88E6XXX_FAMILY_6341 &&
+	    chip->info->ops->atu_set_hash) {
+		u8 hash = MV88E6161_G1_ATU_CTL_HASH_DIRECT;
+
+		err = chip->info->ops->atu_set_hash(chip, hash);
+		if (err)
+			return err;
+
+		dev_info(chip->dev, "ATU hash mode set to 0x%x\n", hash);
+	}
+
 	err = mv88e6xxx_g1_atu_flush(chip, 0, true);
 	if (err)
 		return err;
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 3dbb7a1b8fe11..e2a54eac64832 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -112,7 +112,11 @@
 /* Offset 0x0A: ATU Control Register */
 #define MV88E6XXX_G1_ATU_CTL		0x0a
 #define MV88E6XXX_G1_ATU_CTL_LEARN2ALL	0x0008
-#define MV88E6161_G1_ATU_CTL_HASH_MASK	0x0003
+#define MV88E6161_G1_ATU_CTL_HASH_MASK		0x0003
+#define MV88E6161_G1_ATU_CTL_HASH_RESERVED_0	0x0000
+#define MV88E6161_G1_ATU_CTL_HASH_DEFAULT	0x0001
+#define MV88E6161_G1_ATU_CTL_HASH_RESERVED_1	0x0002
+#define MV88E6161_G1_ATU_CTL_HASH_DIRECT	0x0003
 
 /* Offset 0x0B: ATU Operation Register */
 #define MV88E6XXX_G1_ATU_OP				0x0b

-- 
2.43.0

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

[Andrew Lunn]

On Fri, Jul 03, 2026 at 04:42:56PM +1000, Luke Howard wrote:
> The default 88E6341/88E6141 ATU hash algorithm appears to result
> in frequent collisions, evicting permanent registrations (including
> the broadcast address) out of the ATU.

Is there any documentation about how the 88E6341/88E6141 hashing
algorithm is different to all the other chips? How is it special?

> This workaround disables hasing in the ATU control register. It may

hashing.

> have a performance impact (the data sheet notes this is for testing
> only), but it also enables correctness, at least in local testing.

How do you define correctness? Are you using a well defined test?

Why is the devlink parameter not sufficient.

    Andrew

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

[Luke Howard]

> On 4 Jul 2026, at 1:40 am, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Fri, Jul 03, 2026 at 04:42:56PM +1000, Luke Howard wrote:
>> The default 88E6341/88E6141 ATU hash algorithm appears to result
>> in frequent collisions, evicting permanent registrations (including
>> the broadcast address) out of the ATU.
> 
> Is there any documentation about how the 88E6341/88E6141 hashing
> algorithm is different to all the other chips? How is it special?

No documentation, just my experience, which is why I asked in the cover for others to test. I can include test results with the next revision.

>> have a performance impact (the data sheet notes this is for testing
>> only), but it also enables correctness, at least in local testing.
> 
> How do you define correctness? Are you using a well defined test?
> 
> Why is the devlink parameter not sufficient.

Devlink would indeed be better, but changing hash mode with a non-empty ATU causes it to be corrupt on read back. Perhaps the right solution is to flus the ATU when changing hash mode via devlink.

Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

[Andrew Lunn]

> No documentation, just my experience, which is why I asked in the
> cover for others to test. I can include test results with the next
> revision.

Not so much the test results, but the test case.

The reason i added the devlink option was because the network
contained only devices from one vendor. So the OUI in the MAC
addresses was the same, the usable number of bits in the MAC address
being cut in half, in the best case. As a result, there was a lot of
hash collisions. For this special case network, the test mode hash was
better.

If you are also seeing a lot of collisions, it makes me wounder what
your distribution of MAC addresses is.

> >> have a performance impact (the data sheet notes this is for testing
> >> only), but it also enables correctness, at least in local testing.
> > 
> > How do you define correctness? Are you using a well defined test?
> > 
> > Why is the devlink parameter not sufficient.
> 

> Devlink would indeed be better, but changing hash mode with a
> non-empty ATU causes it to be corrupt on read back. Perhaps the
> right solution is to flus the ATU when changing hash mode via
> devlink.

Flushing the ATU would make sense. It could well be in the system i
was working on, once it was shown to help, the EEPROM contents was set
to configure the ATU hash at hardware boot time, and devlink was not
used in production. The vendor had a bad habit of using the EEPROM.

     Andrew

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341

[Luke Howard]

> On 4 Jul 2026, at 7:43 am, Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> No documentation, just my experience, which is why I asked in the
>> cover for others to test. I can include test results with the next
>> revision.
> 
> Not so much the test results, but the test case.
> 
> The reason i added the devlink option was because the network
> contained only devices from one vendor. So the OUI in the MAC
> addresses was the same, the usable number of bits in the MAC address
> being cut in half, in the best case. As a result, there was a lot of
> hash collisions. For this special case network, the test mode hash was
> better.
> 
> If you are also seeing a lot of collisions, it makes me wounder what
> your distribution of MAC addresses is.

Well, a bunch of unicast addresses along with MAAP for AVB.

But what triggered it was multiple FIDs (VLAN-aware bridge), which wouldn’t have been visible without the first patch to configure the ATU FID register correctly.

Luke