[PATCH net v2 0/2] net: dsa: Learning fixes for b53/bcm_sf2

[PATCH net v2 0/2] net: dsa: Learning fixes for b53/bcm_sf2

[Florian Fainelli]

Hi David, Jakub,

This patch series contains a couple of fixes for the b53/bcm_sf2 drivers
with respect to configuring learning.

The first patch is wiring-up the necessary dsa_switch_ops operations in
order to support the offloading of bridge flags.

The second patch corrects the switch driver's default learning behavior
which was unfortunately wrong from day one.

This is submitted against "net" because this is technically a bug fix
since ports should not have had learning enabled by default but given
this is dependent upon Vladimir's recent br_flags series, there is no
Fixes tag provided.

I will be providing targeted stable backports that look a bit
difference.

Changes in v2:

- added first patch
- updated second patch to include BR_LEARNING check in br_flags_pre as
  a support bridge flag to offload

Florian Fainelli (2):
  net: dsa: bcm_sf2: Wire-up br_flags_pre, br_flags and set_mrouter
  net: dsa: b53: Support setting learning on port

 drivers/net/dsa/b53/b53_common.c | 39 ++++++++++++++++++++++++--------
 drivers/net/dsa/b53/b53_priv.h   |  8 +++++++
 drivers/net/dsa/b53/b53_regs.h   |  1 +
 drivers/net/dsa/bcm_sf2.c        | 18 ++++-----------
 4 files changed, 43 insertions(+), 23 deletions(-)

-- 
2.25.1

[PATCH net v2 1/2] net: dsa: bcm_sf2: Wire-up br_flags_pre, br_flags and set_mrouter

[Florian Fainelli]

Because bcm_sf2 implements its own dsa_switch_ops we need to export the
b53_br_flags_pre(), b53_br_flags() and b53_set_mrouter so we can wire-up
them up like they used to be with the former b53_br_egress_floods().

Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 19 +++++++++++--------
 drivers/net/dsa/b53/b53_priv.h   |  8 ++++++++
 drivers/net/dsa/bcm_sf2.c        |  3 +++
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ae86ded1e2a1..fceca3f5b6a5 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1953,19 +1953,20 @@ void b53_br_fast_age(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_br_fast_age);
 
-static int b53_br_flags_pre(struct dsa_switch *ds, int port,
-			    struct switchdev_brport_flags flags,
-			    struct netlink_ext_ack *extack)
+int b53_br_flags_pre(struct dsa_switch *ds, int port,
+		     struct switchdev_brport_flags flags,
+		     struct netlink_ext_ack *extack)
 {
 	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
 		return -EINVAL;
 
 	return 0;
 }
+EXPORT_SYMBOL(b53_br_flags_pre);
 
-static int b53_br_flags(struct dsa_switch *ds, int port,
-			struct switchdev_brport_flags flags,
-			struct netlink_ext_ack *extack)
+int b53_br_flags(struct dsa_switch *ds, int port,
+		 struct switchdev_brport_flags flags,
+		 struct netlink_ext_ack *extack)
 {
 	if (flags.mask & BR_FLOOD)
 		b53_port_set_ucast_flood(ds->priv, port,
@@ -1976,14 +1977,16 @@ static int b53_br_flags(struct dsa_switch *ds, int port,
 
 	return 0;
 }
+EXPORT_SYMBOL(b53_br_flags);
 
-static int b53_set_mrouter(struct dsa_switch *ds, int port, bool mrouter,
-			   struct netlink_ext_ack *extack)
+int b53_set_mrouter(struct dsa_switch *ds, int port, bool mrouter,
+		    struct netlink_ext_ack *extack)
 {
 	b53_port_set_mcast_flood(ds->priv, port, mrouter);
 
 	return 0;
 }
+EXPORT_SYMBOL(b53_set_mrouter);
 
 static bool b53_possible_cpu_port(struct dsa_switch *ds, int port)
 {
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index faf983fbca82..8419bb7f4505 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -326,6 +326,14 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
 void b53_br_fast_age(struct dsa_switch *ds, int port);
+int b53_br_flags_pre(struct dsa_switch *ds, int port,
+		     struct switchdev_brport_flags flags,
+		     struct netlink_ext_ack *extack);
+int b53_br_flags(struct dsa_switch *ds, int port,
+		 struct switchdev_brport_flags flags,
+		 struct netlink_ext_ack *extack);
+int b53_set_mrouter(struct dsa_switch *ds, int port, bool mrouter,
+		    struct netlink_ext_ack *extack);
 int b53_setup_devlink_resources(struct dsa_switch *ds);
 void b53_port_event(struct dsa_switch *ds, int port);
 void b53_phylink_validate(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 1857aa9aa84a..3eaedbb12815 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1117,7 +1117,10 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
 	.set_mac_eee		= b53_set_mac_eee,
 	.port_bridge_join	= b53_br_join,
 	.port_bridge_leave	= b53_br_leave,
+	.port_pre_bridge_flags	= b53_br_flags_pre,
+	.port_bridge_flags	= b53_br_flags,
 	.port_stp_state_set	= b53_br_set_stp_state,
+	.port_set_mrouter	= b53_set_mrouter,
 	.port_fast_age		= b53_br_fast_age,
 	.port_vlan_filtering	= b53_vlan_filtering,
 	.port_vlan_add		= b53_vlan_add,
-- 
2.25.1

Re: [PATCH net v2 1/2] net: dsa: bcm_sf2: Wire-up br_flags_pre, br_flags and set_mrouter

[Vladimir Oltean]

On Mon, Feb 22, 2021 at 02:30:09PM -0800, Florian Fainelli wrote:
> Because bcm_sf2 implements its own dsa_switch_ops we need to export the
> b53_br_flags_pre(), b53_br_flags() and b53_set_mrouter so we can wire-up
> them up like they used to be with the former b53_br_egress_floods().
> 
> Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---

To be clear, I did not miss migrating .port_egress_floods towards
.port_bridge_flags, I just migrated what existed, and sf2 didn't have
it. So from a blame perspective, this patch should have:

Fixes: 53568438e381 ("net: dsa: b53: Add support for port_egress_floods callback")

However, your fix cannot be backported past the patch you blamed, so in
practice it makes no difference.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[PATCH net v2 2/2] net: dsa: b53: Support setting learning on port

[Florian Fainelli]

Add support for being able to set the learning attribute on port, and
make sure that the standalone ports start up with learning disabled.

We can remove the code in bcm_sf2 that configured the ports learning
attribute because we want the standalone ports to have learning disabled
by default and port 7 cannot be bridged, so its learning attribute will
not change past its initial configuration.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 20 +++++++++++++++++++-
 drivers/net/dsa/b53/b53_regs.h   |  1 +
 drivers/net/dsa/bcm_sf2.c        | 15 +--------------
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index fceca3f5b6a5..a162499bcafc 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -543,6 +543,19 @@ static void b53_port_set_mcast_flood(struct b53_device *dev, int port,
 	b53_write16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, mc);
 }
 
+static void b53_port_set_learning(struct b53_device *dev, int port,
+				  bool learning)
+{
+	u16 reg;
+
+	b53_read16(dev, B53_CTRL_PAGE, B53_DIS_LEARNING, &reg);
+	if (learning)
+		reg &= ~BIT(port);
+	else
+		reg |= BIT(port);
+	b53_write16(dev, B53_CTRL_PAGE, B53_DIS_LEARNING, reg);
+}
+
 int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 {
 	struct b53_device *dev = ds->priv;
@@ -557,6 +570,7 @@ int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 
 	b53_port_set_ucast_flood(dev, port, true);
 	b53_port_set_mcast_flood(dev, port, true);
+	b53_port_set_learning(dev, port, false);
 
 	if (dev->ops->irq_enable)
 		ret = dev->ops->irq_enable(dev, port);
@@ -691,6 +705,7 @@ static void b53_enable_cpu_port(struct b53_device *dev, int port)
 
 	b53_port_set_ucast_flood(dev, port, true);
 	b53_port_set_mcast_flood(dev, port, true);
+	b53_port_set_learning(dev, port, false);
 }
 
 static void b53_enable_mib(struct b53_device *dev)
@@ -1957,7 +1972,7 @@ int b53_br_flags_pre(struct dsa_switch *ds, int port,
 		     struct switchdev_brport_flags flags,
 		     struct netlink_ext_ack *extack)
 {
-	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
+	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD | BR_LEARNING))
 		return -EINVAL;
 
 	return 0;
@@ -1974,6 +1989,9 @@ int b53_br_flags(struct dsa_switch *ds, int port,
 	if (flags.mask & BR_MCAST_FLOOD)
 		b53_port_set_mcast_flood(ds->priv, port,
 					 !!(flags.val & BR_MCAST_FLOOD));
+	if (flags.mask & BR_LEARNING)
+		b53_port_set_learning(ds->priv, port,
+				      !!(flags.val & BR_LEARNING));
 
 	return 0;
 }
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index c90985c294a2..b2c539a42154 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -115,6 +115,7 @@
 #define B53_UC_FLOOD_MASK		0x32
 #define B53_MC_FLOOD_MASK		0x34
 #define B53_IPMC_FLOOD_MASK		0x36
+#define B53_DIS_LEARNING		0x3c
 
 /*
  * Override Ports 0-7 State on devices with xMII interfaces (8 bit)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 3eaedbb12815..5ee8103b8e9c 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -223,23 +223,10 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 	reg &= ~P_TXQ_PSM_VDD(port);
 	core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
 
-	/* Enable learning */
-	reg = core_readl(priv, CORE_DIS_LEARN);
-	reg &= ~BIT(port);
-	core_writel(priv, reg, CORE_DIS_LEARN);
-
 	/* Enable Broadcom tags for that port if requested */
-	if (priv->brcm_tag_mask & BIT(port)) {
+	if (priv->brcm_tag_mask & BIT(port))
 		b53_brcm_hdr_setup(ds, port);
 
-		/* Disable learning on ASP port */
-		if (port == 7) {
-			reg = core_readl(priv, CORE_DIS_LEARN);
-			reg |= BIT(port);
-			core_writel(priv, reg, CORE_DIS_LEARN);
-		}
-	}
-
 	/* Configure Traffic Class to QoS mapping, allow each priority to map
 	 * to a different queue number
 	 */
-- 
2.25.1

Re: [PATCH net v2 2/2] net: dsa: b53: Support setting learning on port

[Vladimir Oltean]

On Mon, Feb 22, 2021 at 02:30:10PM -0800, Florian Fainelli wrote:
> diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
> index c90985c294a2..b2c539a42154 100644
> --- a/drivers/net/dsa/b53/b53_regs.h
> +++ b/drivers/net/dsa/b53/b53_regs.h
> @@ -115,6 +115,7 @@
>  #define B53_UC_FLOOD_MASK		0x32
>  #define B53_MC_FLOOD_MASK		0x34
>  #define B53_IPMC_FLOOD_MASK		0x36
> +#define B53_DIS_LEARNING		0x3c
>  
>  /*
>   * Override Ports 0-7 State on devices with xMII interfaces (8 bit)
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index 3eaedbb12815..5ee8103b8e9c 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -223,23 +223,10 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
>  	reg &= ~P_TXQ_PSM_VDD(port);
>  	core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
>  
> -	/* Enable learning */
> -	reg = core_readl(priv, CORE_DIS_LEARN);
> -	reg &= ~BIT(port);
> -	core_writel(priv, reg, CORE_DIS_LEARN);
> -
>  	/* Enable Broadcom tags for that port if requested */
> -	if (priv->brcm_tag_mask & BIT(port)) {
> +	if (priv->brcm_tag_mask & BIT(port))
>  		b53_brcm_hdr_setup(ds, port);
>  
> -		/* Disable learning on ASP port */
> -		if (port == 7) {
> -			reg = core_readl(priv, CORE_DIS_LEARN);
> -			reg |= BIT(port);
> -			core_writel(priv, reg, CORE_DIS_LEARN);
> -		}
> -	}
> -

In sf2, CORE_DIS_LEARN is at address 0xf0, while in b53, B53_DIS_LEARN
is at 0x3c. Are they even configuring the same thing?

Re: [PATCH net v2 2/2] net: dsa: b53: Support setting learning on port

[Florian Fainelli]

On 2/22/2021 3:18 PM, Vladimir Oltean wrote:
> On Mon, Feb 22, 2021 at 02:30:10PM -0800, Florian Fainelli wrote:
>> diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
>> index c90985c294a2..b2c539a42154 100644
>> --- a/drivers/net/dsa/b53/b53_regs.h
>> +++ b/drivers/net/dsa/b53/b53_regs.h
>> @@ -115,6 +115,7 @@
>>  #define B53_UC_FLOOD_MASK		0x32
>>  #define B53_MC_FLOOD_MASK		0x34
>>  #define B53_IPMC_FLOOD_MASK		0x36
>> +#define B53_DIS_LEARNING		0x3c
>>  
>>  /*
>>   * Override Ports 0-7 State on devices with xMII interfaces (8 bit)
>> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
>> index 3eaedbb12815..5ee8103b8e9c 100644
>> --- a/drivers/net/dsa/bcm_sf2.c
>> +++ b/drivers/net/dsa/bcm_sf2.c
>> @@ -223,23 +223,10 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
>>  	reg &= ~P_TXQ_PSM_VDD(port);
>>  	core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
>>  
>> -	/* Enable learning */
>> -	reg = core_readl(priv, CORE_DIS_LEARN);
>> -	reg &= ~BIT(port);
>> -	core_writel(priv, reg, CORE_DIS_LEARN);
>> -
>>  	/* Enable Broadcom tags for that port if requested */
>> -	if (priv->brcm_tag_mask & BIT(port)) {
>> +	if (priv->brcm_tag_mask & BIT(port))
>>  		b53_brcm_hdr_setup(ds, port);
>>  
>> -		/* Disable learning on ASP port */
>> -		if (port == 7) {
>> -			reg = core_readl(priv, CORE_DIS_LEARN);
>> -			reg |= BIT(port);
>> -			core_writel(priv, reg, CORE_DIS_LEARN);
>> -		}
>> -	}
>> -
> 
> In sf2, CORE_DIS_LEARN is at address 0xf0, while in b53, B53_DIS_LEARN
> is at 0x3c. Are they even configuring the same thing?

They are the SF2 switch was integrated with a bridge that would flatten
its address space such that there would be no need to access the
registers indirectly like what b53_srab does.

This is the reason why we have the SF2_PAGE_REG_MKADDR() macro to
convert from a {page, offset} tuple to a memory mapped address and here
0x3c << 2 = 0xf0.
-- 
Florian

Re: [PATCH net v2 2/2] net: dsa: b53: Support setting learning on port

[Vladimir Oltean]

On Mon, Feb 22, 2021 at 03:44:21PM -0800, Florian Fainelli wrote:
> > In sf2, CORE_DIS_LEARN is at address 0xf0, while in b53, B53_DIS_LEARN
> > is at 0x3c. Are they even configuring the same thing?
> 
> They are the SF2 switch was integrated with a bridge that would flatten
> its address space such that there would be no need to access the
> registers indirectly like what b53_srab does.
> 
> This is the reason why we have the SF2_PAGE_REG_MKADDR() macro to
> convert from a {page, offset} tuple to a memory mapped address and here
> 0x3c << 2 = 0xf0.

Thanks, now I have more context to understand why sf2 uses one kind of
I/O and b53 another, and also how the paged address map managed by the
b53 driver gets linearized before accessing the sf2 registers.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Re: [PATCH net v2 0/2] net: dsa: Learning fixes for b53/bcm_sf2

[Jakub Kicinski]

On Mon, 22 Feb 2021 14:30:08 -0800 Florian Fainelli wrote:
> This patch series contains a couple of fixes for the b53/bcm_sf2 drivers
> with respect to configuring learning.
> 
> The first patch is wiring-up the necessary dsa_switch_ops operations in
> order to support the offloading of bridge flags.
> 
> The second patch corrects the switch driver's default learning behavior
> which was unfortunately wrong from day one.
> 
> This is submitted against "net" because this is technically a bug fix
> since ports should not have had learning enabled by default but given
> this is dependent upon Vladimir's recent br_flags series, there is no
> Fixes tag provided.
> 
> I will be providing targeted stable backports that look a bit
> difference.

Applied, thanks!