[PATCH 1/2] ARM: dts: BCM5301X: Explicitly disable unused switch CPU ports

[PATCH 1/2] ARM: dts: BCM5301X: Explicitly disable unused switch CPU ports

[Rafał Miłecki]

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

When redescribing ports I assumed that missing "label" (like "cpu")
means switch port isn't used. That was incorrect and I realized my
change made Linux always use the first (5) CPU port (there are 3 of
them).

While above should technically be possible it often isn't correct:
1. Non-default switch ports are often connected to Ethernet interfaces
   not fully covered by vendor setup (they may miss MACs)
2. On some devices non-default ports require specifying fixed link

This fixes network connectivity for some devices. It was reported &
tested for Netgear R8000. It also affects Linksys EA9200 with its
downstream DTS.

Fixes: ba4aebce23b2 ("ARM: dts: BCM5301X: Describe switch ports in the main DTS")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../dts/broadcom/bcm4708-buffalo-wzr-1166dhp-common.dtsi  | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm4708-luxul-xap-1510.dts     | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm4708-luxul-xwc-1000.dts     | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm4708-netgear-r6250.dts      | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm4708-smartrg-sr400ac.dts    | 8 ++++++++
 .../boot/dts/broadcom/bcm47081-buffalo-wzr-600dhp2.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47081-luxul-xap-1410.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47081-luxul-xwr-1200.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts      | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-885l.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-890l.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-abr-4500.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-xap-1610.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-xbr-4500.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-xwc-2000.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3100.dts    | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3150-v1.dts | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm53015-meraki-mr26.dts       | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm53016-meraki-mr32.dts       | 8 ++++++++
 arch/arm/boot/dts/broadcom/bcm953012er.dts                | 8 ++++++++
 20 files changed, 160 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wzr-1166dhp-common.dtsi b/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wzr-1166dhp-common.dtsi
index 42bcbf10957c..9f9084269ef5 100644
--- a/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wzr-1166dhp-common.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wzr-1166dhp-common.dtsi
@@ -181,5 +181,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm4708-luxul-xap-1510.dts b/arch/arm/boot/dts/broadcom/bcm4708-luxul-xap-1510.dts
index e04d2e5ea51a..72e960c888ac 100644
--- a/arch/arm/boot/dts/broadcom/bcm4708-luxul-xap-1510.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4708-luxul-xap-1510.dts
@@ -85,5 +85,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm4708-luxul-xwc-1000.dts b/arch/arm/boot/dts/broadcom/bcm4708-luxul-xwc-1000.dts
index a399800139d9..750e17482371 100644
--- a/arch/arm/boot/dts/broadcom/bcm4708-luxul-xwc-1000.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4708-luxul-xwc-1000.dts
@@ -88,5 +88,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm4708-netgear-r6250.dts b/arch/arm/boot/dts/broadcom/bcm4708-netgear-r6250.dts
index fad3473810a2..2bdbc7d18b0e 100644
--- a/arch/arm/boot/dts/broadcom/bcm4708-netgear-r6250.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4708-netgear-r6250.dts
@@ -122,5 +122,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm4708-smartrg-sr400ac.dts b/arch/arm/boot/dts/broadcom/bcm4708-smartrg-sr400ac.dts
index 5b2b7b8b3b12..b226bef3369c 100644
--- a/arch/arm/boot/dts/broadcom/bcm4708-smartrg-sr400ac.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4708-smartrg-sr400ac.dts
@@ -145,6 +145,14 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/broadcom/bcm47081-buffalo-wzr-600dhp2.dts b/arch/arm/boot/dts/broadcom/bcm47081-buffalo-wzr-600dhp2.dts
index d0a26b643b82..192b8db5a89c 100644
--- a/arch/arm/boot/dts/broadcom/bcm47081-buffalo-wzr-600dhp2.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47081-buffalo-wzr-600dhp2.dts
@@ -145,5 +145,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47081-luxul-xap-1410.dts b/arch/arm/boot/dts/broadcom/bcm47081-luxul-xap-1410.dts
index 9f21d6d6d35b..0198b5f9e4a7 100644
--- a/arch/arm/boot/dts/broadcom/bcm47081-luxul-xap-1410.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47081-luxul-xap-1410.dts
@@ -81,5 +81,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47081-luxul-xwr-1200.dts b/arch/arm/boot/dts/broadcom/bcm47081-luxul-xwr-1200.dts
index 256107291702..73ff1694a4a0 100644
--- a/arch/arm/boot/dts/broadcom/bcm47081-luxul-xwr-1200.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47081-luxul-xwr-1200.dts
@@ -148,5 +148,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
index 707c561703ed..55fc9f44cbc7 100644
--- a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
@@ -227,6 +227,14 @@ port@4 {
 			label = "wan";
 		};
 
+		port@5 {
+			status = "disabled";
+		};
+
+		port@7 {
+			status = "disabled";
+		};
+
 		port@8 {
 			label = "cpu";
 		};
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-885l.dts b/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-885l.dts
index abe0cb245c7e..c5099defe9f9 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-885l.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-885l.dts
@@ -160,6 +160,14 @@ port@4 {
 			nvmem-cell-names = "mac-address";
 		};
 
+		port@5 {
+			status = "disabled";
+		};
+
+		port@7 {
+			status = "disabled";
+		};
+
 		port@8 {
 			label = "cpu";
 		};
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-890l.dts b/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-890l.dts
index f050acbea0b2..3124dfd01b94 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-890l.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-dlink-dir-890l.dts
@@ -192,6 +192,14 @@ port@4 {
 			label = "wan";
 		};
 
+		port@5 {
+			status = "disabled";
+		};
+
+		port@7 {
+			status = "disabled";
+		};
+
 		port@8 {
 			label = "cpu";
 			phy-mode = "rgmii";
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-abr-4500.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-abr-4500.dts
index e8991d4e248c..e374062eb5b7 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-abr-4500.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-abr-4500.dts
@@ -107,5 +107,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xap-1610.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xap-1610.dts
index afc635c8cdeb..badafa024d24 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xap-1610.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xap-1610.dts
@@ -120,5 +120,13 @@ port@1 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xbr-4500.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xbr-4500.dts
index 7cfa4607ef31..cf95af9db1e6 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xbr-4500.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xbr-4500.dts
@@ -107,5 +107,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwc-2000.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwc-2000.dts
index d55e10095eae..992c19e1cfa1 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwc-2000.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwc-2000.dts
@@ -75,5 +75,13 @@ port@0 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3100.dts b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3100.dts
index ccf031c0e276..4d0ba315a204 100644
--- a/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3100.dts
+++ b/arch/arm/boot/dts/broadcom/bcm47094-luxul-xwr-3100.dts
@@ -147,5 +147,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
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 e28f7a350117..83c429afc297 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
@@ -158,5 +158,13 @@ port@4 {
 		port@5 {
 			label = "cpu";
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/broadcom/bcm53015-meraki-mr26.dts b/arch/arm/boot/dts/broadcom/bcm53015-meraki-mr26.dts
index 03ad614e6b72..0bf5106f7012 100644
--- a/arch/arm/boot/dts/broadcom/bcm53015-meraki-mr26.dts
+++ b/arch/arm/boot/dts/broadcom/bcm53015-meraki-mr26.dts
@@ -124,6 +124,14 @@ fixed-link {
 				full-duplex;
 			};
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/broadcom/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/broadcom/bcm53016-meraki-mr32.dts
index 26c12bfb0bdd..25eeacf6a248 100644
--- a/arch/arm/boot/dts/broadcom/bcm53016-meraki-mr32.dts
+++ b/arch/arm/boot/dts/broadcom/bcm53016-meraki-mr32.dts
@@ -185,6 +185,14 @@ fixed-link {
 				full-duplex;
 			};
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/broadcom/bcm953012er.dts b/arch/arm/boot/dts/broadcom/bcm953012er.dts
index 4fe3b3653376..d939ec9f4a9e 100644
--- a/arch/arm/boot/dts/broadcom/bcm953012er.dts
+++ b/arch/arm/boot/dts/broadcom/bcm953012er.dts
@@ -84,6 +84,14 @@ port@5 {
 			label = "cpu";
 			ethernet = <&gmac0>;
 		};
+
+		port@7 {
+			status = "disabled";
+		};
+
+		port@8 {
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.35.3

[PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Rafał Miłecki]

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

While switch ports 5 and 7 are disabled (vendor designed port 8 to be
used for CPU traffic) they could be used strictly technically. For some
reason however both those ports need forcing link to be usable.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
index 55fc9f44cbc7..127ca8741220 100644
--- a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r8000.dts
@@ -229,10 +229,20 @@ port@4 {
 
 		port@5 {
 			status = "disabled";
+
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
 		};
 
 		port@7 {
 			status = "disabled";
+
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
 		};
 
 		port@8 {
-- 
2.35.3

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Andrew Lunn]

On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
> used for CPU traffic) they could be used strictly technically. For some
> reason however both those ports need forcing link to be usable.

This explanation is not making much sense to me.

I assume this board does not have an RJ45 for these two ports? But
does it have a header so you can access the MII interface?

     Andrew
 

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Rafał Miłecki]

On 2023-10-14 18:50, Andrew Lunn wrote:
> On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
>> used for CPU traffic) they could be used strictly technically. For 
>> some
>> reason however both those ports need forcing link to be usable.
> 
> This explanation is not making much sense to me.
> 
> I assume this board does not have an RJ45 for these two ports? But
> does it have a header so you can access the MII interface?

This PATCH as it is requires a basic familiarity with Northstar platform
or checking bcm-ns.dtsi.

All Northstar (BCM5301X) devices have 3 Ethernet interfaces. 99% of them
have:
1. gmac0 connected to port 5
2. gmac1 connected to port 7
3. gmac2 connected to port 8
(it's described in bcm-ns.dtsi).


Some vendors decide to use gmac0 and switch port 5. They fill NVRAM with
MAC for gmac0.

Some vendors decide to use gmac2 & port 8. They set MAC for gmac2.


Netgear decided to use gmac2 & port 8 for R8000. They fill NVRAM with
MAC for gmac2.

If you however insist on using gmac0 you could do that. That just
requires setting up gmac0 with a custom/random MAC and forcing link for
switch ports as described in this PATCH.


Does it make sense now? Should I reword this commit somehow?

-- 
Rafał Miłecki

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Andrew Lunn]

On Mon, Oct 16, 2023 at 05:36:24PM +0200, Rafał Miłecki wrote:
> On 2023-10-14 18:50, Andrew Lunn wrote:
> > On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > While switch ports 5 and 7 are disabled (vendor designed port 8 to be
> > > used for CPU traffic) they could be used strictly technically. For
> > > some
> > > reason however both those ports need forcing link to be usable.
> > 
> > This explanation is not making much sense to me.
> > 
> > I assume this board does not have an RJ45 for these two ports? But
> > does it have a header so you can access the MII interface?
> 
> This PATCH as it is requires a basic familiarity with Northstar platform
> or checking bcm-ns.dtsi.
> 
> All Northstar (BCM5301X) devices have 3 Ethernet interfaces. 99% of them
> have:
> 1. gmac0 connected to port 5
> 2. gmac1 connected to port 7
> 3. gmac2 connected to port 8
> (it's described in bcm-ns.dtsi).
> 
> 
> Some vendors decide to use gmac0 and switch port 5. They fill NVRAM with
> MAC for gmac0.
> 
> Some vendors decide to use gmac2 & port 8. They set MAC for gmac2.
> 
> 
> Netgear decided to use gmac2 & port 8 for R8000. They fill NVRAM with
> MAC for gmac2.
> 
> If you however insist on using gmac0 you could do that. That just
> requires setting up gmac0 with a custom/random MAC and forcing link for
> switch ports as described in this PATCH.

If the ports are not used, you have them set to disabled, why do they
need a fixed-link? That is what i don't understand yet.

     Andrew

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Florian Fainelli]

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

On 10/16/23 08:45, Andrew Lunn wrote:
> On Mon, Oct 16, 2023 at 05:36:24PM +0200, Rafał Miłecki wrote:
>> On 2023-10-14 18:50, Andrew Lunn wrote:
>>> On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
>>>> used for CPU traffic) they could be used strictly technically. For
>>>> some
>>>> reason however both those ports need forcing link to be usable.
>>>
>>> This explanation is not making much sense to me.
>>>
>>> I assume this board does not have an RJ45 for these two ports? But
>>> does it have a header so you can access the MII interface?
>>
>> This PATCH as it is requires a basic familiarity with Northstar platform
>> or checking bcm-ns.dtsi.
>>
>> All Northstar (BCM5301X) devices have 3 Ethernet interfaces. 99% of them
>> have:
>> 1. gmac0 connected to port 5
>> 2. gmac1 connected to port 7
>> 3. gmac2 connected to port 8
>> (it's described in bcm-ns.dtsi).
>>
>>
>> Some vendors decide to use gmac0 and switch port 5. They fill NVRAM with
>> MAC for gmac0.
>>
>> Some vendors decide to use gmac2 & port 8. They set MAC for gmac2.
>>
>>
>> Netgear decided to use gmac2 & port 8 for R8000. They fill NVRAM with
>> MAC for gmac2.
>>
>> If you however insist on using gmac0 you could do that. That just
>> requires setting up gmac0 with a custom/random MAC and forcing link for
>> switch ports as described in this PATCH.
> 
> If the ports are not used, you have them set to disabled, why do they
> need a fixed-link? That is what i don't understand yet.

It seems to me like the commit message could be reworded such that:

Even though ports 5 and 7 are disabled and the system is intended to use 
port 8, make it possible for users to experiment with using ports 5 
and/or 7 if they desire so by ensuring that they have the necessary 
'fixed-link' properties to describe the internal connection within the 
SoC between the switch ports and the two Ethernet controllers.

Rafal, does that capture the intent? If so I can amend the commit 
message while applying.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 1/2] ARM: dts: BCM5301X: Explicitly disable unused switch CPU ports

[Florian Fainelli]

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

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

On Fri, 13 Oct 2023 12:33:13 +0200, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> When redescribing ports I assumed that missing "label" (like "cpu")
> means switch port isn't used. That was incorrect and I realized my
> change made Linux always use the first (5) CPU port (there are 3 of
> them).
> 
> While above should technically be possible it often isn't correct:
> 1. Non-default switch ports are often connected to Ethernet interfaces
>    not fully covered by vendor setup (they may miss MACs)
> 2. On some devices non-default ports require specifying fixed link
> 
> This fixes network connectivity for some devices. It was reported &
> tested for Netgear R8000. It also affects Linksys EA9200 with its
> downstream DTS.
> 
> Fixes: ba4aebce23b2 ("ARM: dts: BCM5301X: Describe switch ports in the main DTS")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
--
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Rafał Miłecki]

On 2023-10-16 17:45, Andrew Lunn wrote:
> On Mon, Oct 16, 2023 at 05:36:24PM +0200, Rafał Miłecki wrote:
>> On 2023-10-14 18:50, Andrew Lunn wrote:
>> > On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
>> > > From: Rafał Miłecki <rafal@milecki.pl>
>> > >
>> > > While switch ports 5 and 7 are disabled (vendor designed port 8 to be
>> > > used for CPU traffic) they could be used strictly technically. For
>> > > some
>> > > reason however both those ports need forcing link to be usable.
>> >
>> > This explanation is not making much sense to me.
>> >
>> > I assume this board does not have an RJ45 for these two ports? But
>> > does it have a header so you can access the MII interface?
>> 
>> This PATCH as it is requires a basic familiarity with Northstar 
>> platform
>> or checking bcm-ns.dtsi.
>> 
>> All Northstar (BCM5301X) devices have 3 Ethernet interfaces. 99% of 
>> them
>> have:
>> 1. gmac0 connected to port 5
>> 2. gmac1 connected to port 7
>> 3. gmac2 connected to port 8
>> (it's described in bcm-ns.dtsi).
>> 
>> 
>> Some vendors decide to use gmac0 and switch port 5. They fill NVRAM 
>> with
>> MAC for gmac0.
>> 
>> Some vendors decide to use gmac2 & port 8. They set MAC for gmac2.
>> 
>> 
>> Netgear decided to use gmac2 & port 8 for R8000. They fill NVRAM with
>> MAC for gmac2.
>> 
>> If you however insist on using gmac0 you could do that. That just
>> requires setting up gmac0 with a custom/random MAC and forcing link 
>> for
>> switch ports as described in this PATCH.
> 
> If the ports are not used, you have them set to disabled, why do they
> need a fixed-link? That is what i don't understand yet.

So developers/hackers can use them for custom needs by just dropping
"disabled" bit. That's a pretty simple step compared to figuring out
that a fixed link is needed.

I can imagine advanced users using extra ports and interfaces to get
higher speeds. If you use a single switch port and single interface
you're limited to 1 Gbps. By using two you can exceed that limitation.

This is clearly some corner case but I don't think it really violates
what DT is for. We just describe hardware more clearly. There is a fixed
link after all. That port just happens to be disabled.

-- 
Rafał Miłecki

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Rafał Miłecki]

On 2023-10-18 20:10, Florian Fainelli wrote:
> On 10/16/23 08:45, Andrew Lunn wrote:
>> On Mon, Oct 16, 2023 at 05:36:24PM +0200, Rafał Miłecki wrote:
>>> On 2023-10-14 18:50, Andrew Lunn wrote:
>>>> On Fri, Oct 13, 2023 at 12:33:14PM +0200, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>> 
>>>>> While switch ports 5 and 7 are disabled (vendor designed port 8 to 
>>>>> be
>>>>> used for CPU traffic) they could be used strictly technically. For
>>>>> some
>>>>> reason however both those ports need forcing link to be usable.
>>>> 
>>>> This explanation is not making much sense to me.
>>>> 
>>>> I assume this board does not have an RJ45 for these two ports? But
>>>> does it have a header so you can access the MII interface?
>>> 
>>> This PATCH as it is requires a basic familiarity with Northstar 
>>> platform
>>> or checking bcm-ns.dtsi.
>>> 
>>> All Northstar (BCM5301X) devices have 3 Ethernet interfaces. 99% of 
>>> them
>>> have:
>>> 1. gmac0 connected to port 5
>>> 2. gmac1 connected to port 7
>>> 3. gmac2 connected to port 8
>>> (it's described in bcm-ns.dtsi).
>>> 
>>> 
>>> Some vendors decide to use gmac0 and switch port 5. They fill NVRAM 
>>> with
>>> MAC for gmac0.
>>> 
>>> Some vendors decide to use gmac2 & port 8. They set MAC for gmac2.
>>> 
>>> 
>>> Netgear decided to use gmac2 & port 8 for R8000. They fill NVRAM with
>>> MAC for gmac2.
>>> 
>>> If you however insist on using gmac0 you could do that. That just
>>> requires setting up gmac0 with a custom/random MAC and forcing link 
>>> for
>>> switch ports as described in this PATCH.
>> 
>> If the ports are not used, you have them set to disabled, why do they
>> need a fixed-link? That is what i don't understand yet.
> 
> It seems to me like the commit message could be reworded such that:
> 
> Even though ports 5 and 7 are disabled and the system is intended to
> use port 8, make it possible for users to experiment with using ports
> 5 and/or 7 if they desire so by ensuring that they have the necessary
> 'fixed-link' properties to describe the internal connection within the
> SoC between the switch ports and the two Ethernet controllers.
> 
> Rafal, does that capture the intent? If so I can amend the commit
> message while applying.

I believe so. You're correct that in practice it's for experimenting
mainly. Formally it also describes hardware which DT is for.

I'm sure we can find a lot of *disabled* hardware blocks in Linux's DTS
files that are still described for the sake of documenting it.

-- 
Rafał Miłecki

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Andrew Lunn]

> So developers/hackers can use them for custom needs by just dropping
> "disabled" bit. That's a pretty simple step compared to figuring out
> that a fixed link is needed.
> 
> I can imagine advanced users using extra ports and interfaces to get
> higher speeds. If you use a single switch port and single interface
> you're limited to 1 Gbps. By using two you can exceed that limitation.
> 
> This is clearly some corner case but I don't think it really violates
> what DT is for. We just describe hardware more clearly. There is a fixed
> link after all. That port just happens to be disabled.

Please reformulate this into the commit message. Then it becomes a
good answer to the question `Why?` which is what the commit message is
all about.

    Andrew

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Florian Fainelli]

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

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

On Fri, 13 Oct 2023 12:33:14 +0200, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
> used for CPU traffic) they could be used strictly technically. For some
> reason however both those ports need forcing link to be usable.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
--
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Florian Fainelli]

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

On 10/23/23 11:33, Florian Fainelli wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> 
> On Fri, 13 Oct 2023 12:33:14 +0200, Rafał Miłecki <zajec5@gmail.com> wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
>> used for CPU traffic) they could be used strictly technically. For some
>> reason however both those ports need forcing link to be usable.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
> 
> Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!

Ended up with the following commit message:

Ports 5 and 7 are disabled by default because the standard use case is
for port 8 to manage all CPU directed traffic. For experimentation
purposes however it is desirable to provide adequate properties such
that people can experiment with using different ports without having to
figure out their configuration. Some of the use cases include but are
not limited to doubling or tripling the bandwidth by leveraging the
additional ports/Ethernet MAC combinations.


let me know if I should rephrase it before pushing this to the the ARM 
SoC maintainers (tomorrow).
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 2/2] ARM: dts: BCM5301X: Set fixed-link for extra Netgear R8000 CPU ports

[Rafał Miłecki]

On 2023-10-23 20:36, Florian Fainelli wrote:
> On 10/23/23 11:33, Florian Fainelli wrote:
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> 
>> On Fri, 13 Oct 2023 12:33:14 +0200, Rafał Miłecki <zajec5@gmail.com> 
>> wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>> 
>>> While switch ports 5 and 7 are disabled (vendor designed port 8 to be
>>> used for CPU traffic) they could be used strictly technically. For 
>>> some
>>> reason however both those ports need forcing link to be usable.
>>> 
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>> 
>> Applied to 
>> https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
> 
> Ended up with the following commit message:
> 
> Ports 5 and 7 are disabled by default because the standard use case is
> for port 8 to manage all CPU directed traffic. For experimentation
> purposes however it is desirable to provide adequate properties such
> that people can experiment with using different ports without having to
> figure out their configuration. Some of the use cases include but are
> not limited to doubling or tripling the bandwidth by leveraging the
> additional ports/Ethernet MAC combinations.
> 
> 
> let me know if I should rephrase it before pushing this to the the ARM
> SoC maintainers (tomorrow).

Looks good, thank you!

-- 
Rafał Miłecki