[PATCH V2 net 1/2] b53: deal with Northstar devices using CPU port 5 or 7

[PATCH V2 net 1/2] b53: deal with Northstar devices using CPU port 5 or 7

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Northstar devices have 3 CPU ports. Ports 5 and 7 may be used but have
their limitations. Warn user when CPU port 8 isn't used and add EAP
setup workaround to avoid non-functional standalone ports.

Fixes: 4227ea91e265 ("net: dsa: b53: prevent standalone from trying to forward to other ports")
Cc: Semih Baskan <strst.gs@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Tested-by: Semih Baskan <strst.gs@gmail.com>
---
V2: Print warning on switch setup instead of on actual EAP mode change

I added PATCH 2/2 as requested but I'm not sure what DT maintainers will
say about it. I'd suggest to take this fix if there are not more
objections on net changes so regression can be fixed. I believe we can
take more time to improve Linux dealing with switch to be more optimal.

 drivers/net/dsa/b53/b53_common.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 3f5b9592794d..a9bc90b2ffd2 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -332,6 +332,14 @@ static void b53_set_eap_mode(struct b53_device *dev, int port, int mode)
 {
 	u64 eap_conf;
 
+	/*
+	 * Setting EAP_MODE_SIMPLIFIED with port 5/7 used as CPU port breaks
+	 * standalone ports, see:
+	 * https://lore.kernel.org/netdev/ce4d9b7b-aaf6-4796-94fb-8c3d6a1dcd4d@gmail.com/
+	 */
+	if (is5301x(dev) && !dsa_is_cpu_port(dev->ds, B53_CPU_PORT))
+		return;
+
 	if (is5325(dev) || is5365(dev) || dev->chip_id == BCM5389_DEVICE_ID)
 		return;
 
@@ -1281,6 +1289,18 @@ static int b53_setup(struct dsa_switch *ds)
 	u16 pvid;
 	int ret;
 
+	/*
+	 * Northstar SoCs devices have 3 Ethernet controllers and 3 CPU ports:
+	 * 5 (IMP1), 7, 8 (IMP0). This design was meant for dual IMP setups when
+	 * WAN traffic goes to port 5 and LAN traffic goes to port 8.
+	 *
+	 * While all 3 ports support Broadcom header, ports 5 and 7 have their
+	 * limitations. Features that relay on trapping to CPU (e.g. EAP, STP)
+	 * won't work when using those as CPU ones.
+	 */
+	if (is5301x(dev) && !dsa_is_cpu_port(dev->ds, B53_CPU_PORT))
+		dev_warn(dev->dev, "switch to CPU port 8 to use all switch features\n");
+
 	/* Request bridge PVID untagged when DSA_TAG_PROTO_NONE is set
 	 * which forces the CPU port to be tagged in all VLANs.
 	 */
-- 
2.51.0

[PATCH V2 RFC 2/2] ARM: dts: BCM5301X: change Luxul XWR-3150 CPU port from 5 do 8

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Northstar devices have 3 CPU ports (each connected to a different
Ethernet interface). This design was meant for dual IMP setups when
WAN traffic goes to port 5 and LAN traffic goes to port 8.

For practical reasons we label only one of those ports as "cpu" (the
rest remains disabled). It's because of Linux behaviour (and it's
probably a bad thing since DT shouldn't "care" that that).

So far the choice of "cpu" port was based on how vendor decided to setup
given device originally: which of three Ethernet interfaces got MAC
assigned and which port was used by vendor original firmware.

Most vendors decided to use CPU port 5 & relevant Ethernet interface.
The problem is that only switch port 8 provides full functionality.

Change the choice of "cpu" port to 8 and make the third Ethernet
interface (connected to port 8) use MAC designed for the first Ethernet
interface (connected to port 5). This makes Linux use switch in a
feature full way.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This seems to be a slight abuse of DT. Instead of describing hardware we
make it steer Linux into using a more appropriate switch port. This
isn't strictly a setback on its own. It's a change from one choice to
another.

I'm not sure about that MAC however. We say that the third Ethernet
interface should use MAC that was designed for the first one.

Is all of that acceptable?

 .../arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts
index 8e487f60a2cc..7969f0f331b5 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts
@@ -81,6 +81,11 @@ &gmac0 {
 	nvmem-cell-names = "mac-address";
 };
 
+&gmac2 {
+	nvmem-cells = <&et0macaddr 0>;
+	nvmem-cell-names = "mac-address";
+};
+
 &pcie_bridge0 {
 	wifi@0,0 {
 		compatible = "brcm,bcm4366-fmac", "brcm,bcm4329-fmac";
@@ -136,7 +141,7 @@ port@4 {
 		};
 
 		port@5 {
-			label = "cpu";
+			status = "disabled";
 		};
 
 		port@7 {
@@ -144,7 +149,7 @@ port@7 {
 		};
 
 		port@8 {
-			status = "disabled";
+			label = "cpu";
 		};
 	};
 };
-- 
2.51.0

Re: [PATCH V2 RFC 2/2] ARM: dts: BCM5301X: change Luxul XWR-3150 CPU port from 5 do 8

[Andrew Lunn]

> Change the choice of "cpu" port to 8 and make the third Ethernet
> interface (connected to port 8) use MAC designed for the first Ethernet
> interface (connected to port 5). This makes Linux use switch in a
> feature full way.

> This seems to be a slight abuse of DT. Instead of describing hardware we
> make it steer Linux into using a more appropriate switch port. This
> isn't strictly a setback on its own. It's a change from one choice to
> another.

It is a bit fuzzy, but the hardware of port 8 is better designed for
the function of being the CPU port. We are expressing this hardware
property via actually using it as the CPU port.

> I'm not sure about that MAC however. We say that the third Ethernet
> interface should use MAC that was designed for the first one.
> 
> Is all of that acceptable?
> 
>  .../arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts | 9 +++++++--

Is this the only device in mainline which is affected?

   Andrew