[PATCH net-next v3 0/3] net: dsa: b53: Turn on Broadcom tags

[PATCH net-next v3 0/3] net: dsa: b53: Turn on Broadcom tags

[Florian Fainelli]

Hi all,

This was long overdue, with this patch series, the b53 driver now
turns on Broadcom tags except for 5325 and 5365 which use an older
format that we do not support yet (TBD).

First patch is necessary in order for bgmac, used on BCM5301X and Northstar
Plus to work correctly and successfully send ARP packets back to the requsester.

Second patch is actually a bug fix, but because net/master and net-next/master
diverge in that area, I am targeting net-next/master here.

Finally, the last patch enables Broadcom tags after checking that the CPU port
selected is either, 5, 7 or 8, since those are the only valid combinations
given currently supported HW.

Changes in v3:

- guarded padding with netdev_uses_dsa() to let the non-DSA use cases
  not have a performance hit for smaller packets

- added missing select NET_DSA_TAG_BRCM to drivers/net/dsa/b53/Kconfig

Changes in v2:

- moved a hunk between patch 2 and patch 3 to avoid a bisectability issue

Florian Fainelli (3):
  net: bgmac: Pad packets to a minimum size
  net: dsa: b53: Stop using dev->cpu_port incorrectly
  net: dsa: b53: Turn on Broadcom tags

 drivers/net/dsa/b53/Kconfig           |  1 +
 drivers/net/dsa/b53/b53_common.c      | 58 ++++++++++++++++++++++++++---------
 drivers/net/ethernet/broadcom/bgmac.c | 16 ++++++++++
 3 files changed, 60 insertions(+), 15 deletions(-)

-- 
2.9.3

[PATCH net-next v3 1/3] net: bgmac: Pad packets to a minimum size

[Florian Fainelli]

In preparation for enabling Broadcom tags with b53, pad packets to a
minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
accept ingressing frames. Without this, we would typically be able to
DHCP, but not resolve with ARP because packets are too small and get
rejected by the switch.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 48d672b204a4..1d96cd594ade 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -15,6 +15,7 @@
 #include <linux/bcm47xx_nvram.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
+#include <net/dsa.h>
 #include "bgmac.h"
 
 static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask,
@@ -127,6 +128,8 @@ bgmac_dma_tx_add_buf(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	dma_desc->ctl1 = cpu_to_le32(ctl1);
 }
 
+#define ENET_BRCM_TAG_LEN	4
+
 static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 				    struct bgmac_dma_ring *ring,
 				    struct sk_buff *skb)
@@ -139,6 +142,18 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	u32 flags;
 	int i;
 
+	/* The Ethernet switch we are interfaced with needs packets to be at
+	 * least 64 bytes (including FCS) otherwise they will be discarded when
+	 * they enter the switch port logic. When Broadcom tags are enabled, we
+	 * need to make sure that packets are at least 68 bytes
+	 * (including FCS and tag) because the length verification is done after
+	 * the Broadcom tag is stripped off the ingress packet.
+	 */
+	if (netdev_uses_dsa(net_dev)) {
+		if (skb_put_padto(skb, ETH_ZLEN + ENET_BRCM_TAG_LEN))
+			goto err_stats;
+	}
+
 	if (skb->len > BGMAC_DESC_CTL1_LEN) {
 		netdev_err(bgmac->net_dev, "Too long skb (%d)\n", skb->len);
 		goto err_drop;
@@ -225,6 +240,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 
 err_drop:
 	dev_kfree_skb(skb);
+err_stats:
 	net_dev->stats.tx_dropped++;
 	net_dev->stats.tx_errors++;
 	return NETDEV_TX_OK;
-- 
2.9.3

[PATCH net-next v3 2/3] net: dsa: b53: Stop using dev->cpu_port incorrectly

[Florian Fainelli]

dev->cpu_port is the driver local information that should only be used
to look up register offsets for a particular port, when they differ
(e.g: IMP port override), but it should certainly not be used in place
of the DSA configured CPU port.

Since the DSA switch layer calls port_vlan_{add,del}() on the CPU port
as well, we can remove the specific setting of the CPU port within
port_vlan_{add,del}.

Fixes: ff39c2d68679 ("net: dsa: b53: Add bridge support")
Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a7ca62ba27b7..17f12484ce24 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -505,7 +505,7 @@ EXPORT_SYMBOL(b53_imp_vlan_setup);
 int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 {
 	struct b53_device *dev = ds->priv;
-	unsigned int cpu_port = dev->cpu_port;
+	unsigned int cpu_port = ds->ports[port].cpu_dp->index;
 	u16 pvlan;
 
 	/* Clear the Rx and Tx disable bits and set to no spanning tree */
@@ -1054,7 +1054,6 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
 	struct b53_device *dev = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
-	unsigned int cpu_port = dev->cpu_port;
 	struct b53_vlan *vl;
 	u16 vid;
 
@@ -1063,12 +1062,11 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
 
 		b53_get_vlan_entry(dev, vid, vl);
 
-		vl->members |= BIT(port) | BIT(cpu_port);
+		vl->members |= BIT(port);
 		if (untagged)
 			vl->untag |= BIT(port);
 		else
 			vl->untag &= ~BIT(port);
-		vl->untag &= ~BIT(cpu_port);
 
 		b53_set_vlan_entry(dev, vid, vl);
 		b53_fast_age_vlan(dev, vid);
@@ -1432,8 +1430,8 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 		b53_write16(dev, B53_VLAN_PAGE, B53_JOIN_ALL_VLAN_EN, reg);
 	} else {
 		b53_get_vlan_entry(dev, pvid, vl);
-		vl->members |= BIT(port) | BIT(dev->cpu_port);
-		vl->untag |= BIT(port) | BIT(dev->cpu_port);
+		vl->members |= BIT(port) | BIT(cpu_port);
+		vl->untag |= BIT(port) | BIT(cpu_port);
 		b53_set_vlan_entry(dev, pvid, vl);
 	}
 }
-- 
2.9.3

[PATCH net-next v3 3/3] net: dsa: b53: Turn on Broadcom tags

[Florian Fainelli]

Enable Broadcom tags for b53 devices, except 5325 and 5365 which use a
different Broadcom tag format not yet supported by net/dsa/tag_brcm.c.

We also make sure that we can turn on Broadcom tags on a CPU port number
that is capable of that: 5, 7 or 8.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/Kconfig      |  1 +
 drivers/net/dsa/b53/b53_common.c | 48 ++++++++++++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/b53/Kconfig b/drivers/net/dsa/b53/Kconfig
index 27f32a50df57..b413d100c6b3 100644
--- a/drivers/net/dsa/b53/Kconfig
+++ b/drivers/net/dsa/b53/Kconfig
@@ -1,6 +1,7 @@
 menuconfig B53
 	tristate "Broadcom BCM53xx managed switch support"
 	depends on NET_DSA
+	select NET_DSA_TAG_BRCM
 	help
 	  This driver adds support for Broadcom managed switch chips. It supports
 	  BCM5325E, BCM5365, BCM539x, BCM53115 and BCM53125 as well as BCM63XX
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 17f12484ce24..44a9a03bff55 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -325,7 +325,6 @@ static void b53_get_vlan_entry(struct b53_device *dev, u16 vid,
 
 static void b53_set_forwarding(struct b53_device *dev, int enable)
 {
-	struct dsa_switch *ds = dev->ds;
 	u8 mgmt;
 
 	b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, &mgmt);
@@ -337,14 +336,11 @@ 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 when no tagging protocol is
-	 * set
+	/* Include IMP port in dumb forwarding mode
 	 */
-	if (ds->ops->get_tag_protocol(ds) == DSA_TAG_PROTO_NONE) {
-		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);
-	}
+	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);
 }
 
 static void b53_enable_vlan(struct b53_device *dev, bool enable)
@@ -612,6 +608,8 @@ static void b53_enable_cpu_port(struct b53_device *dev, int port)
 		    PORT_CTRL_RX_MCST_EN |
 		    PORT_CTRL_RX_UCST_EN;
 	b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), port_ctrl);
+
+	b53_brcm_hdr_setup(dev->ds, port);
 }
 
 static void b53_enable_mib(struct b53_device *dev)
@@ -1480,9 +1478,41 @@ void b53_br_fast_age(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_br_fast_age);
 
+static bool b53_can_enable_brcm_tags(struct dsa_switch *ds)
+{
+	unsigned int brcm_tag_mask;
+	unsigned int i;
+
+	/* Broadcom switches will accept enabling Broadcom tags on the
+	 * following ports: 5, 7 and 8, any other port is not supported
+	 */
+	brcm_tag_mask = BIT(B53_CPU_PORT_25) | BIT(7) | BIT(B53_CPU_PORT);
+
+	for (i = 0; i < ds->num_ports; i++) {
+		if (dsa_is_cpu_port(ds, i)) {
+			if (!(BIT(i) & brcm_tag_mask)) {
+				dev_warn(ds->dev,
+					 "Port %d is not Broadcom tag capable\n",
+					 i);
+				return false;
+			}
+		}
+	}
+
+	return true;
+}
+
 static enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds)
 {
-	return DSA_TAG_PROTO_NONE;
+	struct b53_device *dev = ds->priv;
+
+	/* Older models support a different tag format that we do not
+	 * support in net/dsa/tag_brcm.c yet.
+	 */
+	if (is5325(dev) || is5365(dev) || !b53_can_enable_brcm_tags(ds))
+		return DSA_TAG_PROTO_NONE;
+	else
+		return DSA_TAG_PROTO_BRCM;
 }
 
 int b53_mirror_add(struct dsa_switch *ds, int port,
-- 
2.9.3

Re: [PATCH net-next v3 1/3] net: bgmac: Pad packets to a minimum size

[Andrew Lunn]

On Fri, Nov 10, 2017 at 11:33:25AM -0800, Florian Fainelli wrote:
> In preparation for enabling Broadcom tags with b53, pad packets to a
> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
> accept ingressing frames. Without this, we would typically be able to
> DHCP, but not resolve with ARP because packets are too small and get
> rejected by the switch.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v3 2/3] net: dsa: b53: Stop using dev->cpu_port incorrectly

[Andrew Lunn]

On Fri, Nov 10, 2017 at 11:33:26AM -0800, Florian Fainelli wrote:
> dev->cpu_port is the driver local information that should only be used
> to look up register offsets for a particular port, when they differ
> (e.g: IMP port override), but it should certainly not be used in place
> of the DSA configured CPU port.
> 
> Since the DSA switch layer calls port_vlan_{add,del}() on the CPU port
> as well, we can remove the specific setting of the CPU port within
> port_vlan_{add,del}.
> 
> Fixes: ff39c2d68679 ("net: dsa: b53: Add bridge support")
> Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v3 3/3] net: dsa: b53: Turn on Broadcom tags

[Andrew Lunn]

On Fri, Nov 10, 2017 at 11:33:27AM -0800, Florian Fainelli wrote:
> Enable Broadcom tags for b53 devices, except 5325 and 5365 which use a
> different Broadcom tag format not yet supported by net/dsa/tag_brcm.c.
> 
> We also make sure that we can turn on Broadcom tags on a CPU port number
> that is capable of that: 5, 7 or 8.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v3 0/3] net: dsa: b53: Turn on Broadcom tags

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 10 Nov 2017 11:33:24 -0800

> Hi all,
> 
> This was long overdue, with this patch series, the b53 driver now
> turns on Broadcom tags except for 5325 and 5365 which use an older
> format that we do not support yet (TBD).
> 
> First patch is necessary in order for bgmac, used on BCM5301X and Northstar
> Plus to work correctly and successfully send ARP packets back to the requsester.
> 
> Second patch is actually a bug fix, but because net/master and net-next/master
> diverge in that area, I am targeting net-next/master here.
> 
> Finally, the last patch enables Broadcom tags after checking that the CPU port
> selected is either, 5, 7 or 8, since those are the only valid combinations
> given currently supported HW.
 ...

Series applied.