Fix Flooding: Disable by default on User ports and Enable on CPU ports

Commit Message

Richard van Schagen Feb. 12, 2023, 9:40 p.m. UTC

---
 drivers/net/dsa/mt7530.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Vladimir Oltean Feb. 13, 2023, 1:48 p.m. UTC | #1

NACK.
- the change needs to be localized to the file it touches by specifying the
  "net: dsa: mt7530: " prefix.
- the rest of the 80-character short commit message needs to be a well
  summarized description of the change.
- any time the "fix", "broken", etc words are used, a description of the
  breakage is expected, and its projected impact upon users of the driver
- patches which solve a bug must have a Fixes: tag pointing to the patch
  that introduced the problem
- if that patch being fixed is in the mainline kernel already, the
  --subject-prefix of the patch must be "PATCH net", otherwise "PATCH
  net-next". These correspond to the net.git and net-next.git trees.
- your signed-off tag is missing
- maybe more problems

Arınç ÜNAL March 8, 2023, 1:34 p.m. UTC | #2

Richard, will you send a new patch series for this and your other two 
patches? They are essential for the port5 <-> gmac1 link to work 
properly. Without them, port5 as a CPU port won't work properly.

Arınç

Russell King (Oracle) March 8, 2023, 1:44 p.m. UTC | #3

On Wed, Mar 08, 2023 at 04:34:11PM +0300, Arınç ÜNAL wrote:
> Richard, will you send a new patch series for this and your other two
> patches? They are essential for the port5 <-> gmac1 link to work properly.
> Without them, port5 as a CPU port won't work properly.

... and maybe detail exactly what the effect of this is.

For example, does it affect multicast frames received on user ports
being multicast to other user ports when they are part of the same
bridge device (which is essential for IPv6 to function.)

Thanks.

Patch

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f98a94361c84..f456541a68bc 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -997,6 +997,7 @@  mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 {
 	struct mt7530_priv *priv = ds->priv;
 	int ret;
+	u32 val;
 
 	/* Setup max capability of CPU port at first */
 	if (priv->info->cpu_port_config) {
@@ -1009,9 +1010,10 @@  mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 	mt7530_write(priv, MT7530_PVC_P(port),
 		     PORT_SPEC_TAG);
 
-	/* Disable flooding by default */
-	mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK,
-		   BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port)));
+	/* Enable flooding on CPU */
+	val = mt7530_read(priv, MT7530_MFC);
+	val |= BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port));
+	mt7530_write(priv, MT7530_MFC, val);
 
 	/* Set CPU port number */
 	if (priv->id == ID_MT7621)
@@ -2155,6 +2157,9 @@  mt7530_setup(struct dsa_switch *ds)
 
 	priv->p6_interface = PHY_INTERFACE_MODE_NA;
 
+	/* Disable flooding by default */
+	mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, 0);
+
 	/* Enable and reset MIB counters */
 	mt7530_mib_reset(ds);
 
@@ -2346,6 +2351,9 @@  mt7531_setup(struct dsa_switch *ds)
 	mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
 		   MT753X_BPDU_CPU_ONLY);
 
+	/* Disable flooding by default */
+	mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, 0);
+
 	/* Enable and reset MIB counters */
 	mt7530_mib_reset(ds);