[RFC PATCH 00/10] net: dsa: b53: fix BCM5325 support

[RFC PATCH 00/10] net: dsa: b53: fix BCM5325 support

[Álvaro Fernández Rojas]

These patches get the BCM5325 switch working with b53.

There are still some sporadic errors related to FDB, but at least the
switch is working now:
bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add d6:67:0c:XX:XX:XX vid 1 to fdb: -28
bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add 5c:4c:a9:XX:XX:XX vid 0 to fdb: -28
bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add 5c:4c:a9:XX:XX:XX vid 1 to fdb: -28
bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to delete d6:67:0c:XX:XX:XX vid 1 from fdb: -2

I'm not really sure that everything here is correct since I don't work for
Broadcom and all this is based on the public datasheet available for the
BCM5325 and my own experiments with a Huawei HG556a (BCM6358).

Florian Fainelli (1):
  net: dsa: b53: add support for FDB operations on 5325/5365

Álvaro Fernández Rojas (9):
  net: dsa: b53: prevent FAST_AGE access on BCM5325
  net: dsa: b53: prevent SWITCH_CTRL access on BCM5325
  net: dsa: b53: fix IP_MULTICAST_CTRL on BCM5325
  net: dsa: b53: prevent DIS_LEARNING access on BCM5325
  net: dsa: b53: prevent BRCM_HDR access on BCM5325
  net: dsa: b53: prevent GMII_PORT_OVERRIDE_CTRL access on BCM5325
  net: dsa: b53: fix unicast/multicast flooding on BCM5325
  net: dsa: b53: fix b53_imp_vlan_setup for BCM5325
  net: dsa: b53: ensure BCM5325 PHYs are enabled

 drivers/net/dsa/b53/b53_common.c | 213 +++++++++++++++++++++++--------
 drivers/net/dsa/b53/b53_priv.h   |  57 ++++++---
 drivers/net/dsa/b53/b53_regs.h   |  47 ++++++-
 3 files changed, 246 insertions(+), 71 deletions(-)

-- 
2.39.5

[RFC PATCH 01/10] net: dsa: b53: add support for FDB operations on 5325/5365

[Álvaro Fernández Rojas]

From: Florian Fainelli <f.fainelli@gmail.com>

BCM5325 and BCM5365 are part of a much older generation of switches which,
due to their limited number of ports and VLAN entries (up to 256) allowed
a single 64-bit register to hold a full ARL entry.
This requires a little bit of massaging when reading, writing and
converting ARL entries in both directions.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 60 +++++++++++++++++++++-----------
 drivers/net/dsa/b53/b53_priv.h   | 57 +++++++++++++++++++++---------
 drivers/net/dsa/b53/b53_regs.h   |  7 ++--
 3 files changed, 84 insertions(+), 40 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 132683ed3abe..03c1e2e75061 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1760,9 +1760,11 @@ static int b53_arl_read(struct b53_device *dev, u64 mac,
 
 		b53_read64(dev, B53_ARLIO_PAGE,
 			   B53_ARLTBL_MAC_VID_ENTRY(i), &mac_vid);
-		b53_read32(dev, B53_ARLIO_PAGE,
-			   B53_ARLTBL_DATA_ENTRY(i), &fwd_entry);
-		b53_arl_to_entry(ent, mac_vid, fwd_entry);
+
+		if (!is5325(dev) && !is5365(dev))
+			b53_read32(dev, B53_ARLIO_PAGE,
+				   B53_ARLTBL_DATA_ENTRY(i), &fwd_entry);
+		b53_arl_to_entry(dev, ent, mac_vid, fwd_entry);
 
 		if (!(fwd_entry & ARLTBL_VALID)) {
 			set_bit(i, free_bins);
@@ -1795,7 +1797,8 @@ 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);
-	b53_write16(dev, B53_ARLIO_PAGE, B53_VLAN_ID_IDX, vid);
+	if (!is5325(dev))
+		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);
@@ -1846,12 +1849,14 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
 	ent.is_static = true;
 	ent.is_age = false;
 	memcpy(ent.mac, addr, ETH_ALEN);
-	b53_arl_from_entry(&mac_vid, &fwd_entry, &ent);
+	b53_arl_from_entry(dev, &mac_vid, &fwd_entry, &ent);
 
 	b53_write64(dev, B53_ARLIO_PAGE,
 		    B53_ARLTBL_MAC_VID_ENTRY(idx), mac_vid);
-	b53_write32(dev, B53_ARLIO_PAGE,
-		    B53_ARLTBL_DATA_ENTRY(idx), fwd_entry);
+
+	if (!is5325(dev) && !is5365(dev))
+		b53_write32(dev, B53_ARLIO_PAGE,
+			    B53_ARLTBL_DATA_ENTRY(idx), fwd_entry);
 
 	return b53_arl_rw_op(dev, 0);
 }
@@ -1863,12 +1868,6 @@ int b53_fdb_add(struct dsa_switch *ds, int port,
 	struct b53_device *priv = ds->priv;
 	int ret;
 
-	/* 5325 and 5365 require some more massaging, but could
-	 * be supported eventually
-	 */
-	if (is5325(priv) || is5365(priv))
-		return -EOPNOTSUPP;
-
 	mutex_lock(&priv->arl_mutex);
 	ret = b53_arl_op(priv, 0, port, addr, vid, true);
 	mutex_unlock(&priv->arl_mutex);
@@ -1895,10 +1894,15 @@ EXPORT_SYMBOL(b53_fdb_del);
 static int b53_arl_search_wait(struct b53_device *dev)
 {
 	unsigned int timeout = 1000;
-	u8 reg;
+	u8 reg, offset;
+
+	if (is5325(dev) || is5365(dev))
+		offset = B53_ARL_SRCH_CTL_25;
+	else
+		offset = B53_ARL_SRCH_CTL;
 
 	do {
-		b53_read8(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_CTL, &reg);
+		b53_read8(dev, B53_ARLIO_PAGE, offset, &reg);
 		if (!(reg & ARL_SRCH_STDN))
 			return 0;
 
@@ -1917,11 +1921,19 @@ static void b53_arl_search_rd(struct b53_device *dev, u8 idx,
 	u64 mac_vid;
 	u32 fwd_entry;
 
-	b53_read64(dev, B53_ARLIO_PAGE,
-		   B53_ARL_SRCH_RSTL_MACVID(idx), &mac_vid);
-	b53_read32(dev, B53_ARLIO_PAGE,
-		   B53_ARL_SRCH_RSTL(idx), &fwd_entry);
-	b53_arl_to_entry(ent, mac_vid, fwd_entry);
+	if (is5325(dev)) {
+		b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_25,
+			   &mac_vid);
+	} else if (is5365(dev)) {
+		b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_65,
+			   &mac_vid);
+	} else {
+		b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_MACVID(idx),
+			   &mac_vid);
+		b53_read32(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL(idx),
+			   &fwd_entry);
+	}
+	b53_arl_to_entry(dev, ent, mac_vid, fwd_entry);
 }
 
 static int b53_fdb_copy(int port, const struct b53_arl_entry *ent,
@@ -1942,14 +1954,20 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
 	struct b53_device *priv = ds->priv;
 	struct b53_arl_entry results[2];
 	unsigned int count = 0;
+	u8 offset;
 	int ret;
 	u8 reg;
 
 	mutex_lock(&priv->arl_mutex);
 
+	if (is5325(priv) || is5365(priv))
+		offset = B53_ARL_SRCH_CTL_25;
+	else
+		offset = B53_ARL_SRCH_CTL;
+
 	/* Start search operation */
 	reg = ARL_SRCH_STDN;
-	b53_write8(priv, B53_ARLIO_PAGE, B53_ARL_SRCH_CTL, reg);
+	b53_write8(priv, offset, B53_ARL_SRCH_CTL, reg);
 
 	do {
 		ret = b53_arl_search_wait(priv);
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index a5ef7071ba07..05c5b9239bda 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -286,30 +286,55 @@ struct b53_arl_entry {
 	u8 is_static:1;
 };
 
-static inline void b53_arl_to_entry(struct b53_arl_entry *ent,
+static inline void b53_arl_to_entry(struct b53_device *dev,
+				    struct b53_arl_entry *ent,
 				    u64 mac_vid, u32 fwd_entry)
 {
 	memset(ent, 0, sizeof(*ent));
-	ent->port = fwd_entry & ARLTBL_DATA_PORT_ID_MASK;
-	ent->is_valid = !!(fwd_entry & ARLTBL_VALID);
-	ent->is_age = !!(fwd_entry & ARLTBL_AGE);
-	ent->is_static = !!(fwd_entry & ARLTBL_STATIC);
-	u64_to_ether_addr(mac_vid, ent->mac);
-	ent->vid = mac_vid >> ARLTBL_VID_S;
+	if (is5325(dev) || is5365(dev)) {
+		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->vid = mac_vid >> ARLTBL_VID_S_65;
+	} else {
+		ent->port = fwd_entry & ARLTBL_DATA_PORT_ID_MASK;
+		ent->is_valid = !!(fwd_entry & ARLTBL_VALID);
+		ent->is_age = !!(fwd_entry & ARLTBL_AGE);
+		ent->is_static = !!(fwd_entry & ARLTBL_STATIC);
+		u64_to_ether_addr(mac_vid, ent->mac);
+		ent->vid = mac_vid >> ARLTBL_VID_S;
+	}
 }
 
-static inline void b53_arl_from_entry(u64 *mac_vid, u32 *fwd_entry,
+static inline void b53_arl_from_entry(struct b53_device *dev,
+				      u64 *mac_vid, u32 *fwd_entry,
 				      const struct b53_arl_entry *ent)
 {
 	*mac_vid = ether_addr_to_u64(ent->mac);
-	*mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK) << ARLTBL_VID_S;
-	*fwd_entry = ent->port & ARLTBL_DATA_PORT_ID_MASK;
-	if (ent->is_valid)
-		*fwd_entry |= ARLTBL_VALID;
-	if (ent->is_static)
-		*fwd_entry |= ARLTBL_STATIC;
-	if (ent->is_age)
-		*fwd_entry |= ARLTBL_AGE;
+	if (is5325(dev) || is5365(dev)) {
+		*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)
+			*mac_vid |= ARLTBL_VALID_25;
+		if (ent->is_static)
+			*mac_vid |= ARLTBL_STATIC_25;
+		if (ent->is_age)
+			*mac_vid |= ARLTBL_AGE_25;
+	} else {
+		*mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK) << ARLTBL_VID_S;
+		*fwd_entry = ent->port & ARLTBL_DATA_PORT_ID_MASK;
+		if (ent->is_valid)
+			*fwd_entry |= ARLTBL_VALID;
+		if (ent->is_static)
+			*fwd_entry |= ARLTBL_STATIC;
+		if (ent->is_age)
+			*fwd_entry |= ARLTBL_AGE;
+	}
 }
 
 #ifdef CONFIG_BCM47XX
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 1fbc5a204bc7..1f15332fb2a7 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -324,9 +324,10 @@
 #define   ARLTBL_VID_MASK		0xfff
 #define   ARLTBL_DATA_PORT_ID_S_25	48
 #define   ARLTBL_DATA_PORT_ID_MASK_25	0xf
-#define   ARLTBL_AGE_25			BIT(61)
-#define   ARLTBL_STATIC_25		BIT(62)
-#define   ARLTBL_VALID_25		BIT(63)
+#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)
 
 /* ARL Table Data Entry N Registers (32 bit) */
 #define B53_ARLTBL_DATA_ENTRY(n)	((0x10 * (n)) + 0x18)
-- 
2.39.5

[RFC PATCH 02/10] net: dsa: b53: prevent FAST_AGE access on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement FAST_AGE registers so we should avoid reading or
writing them.

Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 03c1e2e75061..d1249aac6136 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -486,6 +486,9 @@ static int b53_flush_arl(struct b53_device *dev, u8 mask)
 {
 	unsigned int i;
 
+	if (is5325(dev))
+		return 0;
+
 	b53_write8(dev, B53_CTRL_PAGE, B53_FAST_AGE_CTRL,
 		   FAST_AGE_DONE | FAST_AGE_DYNAMIC | mask);
 
@@ -510,6 +513,9 @@ static int b53_flush_arl(struct b53_device *dev, u8 mask)
 
 static int b53_fast_age_port(struct b53_device *dev, int port)
 {
+	if (is5325(dev))
+		return 0;
+
 	b53_write8(dev, B53_CTRL_PAGE, B53_FAST_AGE_PORT_CTRL, port);
 
 	return b53_flush_arl(dev, FAST_AGE_PORT);
@@ -517,6 +523,9 @@ static int b53_fast_age_port(struct b53_device *dev, int port)
 
 static int b53_fast_age_vlan(struct b53_device *dev, u16 vid)
 {
+	if (is5325(dev))
+		return 0;
+
 	b53_write16(dev, B53_CTRL_PAGE, B53_FAST_AGE_VID_CTRL, vid);
 
 	return b53_flush_arl(dev, FAST_AGE_VLAN);
-- 
2.39.5

[RFC PATCH 03/10] net: dsa: b53: prevent SWITCH_CTRL access on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement SWITCH_CTRL register so we should avoid reading
or writing it.

Fixes: a424f0de6163 ("net: dsa: b53: Include IMP/CPU port in dumb forwarding mode")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index d1249aac6136..f314aeb81643 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -360,11 +360,12 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 
 	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, mgmt);
 
-	/* Include IMP port in dumb forwarding mode
-	 */
-	b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, &mgmt);
-	mgmt |= B53_MII_DUMB_FWDG_EN;
-	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
+	if (!is5325(dev)) {
+		/* Include IMP port in dumb forwarding mode */
+		b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, &mgmt);
+		mgmt |= B53_MII_DUMB_FWDG_EN;
+		b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
+	}
 
 	/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
 	 * frames should be flooded or not.
-- 
2.39.5

[RFC PATCH 04/10] net: dsa: b53: fix IP_MULTICAST_CTRL on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement B53_UC_FWD_EN, B53_MC_FWD_EN or B53_IPMC_FWD_EN.

Fixes: 53568438e381 ("net: dsa: b53: Add support for port_egress_floods callback")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 13 +++++++++----
 drivers/net/dsa/b53/b53_regs.h   |  1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index f314aeb81643..6b2ad82aa95f 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -367,11 +367,16 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 		b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
 	}
 
-	/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
-	 * frames should be flooded or not.
-	 */
 	b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
-	mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
+	if (is5325(dev)) {
+		/* Enable IP multicast address scheme. */
+		mgmt |= B53_IP_MCAST_25;
+	} else {
+		/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
+		 * frames should be flooded or not.
+		 */
+		mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
+	}
 	b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
 }
 
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 1f15332fb2a7..896684d7f594 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -106,6 +106,7 @@
 
 /* IP Multicast control (8 bit) */
 #define B53_IP_MULTICAST_CTRL		0x21
+#define  B53_IP_MCAST_25		BIT(0)
 #define  B53_IPMC_FWD_EN		BIT(1)
 #define  B53_UC_FWD_EN			BIT(6)
 #define  B53_MC_FWD_EN			BIT(7)
-- 
2.39.5

[RFC PATCH 05/10] net: dsa: b53: prevent DIS_LEARNING access on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement DIS_LEARNING register so we should avoid reading
or writing it.

Fixes: f9b3827ee66c ("net: dsa: b53: Support setting learning on port")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 6b2ad82aa95f..9667d4107139 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -593,6 +593,9 @@ static void b53_port_set_learning(struct b53_device *dev, int port,
 {
 	u16 reg;
 
+	if (is5325(dev))
+		return;
+
 	b53_read16(dev, B53_CTRL_PAGE, B53_DIS_LEARNING, &reg);
 	if (learning)
 		reg &= ~BIT(port);
-- 
2.39.5

[RFC PATCH 06/10] net: dsa: b53: prevent BRCM_HDR access on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement BRCM_HDR register so we should avoid reading or
writing it.

Fixes: b409a9efa183 ("net: dsa: b53: Move Broadcom header setup to b53")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 9667d4107139..fd0752100df8 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -730,6 +730,10 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
 		hdr_ctl |= GC_FRM_MGMT_PORT_M;
 	b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, hdr_ctl);
 
+	/* B53_BRCM_HDR not present on BCM5325 */
+	if (is5325(dev))
+		return;
+
 	/* Enable Broadcom tags for IMP port */
 	b53_read8(dev, B53_MGMT_PAGE, B53_BRCM_HDR, &hdr_ctl);
 	if (tag_en)
-- 
2.39.5

[RFC PATCH 07/10] net: dsa: b53: prevent GMII_PORT_OVERRIDE_CTRL access on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement GMII_PORT_OVERRIDE_CTRL register so we should
avoid reading or writing it.
PORT_OVERRIDE_RX_FLOW and PORT_OVERRIDE_TX_FLOW aren't defined on BCM5325
and we should use PORT_OVERRIDE_LP_FLOW_25 instead.

Fixes: 5e004460f874 ("net: dsa: b53: Add helper to set link parameters")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 21 +++++++++++++++++----
 drivers/net/dsa/b53/b53_regs.h   |  1 +
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index fd0752100df8..387e1e7ec749 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1278,6 +1278,8 @@ static void b53_force_link(struct b53_device *dev, int port, int link)
 	if (port == dev->imp_port) {
 		off = B53_PORT_OVERRIDE_CTRL;
 		val = PORT_OVERRIDE_EN;
+	} else if (is5325(dev)) {
+		return;
 	} else {
 		off = B53_GMII_PORT_OVERRIDE_CTRL(port);
 		val = GMII_PO_EN;
@@ -1302,6 +1304,8 @@ static void b53_force_port_config(struct b53_device *dev, int port,
 	if (port == dev->imp_port) {
 		off = B53_PORT_OVERRIDE_CTRL;
 		val = PORT_OVERRIDE_EN;
+	} else if (is5325(dev)) {
+		return;
 	} else {
 		off = B53_GMII_PORT_OVERRIDE_CTRL(port);
 		val = GMII_PO_EN;
@@ -1332,10 +1336,19 @@ static void b53_force_port_config(struct b53_device *dev, int port,
 		return;
 	}
 
-	if (rx_pause)
-		reg |= PORT_OVERRIDE_RX_FLOW;
-	if (tx_pause)
-		reg |= PORT_OVERRIDE_TX_FLOW;
+	if (rx_pause) {
+		if (is5325(dev))
+			reg |= PORT_OVERRIDE_LP_FLOW_25;
+		else
+			reg |= PORT_OVERRIDE_RX_FLOW;
+	}
+
+	if (tx_pause) {
+		if (is5325(dev))
+			reg |= PORT_OVERRIDE_LP_FLOW_25;
+		else
+			reg |= PORT_OVERRIDE_TX_FLOW;
+	}
 
 	b53_write8(dev, B53_CTRL_PAGE, off, reg);
 }
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 896684d7f594..ab15f36a135a 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -95,6 +95,7 @@
 #define   PORT_OVERRIDE_SPEED_10M	(0 << PORT_OVERRIDE_SPEED_S)
 #define   PORT_OVERRIDE_SPEED_100M	(1 << PORT_OVERRIDE_SPEED_S)
 #define   PORT_OVERRIDE_SPEED_1000M	(2 << PORT_OVERRIDE_SPEED_S)
+#define   PORT_OVERRIDE_LP_FLOW_25	BIT(3) /* BCM5325 only */
 #define   PORT_OVERRIDE_RV_MII_25	BIT(4) /* BCM5325 only */
 #define   PORT_OVERRIDE_RX_FLOW		BIT(4)
 #define   PORT_OVERRIDE_TX_FLOW		BIT(5)
-- 
2.39.5

[RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Álvaro Fernández Rojas]

BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
registers.
This has to be handled differently with other pages and registers.

Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 85 +++++++++++++++++++++++++-------
 drivers/net/dsa/b53/b53_regs.h   | 38 ++++++++++++++
 2 files changed, 105 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 387e1e7ec749..d5216ea2c984 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -560,12 +560,36 @@ static void b53_port_set_ucast_flood(struct b53_device *dev, int port,
 {
 	u16 uc;
 
-	b53_read16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, &uc);
-	if (unicast)
-		uc |= BIT(port);
-	else
-		uc &= ~BIT(port);
-	b53_write16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, uc);
+	if (is5325(dev)) {
+		u8 rc;
+
+		if (port == B53_CPU_PORT_25)
+			port = B53_CPU_PORT;
+
+		b53_read16(dev, B53_IEEE_PAGE, B53_IEEE_UCAST_DLF, &uc);
+		if (unicast)
+			uc |= BIT(port) | B53_IEEE_UCAST_DROP_EN;
+		else
+			uc &= ~BIT(port);
+		b53_write16(dev, B53_IEEE_PAGE, B53_IEEE_UCAST_DLF, uc);
+
+		if (port >= B53_CPU_PORT_25)
+			return;
+
+		b53_read8(dev, B53_RATE_CTL_PAGE, B53_RATE_CONTROL(port), &rc);
+		if (unicast)
+			rc |= (RC_DLF_EN | RC_BKT_SIZE_8K | RC_PERCENT_40);
+		else
+			rc &= ~(RC_DLF_EN);
+		b53_write8(dev, B53_RATE_CTL_PAGE, B53_RATE_CONTROL(port), rc);
+	} else {
+		b53_read16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, &uc);
+		if (unicast)
+			uc |= BIT(port);
+		else
+			uc &= ~BIT(port);
+		b53_write16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, uc);
+	}
 }
 
 static void b53_port_set_mcast_flood(struct b53_device *dev, int port,
@@ -573,19 +597,44 @@ static void b53_port_set_mcast_flood(struct b53_device *dev, int port,
 {
 	u16 mc;
 
-	b53_read16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, &mc);
-	if (multicast)
-		mc |= BIT(port);
-	else
-		mc &= ~BIT(port);
-	b53_write16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, mc);
+	if (is5325(dev)) {
+		u8 rc;
 
-	b53_read16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, &mc);
-	if (multicast)
-		mc |= BIT(port);
-	else
-		mc &= ~BIT(port);
-	b53_write16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, mc);
+		if (port == B53_CPU_PORT_25)
+			port = B53_CPU_PORT;
+
+		b53_read16(dev, B53_IEEE_PAGE, B53_IEEE_MCAST_DLF, &mc);
+		if (multicast)
+			mc |= BIT(port) | B53_IEEE_MCAST_DROP_EN;
+		else
+			mc &= ~BIT(port);
+		b53_write16(dev, B53_IEEE_PAGE, B53_IEEE_MCAST_DLF, mc);
+
+		if (port >= B53_CPU_PORT_25)
+			return;
+
+		b53_read8(dev, B53_RATE_CTL_PAGE, B53_RATE_CONTROL(port), &rc);
+		if (multicast)
+			rc |= (RC_BCAST_EN | RC_MCAST_EN | RC_BKT_SIZE_8K |
+			       RC_PERCENT_40);
+		else
+			rc &= ~(RC_BCAST_EN | RC_MCAST_EN);
+		b53_write8(dev, B53_RATE_CTL_PAGE, B53_RATE_CONTROL(port), rc);
+	} else {
+		b53_read16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, &mc);
+		if (multicast)
+			mc |= BIT(port);
+		else
+			mc &= ~BIT(port);
+		b53_write16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, mc);
+
+		b53_read16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, &mc);
+		if (multicast)
+			mc |= BIT(port);
+		else
+			mc &= ~BIT(port);
+		b53_write16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, mc);
+	}
 }
 
 static void b53_port_set_learning(struct b53_device *dev, int port,
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index ab15f36a135a..b0a7ba3d9b65 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -29,6 +29,7 @@
 #define B53_ARLIO_PAGE			0x05 /* ARL Access */
 #define B53_FRAMEBUF_PAGE		0x06 /* Management frame access */
 #define B53_MEM_ACCESS_PAGE		0x08 /* Memory access */
+#define B53_IEEE_PAGE			0x0a /* IEEE 802.1X */
 
 /* PHY Registers */
 #define B53_PORT_MII_PAGE(i)		(0x10 + (i)) /* Port i MII Registers */
@@ -47,6 +48,9 @@
 /* VLAN Registers */
 #define B53_VLAN_PAGE			0x34
 
+/* Rate Control Registers */
+#define B53_RATE_CTL_PAGE		0x35
+
 /* Jumbo Frame Registers */
 #define B53_JUMBO_PAGE			0x40
 
@@ -368,6 +372,18 @@
 #define B53_ARL_SRCH_RSTL_MACVID(x)	(B53_ARL_SRCH_RSTL_0_MACVID + ((x) * 0x10))
 #define B53_ARL_SRCH_RSTL(x)		(B53_ARL_SRCH_RSTL_0 + ((x) * 0x10))
 
+/*************************************************************************
+ * IEEE 802.1X Registers
+ *************************************************************************/
+
+/* Multicast DLF Drop Control register (16 bit) */
+#define B53_IEEE_MCAST_DLF		0x94
+#define B53_IEEE_MCAST_DROP_EN		BIT(11)
+
+/* Unicast DLF Drop Control register (16 bit) */
+#define B53_IEEE_UCAST_DLF		0x96
+#define B53_IEEE_UCAST_DROP_EN		BIT(11)
+
 /*************************************************************************
  * Port VLAN Registers
  *************************************************************************/
@@ -478,6 +494,28 @@
 /* VLAN Port Default Tag (16 bit) */
 #define B53_VLAN_PORT_DEF_TAG(i)	(0x10 + 2 * (i))
 
+/*************************************************************************
+ * Rate Control Page Registers
+ *************************************************************************/
+
+#define B53_RATE_CONTROL(i)		(0x00 + (i))
+#define   RC_PERCENT_S			0
+#define   RC_PERCENT_10			(0 << RC_PERCENT_S)
+#define   RC_PERCENT_20			(1 << RC_PERCENT_S)
+#define   RC_PERCENT_30			(2 << RC_PERCENT_S)
+#define   RC_PERCENT_40			(3 << RC_PERCENT_S)
+#define   RC_PERCENT_MASK		(3 << RC_PERCENT_S)
+#define   RC_BKT_SIZE_S			2
+#define   RC_BKT_SIZE_2K		(0 << RC_BKT_SIZE_S)
+#define   RC_BKT_SIZE_4K		(1 << RC_BKT_SIZE_S)
+#define   RC_BKT_SIZE_6K		(2 << RC_BKT_SIZE_S)
+#define   RC_BKT_SIZE_8K		(3 << RC_BKT_SIZE_S)
+#define   RC_BKT_SIZE_MASK		(3 << RC_BKT_SIZE_S)
+#define   RC_DLF_EN			BIT(4)
+#define   RC_BCAST_EN			BIT(5)
+#define   RC_MCAST_EN			BIT(6)
+#define   RC_DROP_FRAME			BIT(7)
+
 /*************************************************************************
  * Jumbo Frame Page Registers
  *************************************************************************/
-- 
2.39.5

[RFC PATCH 09/10] net: dsa: b53: fix b53_imp_vlan_setup for BCM5325

[Álvaro Fernández Rojas]

CPU port should be B53_CPU_PORT instead of B53_CPU_PORT_25 for
B53_PVLAN_PORT_MASK register.

Fixes: ff39c2d68679 ("net: dsa: b53: Add bridge support")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index d5216ea2c984..802020eaea44 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -543,6 +543,10 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 	unsigned int i;
 	u16 pvlan;
 
+	/* BCM5325 CPU port is at 8 */
+	if ((is5325(dev) || is5365(dev)) && cpu_port == B53_CPU_PORT_25)
+		cpu_port = B53_CPU_PORT;
+
 	/* Enable the IMP port to be in the same VLAN as the other ports
 	 * on a per-port basis such that we only have Port i and IMP in
 	 * the same VLAN.
-- 
2.39.5

[RFC PATCH 10/10] net: dsa: b53: ensure BCM5325 PHYs are enabled

[Álvaro Fernández Rojas]

According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
disable clocking to individual PHYs.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 802020eaea44..7b786d0d14cd 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1299,6 +1299,9 @@ static int b53_setup(struct dsa_switch *ds)
 
 	b53_reset_mib(dev);
 
+	if (is5325(dev))
+		b53_write8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, 0);
+
 	ret = b53_apply_config(dev);
 	if (ret) {
 		dev_err(ds->dev, "failed to apply configuration\n");
-- 
2.39.5

Re: [RFC PATCH 02/10] net: dsa: b53: prevent FAST_AGE access on BCM5325

[Vladimir Oltean]

Hello,

On Sat, May 31, 2025 at 12:13:00PM +0200, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement FAST_AGE registers so we should avoid reading or
> writing them.
> 
> Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---

How about implementing a "slow age" procedure instead? Walk through the
FDB, and delete the dynamically learned entries for the port?

Address aging is important for STP state transitions.

Re: [RFC PATCH 05/10] net: dsa: b53: prevent DIS_LEARNING access on BCM5325

[Vladimir Oltean]

On Sat, May 31, 2025 at 12:13:03PM +0200, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement DIS_LEARNING register so we should avoid reading
> or writing it.
> 
> Fixes: f9b3827ee66c ("net: dsa: b53: Support setting learning on port")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 6b2ad82aa95f..9667d4107139 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -593,6 +593,9 @@ static void b53_port_set_learning(struct b53_device *dev, int port,
>  {
>  	u16 reg;
>  
> +	if (is5325(dev))
> +		return;
> +
>  	b53_read16(dev, B53_CTRL_PAGE, B53_DIS_LEARNING, &reg);
>  	if (learning)
>  		reg &= ~BIT(port);
> -- 
> 2.39.5
> 

I think if this switch family has that limitation, you should also patch
b53_br_flags_pre() to not permit BR_LEARNING in flags.val, and return
-EINVAL in that case. Otherwise, the "ip link set swp0 type bridge_slave
learning off" command will go through.

Re: [RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Vladimir Oltean]

On Sat, May 31, 2025 at 12:13:06PM +0200, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
> registers.
> This has to be handled differently with other pages and registers.
> 
> Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 85 +++++++++++++++++++++++++-------
>  drivers/net/dsa/b53/b53_regs.h   | 38 ++++++++++++++
>  2 files changed, 105 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 387e1e7ec749..d5216ea2c984 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -560,12 +560,36 @@ static void b53_port_set_ucast_flood(struct b53_device *dev, int port,
>  {
>  	u16 uc;
>  
> -	b53_read16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, &uc);
> -	if (unicast)
> -		uc |= BIT(port);
> -	else
> -		uc &= ~BIT(port);
> -	b53_write16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, uc);
> +	if (is5325(dev)) {

Maybe instead of a big "if (is5325(dev)) else", you could have this instead?

	if (is5325(dev))
		return b5325_port_set_ucast_flood();

	... // go on with regular procedure

Here and in other places.

Re: [RFC PATCH 10/10] net: dsa: b53: ensure BCM5325 PHYs are enabled

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
> disable clocking to individual PHYs.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Would this be more natural and power efficient to move to 
b53_port_setup() instead?
-- 
Florian

Re: [RFC PATCH 02/10] net: dsa: b53: prevent FAST_AGE access on BCM5325

[Florian Fainelli]

On 6/2/25 02:37, Vladimir Oltean wrote:
> Hello,
> 
> On Sat, May 31, 2025 at 12:13:00PM +0200, Álvaro Fernández Rojas wrote:
>> BCM5325 doesn't implement FAST_AGE registers so we should avoid reading or
>> writing them.
>>
>> Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
> 
> How about implementing a "slow age" procedure instead? Walk through the
> FDB, and delete the dynamically learned entries for the port?
> 
> Address aging is important for STP state transitions.

That's a good suggestion, I suppose for now this can be b53 specific 
until we encounter another 20 year old switch and then we move that 
logic within the DSA framework?
-- 
Florian

Re: [RFC PATCH 03/10] net: dsa: b53: prevent SWITCH_CTRL access on BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement SWITCH_CTRL register so we should avoid reading
> or writing it.
> 
> Fixes: a424f0de6163 ("net: dsa: b53: Include IMP/CPU port in dumb forwarding mode")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

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

Re: [RFC PATCH 04/10] net: dsa: b53: fix IP_MULTICAST_CTRL on BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement B53_UC_FWD_EN, B53_MC_FWD_EN or B53_IPMC_FWD_EN.
> 
> Fixes: 53568438e381 ("net: dsa: b53: Add support for port_egress_floods callback")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>   drivers/net/dsa/b53/b53_common.c | 13 +++++++++----
>   drivers/net/dsa/b53/b53_regs.h   |  1 +
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index f314aeb81643..6b2ad82aa95f 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -367,11 +367,16 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
>   		b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
>   	}
>   
> -	/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> -	 * frames should be flooded or not.
> -	 */
>   	b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
> -	mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> +	if (is5325(dev)) {
> +		/* Enable IP multicast address scheme. */
> +		mgmt |= B53_IP_MCAST_25;
> +	} else {
> +		/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> +		 * frames should be flooded or not.
> +		 */
> +		mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> +	}
>   	b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);

I don't think B53_IPM_MULTICAST_CTRL is a valid register offset within 
B53_CTRL_PAGE, or elsewhere for that matter, do you have a datasheet 
that says this exists?
-- 
Florian

Re: [RFC PATCH 06/10] net: dsa: b53: prevent BRCM_HDR access on BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement BRCM_HDR register so we should avoid reading or
> writing it.
> 
> Fixes: b409a9efa183 ("net: dsa: b53: Move Broadcom header setup to b53")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

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

Re: [RFC PATCH 07/10] net: dsa: b53: prevent GMII_PORT_OVERRIDE_CTRL access on BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement GMII_PORT_OVERRIDE_CTRL register so we should
> avoid reading or writing it.
> PORT_OVERRIDE_RX_FLOW and PORT_OVERRIDE_TX_FLOW aren't defined on BCM5325
> and we should use PORT_OVERRIDE_LP_FLOW_25 instead.
> 
> Fixes: 5e004460f874 ("net: dsa: b53: Add helper to set link parameters")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

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

Re: [RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
> registers.
> This has to be handled differently with other pages and registers.
> 
> Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---

[snip]

> +/*************************************************************************
> + * IEEE 802.1X Registers
> + *************************************************************************/
> +
> +/* Multicast DLF Drop Control register (16 bit) */
> +#define B53_IEEE_MCAST_DLF		0x94
> +#define B53_IEEE_MCAST_DROP_EN		BIT(11)
> +
> +/* Unicast DLF Drop Control register (16 bit) */
> +#define B53_IEEE_UCAST_DLF		0x96
> +#define B53_IEEE_UCAST_DROP_EN		BIT(11)

Are you positive the 5325 implements all of those registers? They are 
not documented in my databook.
-- 
Florian

Re: [RFC PATCH 09/10] net: dsa: b53: fix b53_imp_vlan_setup for BCM5325

[Florian Fainelli]

On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> CPU port should be B53_CPU_PORT instead of B53_CPU_PORT_25 for
> B53_PVLAN_PORT_MASK register.
> 
> Fixes: ff39c2d68679 ("net: dsa: b53: Add bridge support")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>   drivers/net/dsa/b53/b53_common.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index d5216ea2c984..802020eaea44 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -543,6 +543,10 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
>   	unsigned int i;
>   	u16 pvlan;
>   
> +	/* BCM5325 CPU port is at 8 */
> +	if ((is5325(dev) || is5365(dev)) && cpu_port == B53_CPU_PORT_25)
> +		cpu_port = B53_CPU_PORT;

Don't we get to that point only if we have invalid Device Tree settings? 
In which case wouldn't a WARN_ON() be more adequate?
-- 
Florian

Re: [RFC PATCH 01/10] net: dsa: b53: add support for FDB operations on 5325/5365

[Florian Fainelli]

On 5/31/25 03:12, Álvaro Fernández Rojas wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> 
> BCM5325 and BCM5365 are part of a much older generation of switches which,
> due to their limited number of ports and VLAN entries (up to 256) allowed
> a single 64-bit register to hold a full ARL entry.
> This requires a little bit of massaging when reading, writing and
> converting ARL entries in both directions.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Regarding your error in the cover letter which are mostly -ENOSPC, I 
believe the problem is that b53_arl_read() is still looking for 
ARLTBL_VALID rather than ARLTBL_VALID_25.

Given there is no VID returned upon an ARL ready with the 5325, I don't 
know how to also reconcile that line:

                 if (dev->vlan_enabled &&
                     ((mac_vid >> ARLTBL_VID_S) & ARLTBL_VID_MASK) != vid)
                         continue;


-- 
Florian

Re: [RFC PATCH 04/10] net: dsa: b53: fix IP_MULTICAST_CTRL on BCM5325

[Jonas Gorski]

On Mon, Jun 2, 2025 at 8:06 PM Florian Fainelli
<florian.fainelli@broadcom.com> wrote:
>
> On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > BCM5325 doesn't implement B53_UC_FWD_EN, B53_MC_FWD_EN or B53_IPMC_FWD_EN.
> >
> > Fixes: 53568438e381 ("net: dsa: b53: Add support for port_egress_floods callback")
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >   drivers/net/dsa/b53/b53_common.c | 13 +++++++++----
> >   drivers/net/dsa/b53/b53_regs.h   |  1 +
> >   2 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index f314aeb81643..6b2ad82aa95f 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -367,11 +367,16 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
> >               b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
> >       }
> >
> > -     /* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> > -      * frames should be flooded or not.
> > -      */
> >       b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
> > -     mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> > +     if (is5325(dev)) {
> > +             /* Enable IP multicast address scheme. */
> > +             mgmt |= B53_IP_MCAST_25;
> > +     } else {
> > +             /* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> > +              * frames should be flooded or not.
> > +              */
> > +             mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> > +     }
> >       b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);

Since the only common thing is the register name, maybe it would make
more sense to have the flow here

if (is5325) {
    enable IP_MULTICAST
}  else {
    enable DUMB_FWD_EN
    enable {UC,MC,IPMC}_FWD_EN
}

>
> I don't think B53_IPM_MULTICAST_CTRL is a valid register offset within
> B53_CTRL_PAGE, or elsewhere for that matter, do you have a datasheet
> that says this exists?

5325E-DS14-R, page 83 (or 105 in pdf paging) on the top.

Regards,
Jonas

Re: [RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Jonas Gorski]

On Mon, Jun 2, 2025 at 8:09 PM Florian Fainelli
<florian.fainelli@broadcom.com> wrote:
>
> On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
> > registers.
> > This has to be handled differently with other pages and registers.
> >
> > Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
>
> [snip]
>
> > +/*************************************************************************
> > + * IEEE 802.1X Registers
> > + *************************************************************************/
> > +
> > +/* Multicast DLF Drop Control register (16 bit) */
> > +#define B53_IEEE_MCAST_DLF           0x94
> > +#define B53_IEEE_MCAST_DROP_EN               BIT(11)
> > +
> > +/* Unicast DLF Drop Control register (16 bit) */
> > +#define B53_IEEE_UCAST_DLF           0x96
> > +#define B53_IEEE_UCAST_DROP_EN               BIT(11)
>
> Are you positive the 5325 implements all of those registers? They are
> not documented in my databook.

They are in 5325E-DS14-R pages 112 - 112 (134/135)

That being said, I don't thing we need to touch the MC/BC/DLF rate
control registers when enabling/disabling flooding - these only limit
how much traffic may be UC / MC  on a port, but apart from that they
do not limit flooding. We don't limit this on other switch models
either.

Regards,
Jonas

Re: [RFC PATCH 01/10] net: dsa: b53: add support for FDB operations on 5325/5365

[Jonas Gorski]

On Sat, May 31, 2025 at 12:13 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> From: Florian Fainelli <f.fainelli@gmail.com>
>
> BCM5325 and BCM5365 are part of a much older generation of switches which,
> due to their limited number of ports and VLAN entries (up to 256) allowed
> a single 64-bit register to hold a full ARL entry.
> This requires a little bit of massaging when reading, writing and
> converting ARL entries in both directions.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 60 +++++++++++++++++++++-----------
>  drivers/net/dsa/b53/b53_priv.h   | 57 +++++++++++++++++++++---------
>  drivers/net/dsa/b53/b53_regs.h   |  7 ++--
>  3 files changed, 84 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 132683ed3abe..03c1e2e75061 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1760,9 +1760,11 @@ static int b53_arl_read(struct b53_device *dev, u64 mac,
>
>                 b53_read64(dev, B53_ARLIO_PAGE,
>                            B53_ARLTBL_MAC_VID_ENTRY(i), &mac_vid);
> -               b53_read32(dev, B53_ARLIO_PAGE,
> -                          B53_ARLTBL_DATA_ENTRY(i), &fwd_entry);
> -               b53_arl_to_entry(ent, mac_vid, fwd_entry);
> +
> +               if (!is5325(dev) && !is5365(dev))
> +                       b53_read32(dev, B53_ARLIO_PAGE,
> +                                  B53_ARLTBL_DATA_ENTRY(i), &fwd_entry);
> +               b53_arl_to_entry(dev, ent, mac_vid, fwd_entry);
>
>                 if (!(fwd_entry & ARLTBL_VALID)) {
>                         set_bit(i, free_bins);
> @@ -1795,7 +1797,8 @@ 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);
> -       b53_write16(dev, B53_ARLIO_PAGE, B53_VLAN_ID_IDX, vid);
> +       if (!is5325(dev))
> +               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);
> @@ -1846,12 +1849,14 @@ static int b53_arl_op(struct b53_device *dev, int op, int port,
>         ent.is_static = true;
>         ent.is_age = false;
>         memcpy(ent.mac, addr, ETH_ALEN);
> -       b53_arl_from_entry(&mac_vid, &fwd_entry, &ent);
> +       b53_arl_from_entry(dev, &mac_vid, &fwd_entry, &ent);
>
>         b53_write64(dev, B53_ARLIO_PAGE,
>                     B53_ARLTBL_MAC_VID_ENTRY(idx), mac_vid);
> -       b53_write32(dev, B53_ARLIO_PAGE,
> -                   B53_ARLTBL_DATA_ENTRY(idx), fwd_entry);
> +
> +       if (!is5325(dev) && !is5365(dev))
> +               b53_write32(dev, B53_ARLIO_PAGE,
> +                           B53_ARLTBL_DATA_ENTRY(idx), fwd_entry);
>
>         return b53_arl_rw_op(dev, 0);
>  }
> @@ -1863,12 +1868,6 @@ int b53_fdb_add(struct dsa_switch *ds, int port,
>         struct b53_device *priv = ds->priv;
>         int ret;
>
> -       /* 5325 and 5365 require some more massaging, but could
> -        * be supported eventually
> -        */
> -       if (is5325(priv) || is5365(priv))
> -               return -EOPNOTSUPP;
> -
>         mutex_lock(&priv->arl_mutex);
>         ret = b53_arl_op(priv, 0, port, addr, vid, true);
>         mutex_unlock(&priv->arl_mutex);
> @@ -1895,10 +1894,15 @@ EXPORT_SYMBOL(b53_fdb_del);
>  static int b53_arl_search_wait(struct b53_device *dev)
>  {
>         unsigned int timeout = 1000;
> -       u8 reg;
> +       u8 reg, offset;
> +
> +       if (is5325(dev) || is5365(dev))
> +               offset = B53_ARL_SRCH_CTL_25;
> +       else
> +               offset = B53_ARL_SRCH_CTL;
>
>         do {
> -               b53_read8(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_CTL, &reg);
> +               b53_read8(dev, B53_ARLIO_PAGE, offset, &reg);
>                 if (!(reg & ARL_SRCH_STDN))
>                         return 0;
>
> @@ -1917,11 +1921,19 @@ static void b53_arl_search_rd(struct b53_device *dev, u8 idx,
>         u64 mac_vid;
>         u32 fwd_entry;
>
> -       b53_read64(dev, B53_ARLIO_PAGE,
> -                  B53_ARL_SRCH_RSTL_MACVID(idx), &mac_vid);
> -       b53_read32(dev, B53_ARLIO_PAGE,
> -                  B53_ARL_SRCH_RSTL(idx), &fwd_entry);
> -       b53_arl_to_entry(ent, mac_vid, fwd_entry);
> +       if (is5325(dev)) {
> +               b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_25,
> +                          &mac_vid);
> +       } else if (is5365(dev)) {
> +               b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_0_MACVID_65,
> +                          &mac_vid);
> +       } else {
> +               b53_read64(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL_MACVID(idx),
> +                          &mac_vid);
> +               b53_read32(dev, B53_ARLIO_PAGE, B53_ARL_SRCH_RSTL(idx),
> +                          &fwd_entry);
> +       }
> +       b53_arl_to_entry(dev, ent, mac_vid, fwd_entry);
>  }
>
>  static int b53_fdb_copy(int port, const struct b53_arl_entry *ent,
> @@ -1942,14 +1954,20 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
>         struct b53_device *priv = ds->priv;
>         struct b53_arl_entry results[2];
>         unsigned int count = 0;
> +       u8 offset;
>         int ret;
>         u8 reg;
>
>         mutex_lock(&priv->arl_mutex);
>
> +       if (is5325(priv) || is5365(priv))
> +               offset = B53_ARL_SRCH_CTL_25;
> +       else
> +               offset = B53_ARL_SRCH_CTL;
> +
>         /* Start search operation */
>         reg = ARL_SRCH_STDN;
> -       b53_write8(priv, B53_ARLIO_PAGE, B53_ARL_SRCH_CTL, reg);
> +       b53_write8(priv, offset, B53_ARL_SRCH_CTL, reg);
>
>         do {
>                 ret = b53_arl_search_wait(priv);
> diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> index a5ef7071ba07..05c5b9239bda 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -286,30 +286,55 @@ struct b53_arl_entry {
>         u8 is_static:1;
>  };
>
> -static inline void b53_arl_to_entry(struct b53_arl_entry *ent,
> +static inline void b53_arl_to_entry(struct b53_device *dev,
> +                                   struct b53_arl_entry *ent,
>                                     u64 mac_vid, u32 fwd_entry)
>  {
>         memset(ent, 0, sizeof(*ent));
> -       ent->port = fwd_entry & ARLTBL_DATA_PORT_ID_MASK;
> -       ent->is_valid = !!(fwd_entry & ARLTBL_VALID);
> -       ent->is_age = !!(fwd_entry & ARLTBL_AGE);
> -       ent->is_static = !!(fwd_entry & ARLTBL_STATIC);
> -       u64_to_ether_addr(mac_vid, ent->mac);
> -       ent->vid = mac_vid >> ARLTBL_VID_S;
> +       if (is5325(dev) || is5365(dev)) {
> +               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->vid = mac_vid >> ARLTBL_VID_S_65;
> +       } else {
> +               ent->port = fwd_entry & ARLTBL_DATA_PORT_ID_MASK;
> +               ent->is_valid = !!(fwd_entry & ARLTBL_VALID);
> +               ent->is_age = !!(fwd_entry & ARLTBL_AGE);
> +               ent->is_static = !!(fwd_entry & ARLTBL_STATIC);
> +               u64_to_ether_addr(mac_vid, ent->mac);
> +               ent->vid = mac_vid >> ARLTBL_VID_S;
> +       }
>  }
>
> -static inline void b53_arl_from_entry(u64 *mac_vid, u32 *fwd_entry,
> +static inline void b53_arl_from_entry(struct b53_device *dev,
> +                                     u64 *mac_vid, u32 *fwd_entry,
>                                       const struct b53_arl_entry *ent)
>  {
>         *mac_vid = ether_addr_to_u64(ent->mac);
> -       *mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK) << ARLTBL_VID_S;
> -       *fwd_entry = ent->port & ARLTBL_DATA_PORT_ID_MASK;
> -       if (ent->is_valid)
> -               *fwd_entry |= ARLTBL_VALID;
> -       if (ent->is_static)
> -               *fwd_entry |= ARLTBL_STATIC;
> -       if (ent->is_age)
> -               *fwd_entry |= ARLTBL_AGE;
> +       if (is5325(dev) || is5365(dev)) {
> +               *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)
> +                       *mac_vid |= ARLTBL_VALID_25;
> +               if (ent->is_static)
> +                       *mac_vid |= ARLTBL_STATIC_25;
> +               if (ent->is_age)
> +                       *mac_vid |= ARLTBL_AGE_25;
> +       } else {
> +               *mac_vid |= (u64)(ent->vid & ARLTBL_VID_MASK) << ARLTBL_VID_S;
> +               *fwd_entry = ent->port & ARLTBL_DATA_PORT_ID_MASK;
> +               if (ent->is_valid)
> +                       *fwd_entry |= ARLTBL_VALID;
> +               if (ent->is_static)
> +                       *fwd_entry |= ARLTBL_STATIC;
> +               if (ent->is_age)
> +                       *fwd_entry |= ARLTBL_AGE;
> +       }
>  }

Looking at the low amount of shared code in all of these changes,
maybe it would make more sense to have separate functions for 5325
instead, e.g. have a  b53_arl_from_entry_25() that doesn't take a
fwd_entry etc.

Regards,
Jonas

Re: [RFC PATCH 00/10] net: dsa: b53: fix BCM5325 support

[Jonas Gorski]

On Sat, May 31, 2025 at 12:13 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> These patches get the BCM5325 switch working with b53.
>
> There are still some sporadic errors related to FDB, but at least the
> switch is working now:
> bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add d6:67:0c:XX:XX:XX vid 1 to fdb: -28
> bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add 5c:4c:a9:XX:XX:XX vid 0 to fdb: -28
> bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to add 5c:4c:a9:XX:XX:XX vid 1 to fdb: -28
> bcm53xx fffe4800.ethernet-mii:1e: port 0 failed to delete d6:67:0c:XX:XX:XX vid 1 from fdb: -2
>
> I'm not really sure that everything here is correct since I don't work for
> Broadcom and all this is based on the public datasheet available for the
> BCM5325 and my own experiments with a Huawei HG556a (BCM6358).

The number of buckets is wrong for 5325, and it actually has only half
of that: "The address table is organized into 512 buckets with two
entries in each bucket."

Might be related, might be not.

Regards,
Jonas

Re: [RFC PATCH 09/10] net: dsa: b53: fix b53_imp_vlan_setup for BCM5325

[Álvaro Fernández Rojas]

Hi Florian,

El lun, 2 jun 2025 a las 20:11, Florian Fainelli
(<florian.fainelli@broadcom.com>) escribió:
>
> On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > CPU port should be B53_CPU_PORT instead of B53_CPU_PORT_25 for
> > B53_PVLAN_PORT_MASK register.
> >
> > Fixes: ff39c2d68679 ("net: dsa: b53: Add bridge support")
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >   drivers/net/dsa/b53/b53_common.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index d5216ea2c984..802020eaea44 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -543,6 +543,10 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
> >       unsigned int i;
> >       u16 pvlan;
> >
> > +     /* BCM5325 CPU port is at 8 */
> > +     if ((is5325(dev) || is5365(dev)) && cpu_port == B53_CPU_PORT_25)
> > +             cpu_port = B53_CPU_PORT;
>
> Don't we get to that point only if we have invalid Device Tree settings?
> In which case wouldn't a WARN_ON() be more adequate?

I just copied the same code that's already present on b53_enable_cpu_port:
https://github.com/torvalds/linux/blob/master/drivers/net/dsa/b53/b53_common.c#L753-L755

I believe that the correct configuration should have the CPU port at
#5, but certain registers expect it at #8:
https://github.com/openwrt/openwrt/blob/cc5421128e44effd5df05227cec4d4c5d05be8dc/target/linux/bmips/dts/bcm6358-huawei-hg556a-b.dts#L155-L204

> --
> Florian

Best regards,
Álvaro.

Re: [RFC PATCH 10/10] net: dsa: b53: ensure BCM5325 PHYs are enabled

[Álvaro Fernández Rojas]

Hi Florian,

El lun, 2 jun 2025 a las 18:00, Florian Fainelli
(<florian.fainelli@broadcom.com>) escribió:
>
> On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
> > disable clocking to individual PHYs.
> >
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>
> Would this be more natural and power efficient to move to
> b53_port_setup() instead?

OK, I will move this to b53_setup_port on v2.

> --
> Florian
>

Best regards,
Álvaro.

Re: [RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Álvaro Fernández Rojas]

Hi Jonas,

El lun, 2 jun 2025 a las 22:08, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> On Mon, Jun 2, 2025 at 8:09 PM Florian Fainelli
> <florian.fainelli@broadcom.com> wrote:
> >
> > On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > > BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
> > > registers.
> > > This has to be handled differently with other pages and registers.
> > >
> > > Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > ---
> >
> > [snip]
> >
> > > +/*************************************************************************
> > > + * IEEE 802.1X Registers
> > > + *************************************************************************/
> > > +
> > > +/* Multicast DLF Drop Control register (16 bit) */
> > > +#define B53_IEEE_MCAST_DLF           0x94
> > > +#define B53_IEEE_MCAST_DROP_EN               BIT(11)
> > > +
> > > +/* Unicast DLF Drop Control register (16 bit) */
> > > +#define B53_IEEE_UCAST_DLF           0x96
> > > +#define B53_IEEE_UCAST_DROP_EN               BIT(11)
> >
> > Are you positive the 5325 implements all of those registers? They are
> > not documented in my databook.
>
> They are in 5325E-DS14-R pages 112 - 112 (134/135)
>
> That being said, I don't thing we need to touch the MC/BC/DLF rate
> control registers when enabling/disabling flooding - these only limit
> how much traffic may be UC / MC  on a port, but apart from that they
> do not limit flooding. We don't limit this on other switch models
> either.

In that case there's nothing to enable/disable on the BCM5325 and we
should do an early return on b53_port_set_ucast_flood and
b53_port_set_mcast_flood since UC_FLOOD_MASK, MC_FLOOD_MASK and
IPMC_FLOOD_MASK don't exist.

>
> Regards,
> Jonas

Best regards,
Álvaro.

Re: [RFC PATCH 02/10] net: dsa: b53: prevent FAST_AGE access on BCM5325

[Vladimir Oltean]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

On Mon, Jun 02, 2025 at 11:01:40AM -0700, Florian Fainelli wrote:
> On 6/2/25 02:37, Vladimir Oltean wrote:
> > Hello,
> > 
> > On Sat, May 31, 2025 at 12:13:00PM +0200, Álvaro Fernández Rojas wrote:
> > > BCM5325 doesn't implement FAST_AGE registers so we should avoid reading or
> > > writing them.
> > > 
> > > Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
> > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > ---
> > 
> > How about implementing a "slow age" procedure instead? Walk through the
> > FDB, and delete the dynamically learned entries for the port?
> > 
> > Address aging is important for STP state transitions.
> 
> That's a good suggestion, I suppose for now this can be b53 specific until
> we encounter another 20 year old switch and then we move that logic within
> the DSA framework?
> -- 
> Florian

Hmm, thank you for saying that, I didn't even consider consolidating the
logic in the DSA framework, but it sure makes sense and we already have
almost all API required to do that.

So I now have a WIP patch attached, but the consolidation into the
framework is definitely a net-next activity, since it will also affect
mt7530, hellcreek and vsc73xx, and those need testing. Also, for b53 it
further requires changing the port_fast_age() prototype.

The question further becomes whether for stable kernels we should
implement a local slow age procedure in b53 like the one from sja1105,
only to delete it in net-next, or to skip it altogether, go with
Álvaro's patch as is and concentrate on the net-next implementation
directly. I'm ok both ways.

[-- Attachment #2: 0001-net-dsa-centralize-the-slow-aging-procedure-from-sja.patch --]
[-- Type: text/x-diff, Size: 7257 bytes --]

From ceb0f0b25ecd3bae6629bbb74fe30f030ddd3a0a Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Tue, 3 Jun 2025 12:40:30 +0300
Subject: [PATCH] net: dsa: centralize the slow aging procedure from sja1105
 (WIP)

With more hardware drivers which are unable to perform a dynamic FDB
flush on a port (see
https://lore.kernel.org/netdev/20250531101308.155757-3-noltari@gmail.com/),
it makes sense to move the sja1105 logic to the framework level, so that
more drivers can flush out dynamically learned entries (relevant when
transitioning to a bridge port STP state incompatible with learning).

The drivers which have .port_fdb_dump() and .port_fdb_del() but not
.port_fast_age() are mt7530, hellcreek, vsc73xx. These will go through
dsa_port_slow_age() now.

TODO: multi-generational drivers like b53 cannot signal that old
hardware cannot do fast ageing and should fall back to
dsa_port_slow_age(), because they offer a single ds->ops->port_fast_age()
to the framework (which returns void), and the framework relies purely
on the presence of the function pointer to determine that the function
is implemented. We should change ds->ops->port_fast_age() to return int,
and treat -EOPNOTSUPP, so as to fall back to slow aging even in that
case.

Also, this change also has squashed the conversion of some function
prototypes from "struct dsa_port *" to "const struct dsa_port *", to be
compatible with the dsa_port_fast_age() caller where dp is a const
pointer. Eventualy, these changes should be split out into a preparatory
change.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 53 --------------------------
 net/dsa/port.c                         | 50 +++++++++++++++++++++---
 net/dsa/port.h                         |  5 ++-
 3 files changed, 48 insertions(+), 60 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index f8454f3b6f9c..77faa43880ed 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1910,58 +1910,6 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void sja1105_fast_age(struct dsa_switch *ds, int port)
-{
-	struct dsa_port *dp = dsa_to_port(ds, port);
-	struct sja1105_private *priv = ds->priv;
-	struct dsa_db db = {
-		.type = DSA_DB_BRIDGE,
-		.bridge = {
-			.dev = dsa_port_bridge_dev_get(dp),
-			.num = dsa_port_bridge_num_get(dp),
-		},
-	};
-	int i;
-
-	mutex_lock(&priv->fdb_lock);
-
-	for (i = 0; i < SJA1105_MAX_L2_LOOKUP_COUNT; i++) {
-		struct sja1105_l2_lookup_entry l2_lookup = {0};
-		u8 macaddr[ETH_ALEN];
-		int rc;
-
-		rc = sja1105_dynamic_config_read(priv, BLK_IDX_L2_LOOKUP,
-						 i, &l2_lookup);
-		/* No fdb entry at i, not an issue */
-		if (rc == -ENOENT)
-			continue;
-		if (rc) {
-			dev_err(ds->dev, "Failed to read FDB: %pe\n",
-				ERR_PTR(rc));
-			break;
-		}
-
-		if (!(l2_lookup.destports & BIT(port)))
-			continue;
-
-		/* Don't delete static FDB entries */
-		if (l2_lookup.lockeds)
-			continue;
-
-		u64_to_ether_addr(l2_lookup.macaddr, macaddr);
-
-		rc = __sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
-		if (rc) {
-			dev_err(ds->dev,
-				"Failed to delete FDB entry %pM vid %lld: %pe\n",
-				macaddr, l2_lookup.vlanid, ERR_PTR(rc));
-			break;
-		}
-	}
-
-	mutex_unlock(&priv->fdb_lock);
-}
-
 static int sja1105_mdb_add(struct dsa_switch *ds, int port,
 			   const struct switchdev_obj_port_mdb *mdb,
 			   struct dsa_db db)
@@ -3222,7 +3170,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.port_fdb_dump		= sja1105_fdb_dump,
 	.port_fdb_add		= sja1105_fdb_add,
 	.port_fdb_del		= sja1105_fdb_del,
-	.port_fast_age		= sja1105_fast_age,
 	.port_bridge_join	= sja1105_bridge_join,
 	.port_bridge_leave	= sja1105_bridge_leave,
 	.port_pre_bridge_flags	= sja1105_port_pre_bridge_flags,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 082573ae6864..2875bda2603f 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -52,14 +52,53 @@ static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
 				 brport_dev, &info.info, NULL);
 }
 
+struct dsa_port_slow_age_ctx {
+	const struct dsa_port *dp;
+};
+
+static int
+dsa_port_slow_age_entry(const unsigned char *addr, u16 vid,
+			bool is_static, void *data)
+{
+	struct dsa_port_slow_age_ctx *ctx = data;
+	const struct dsa_port *dp = ctx->dp;
+
+	if (is_static)
+		return 0;
+
+	dev_dbg(dp->ds->dev,
+		"Flushing dynamic FDB entry %pM vid %u on port %d\n",
+		addr, vid, dp->index);
+
+	return dsa_port_fdb_del(dp, addr, vid);
+}
+
+static int dsa_port_slow_age(const struct dsa_port *dp)
+{
+	struct dsa_port_slow_age_ctx ctx = {
+		.dp = dp,
+	};
+
+	return dsa_port_fdb_dump(dp, dsa_port_slow_age_entry, &ctx);
+}
+
 static void dsa_port_fast_age(const struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
+	int err = 0;
 
-	if (!ds->ops->port_fast_age)
-		return;
+	if (ds->ops->port_fast_age)
+		ds->ops->port_fast_age(ds, dp->index);
+	else
+		err = dsa_port_slow_age(dp);
 
-	ds->ops->port_fast_age(ds, dp->index);
+	if (err && err != -EOPNOTSUPP) {
+		dev_err(ds->dev,
+			"Port %d failed to age dynamic FDB entries: %pe\n",
+			dp->index, ERR_PTR(err));
+	}
+	if (err)
+		return;
 
 	/* flush all VLANs */
 	dsa_port_notify_bridge_fdb_flush(dp, 0);
@@ -996,7 +1035,7 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_ADD, &info);
 }
 
-int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+int dsa_port_fdb_del(const struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid)
 {
 	struct dsa_notifier_fdb_info info = {
@@ -1151,7 +1190,8 @@ int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
 }
 
-int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data)
+int dsa_port_fdb_dump(const struct dsa_port *dp, dsa_fdb_dump_cb_t *cb,
+		      void *data)
 {
 	struct dsa_switch *ds = dp->ds;
 	int port = dp->index;
diff --git a/net/dsa/port.h b/net/dsa/port.h
index 6bc3291573c0..ea20ed6d706e 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -48,7 +48,7 @@ int dsa_port_vlan_msti(struct dsa_port *dp,
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid);
-int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+int dsa_port_fdb_del(const struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid);
 int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
 				     const unsigned char *addr, u16 vid);
@@ -62,7 +62,8 @@ int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 			 u16 vid);
 int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 			 u16 vid);
-int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data);
+int dsa_port_fdb_dump(const struct dsa_port *dp, dsa_fdb_dump_cb_t *cb,
+		      void *data);
 int dsa_port_mdb_add(const struct dsa_port *dp,
 		     const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_mdb_del(const struct dsa_port *dp,
-- 
2.43.0

Re: [RFC PATCH 08/10] net: dsa: b53: fix unicast/multicast flooding on BCM5325

[Jonas Gorski]

On Tue, Jun 3, 2025 at 12:18 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Hi Jonas,
>
> El lun, 2 jun 2025 a las 22:08, Jonas Gorski
> (<jonas.gorski@gmail.com>) escribió:
> >
> > On Mon, Jun 2, 2025 at 8:09 PM Florian Fainelli
> > <florian.fainelli@broadcom.com> wrote:
> > >
> > > On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > > > BCM5325 doesn't implement UC_FLOOD_MASK, MC_FLOOD_MASK and IPMC_FLOOD_MASK
> > > > registers.
> > > > This has to be handled differently with other pages and registers.
> > > >
> > > > Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> > > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > > ---
> > >
> > > [snip]
> > >
> > > > +/*************************************************************************
> > > > + * IEEE 802.1X Registers
> > > > + *************************************************************************/
> > > > +
> > > > +/* Multicast DLF Drop Control register (16 bit) */
> > > > +#define B53_IEEE_MCAST_DLF           0x94
> > > > +#define B53_IEEE_MCAST_DROP_EN               BIT(11)
> > > > +
> > > > +/* Unicast DLF Drop Control register (16 bit) */
> > > > +#define B53_IEEE_UCAST_DLF           0x96
> > > > +#define B53_IEEE_UCAST_DROP_EN               BIT(11)
> > >
> > > Are you positive the 5325 implements all of those registers? They are
> > > not documented in my databook.
> >
> > They are in 5325E-DS14-R pages 112 - 112 (134/135)
> >
> > That being said, I don't thing we need to touch the MC/BC/DLF rate
> > control registers when enabling/disabling flooding - these only limit
> > how much traffic may be UC / MC  on a port, but apart from that they
> > do not limit flooding. We don't limit this on other switch models
> > either.
>
> In that case there's nothing to enable/disable on the BCM5325 and we
> should do an early return on b53_port_set_ucast_flood and
> b53_port_set_mcast_flood since UC_FLOOD_MASK, MC_FLOOD_MASK and
> IPMC_FLOOD_MASK don't exist.

You are adding calls to modify the B53_IEEE_UCAST_DLF and
B53_IEEE_MCAST_DLF registers, which are fine and look correct. And
AFAICT these are the equivalent to UC_FLOOD_MASK and MC_FLOOD_MASK,
with no IPMC_FLOOD_MASK equivalent.

It's only the rate control ones I'm not sure are the right thing to
do, at least in the same patch.

Regards,
Jonas

Re: [RFC PATCH 04/10] net: dsa: b53: fix IP_MULTICAST_CTRL on BCM5325

[Álvaro Fernández Rojas]

Hi Jonas,

El lun, 2 jun 2025 a las 21:59, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> On Mon, Jun 2, 2025 at 8:06 PM Florian Fainelli
> <florian.fainelli@broadcom.com> wrote:
> >
> > On 5/31/25 03:13, Álvaro Fernández Rojas wrote:
> > > BCM5325 doesn't implement B53_UC_FWD_EN, B53_MC_FWD_EN or B53_IPMC_FWD_EN.
> > >
> > > Fixes: 53568438e381 ("net: dsa: b53: Add support for port_egress_floods callback")
> > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > ---
> > >   drivers/net/dsa/b53/b53_common.c | 13 +++++++++----
> > >   drivers/net/dsa/b53/b53_regs.h   |  1 +
> > >   2 files changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > > index f314aeb81643..6b2ad82aa95f 100644
> > > --- a/drivers/net/dsa/b53/b53_common.c
> > > +++ b/drivers/net/dsa/b53/b53_common.c
> > > @@ -367,11 +367,16 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
> > >               b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
> > >       }
> > >
> > > -     /* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> > > -      * frames should be flooded or not.
> > > -      */
> > >       b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
> > > -     mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> > > +     if (is5325(dev)) {
> > > +             /* Enable IP multicast address scheme. */
> > > +             mgmt |= B53_IP_MCAST_25;
> > > +     } else {
> > > +             /* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
> > > +              * frames should be flooded or not.
> > > +              */
> > > +             mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
> > > +     }
> > >       b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
>
> Since the only common thing is the register name, maybe it would make
> more sense to have the flow here
>
> if (is5325) {
>     enable IP_MULTICAST
> }  else {
>     enable DUMB_FWD_EN
>     enable {UC,MC,IPMC}_FWD_EN
> }

OK, then I will do that by merging this patch with #3.

>
> >
> > I don't think B53_IPM_MULTICAST_CTRL is a valid register offset within
> > B53_CTRL_PAGE, or elsewhere for that matter, do you have a datasheet
> > that says this exists?
>
> 5325E-DS14-R, page 83 (or 105 in pdf paging) on the top.
>
> Regards,
> Jonas

Best regards,
Álvaro.