[PATCH net-next 0/7] net: dsa: b53: fix ARL accesses for BCM5325/65 and allow VID 0

[PATCH net-next 0/7] net: dsa: b53: fix ARL accesses for BCM5325/65 and allow VID 0

[Jonas Gorski]

ARL entries on BCM5325 and BCM5365 were broken significantly in two
ways:

- Entries for the CPU port were using the wrong port id, pointing to a
  non existing port.
- Setting the VLAN ID for entries was not done, adding them all to VLAN
  0 instead.

While the former technically broke any communication to the CPU port,
with the latter they were added to the currently unused VID 0, so they
never became effective. Presumably the default PVID was set to 1 because
of these issues 0 was broken (and the root cause not found).

So fix writing and reading entries on BCM5325/65 by first fixing the CPU
port entries, then fixing setting the VLAN ID for entries.

Finally, re-allow VID 0 for BCM5325/65 to allow the whole 1-15 VLAN ID
range to be available to users, and align VLAN handling with all other
switch chips.

Sent to net-next as it would cause an ugly, non trivial merge conflict
with net-next when added to net, and I don't want to subject the
maintainers to that. I will take care of sending adapted versions to
stable once it hit linus' tree.

Jonas Gorski (7):
  net: dsa: b53: fix VLAN_ID_IDX write size for BCM5325/65
  net: dsa: b53: fix extracting VID from entry for BCM5325/65
  net: dsa: b53: use same ARL search result offset for BCM5325/65
  net: dsa: b53: fix CPU port unicast ARL entries for BCM5325/65
  net: dsa: b53: fix BCM5325/65 ARL entry multicast port masks
  net: dsa: b53: fix BCM5325/65 ARL entry VIDs
  net: dsa: b53: allow VID 0 for BCM5325/65

 drivers/net/dsa/b53/b53_common.c | 88 ++++++++++++--------------------
 drivers/net/dsa/b53/b53_priv.h   | 40 +++++++++++----
 drivers/net/dsa/b53/b53_regs.h   | 19 ++++---
 3 files changed, 78 insertions(+), 69 deletions(-)


base-commit: cc1b62512abf19c635fe304e253953ca3b33ffa2
-- 
2.43.0

[PATCH net-next 1/7] net: dsa: b53: fix VLAN_ID_IDX write size for BCM5325/65

[Jonas Gorski]

Since BCM5325 and BCM5365 only support up to 256 VLANs, the VLAN_ID_IDX
register is only 8 bit wide, not 16 bit, so use an appropriate accessor.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 72c85cd34a4e..7f24d2d8f938 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1966,8 +1966,12 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
 
 	/* Perform a read for the given MAC and VID */
 	b53_write48(dev, B53_ARLIO_PAGE, B53_MAC_ADDR_IDX, mac);
-	if (!is5325m(dev))
-		b53_write16(dev, B53_ARLIO_PAGE, B53_VLAN_ID_IDX, vid);
+	if (!is5325m(dev)) {
+		if (is5325(dev) || is5365(dev))
+			b53_write8(dev, B53_ARLIO_PAGE, B53_VLAN_ID_IDX, vid);
+		else
+			b53_write16(dev, B53_ARLIO_PAGE, B53_VLAN_ID_IDX, vid);
+	}
 
 	/* Issue a read operation for this MAC */
 	ret = b53_arl_rw_op(dev, 1);
-- 
2.43.0

[PATCH net-next 2/7] net: dsa: b53: fix extracting VID from entry for BCM5325/65

[Jonas Gorski]

BCM5325/65's Entry register uses the highest three bits for
VALID/STATIC/AGE, so shifting by 53 only will add these to
b53_arl_entry::vid.

So make sure to mask the vid value as well, to not get invalid VIDs.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 2bfd0e7c95c9..3b22e817fb28 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -350,7 +350,7 @@ static inline void b53_arl_to_entry_25(struct b53_arl_entry *ent,
 	ent->is_age = !!(mac_vid & ARLTBL_AGE_25);
 	ent->is_static = !!(mac_vid & ARLTBL_STATIC_25);
 	u64_to_ether_addr(mac_vid, ent->mac);
-	ent->vid = mac_vid >> ARLTBL_VID_S_65;
+	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
 }
 
 static inline void b53_arl_to_entry_89(struct b53_arl_entry *ent,
-- 
2.43.0

[PATCH net-next 3/7] net: dsa: b53: use same ARL search result offset for BCM5325/65

[Jonas Gorski]

BCM5365's search result is at the same offset as BCM5325's search
result, and they (mostly) share the same format, so switch BCM5365 to
BCM5325's arl ops.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 18 +-----------------
 drivers/net/dsa/b53/b53_regs.h   |  4 +---
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 7f24d2d8f938..91b0b4de475f 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2125,16 +2125,6 @@ static void b53_arl_search_read_25(struct b53_device *dev, u8 idx,
 	b53_arl_to_entry_25(ent, mac_vid);
 }
 
-static void b53_arl_search_read_65(struct b53_device *dev, u8 idx,
-				   struct b53_arl_entry *ent)
-{
-	u64 mac_vid;
-
-	b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_65,
-		   &mac_vid);
-	b53_arl_to_entry_25(ent, mac_vid);
-}
-
 static void b53_arl_search_read_89(struct b53_device *dev, u8 idx,
 				   struct b53_arl_entry *ent)
 {
@@ -2746,12 +2736,6 @@ static const struct b53_arl_ops b53_arl_ops_25 = {
 	.arl_search_read = b53_arl_search_read_25,
 };
 
-static const struct b53_arl_ops b53_arl_ops_65 = {
-	.arl_read_entry = b53_arl_read_entry_25,
-	.arl_write_entry = b53_arl_write_entry_25,
-	.arl_search_read = b53_arl_search_read_65,
-};
-
 static const struct b53_arl_ops b53_arl_ops_89 = {
 	.arl_read_entry = b53_arl_read_entry_89,
 	.arl_write_entry = b53_arl_write_entry_89,
@@ -2814,7 +2798,7 @@ static const struct b53_chip_data b53_switch_chips[] = {
 		.arl_buckets = 1024,
 		.imp_port = 5,
 		.duplex_reg = B53_DUPLEX_STAT_FE,
-		.arl_ops = &b53_arl_ops_65,
+		.arl_ops = &b53_arl_ops_25,
 	},
 	{
 		.chip_id = BCM5389_DEVICE_ID,
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 69ebbec932f6..505979102ed5 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -376,10 +376,8 @@
 #define B53_ARL_SRCH_RSLT_MACVID_89	0x33
 #define B53_ARL_SRCH_RSLT_MACVID_63XX	0x34
 
-/* Single register search result on 5325 */
+/* Single register search result on 5325/5365 */
 #define B53_ARL_SRCH_RSTL_0_MACVID_25	0x24
-/* Single register search result on 5365 */
-#define B53_ARL_SRCH_RSTL_0_MACVID_65	0x30
 
 /* ARL Search Data Result (32 bit) */
 #define B53_ARL_SRCH_RSTL_0		0x68
-- 
2.43.0

[PATCH net-next 4/7] net: dsa: b53: fix CPU port unicast ARL entries for BCM5325/65

[Jonas Gorski]

On BCM5325 and BCM5365, unicast ARL entries use 8 as the value for the
CPU port, so we need to translate it to/from 5 as used for the CPU port
at most other places.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_priv.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 3b22e817fb28..bd821d60ac90 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -344,12 +344,14 @@ static inline void b53_arl_to_entry_25(struct b53_arl_entry *ent,
 				       u64 mac_vid)
 {
 	memset(ent, 0, sizeof(*ent));
-	ent->port = (mac_vid >> ARLTBL_DATA_PORT_ID_S_25) &
-		     ARLTBL_DATA_PORT_ID_MASK_25;
 	ent->is_valid = !!(mac_vid & ARLTBL_VALID_25);
 	ent->is_age = !!(mac_vid & ARLTBL_AGE_25);
 	ent->is_static = !!(mac_vid & ARLTBL_STATIC_25);
 	u64_to_ether_addr(mac_vid, ent->mac);
+	ent->port = (mac_vid >> ARLTBL_DATA_PORT_ID_S_25) &
+		     ARLTBL_DATA_PORT_ID_MASK_25;
+	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
+		ent->port = B53_CPU_PORT_25;
 	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
 }
 
@@ -383,8 +385,11 @@ static inline void b53_arl_from_entry_25(u64 *mac_vid,
 					 const struct b53_arl_entry *ent)
 {
 	*mac_vid = ether_addr_to_u64(ent->mac);
-	*mac_vid |= (u64)(ent->port & ARLTBL_DATA_PORT_ID_MASK_25) <<
-			  ARLTBL_DATA_PORT_ID_S_25;
+	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT_25)
+		*mac_vid |= (u64)B53_CPU_PORT << ARLTBL_DATA_PORT_ID_S_25;
+	else
+		*mac_vid |= (u64)(ent->port & ARLTBL_DATA_PORT_ID_MASK_25) <<
+				  ARLTBL_DATA_PORT_ID_S_25;
 	*mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK_25) <<
 			  ARLTBL_VID_S_65;
 	if (ent->is_valid)
-- 
2.43.0

[PATCH net-next 5/7] net: dsa: b53: fix BCM5325/65 ARL entry multicast port masks

[Jonas Gorski]

We currently use the mask 0xf for writing and reading b53_entry::port,
but this is only correct for unicast ARL entries. Multicast ARL entries
use a bitmask, and 0xf is not enough space for ports > 3, which includes
the CPU port.

So extend the mask accordingly to also fit port 4 (bit 4) and MII (bit
5). According to the datasheet the multicast port mask is [60:48],
making it 12 bit wide, but bits 60-55 are reserved anyway, and collide
with the priority field at [60:59], so I am not sure if this is valid.
Therefore leave it at the actual used range, [53:48].

The ARL search result register differs a bit, and there the mask is only
[52:48], so only spanning the user ports. The MII port bit is
contained in the Search Result Extension register. So create a separate
search result parse function that properly handles this.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  4 +++-
 drivers/net/dsa/b53/b53_priv.h   | 25 +++++++++++++++++++++----
 drivers/net/dsa/b53/b53_regs.h   |  8 +++++++-
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 91b0b4de475f..09a64812cd84 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2119,10 +2119,12 @@ static void b53_arl_search_read_25(struct b53_device *dev, u8 idx,
 				   struct b53_arl_entry *ent)
 {
 	u64 mac_vid;
+	u8 ext;
 
+	b53_read8(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSLT_EXT_25, &ext);
 	b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_25,
 		   &mac_vid);
-	b53_arl_to_entry_25(ent, mac_vid);
+	b53_arl_search_to_entry_25(ent, mac_vid, ext);
 }
 
 static void b53_arl_search_read_89(struct b53_device *dev, u8 idx,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index bd821d60ac90..24df3ab64395 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -348,8 +348,8 @@ static inline void b53_arl_to_entry_25(struct b53_arl_entry *ent,
 	ent->is_age = !!(mac_vid & ARLTBL_AGE_25);
 	ent->is_static = !!(mac_vid & ARLTBL_STATIC_25);
 	u64_to_ether_addr(mac_vid, ent->mac);
-	ent->port = (mac_vid >> ARLTBL_DATA_PORT_ID_S_25) &
-		     ARLTBL_DATA_PORT_ID_MASK_25;
+	ent->port = (mac_vid & ARLTBL_DATA_PORT_ID_MASK_25) >>
+		     ARLTBL_DATA_PORT_ID_S_25;
 	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
 		ent->port = B53_CPU_PORT_25;
 	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
@@ -388,8 +388,8 @@ static inline void b53_arl_from_entry_25(u64 *mac_vid,
 	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT_25)
 		*mac_vid |= (u64)B53_CPU_PORT << ARLTBL_DATA_PORT_ID_S_25;
 	else
-		*mac_vid |= (u64)(ent->port & ARLTBL_DATA_PORT_ID_MASK_25) <<
-				  ARLTBL_DATA_PORT_ID_S_25;
+		*mac_vid |= ((u64)ent->port << ARLTBL_DATA_PORT_ID_S_25) &
+			    ARLTBL_DATA_PORT_ID_MASK_25;
 	*mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK_25) <<
 			  ARLTBL_VID_S_65;
 	if (ent->is_valid)
@@ -414,6 +414,23 @@ static inline void b53_arl_from_entry_89(u64 *mac_vid, u32 *fwd_entry,
 		*fwd_entry |= ARLTBL_AGE_89;
 }
 
+static inline void b53_arl_search_to_entry_25(struct b53_arl_entry *ent,
+					      u64 mac_vid, u8 ext)
+{
+	memset(ent, 0, sizeof(*ent));
+	ent->is_valid = !!(mac_vid & ARLTBL_VALID_25);
+	ent->is_age = !!(mac_vid & ARLTBL_AGE_25);
+	ent->is_static = !!(mac_vid & ARLTBL_STATIC_25);
+	u64_to_ether_addr(mac_vid, ent->mac);
+	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
+	ent->port = (mac_vid & ARL_SRCH_RSLT_PORT_ID_MASK_25) >>
+		    ARL_SRCH_RSLT_PORT_ID_S_25;
+	if (is_multicast_ether_addr(ent->mac) && (ext & ARL_SRCH_RSLT_EXT_MC_MII))
+		ent->port |= BIT(B53_CPU_PORT_25);
+	else if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
+		ent->port = B53_CPU_PORT_25;
+}
+
 static inline void b53_arl_search_to_entry_63xx(struct b53_arl_entry *ent,
 						u64 mac_vid, u16 fwd_entry)
 {
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 505979102ed5..54b1016eb7eb 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -332,7 +332,7 @@
 #define   ARLTBL_VID_MASK_25		0xff
 #define   ARLTBL_VID_MASK		0xfff
 #define   ARLTBL_DATA_PORT_ID_S_25	48
-#define   ARLTBL_DATA_PORT_ID_MASK_25	0xf
+#define   ARLTBL_DATA_PORT_ID_MASK_25	GENMASK_ULL(53, 48)
 #define   ARLTBL_VID_S_65		53
 #define   ARLTBL_AGE_25			BIT_ULL(61)
 #define   ARLTBL_STATIC_25		BIT_ULL(62)
@@ -378,6 +378,12 @@
 
 /* Single register search result on 5325/5365 */
 #define B53_ARL_SRCH_RSTL_0_MACVID_25	0x24
+#define   ARL_SRCH_RSLT_PORT_ID_S_25	48
+#define   ARL_SRCH_RSLT_PORT_ID_MASK_25	GENMASK_ULL(52, 48)
+
+/* BCM5325/5365 Search result extend register (8 bit) */
+#define B53_ARL_SRCH_RSLT_EXT_25	0x2c
+#define   ARL_SRCH_RSLT_EXT_MC_MII	BIT(2)
 
 /* ARL Search Data Result (32 bit) */
 #define B53_ARL_SRCH_RSTL_0		0x68
-- 
2.43.0

[PATCH net-next 6/7] net: dsa: b53: fix BCM5325/65 ARL entry VIDs

[Jonas Gorski]

BCM5325/65's ARL entry registers do not contain the VID, only the search
result register does. ARL entries have a separate VID entry register for
the index into the VLAN table.

So make ARL entry accessors use the VID entry registers instead, and
move the VLAN ID field definition to the search register definition.

Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  9 +++++++--
 drivers/net/dsa/b53/b53_priv.h   | 12 ++++++------
 drivers/net/dsa/b53/b53_regs.h   |  7 +++++--
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 09a64812cd84..ac995f36ed95 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1853,19 +1853,24 @@ static int b53_arl_rw_op(struct b53_device *dev, unsigned int op)
 static void b53_arl_read_entry_25(struct b53_device *dev,
 				  struct b53_arl_entry *ent, u8 idx)
 {
+	u8 vid_entry;
 	u64 mac_vid;
 
+	b53_read8(dev, B53_ARLIO_PAGE, B53_ARLTBL_VID_ENTRY_25(idx),
+		  &vid_entry);
 	b53_read64(dev, B53_ARLIO_PAGE, B53_ARLTBL_MAC_VID_ENTRY(idx),
 		   &mac_vid);
-	b53_arl_to_entry_25(ent, mac_vid);
+	b53_arl_to_entry_25(ent, mac_vid, vid_entry);
 }
 
 static void b53_arl_write_entry_25(struct b53_device *dev,
 				   const struct b53_arl_entry *ent, u8 idx)
 {
+	u8 vid_entry;
 	u64 mac_vid;
 
-	b53_arl_from_entry_25(&mac_vid, ent);
+	b53_arl_from_entry_25(&mac_vid, &vid_entry, ent);
+	b53_write8(dev, B53_ARLIO_PAGE, B53_ARLTBL_VID_ENTRY_25(idx), vid_entry);
 	b53_write64(dev, B53_ARLIO_PAGE, B53_ARLTBL_MAC_VID_ENTRY(idx),
 		    mac_vid);
 }
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 24df3ab64395..ea99e4d322bd 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -341,7 +341,7 @@ static inline void b53_arl_to_entry(struct b53_arl_entry *ent,
 }
 
 static inline void b53_arl_to_entry_25(struct b53_arl_entry *ent,
-				       u64 mac_vid)
+				       u64 mac_vid, u8 vid_entry)
 {
 	memset(ent, 0, sizeof(*ent));
 	ent->is_valid = !!(mac_vid & ARLTBL_VALID_25);
@@ -352,7 +352,7 @@ static inline void b53_arl_to_entry_25(struct b53_arl_entry *ent,
 		     ARLTBL_DATA_PORT_ID_S_25;
 	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
 		ent->port = B53_CPU_PORT_25;
-	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
+	ent->vid = vid_entry;
 }
 
 static inline void b53_arl_to_entry_89(struct b53_arl_entry *ent,
@@ -381,7 +381,7 @@ static inline void b53_arl_from_entry(u64 *mac_vid, u32 *fwd_entry,
 		*fwd_entry |= ARLTBL_AGE;
 }
 
-static inline void b53_arl_from_entry_25(u64 *mac_vid,
+static inline void b53_arl_from_entry_25(u64 *mac_vid, u8 *vid_entry,
 					 const struct b53_arl_entry *ent)
 {
 	*mac_vid = ether_addr_to_u64(ent->mac);
@@ -390,14 +390,13 @@ static inline void b53_arl_from_entry_25(u64 *mac_vid,
 	else
 		*mac_vid |= ((u64)ent->port << ARLTBL_DATA_PORT_ID_S_25) &
 			    ARLTBL_DATA_PORT_ID_MASK_25;
-	*mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK_25) <<
-			  ARLTBL_VID_S_65;
 	if (ent->is_valid)
 		*mac_vid |= ARLTBL_VALID_25;
 	if (ent->is_static)
 		*mac_vid |= ARLTBL_STATIC_25;
 	if (ent->is_age)
 		*mac_vid |= ARLTBL_AGE_25;
+	*vid_entry = ent->vid;
 }
 
 static inline void b53_arl_from_entry_89(u64 *mac_vid, u32 *fwd_entry,
@@ -422,7 +421,8 @@ static inline void b53_arl_search_to_entry_25(struct b53_arl_entry *ent,
 	ent->is_age = !!(mac_vid & ARLTBL_AGE_25);
 	ent->is_static = !!(mac_vid & ARLTBL_STATIC_25);
 	u64_to_ether_addr(mac_vid, ent->mac);
-	ent->vid = (mac_vid >> ARLTBL_VID_S_65) & ARLTBL_VID_MASK_25;
+	ent->vid = (mac_vid & ARL_SRCH_RSLT_VID_MASK_25) >>
+		   ARL_SRCH_RSLT_VID_S_25;
 	ent->port = (mac_vid & ARL_SRCH_RSLT_PORT_ID_MASK_25) >>
 		    ARL_SRCH_RSLT_PORT_ID_S_25;
 	if (is_multicast_ether_addr(ent->mac) && (ext & ARL_SRCH_RSLT_EXT_MC_MII))
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 54b1016eb7eb..54a278db67c9 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -329,11 +329,9 @@
 #define B53_ARLTBL_MAC_VID_ENTRY(n)	((0x10 * (n)) + 0x10)
 #define   ARLTBL_MAC_MASK		0xffffffffffffULL
 #define   ARLTBL_VID_S			48
-#define   ARLTBL_VID_MASK_25		0xff
 #define   ARLTBL_VID_MASK		0xfff
 #define   ARLTBL_DATA_PORT_ID_S_25	48
 #define   ARLTBL_DATA_PORT_ID_MASK_25	GENMASK_ULL(53, 48)
-#define   ARLTBL_VID_S_65		53
 #define   ARLTBL_AGE_25			BIT_ULL(61)
 #define   ARLTBL_STATIC_25		BIT_ULL(62)
 #define   ARLTBL_VALID_25		BIT_ULL(63)
@@ -353,6 +351,9 @@
 #define   ARLTBL_STATIC_89		BIT(14)
 #define   ARLTBL_VALID_89		BIT(15)
 
+/* BCM5325/BCM565 ARL Table VID Entry N Registers (8 bit) */
+#define B53_ARLTBL_VID_ENTRY_25(n)	((0x2 * (n)) + 0x30)
+
 /* Maximum number of bin entries in the ARL for all switches */
 #define B53_ARLTBL_MAX_BIN_ENTRIES	4
 
@@ -380,6 +381,8 @@
 #define B53_ARL_SRCH_RSTL_0_MACVID_25	0x24
 #define   ARL_SRCH_RSLT_PORT_ID_S_25	48
 #define   ARL_SRCH_RSLT_PORT_ID_MASK_25	GENMASK_ULL(52, 48)
+#define   ARL_SRCH_RSLT_VID_S_25	53
+#define   ARL_SRCH_RSLT_VID_MASK_25	GENMASK_ULL(60, 53)
 
 /* BCM5325/5365 Search result extend register (8 bit) */
 #define B53_ARL_SRCH_RSLT_EXT_25	0x2c
-- 
2.43.0

[PATCH net-next 7/7] net: dsa: b53: allow VID 0 for BCM5325/65

[Jonas Gorski]

Now that writing ARL entries works properly, we can actually use VID 0
as the default untagged VLAN for BCM5325 and BCM5365 as well, so use 0
as default PVID always.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 49 +++++++++++---------------------
 1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ac995f36ed95..4eff64204897 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -870,14 +870,6 @@ static void b53_enable_stp(struct b53_device *dev)
 	b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, gc);
 }
 
-static u16 b53_default_pvid(struct b53_device *dev)
-{
-	if (is5325(dev) || is5365(dev))
-		return 1;
-	else
-		return 0;
-}
-
 static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port)
 {
 	struct b53_device *dev = ds->priv;
@@ -906,14 +898,12 @@ int b53_configure_vlan(struct dsa_switch *ds)
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan vl = { 0 };
 	struct b53_vlan *v;
-	int i, def_vid;
 	u16 vid;
-
-	def_vid = b53_default_pvid(dev);
+	int i;
 
 	/* clear all vlan entries */
 	if (is5325(dev) || is5365(dev)) {
-		for (i = def_vid; i < dev->num_vlans; i++)
+		for (i = 0; i < dev->num_vlans; i++)
 			b53_set_vlan_entry(dev, i, &vl);
 	} else {
 		b53_do_vlan_op(dev, VTA_CMD_CLEAR);
@@ -927,7 +917,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
 	 * entry. Do this only when the tagging protocol is not
 	 * DSA_TAG_PROTO_NONE
 	 */
-	v = &dev->vlans[def_vid];
+	v = &dev->vlans[0];
 	b53_for_each_port(dev, i) {
 		if (!b53_vlan_port_may_join_untagged(ds, i))
 			continue;
@@ -935,16 +925,15 @@ int b53_configure_vlan(struct dsa_switch *ds)
 		vl.members |= BIT(i);
 		if (!b53_vlan_port_needs_forced_tagged(ds, i))
 			vl.untag = vl.members;
-		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i),
-			    def_vid);
+		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), 0);
 	}
-	b53_set_vlan_entry(dev, def_vid, &vl);
+	b53_set_vlan_entry(dev, 0, &vl);
 
 	if (dev->vlan_filtering) {
 		/* Upon initial call we have not set-up any VLANs, but upon
 		 * system resume, we need to restore all VLAN entries.
 		 */
-		for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
+		for (vid = 1; vid < dev->num_vlans; vid++) {
 			v = &dev->vlans[vid];
 
 			if (!v->members)
@@ -1280,7 +1269,6 @@ static int b53_setup(struct dsa_switch *ds)
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan *vl;
 	unsigned int port;
-	u16 pvid;
 	int ret;
 
 	/* Request bridge PVID untagged when DSA_TAG_PROTO_NONE is set
@@ -1310,8 +1298,7 @@ static int b53_setup(struct dsa_switch *ds)
 	}
 
 	/* setup default vlan for filtering mode */
-	pvid = b53_default_pvid(dev);
-	vl = &dev->vlans[pvid];
+	vl = &dev->vlans[0];
 	b53_for_each_port(dev, port) {
 		vl->members |= BIT(port);
 		if (!b53_vlan_port_needs_forced_tagged(ds, port))
@@ -1740,7 +1727,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
 	if (pvid)
 		new_pvid = vlan->vid;
 	else if (!pvid && vlan->vid == old_pvid)
-		new_pvid = b53_default_pvid(dev);
+		new_pvid = 0;
 	else
 		new_pvid = old_pvid;
 	dev->ports[port].pvid = new_pvid;
@@ -1790,7 +1777,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
 	vl->members &= ~BIT(port);
 
 	if (pvid == vlan->vid)
-		pvid = b53_default_pvid(dev);
+		pvid = 0;
 	dev->ports[port].pvid = pvid;
 
 	if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
@@ -2269,7 +2256,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan *vl;
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
-	u16 pvlan, reg, pvid;
+	u16 pvlan, reg;
 	unsigned int i;
 
 	/* On 7278, port 7 which connects to the ASP should only receive
@@ -2278,8 +2265,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 	if (dev->chip_id == BCM7278_DEVICE_ID && port == 7)
 		return -EINVAL;
 
-	pvid = b53_default_pvid(dev);
-	vl = &dev->vlans[pvid];
+	vl = &dev->vlans[0];
 
 	if (dev->vlan_filtering) {
 		/* Make this port leave the all VLANs join since we will have
@@ -2295,9 +2281,9 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 				    reg);
 		}
 
-		b53_get_vlan_entry(dev, pvid, vl);
+		b53_get_vlan_entry(dev, 0, vl);
 		vl->members &= ~BIT(port);
-		b53_set_vlan_entry(dev, pvid, vl);
+		b53_set_vlan_entry(dev, 0, vl);
 	}
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
@@ -2336,7 +2322,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 	struct b53_vlan *vl;
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
 	unsigned int i;
-	u16 pvlan, reg, pvid;
+	u16 pvlan, reg;
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
@@ -2361,8 +2347,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 	b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
 	dev->ports[port].vlan_ctl_mask = pvlan;
 
-	pvid = b53_default_pvid(dev);
-	vl = &dev->vlans[pvid];
+	vl = &dev->vlans[0];
 
 	if (dev->vlan_filtering) {
 		/* Make this port join all VLANs without VLAN entries */
@@ -2374,9 +2359,9 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 			b53_write16(dev, B53_VLAN_PAGE, B53_JOIN_ALL_VLAN_EN, reg);
 		}
 
-		b53_get_vlan_entry(dev, pvid, vl);
+		b53_get_vlan_entry(dev, 0, vl);
 		vl->members |= BIT(port);
-		b53_set_vlan_entry(dev, pvid, vl);
+		b53_set_vlan_entry(dev, 0, vl);
 	}
 }
 EXPORT_SYMBOL(b53_br_leave);
-- 
2.43.0

Re: [PATCH net-next 7/7] net: dsa: b53: allow VID 0 for BCM5325/65

[Álvaro Fernández Rojas]

El mar, 25 nov 2025 a las 8:52, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> Now that writing ARL entries works properly, we can actually use VID 0
> as the default untagged VLAN for BCM5325 and BCM5365 as well, so use 0
> as default PVID always.
>
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 49 +++++++++++---------------------
>  1 file changed, 17 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index ac995f36ed95..4eff64204897 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -870,14 +870,6 @@ static void b53_enable_stp(struct b53_device *dev)
>         b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, gc);
>  }
>
> -static u16 b53_default_pvid(struct b53_device *dev)
> -{
> -       if (is5325(dev) || is5365(dev))
> -               return 1;
> -       else
> -               return 0;
> -}
> -
>  static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port)
>  {
>         struct b53_device *dev = ds->priv;
> @@ -906,14 +898,12 @@ int b53_configure_vlan(struct dsa_switch *ds)
>         struct b53_device *dev = ds->priv;
>         struct b53_vlan vl = { 0 };
>         struct b53_vlan *v;
> -       int i, def_vid;
>         u16 vid;
> -
> -       def_vid = b53_default_pvid(dev);
> +       int i;
>
>         /* clear all vlan entries */
>         if (is5325(dev) || is5365(dev)) {
> -               for (i = def_vid; i < dev->num_vlans; i++)
> +               for (i = 0; i < dev->num_vlans; i++)
>                         b53_set_vlan_entry(dev, i, &vl);
>         } else {
>                 b53_do_vlan_op(dev, VTA_CMD_CLEAR);
> @@ -927,7 +917,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
>          * entry. Do this only when the tagging protocol is not
>          * DSA_TAG_PROTO_NONE
>          */
> -       v = &dev->vlans[def_vid];
> +       v = &dev->vlans[0];
>         b53_for_each_port(dev, i) {
>                 if (!b53_vlan_port_may_join_untagged(ds, i))
>                         continue;
> @@ -935,16 +925,15 @@ int b53_configure_vlan(struct dsa_switch *ds)
>                 vl.members |= BIT(i);
>                 if (!b53_vlan_port_needs_forced_tagged(ds, i))
>                         vl.untag = vl.members;
> -               b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i),
> -                           def_vid);
> +               b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), 0);
>         }
> -       b53_set_vlan_entry(dev, def_vid, &vl);
> +       b53_set_vlan_entry(dev, 0, &vl);
>
>         if (dev->vlan_filtering) {
>                 /* Upon initial call we have not set-up any VLANs, but upon
>                  * system resume, we need to restore all VLAN entries.
>                  */
> -               for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
> +               for (vid = 1; vid < dev->num_vlans; vid++) {
>                         v = &dev->vlans[vid];
>
>                         if (!v->members)
> @@ -1280,7 +1269,6 @@ static int b53_setup(struct dsa_switch *ds)
>         struct b53_device *dev = ds->priv;
>         struct b53_vlan *vl;
>         unsigned int port;
> -       u16 pvid;
>         int ret;
>
>         /* Request bridge PVID untagged when DSA_TAG_PROTO_NONE is set
> @@ -1310,8 +1298,7 @@ static int b53_setup(struct dsa_switch *ds)
>         }
>
>         /* setup default vlan for filtering mode */
> -       pvid = b53_default_pvid(dev);
> -       vl = &dev->vlans[pvid];
> +       vl = &dev->vlans[0];
>         b53_for_each_port(dev, port) {
>                 vl->members |= BIT(port);
>                 if (!b53_vlan_port_needs_forced_tagged(ds, port))
> @@ -1740,7 +1727,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
>         if (pvid)
>                 new_pvid = vlan->vid;
>         else if (!pvid && vlan->vid == old_pvid)
> -               new_pvid = b53_default_pvid(dev);
> +               new_pvid = 0;
>         else
>                 new_pvid = old_pvid;
>         dev->ports[port].pvid = new_pvid;
> @@ -1790,7 +1777,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
>         vl->members &= ~BIT(port);
>
>         if (pvid == vlan->vid)
> -               pvid = b53_default_pvid(dev);
> +               pvid = 0;
>         dev->ports[port].pvid = pvid;
>
>         if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
> @@ -2269,7 +2256,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>         struct b53_device *dev = ds->priv;
>         struct b53_vlan *vl;
>         s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> -       u16 pvlan, reg, pvid;
> +       u16 pvlan, reg;
>         unsigned int i;
>
>         /* On 7278, port 7 which connects to the ASP should only receive
> @@ -2278,8 +2265,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>         if (dev->chip_id == BCM7278_DEVICE_ID && port == 7)
>                 return -EINVAL;
>
> -       pvid = b53_default_pvid(dev);
> -       vl = &dev->vlans[pvid];
> +       vl = &dev->vlans[0];
>
>         if (dev->vlan_filtering) {
>                 /* Make this port leave the all VLANs join since we will have
> @@ -2295,9 +2281,9 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>                                     reg);
>                 }
>
> -               b53_get_vlan_entry(dev, pvid, vl);
> +               b53_get_vlan_entry(dev, 0, vl);
>                 vl->members &= ~BIT(port);
> -               b53_set_vlan_entry(dev, pvid, vl);
> +               b53_set_vlan_entry(dev, 0, vl);
>         }
>
>         b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
> @@ -2336,7 +2322,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>         struct b53_vlan *vl;
>         s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
>         unsigned int i;
> -       u16 pvlan, reg, pvid;
> +       u16 pvlan, reg;
>
>         b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
>
> @@ -2361,8 +2347,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>         b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
>         dev->ports[port].vlan_ctl_mask = pvlan;
>
> -       pvid = b53_default_pvid(dev);
> -       vl = &dev->vlans[pvid];
> +       vl = &dev->vlans[0];
>
>         if (dev->vlan_filtering) {
>                 /* Make this port join all VLANs without VLAN entries */
> @@ -2374,9 +2359,9 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>                         b53_write16(dev, B53_VLAN_PAGE, B53_JOIN_ALL_VLAN_EN, reg);
>                 }
>
> -               b53_get_vlan_entry(dev, pvid, vl);
> +               b53_get_vlan_entry(dev, 0, vl);
>                 vl->members |= BIT(port);
> -               b53_set_vlan_entry(dev, pvid, vl);
> +               b53_set_vlan_entry(dev, 0, vl);
>         }
>  }
>  EXPORT_SYMBOL(b53_br_leave);
> --
> 2.43.0
>

Thank you so much for these patches.

I've tested all of them on a Huawei HG556a (bcm6358 + bcm5325).

Tested-by: Álvaro Fernández Rojas <noltari@gmail.com>

Re: [PATCH net-next 7/7] net: dsa: b53: allow VID 0 for BCM5325/65

[Vladimir Oltean]

Hi Jonas,

On Tue, Nov 25, 2025 at 08:51:50AM +0100, Jonas Gorski wrote:
> Now that writing ARL entries works properly, we can actually use VID 0
> as the default untagged VLAN for BCM5325 and BCM5365 as well, so use 0
> as default PVID always.
> 
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 49 +++++++++++---------------------
>  1 file changed, 17 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index ac995f36ed95..4eff64204897 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -870,14 +870,6 @@ static void b53_enable_stp(struct b53_device *dev)
>  	b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, gc);
>  }
>  
> -static u16 b53_default_pvid(struct b53_device *dev)
> -{
> -	if (is5325(dev) || is5365(dev))
> -		return 1;
> -	else
> -		return 0;
> -}
> -

I am in favour of a more minimal change, where b53_default_pvid()
returns 0, and its call sites are kept unmodified, if only and for
code documentation purposes. Other drivers use a macro to avoid
hardcoding the 0 everywhere the default VLAN is meant. This driver
doesn't have a macro but it already has b53_default_pvid().

>  static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port)
>  {
>  	struct b53_device *dev = ds->priv;
> @@ -906,14 +898,12 @@ int b53_configure_vlan(struct dsa_switch *ds)
>  	struct b53_device *dev = ds->priv;
>  	struct b53_vlan vl = { 0 };
>  	struct b53_vlan *v;
> -	int i, def_vid;
>  	u16 vid;
> -
> -	def_vid = b53_default_pvid(dev);
> +	int i;
>  
>  	/* clear all vlan entries */
>  	if (is5325(dev) || is5365(dev)) {
> -		for (i = def_vid; i < dev->num_vlans; i++)
> +		for (i = 0; i < dev->num_vlans; i++)
>  			b53_set_vlan_entry(dev, i, &vl);
>  	} else {
>  		b53_do_vlan_op(dev, VTA_CMD_CLEAR);
> @@ -927,7 +917,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
>  	 * entry. Do this only when the tagging protocol is not
>  	 * DSA_TAG_PROTO_NONE
>  	 */
> -	v = &dev->vlans[def_vid];
> +	v = &dev->vlans[0];
>  	b53_for_each_port(dev, i) {
>  		if (!b53_vlan_port_may_join_untagged(ds, i))
>  			continue;
> @@ -935,16 +925,15 @@ int b53_configure_vlan(struct dsa_switch *ds)
>  		vl.members |= BIT(i);
>  		if (!b53_vlan_port_needs_forced_tagged(ds, i))
>  			vl.untag = vl.members;
> -		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i),
> -			    def_vid);
> +		b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), 0);
>  	}
> -	b53_set_vlan_entry(dev, def_vid, &vl);
> +	b53_set_vlan_entry(dev, 0, &vl);
>  
>  	if (dev->vlan_filtering) {
>  		/* Upon initial call we have not set-up any VLANs, but upon
>  		 * system resume, we need to restore all VLAN entries.
>  		 */
> -		for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
> +		for (vid = 1; vid < dev->num_vlans; vid++) {
>  			v = &dev->vlans[vid];
>  
>  			if (!v->members)
> @@ -1280,7 +1269,6 @@ static int b53_setup(struct dsa_switch *ds)
>  	struct b53_device *dev = ds->priv;
>  	struct b53_vlan *vl;
>  	unsigned int port;
> -	u16 pvid;
>  	int ret;
>  
>  	/* Request bridge PVID untagged when DSA_TAG_PROTO_NONE is set
> @@ -1310,8 +1298,7 @@ static int b53_setup(struct dsa_switch *ds)
>  	}
>  
>  	/* setup default vlan for filtering mode */
> -	pvid = b53_default_pvid(dev);
> -	vl = &dev->vlans[pvid];
> +	vl = &dev->vlans[0];
>  	b53_for_each_port(dev, port) {
>  		vl->members |= BIT(port);
>  		if (!b53_vlan_port_needs_forced_tagged(ds, port))
> @@ -1740,7 +1727,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
>  	if (pvid)
>  		new_pvid = vlan->vid;
>  	else if (!pvid && vlan->vid == old_pvid)
> -		new_pvid = b53_default_pvid(dev);
> +		new_pvid = 0;
>  	else
>  		new_pvid = old_pvid;
>  	dev->ports[port].pvid = new_pvid;
> @@ -1790,7 +1777,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
>  	vl->members &= ~BIT(port);
>  
>  	if (pvid == vlan->vid)
> -		pvid = b53_default_pvid(dev);
> +		pvid = 0;
>  	dev->ports[port].pvid = pvid;
>  
>  	if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
> @@ -2269,7 +2256,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  	struct b53_device *dev = ds->priv;
>  	struct b53_vlan *vl;
>  	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> -	u16 pvlan, reg, pvid;
> +	u16 pvlan, reg;
>  	unsigned int i;
>  
>  	/* On 7278, port 7 which connects to the ASP should only receive
> @@ -2278,8 +2265,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  	if (dev->chip_id == BCM7278_DEVICE_ID && port == 7)
>  		return -EINVAL;
>  
> -	pvid = b53_default_pvid(dev);
> -	vl = &dev->vlans[pvid];
> +	vl = &dev->vlans[0];
>  
>  	if (dev->vlan_filtering) {
>  		/* Make this port leave the all VLANs join since we will have
> @@ -2295,9 +2281,9 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  				    reg);
>  		}
>  
> -		b53_get_vlan_entry(dev, pvid, vl);
> +		b53_get_vlan_entry(dev, 0, vl);
>  		vl->members &= ~BIT(port);
> -		b53_set_vlan_entry(dev, pvid, vl);
> +		b53_set_vlan_entry(dev, 0, vl);
>  	}
>  
>  	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
> @@ -2336,7 +2322,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>  	struct b53_vlan *vl;
>  	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
>  	unsigned int i;
> -	u16 pvlan, reg, pvid;
> +	u16 pvlan, reg;
>  
>  	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
>  
> @@ -2361,8 +2347,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>  	b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
>  	dev->ports[port].vlan_ctl_mask = pvlan;
>  
> -	pvid = b53_default_pvid(dev);
> -	vl = &dev->vlans[pvid];
> +	vl = &dev->vlans[0];
>  
>  	if (dev->vlan_filtering) {
>  		/* Make this port join all VLANs without VLAN entries */
> @@ -2374,9 +2359,9 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>  			b53_write16(dev, B53_VLAN_PAGE, B53_JOIN_ALL_VLAN_EN, reg);
>  		}
>  
> -		b53_get_vlan_entry(dev, pvid, vl);
> +		b53_get_vlan_entry(dev, 0, vl);
>  		vl->members |= BIT(port);
> -		b53_set_vlan_entry(dev, pvid, vl);
> +		b53_set_vlan_entry(dev, 0, vl);
>  	}
>  }
>  EXPORT_SYMBOL(b53_br_leave);
> -- 
> 2.43.0
> 

Not covered in this patch, but I wonder whether it should have been.

This test in b53_vlan_prepare() seems obsolete and a good candidate for
removal:

	if ((is5325(dev) || is5365(dev)) && vlan->vid == 0)
		return -EOPNOTSUPP;

especially since we already have the same check below in b53_vlan_add()
for all chip IDs:

	if (vlan->vid == 0)
		return 0;

Re: [PATCH net-next 4/7] net: dsa: b53: fix CPU port unicast ARL entries for BCM5325/65

[Vladimir Oltean]

On Tue, Nov 25, 2025 at 08:51:47AM +0100, Jonas Gorski wrote:
> On BCM5325 and BCM5365, unicast ARL entries use 8 as the value for the
> CPU port, so we need to translate it to/from 5 as used for the CPU port
> at most other places.
> +	ent->port = (mac_vid >> ARLTBL_DATA_PORT_ID_S_25) &
> +		     ARLTBL_DATA_PORT_ID_MASK_25;
> +	if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
> +		ent->port = B53_CPU_PORT_25;

Why not use is_unicast_ether_addr() to have a more clear correlation
between the commit message and the code?

Re: [PATCH net-next 4/7] net: dsa: b53: fix CPU port unicast ARL entries for BCM5325/65

[Jonas Gorski]

On Tue, Nov 25, 2025 at 9:42 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Nov 25, 2025 at 08:51:47AM +0100, Jonas Gorski wrote:
> > On BCM5325 and BCM5365, unicast ARL entries use 8 as the value for the
> > CPU port, so we need to translate it to/from 5 as used for the CPU port
> > at most other places.
> > +     ent->port = (mac_vid >> ARLTBL_DATA_PORT_ID_S_25) &
> > +                  ARLTBL_DATA_PORT_ID_MASK_25;
> > +     if (!is_multicast_ether_addr(ent->mac) && ent->port == B53_CPU_PORT)
> > +             ent->port = B53_CPU_PORT_25;
>
> Why not use is_unicast_ether_addr() to have a more clear correlation
> between the commit message and the code?

There is a very simple explanation for that: I didn't know it existed.
Will change it for v2.

Thanks for the review,
Jonas

Re: [PATCH net-next 1/7] net: dsa: b53: fix VLAN_ID_IDX write size for BCM5325/65

[Florian Fainelli]

On 11/24/2025 11:51 PM, Jonas Gorski wrote:
> Since BCM5325 and BCM5365 only support up to 256 VLANs, the VLAN_ID_IDX
> register is only 8 bit wide, not 16 bit, so use an appropriate accessor.
> 
> Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next 2/7] net: dsa: b53: fix extracting VID from entry for BCM5325/65

[Florian Fainelli]

On 11/24/2025 11:51 PM, Jonas Gorski wrote:
> BCM5325/65's Entry register uses the highest three bits for
> VALID/STATIC/AGE, so shifting by 53 only will add these to
> b53_arl_entry::vid.
> 
> So make sure to mask the vid value as well, to not get invalid VIDs.
> 
> Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next 3/7] net: dsa: b53: use same ARL search result offset for BCM5325/65

[Florian Fainelli]

On 11/24/2025 11:51 PM, Jonas Gorski wrote:
> BCM5365's search result is at the same offset as BCM5325's search
> result, and they (mostly) share the same format, so switch BCM5365 to
> BCM5325's arl ops.
> 
> Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next 5/7] net: dsa: b53: fix BCM5325/65 ARL entry multicast port masks

[Florian Fainelli]

On 11/24/2025 11:51 PM, Jonas Gorski wrote:
> We currently use the mask 0xf for writing and reading b53_entry::port,
> but this is only correct for unicast ARL entries. Multicast ARL entries
> use a bitmask, and 0xf is not enough space for ports > 3, which includes
> the CPU port.
> 
> So extend the mask accordingly to also fit port 4 (bit 4) and MII (bit
> 5). According to the datasheet the multicast port mask is [60:48],
> making it 12 bit wide, but bits 60-55 are reserved anyway, and collide
> with the priority field at [60:59], so I am not sure if this is valid.
> Therefore leave it at the actual used range, [53:48].
> 
> The ARL search result register differs a bit, and there the mask is only
> [52:48], so only spanning the user ports. The MII port bit is
> contained in the Search Result Extension register. So create a separate
> search result parse function that properly handles this.
> 
> Fixes: c45655386e53 ("net: dsa: b53: add support for FDB operations on 5325/5365")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net-next 7/7] net: dsa: b53: allow VID 0 for BCM5325/65

[Jonas Gorski]

On Tue, Nov 25, 2025 at 9:31 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Jonas,
>
> On Tue, Nov 25, 2025 at 08:51:50AM +0100, Jonas Gorski wrote:
> > Now that writing ARL entries works properly, we can actually use VID 0
> > as the default untagged VLAN for BCM5325 and BCM5365 as well, so use 0
> > as default PVID always.
> >
> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> > ---
> >  drivers/net/dsa/b53/b53_common.c | 49 +++++++++++---------------------
> >  1 file changed, 17 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index ac995f36ed95..4eff64204897 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -870,14 +870,6 @@ static void b53_enable_stp(struct b53_device *dev)
> >       b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, gc);
> >  }
> >
> > -static u16 b53_default_pvid(struct b53_device *dev)
> > -{
> > -     if (is5325(dev) || is5365(dev))
> > -             return 1;
> > -     else
> > -             return 0;
> > -}
> > -
>
> I am in favour of a more minimal change, where b53_default_pvid()
> returns 0, and its call sites are kept unmodified, if only and for
> code documentation purposes. Other drivers use a macro to avoid
> hardcoding the 0 everywhere the default VLAN is meant. This driver
> doesn't have a macro but it already has b53_default_pvid().

Sure, I can do that.

> >  static bool b53_vlan_port_needs_forced_tagged(struct dsa_switch *ds, int port)
> >  {
> >       struct b53_device *dev = ds->priv;
> > @@ -906,14 +898,12 @@ int b53_configure_vlan(struct dsa_switch *ds)
> >       struct b53_device *dev = ds->priv;
> >       struct b53_vlan vl = { 0 };
> >       struct b53_vlan *v;
> > -     int i, def_vid;
> >       u16 vid;
> > -
> > -     def_vid = b53_default_pvid(dev);
> > +     int i;
> >
> >       /* clear all vlan entries */
> >       if (is5325(dev) || is5365(dev)) {
> > -             for (i = def_vid; i < dev->num_vlans; i++)
> > +             for (i = 0; i < dev->num_vlans; i++)
> >                       b53_set_vlan_entry(dev, i, &vl);
> >       } else {
> >               b53_do_vlan_op(dev, VTA_CMD_CLEAR);
> > @@ -927,7 +917,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
> >        * entry. Do this only when the tagging protocol is not
> >        * DSA_TAG_PROTO_NONE
> >        */
> > -     v = &dev->vlans[def_vid];
> > +     v = &dev->vlans[0];
> >       b53_for_each_port(dev, i) {
> >               if (!b53_vlan_port_may_join_untagged(ds, i))
> >                       continue;
> > @@ -935,16 +925,15 @@ int b53_configure_vlan(struct dsa_switch *ds)
> >               vl.members |= BIT(i);
> >               if (!b53_vlan_port_needs_forced_tagged(ds, i))
> >                       vl.untag = vl.members;
> > -             b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i),
> > -                         def_vid);
> > +             b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(i), 0);
> >       }
> > -     b53_set_vlan_entry(dev, def_vid, &vl);
> > +     b53_set_vlan_entry(dev, 0, &vl);
> >
> >       if (dev->vlan_filtering) {
> >               /* Upon initial call we have not set-up any VLANs, but upon
> >                * system resume, we need to restore all VLAN entries.
> >                */
> > -             for (vid = def_vid + 1; vid < dev->num_vlans; vid++) {
> > +             for (vid = 1; vid < dev->num_vlans; vid++) {
> >                       v = &dev->vlans[vid];
> >
> >                       if (!v->members)
> > @@ -1280,7 +1269,6 @@ static int b53_setup(struct dsa_switch *ds)
> >       struct b53_device *dev = ds->priv;
> >       struct b53_vlan *vl;
> >       unsigned int port;
> > -     u16 pvid;
> >       int ret;
> >
> >       /* Request bridge PVID untagged when DSA_TAG_PROTO_NONE is set
> > @@ -1310,8 +1298,7 @@ static int b53_setup(struct dsa_switch *ds)
> >       }
> >
> >       /* setup default vlan for filtering mode */
> > -     pvid = b53_default_pvid(dev);
> > -     vl = &dev->vlans[pvid];
> > +     vl = &dev->vlans[0];
> >       b53_for_each_port(dev, port) {
> >               vl->members |= BIT(port);
> >               if (!b53_vlan_port_needs_forced_tagged(ds, port))
> > @@ -1740,7 +1727,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
> >       if (pvid)
> >               new_pvid = vlan->vid;
> >       else if (!pvid && vlan->vid == old_pvid)
> > -             new_pvid = b53_default_pvid(dev);
> > +             new_pvid = 0;
> >       else
> >               new_pvid = old_pvid;
> >       dev->ports[port].pvid = new_pvid;
> > @@ -1790,7 +1777,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
> >       vl->members &= ~BIT(port);
> >
> >       if (pvid == vlan->vid)
> > -             pvid = b53_default_pvid(dev);
> > +             pvid = 0;
> >       dev->ports[port].pvid = pvid;
> >
> >       if (untagged && !b53_vlan_port_needs_forced_tagged(ds, port))
> > @@ -2269,7 +2256,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> >       struct b53_device *dev = ds->priv;
> >       struct b53_vlan *vl;
> >       s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> > -     u16 pvlan, reg, pvid;
> > +     u16 pvlan, reg;
> >       unsigned int i;
> >
> >       /* On 7278, port 7 which connects to the ASP should only receive
> > @@ -2278,8 +2265,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> >       if (dev->chip_id == BCM7278_DEVICE_ID && port == 7)
> >               return -EINVAL;
> >
> > -     pvid = b53_default_pvid(dev);
> > -     vl = &dev->vlans[pvid];
> > +     vl = &dev->vlans[0];
> >
> >       if (dev->vlan_filtering) {
> >               /* Make this port leave the all VLANs join since we will have
> > @@ -2295,9 +2281,9 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
> >                                   reg);
> >               }
> >
> > -             b53_get_vlan_entry(dev, pvid, vl);
> > +             b53_get_vlan_entry(dev, 0, vl);
> >               vl->members &= ~BIT(port);
> > -             b53_set_vlan_entry(dev, pvid, vl);
> > +             b53_set_vlan_entry(dev, 0, vl);
> >       }
> >
> >       b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
> > @@ -2336,7 +2322,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
> >       struct b53_vlan *vl;
> >       s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
> >       unsigned int i;
> > -     u16 pvlan, reg, pvid;
> > +     u16 pvlan, reg;
> >
> >       b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
> >
> > @@ -2361,8 +2347,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
> >       b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
> >       dev->ports[port].vlan_ctl_mask = pvlan;
> >
> > -     pvid = b53_default_pvid(dev);
> > -     vl = &dev->vlans[pvid];
> > +     vl = &dev->vlans[0];
> >
> >       if (dev->vlan_filtering) {
> >               /* Make this port join all VLANs without VLAN entries */
> > @@ -2374,9 +2359,9 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
> >                       b53_write16(dev, B53_VLAN_PAGE, B53_JOIN_ALL_VLAN_EN, reg);
> >               }
> >
> > -             b53_get_vlan_entry(dev, pvid, vl);
> > +             b53_get_vlan_entry(dev, 0, vl);
> >               vl->members |= BIT(port);
> > -             b53_set_vlan_entry(dev, pvid, vl);
> > +             b53_set_vlan_entry(dev, 0, vl);
> >       }
> >  }
> >  EXPORT_SYMBOL(b53_br_leave);
> > --
> > 2.43.0
> >
>
> Not covered in this patch, but I wonder whether it should have been.
>
> This test in b53_vlan_prepare() seems obsolete and a good candidate for
> removal:
>
>         if ((is5325(dev) || is5365(dev)) && vlan->vid == 0)
>                 return -EOPNOTSUPP;

Ah, good catch. Yeah, that should/can be removed.

>
> especially since we already have the same check below in b53_vlan_add()
> for all chip IDs:
>
>         if (vlan->vid == 0)
>                 return 0;

Arguably the 5325/65 check could have already been dropped at that
time I added this.

I'll send a v2 later.

Best regards,
Jonas