devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node
@ 2022-04-01 10:19 Arınç ÜNAL
  2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 10:19 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree, bcm-kernel-feedback-list,
	Arınç ÜNAL

Remove the unnecessary #address-cells and #size-cells properties on the
nand@0 node to fix the warning below.

Warning (avoid_unnecessary_addr_size): /nand-controller@18028000/nand@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 arch/arm/boot/dts/bcm5301x-nand-cs0.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/bcm5301x-nand-cs0.dtsi b/arch/arm/boot/dts/bcm5301x-nand-cs0.dtsi
index be9a00ff752d..bdf1b4a608e6 100644
--- a/arch/arm/boot/dts/bcm5301x-nand-cs0.dtsi
+++ b/arch/arm/boot/dts/bcm5301x-nand-cs0.dtsi
@@ -10,8 +10,6 @@ nand-controller@18028000 {
 		nandcs: nand@0 {
 			compatible = "brcm,nandcs";
 			reg = <0>;
-			#address-cells = <1>;
-			#size-cells = <1>;
 
 			partitions {
 				compatible = "brcm,bcm947xx-cfe-partitions";
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
@ 2022-04-01 10:19 ` Arınç ÜNAL
  2022-04-01 10:28   ` Rafał Miłecki
  2022-04-04 18:35   ` Florian Fainelli
  2022-04-01 10:20 ` [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch " Arınç ÜNAL
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 10:19 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree, bcm-kernel-feedback-list,
	Arınç ÜNAL

Remove #address-cells and #size-cells properties from the ports node of
&srab. They are already defined on bcm5301x.dtsi, there's no need to define
them again.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
index 82f9629f0abb..cf793c558437 100644
--- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
+++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
 /*
- * Copyright (C) 2021 Arınç ÜNAL <arinc.unal@arinc9.com>
+ * Copyright (C) 2021-2022 Arınç ÜNAL <arinc.unal@arinc9.com>
  */
 
 /dts-v1/;
@@ -177,9 +177,6 @@ &srab {
 	dsa,member = <0 0>;
 
 	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
 		port@0 {
 			reg = <0>;
 			label = "lan4";
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch on Asus RT-AC88U
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
  2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
@ 2022-04-01 10:20 ` Arınç ÜNAL
  2022-04-04 18:36   ` Florian Fainelli
  2022-04-01 10:20 ` [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM " Arınç ÜNAL
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 10:20 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree, bcm-kernel-feedback-list,
	Arınç ÜNAL

Define phy-mode of the Broadcom switch's port@5 as rgmii. This doesn't seem
to matter but let's explicitly define it since phy-mode as rgmii is defined
on the other side which is port@6 of the Realtek switch.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
index cf793c558437..5696dd5fbaf4 100644
--- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
+++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
@@ -205,6 +205,7 @@ port@4 {
 		sw0_p5: port@5 {
 			reg = <5>;
 			label = "extsw";
+			phy-mode = "rgmii";
 
 			fixed-link {
 				speed = <1000>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM on Asus RT-AC88U
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
  2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
  2022-04-01 10:20 ` [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch " Arınç ÜNAL
@ 2022-04-01 10:20 ` Arınç ÜNAL
  2022-04-01 10:32   ` Rafał Miłecki
  2022-04-04 18:36   ` Florian Fainelli
  2022-04-01 10:20 ` [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 " Arınç ÜNAL
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 10:20 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree, bcm-kernel-feedback-list,
	Arınç ÜNAL

The et1macaddr NVRAM variable contains a MAC address for gmac1 on Asus
RT-AC88U. Add NVMEM cell for it and reference it in the gmac1 node.

The Broadcom GBit BCMA driver will issue the MAC address for gmac{0,1,2}
retrieved from et{0,1,2}mac from SPROM without this but let's explicitly
define it as mac-address on the devicetree.
Refer to drivers/net/ethernet/broadcom/bgmac-bcma.c:147.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
index 5696dd5fbaf4..2f944d1c0330 100644
--- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
+++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
@@ -25,6 +25,9 @@ memory@0 {
 	nvram@1c080000 {
 		compatible = "brcm,nvram";
 		reg = <0x1c080000 0x00180000>;
+
+		et1macaddr: et1macaddr {
+		};
 	};
 
 	leds {
@@ -239,6 +242,11 @@ fixed-link {
 	};
 };
 
+&gmac1 {
+	nvmem-cells = <&et1macaddr>;
+	nvmem-cell-names = "mac-address";
+};
+
 &usb2 {
 	vcc-gpio = <&chipcommon 9 GPIO_ACTIVE_HIGH>;
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
                   ` (2 preceding siblings ...)
  2022-04-01 10:20 ` [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM " Arınç ÜNAL
@ 2022-04-01 10:20 ` Arınç ÜNAL
  2022-04-01 10:40   ` Rafał Miłecki
  2022-04-01 10:27 ` [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Rafał Miłecki
  2022-04-04 18:35 ` Florian Fainelli
  5 siblings, 1 reply; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 10:20 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree, bcm-kernel-feedback-list,
	Arınç ÜNAL

Disable gmac0 and gmac2 which are currently not used. This doesn't seem to
be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but this
change is harmless, nonetheless.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
index 2f944d1c0330..0f5c5d576814 100644
--- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
+++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
@@ -242,11 +242,19 @@ fixed-link {
 	};
 };
 
+&gmac0 {
+	status = "disabled";
+};
+
 &gmac1 {
 	nvmem-cells = <&et1macaddr>;
 	nvmem-cell-names = "mac-address";
 };
 
+&gmac2 {
+	status = "disabled";
+};
+
 &usb2 {
 	vcc-gpio = <&chipcommon 9 GPIO_ACTIVE_HIGH>;
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
                   ` (3 preceding siblings ...)
  2022-04-01 10:20 ` [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 " Arınç ÜNAL
@ 2022-04-01 10:27 ` Rafał Miłecki
  2022-04-04 18:35 ` Florian Fainelli
  5 siblings, 0 replies; 16+ messages in thread
From: Rafał Miłecki @ 2022-04-01 10:27 UTC (permalink / raw)
  To: Arınç ÜNAL
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 2022-04-01 12:19, Arınç ÜNAL wrote:
> Remove the unnecessary #address-cells and #size-cells properties on the
> nand@0 node to fix the warning below.
> 
> Warning (avoid_unnecessary_addr_size):
> /nand-controller@18028000/nand@0: unnecessary
> #address-cells/#size-cells without "ranges" or child "reg" property
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Acked-by: Rafał Miłecki <rafal@milecki.pl>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U
  2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
@ 2022-04-01 10:28   ` Rafał Miłecki
  2022-04-04 18:35   ` Florian Fainelli
  1 sibling, 0 replies; 16+ messages in thread
From: Rafał Miłecki @ 2022-04-01 10:28 UTC (permalink / raw)
  To: Arınç ÜNAL
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 2022-04-01 12:19, Arınç ÜNAL wrote:
> Remove #address-cells and #size-cells properties from the ports node of
> &srab. They are already defined on bcm5301x.dtsi, there's no need to 
> define
> them again.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Acked-by: Rafał Miłecki <rafal@milecki.pl>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM on Asus RT-AC88U
  2022-04-01 10:20 ` [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM " Arınç ÜNAL
@ 2022-04-01 10:32   ` Rafał Miłecki
  2022-04-04 18:36   ` Florian Fainelli
  1 sibling, 0 replies; 16+ messages in thread
From: Rafał Miłecki @ 2022-04-01 10:32 UTC (permalink / raw)
  To: Arınç ÜNAL
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 2022-04-01 12:20, Arınç ÜNAL wrote:
> The et1macaddr NVRAM variable contains a MAC address for gmac1 on Asus
> RT-AC88U. Add NVMEM cell for it and reference it in the gmac1 node.
> 
> The Broadcom GBit BCMA driver will issue the MAC address for 
> gmac{0,1,2}
> retrieved from et{0,1,2}mac from SPROM without this but let's 
> explicitly
> define it as mac-address on the devicetree.
> Refer to drivers/net/ethernet/broadcom/bgmac-bcma.c:147.

It doesn't matter how Linux handles that in details. You're working on
hardware binding.

Change is OK of course.

If you need to reference sth it should be
Documentation/devicetree/bindings/net/ethernet-controller.yaml


> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>

I'd drop bgmac-bcma.c reference but nevertheless:

Acked-by: Rafał Miłecki <rafal@milecki.pl>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U
  2022-04-01 10:20 ` [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 " Arınç ÜNAL
@ 2022-04-01 10:40   ` Rafał Miłecki
  2022-04-01 11:36     ` Arınç ÜNAL
  0 siblings, 1 reply; 16+ messages in thread
From: Rafał Miłecki @ 2022-04-01 10:40 UTC (permalink / raw)
  To: Arınç ÜNAL
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 2022-04-01 12:20, Arınç ÜNAL wrote:
> Disable gmac0 and gmac2 which are currently not used. This doesn't seem 
> to
> be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but 
> this
> change is harmless, nonetheless.

It doesn't matter whether Linux respects that.


> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
> b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
> index 2f944d1c0330..0f5c5d576814 100644
> --- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
> +++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
> @@ -242,11 +242,19 @@ fixed-link {
>  	};
>  };
> 
> +&gmac0 {
> +	status = "disabled";
> +};
> +
>  &gmac1 {
>  	nvmem-cells = <&et1macaddr>;
>  	nvmem-cell-names = "mac-address";
>  };
> 
> +&gmac2 {
> +	status = "disabled";
> +};

I don't think that is correct. Those interfaces are still there and
they are actually connected to switch ports. If you configure your
switch properly you can use them.

Someone may want to use e.g. gmac0 & gmac1 with two sets of ports to
speed up network communication.

I think gmac2 is required if you want to enable FA (flow acceleration /
accelerator) - even though there isn't Linux driver for it yet.

They are not disabled / unpopulated / non functional interfaces.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U
  2022-04-01 10:40   ` Rafał Miłecki
@ 2022-04-01 11:36     ` Arınç ÜNAL
  2022-04-06 18:16       ` Florian Fainelli
  0 siblings, 1 reply; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-01 11:36 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 01/04/2022 13:40, Rafał Miłecki wrote:
> On 2022-04-01 12:20, Arınç ÜNAL wrote:
>> Disable gmac0 and gmac2 which are currently not used. This doesn't 
>> seem to
>> be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but this
>> change is harmless, nonetheless.
> 
> It doesn't matter whether Linux respects that.
> 
> 
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>>  arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>> b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>> index 2f944d1c0330..0f5c5d576814 100644
>> --- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>> +++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>> @@ -242,11 +242,19 @@ fixed-link {
>>      };
>>  };
>>
>> +&gmac0 {
>> +    status = "disabled";
>> +};
>> +
>>  &gmac1 {
>>      nvmem-cells = <&et1macaddr>;
>>      nvmem-cell-names = "mac-address";
>>  };
>>
>> +&gmac2 {
>> +    status = "disabled";
>> +};
> 
> I don't think that is correct. Those interfaces are still there and
> they are actually connected to switch ports. If you configure your
> switch properly you can use them.
> 
> Someone may want to use e.g. gmac0 & gmac1 with two sets of ports to
> speed up network communication.
> 
> I think gmac2 is required if you want to enable FA (flow acceleration /
> accelerator) - even though there isn't Linux driver for it yet.
> 
> They are not disabled / unpopulated / non functional interfaces.

I understand your point. However, while we're not supposed to care 
whether the kernel respects the bindings, don't we also need to make the 
bindings work on the version of the Linux kernel we're submitting the 
bindings to?

With the current way DSA works, only one switch port can be used as a 
CPU port. If we were to remove the status = "disabled" property from 
port@8 which connects to gmac2, it'd break the communication between the 
switch and the CPU on the current Linux kernel.

If a new driver or a feature is introduced, we should update the 
bindings accordingly afterwards.

For this reason, I don't see an issue with explaining the driver side of 
it on the commit log for DT bindings.

DT bindings are not exactly static either. Someone could want to use 
gmac2 instead of gmac1. In that case, I think they should change the 
bindings themselves as it's for their own use.

By the way, gmac0 would be wired to port@5 but since port@5 is wired to 
realtek switch's port@6 instead, it's actually non-functional.

Arınç

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node
  2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
                   ` (4 preceding siblings ...)
  2022-04-01 10:27 ` [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Rafał Miłecki
@ 2022-04-04 18:35 ` Florian Fainelli
  5 siblings, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2022-04-04 18:35 UTC (permalink / raw)
  To: bcm-kernel-feedback-list, Arınç ÜNAL
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree

On Fri,  1 Apr 2022 13:19:58 +0300, Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> Remove the unnecessary #address-cells and #size-cells properties on the
> nand@0 node to fix the warning below.
> 
> Warning (avoid_unnecessary_addr_size): /nand-controller@18028000/nand@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U
  2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
  2022-04-01 10:28   ` Rafał Miłecki
@ 2022-04-04 18:35   ` Florian Fainelli
  1 sibling, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2022-04-04 18:35 UTC (permalink / raw)
  To: bcm-kernel-feedback-list, Arınç ÜNAL
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree

On Fri,  1 Apr 2022 13:19:59 +0300, Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> Remove #address-cells and #size-cells properties from the ports node of
> &srab. They are already defined on bcm5301x.dtsi, there's no need to define
> them again.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch on Asus RT-AC88U
  2022-04-01 10:20 ` [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch " Arınç ÜNAL
@ 2022-04-04 18:36   ` Florian Fainelli
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2022-04-04 18:36 UTC (permalink / raw)
  To: bcm-kernel-feedback-list, Arınç ÜNAL
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree

On Fri,  1 Apr 2022 13:20:00 +0300, Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> Define phy-mode of the Broadcom switch's port@5 as rgmii. This doesn't seem
> to matter but let's explicitly define it since phy-mode as rgmii is defined
> on the other side which is port@6 of the Realtek switch.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM on Asus RT-AC88U
  2022-04-01 10:20 ` [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM " Arınç ÜNAL
  2022-04-01 10:32   ` Rafał Miłecki
@ 2022-04-04 18:36   ` Florian Fainelli
  1 sibling, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2022-04-04 18:36 UTC (permalink / raw)
  To: bcm-kernel-feedback-list, Arınç ÜNAL
  Cc: Rafał Miłecki, Hauke Mehrtens, Rob Herring,
	linux-arm-kernel, devicetree

On Fri,  1 Apr 2022 13:20:01 +0300, Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> The et1macaddr NVRAM variable contains a MAC address for gmac1 on Asus
> RT-AC88U. Add NVMEM cell for it and reference it in the gmac1 node.
> 
> The Broadcom GBit BCMA driver will issue the MAC address for gmac{0,1,2}
> retrieved from et{0,1,2}mac from SPROM without this but let's explicitly
> define it as mac-address on the devicetree.
> Refer to drivers/net/ethernet/broadcom/bgmac-bcma.c:147.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U
  2022-04-01 11:36     ` Arınç ÜNAL
@ 2022-04-06 18:16       ` Florian Fainelli
  2022-04-10  9:20         ` Arınç ÜNAL
  0 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2022-04-06 18:16 UTC (permalink / raw)
  To: Arınç ÜNAL, Rafał Miłecki
  Cc: Florian Fainelli, Hauke Mehrtens, Rob Herring, linux-arm-kernel,
	devicetree, bcm-kernel-feedback-list

On 4/1/22 04:36, Arınç ÜNAL wrote:
> On 01/04/2022 13:40, Rafał Miłecki wrote:
>> On 2022-04-01 12:20, Arınç ÜNAL wrote:
>>> Disable gmac0 and gmac2 which are currently not used. This doesn't 
>>> seem to
>>> be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but 
>>> this
>>> change is harmless, nonetheless.
>>
>> It doesn't matter whether Linux respects that.
>>
>>
>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>> ---
>>>  arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>> b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>> index 2f944d1c0330..0f5c5d576814 100644
>>> --- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>> +++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>> @@ -242,11 +242,19 @@ fixed-link {
>>>      };
>>>  };
>>>
>>> +&gmac0 {
>>> +    status = "disabled";
>>> +};
>>> +
>>>  &gmac1 {
>>>      nvmem-cells = <&et1macaddr>;
>>>      nvmem-cell-names = "mac-address";
>>>  };
>>>
>>> +&gmac2 {
>>> +    status = "disabled";
>>> +};
>>
>> I don't think that is correct. Those interfaces are still there and
>> they are actually connected to switch ports. If you configure your
>> switch properly you can use them.
>>
>> Someone may want to use e.g. gmac0 & gmac1 with two sets of ports to
>> speed up network communication.
>>
>> I think gmac2 is required if you want to enable FA (flow acceleration /
>> accelerator) - even though there isn't Linux driver for it yet.
>>
>> They are not disabled / unpopulated / non functional interfaces.
> 
> I understand your point. However, while we're not supposed to care 
> whether the kernel respects the bindings, don't we also need to make the 
> bindings work on the version of the Linux kernel we're submitting the 
> bindings to?
> 
> With the current way DSA works, only one switch port can be used as a 
> CPU port. If we were to remove the status = "disabled" property from 
> port@8 which connects to gmac2, it'd break the communication between the 
> switch and the CPU on the current Linux kernel.

Are you sure? Because DSA still picks up the CPU port from lowest port 
to highest port number. If port 6 is enabled, then it should take 
precedence.

> 
> If a new driver or a feature is introduced, we should update the 
> bindings accordingly afterwards.
> 
> For this reason, I don't see an issue with explaining the driver side of 
> it on the commit log for DT bindings.
> 
> DT bindings are not exactly static either. Someone could want to use 
> gmac2 instead of gmac1. In that case, I think they should change the 
> bindings themselves as it's for their own use.

This is true, but we are not changing the binding here, the binding is 
the contract between the DTB provider and the DTB consumer, it describes 
properties, their shape and size etc. We are just changing the status of 
particular nodes.

> 
> By the way, gmac0 would be wired to port@5 but since port@5 is wired to 
> realtek switch's port@6 instead, it's actually non-functional.



> 
> Arınç


-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U
  2022-04-06 18:16       ` Florian Fainelli
@ 2022-04-10  9:20         ` Arınç ÜNAL
  0 siblings, 0 replies; 16+ messages in thread
From: Arınç ÜNAL @ 2022-04-10  9:20 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki
  Cc: Hauke Mehrtens, Rob Herring, linux-arm-kernel, devicetree,
	bcm-kernel-feedback-list

On 06/04/2022 21:16, Florian Fainelli wrote:
> On 4/1/22 04:36, Arınç ÜNAL wrote:
>> On 01/04/2022 13:40, Rafał Miłecki wrote:
>>> On 2022-04-01 12:20, Arınç ÜNAL wrote:
>>>> Disable gmac0 and gmac2 which are currently not used. This doesn't 
>>>> seem to
>>>> be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but 
>>>> this
>>>> change is harmless, nonetheless.
>>>
>>> It doesn't matter whether Linux respects that.
>>>
>>>
>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>> ---
>>>>  arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> index 2f944d1c0330..0f5c5d576814 100644
>>>> --- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> +++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> @@ -242,11 +242,19 @@ fixed-link {
>>>>      };
>>>>  };
>>>>
>>>> +&gmac0 {
>>>> +    status = "disabled";
>>>> +};
>>>> +
>>>>  &gmac1 {
>>>>      nvmem-cells = <&et1macaddr>;
>>>>      nvmem-cell-names = "mac-address";
>>>>  };
>>>>
>>>> +&gmac2 {
>>>> +    status = "disabled";
>>>> +};
>>>
>>> I don't think that is correct. Those interfaces are still there and
>>> they are actually connected to switch ports. If you configure your
>>> switch properly you can use them.
>>>
>>> Someone may want to use e.g. gmac0 & gmac1 with two sets of ports to
>>> speed up network communication.
>>>
>>> I think gmac2 is required if you want to enable FA (flow acceleration /
>>> accelerator) - even though there isn't Linux driver for it yet.
>>>
>>> They are not disabled / unpopulated / non functional interfaces.
>>
>> I understand your point. However, while we're not supposed to care 
>> whether the kernel respects the bindings, don't we also need to make 
>> the bindings work on the version of the Linux kernel we're submitting 
>> the bindings to?
>>
>> With the current way DSA works, only one switch port can be used as a 
>> CPU port. If we were to remove the status = "disabled" property from 
>> port@8 which connects to gmac2, it'd break the communication between 
>> the switch and the CPU on the current Linux kernel.
> 
> Are you sure? Because DSA still picks up the CPU port from lowest port 
> to highest port number. If port 6 is enabled, then it should take 
> precedence.

I just tested this on the router. You're right. We can keep port@8 
enabled. I also made sure gmac0 is not connected to any switch GMAC. 
I'll send a patch to address these.

> 
>>
>> If a new driver or a feature is introduced, we should update the 
>> bindings accordingly afterwards.
>>
>> For this reason, I don't see an issue with explaining the driver side 
>> of it on the commit log for DT bindings.
>>
>> DT bindings are not exactly static either. Someone could want to use 
>> gmac2 instead of gmac1. In that case, I think they should change the 
>> bindings themselves as it's for their own use.
> 
> This is true, but we are not changing the binding here, the binding is 
> the contract between the DTB provider and the DTB consumer, it describes 
> properties, their shape and size etc. We are just changing the status of 
> particular nodes.

Understood, thank you.

By the way, I don't see my applied patches on the link you referred to.

Arınç

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-04-10  9:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 10:19 [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Arınç ÜNAL
2022-04-01 10:19 ` [PATCH 2/5] ARM: dts: BCM5301X: Remove cell properties from srab ports on Asus RT-AC88U Arınç ÜNAL
2022-04-01 10:28   ` Rafał Miłecki
2022-04-04 18:35   ` Florian Fainelli
2022-04-01 10:20 ` [PATCH 3/5] ARM: dts: BCM5301X: Add rgmii to port@5 of Broadcom switch " Arınç ÜNAL
2022-04-04 18:36   ` Florian Fainelli
2022-04-01 10:20 ` [PATCH 4/5] ARM: dts: BCM5301X: Retrieve gmac1 MAC address from NVRAM " Arınç ÜNAL
2022-04-01 10:32   ` Rafał Miłecki
2022-04-04 18:36   ` Florian Fainelli
2022-04-01 10:20 ` [PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 " Arınç ÜNAL
2022-04-01 10:40   ` Rafał Miłecki
2022-04-01 11:36     ` Arınç ÜNAL
2022-04-06 18:16       ` Florian Fainelli
2022-04-10  9:20         ` Arınç ÜNAL
2022-04-01 10:27 ` [PATCH 1/5] ARM: dts: BCM5301X: Fix DTC warning for NAND node Rafał Miłecki
2022-04-04 18:35 ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).