Subject: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

From: Arınç ÜNAL <[email protected]>

Add the device tree for ASUS RT-AC3200 which is an AC3200 router featuring
5 Ethernet ports over the integrated Broadcom switch.

Hardware info:
* Processor: Broadcom BCM4709A0 dual-core @ 1.0 GHz
* Switch: BCM53012 in BCM4709A0
* DDR3 RAM: 256 MB
* Flash: 128 MB
* 2.4GHz: BCM43602 3x3 single chip 802.11b/g/n SoC
* 5GHz: BCM43602 3x3 two chips 802.11a/n/ac SoC
* Ports: 4 LAN Ports, 1 WAN Port

Co-developed-by: Tom Brautaset <[email protected]>
Signed-off-by: Tom Brautaset <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
arch/arm/boot/dts/broadcom/Makefile | 1 +
.../boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts | 164 +++++++++++++++++++++
2 files changed, 165 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/Makefile b/arch/arm/boot/dts/broadcom/Makefile
index 7099d9560033..c61fca514775 100644
--- a/arch/arm/boot/dts/broadcom/Makefile
+++ b/arch/arm/boot/dts/broadcom/Makefile
@@ -64,6 +64,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm47081-luxul-xap-1410.dtb \
bcm47081-luxul-xwr-1200.dtb \
bcm47081-tplink-archer-c5-v2.dtb \
+ bcm4709-asus-rt-ac3200.dtb \
bcm4709-asus-rt-ac87u.dtb \
bcm4709-buffalo-wxr-1900dhp.dtb \
bcm4709-linksys-ea9200.dtb \
diff --git a/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
new file mode 100644
index 000000000000..8640dda211ae
--- /dev/null
+++ b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Author: Tom Brautaset <[email protected]>
+ */
+
+/dts-v1/;
+
+#include "bcm4709.dtsi"
+#include "bcm5301x-nand-cs0-bch8.dtsi"
+
+#include <dt-bindings/leds/common.h>
+
+/ {
+ compatible = "asus,rt-ac3200", "brcm,bcm4709", "brcm,bcm4708";
+ model = "ASUS RT-AC3200";
+
+ chosen {
+ bootargs = "console=ttyS0,115200 earlycon";
+ };
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0x00000000 0x08000000>,
+ <0x88000000 0x08000000>;
+ };
+
+ nvram@1c080000 {
+ compatible = "brcm,nvram";
+ reg = <0x1c080000 0x00180000>;
+
+ et0macaddr: et0macaddr {
+ #nvmem-cell-cells = <1>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led-power {
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_POWER;
+ gpios = <&chipcommon 3 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "default-on";
+ };
+
+ led-wan-red {
+ color = <LED_COLOR_ID_RED>;
+ function = LED_FUNCTION_WAN;
+ gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
+ };
+
+ led-wps {
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_WPS;
+ gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ button-reset {
+ label = "Reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&chipcommon 11 GPIO_ACTIVE_LOW>;
+ };
+
+ button-wifi {
+ label = "Wi-Fi";
+ linux,code = <KEY_RFKILL>;
+ gpios = <&chipcommon 4 GPIO_ACTIVE_LOW>;
+ };
+
+ button-wps {
+ label = "WPS";
+ linux,code = <KEY_WPS_BUTTON>;
+ gpios = <&chipcommon 7 GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&usb2 {
+ vcc-gpio = <&chipcommon 9 GPIO_ACTIVE_HIGH>;
+};
+
+&usb3_phy {
+ status = "okay";
+};
+
+&gmac0 {
+ nvmem-cells = <&et0macaddr 0>;
+ nvmem-cell-names = "mac-address";
+};
+
+&gmac1 {
+ nvmem-cells = <&et0macaddr 1>;
+ nvmem-cell-names = "mac-address";
+};
+
+&gmac2 {
+ nvmem-cells = <&et0macaddr 2>;
+ nvmem-cell-names = "mac-address";
+};
+
+&srab {
+ status = "okay";
+
+ ports {
+ port@0 {
+ label = "wan";
+ nvmem-cells = <&et0macaddr 0>;
+ nvmem-cell-names = "mac-address";
+ };
+
+ port@1 {
+ label = "lan1";
+ nvmem-cells = <&et0macaddr 1>;
+ nvmem-cell-names = "mac-address";
+ };
+
+ port@2 {
+ label = "lan2";
+ nvmem-cells = <&et0macaddr 2>;
+ nvmem-cell-names = "mac-address";
+ };
+
+ port@3 {
+ label = "lan3";
+ nvmem-cells = <&et0macaddr 3>;
+ nvmem-cell-names = "mac-address";
+ };
+
+ port@4 {
+ label = "lan4";
+ nvmem-cells = <&et0macaddr 4>;
+ nvmem-cell-names = "mac-address";
+ };
+ };
+};
+
+&nandcs {
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "boot";
+ reg = <0x00000000 0x00080000>;
+ read-only;
+ };
+
+ partition@80000 {
+ label = "nvram";
+ reg = <0x00080000 0x00180000>;
+ };
+
+ partition@200000 {
+ label = "firmware";
+ reg = <0x00200000 0x07e00000>;
+ compatible = "brcm,trx";
+ };
+ };
+};

--
2.40.1




2024-04-14 14:13:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 14/04/2024 13:46, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Add the device tree for ASUS RT-AC3200 which is an AC3200 router featuring
> 5 Ethernet ports over the integrated Broadcom switch.
>
> Hardware info:
> * Processor: Broadcom BCM4709A0 dual-core @ 1.0 GHz
> * Switch: BCM53012 in BCM4709A0
> * DDR3 RAM: 256 MB
> * Flash: 128 MB
> * 2.4GHz: BCM43602 3x3 single chip 802.11b/g/n SoC
> * 5GHz: BCM43602 3x3 two chips 802.11a/n/ac SoC
> * Ports: 4 LAN Ports, 1 WAN Port
>
> Co-developed-by: Tom Brautaset <[email protected]>
> Signed-off-by: Tom Brautaset <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> arch/arm/boot/dts/broadcom/Makefile | 1 +
> .../boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts | 164 +++++++++++++++++++++
> 2 files changed, 165 insertions(+)
>
> diff --git a/arch/arm/boot/dts/broadcom/Makefile b/arch/arm/boot/dts/broadcom/Makefile
> index 7099d9560033..c61fca514775 100644
> --- a/arch/arm/boot/dts/broadcom/Makefile
> +++ b/arch/arm/boot/dts/broadcom/Makefile
> @@ -64,6 +64,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
> bcm47081-luxul-xap-1410.dtb \
> bcm47081-luxul-xwr-1200.dtb \
> bcm47081-tplink-archer-c5-v2.dtb \
> + bcm4709-asus-rt-ac3200.dtb \
> bcm4709-asus-rt-ac87u.dtb \
> bcm4709-buffalo-wxr-1900dhp.dtb \
> bcm4709-linksys-ea9200.dtb \
> diff --git a/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
> new file mode 100644
> index 000000000000..8640dda211ae
> --- /dev/null
> +++ b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Author: Tom Brautaset <[email protected]>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm4709.dtsi"
> +#include "bcm5301x-nand-cs0-bch8.dtsi"
> +
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> + compatible = "asus,rt-ac3200", "brcm,bcm4709", "brcm,bcm4708";
> + model = "ASUS RT-AC3200";
> +
> + chosen {
> + bootargs = "console=ttyS0,115200 earlycon";

1. Use stdout.
2. Drop earlycon, it is for debugging, not regular mainline usage.

> + };
> +
> + memory@0 {
> + device_type = "memory";
> + reg = <0x00000000 0x08000000>,
> + <0x88000000 0x08000000>;
> + };
> +
> + nvram@1c080000 {
> + compatible = "brcm,nvram";
> + reg = <0x1c080000 0x00180000>;

Why is this outside of soc? Both soc node and soc DTSI?



Best regards,
Krzysztof


2024-04-14 17:00:16

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 14.04.2024 17:13, Krzysztof Kozlowski wrote:
> On 14/04/2024 13:46, Arınç ÜNAL via B4 Relay wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> Add the device tree for ASUS RT-AC3200 which is an AC3200 router featuring
>> 5 Ethernet ports over the integrated Broadcom switch.
>>
>> Hardware info:
>> * Processor: Broadcom BCM4709A0 dual-core @ 1.0 GHz
>> * Switch: BCM53012 in BCM4709A0
>> * DDR3 RAM: 256 MB
>> * Flash: 128 MB
>> * 2.4GHz: BCM43602 3x3 single chip 802.11b/g/n SoC
>> * 5GHz: BCM43602 3x3 two chips 802.11a/n/ac SoC
>> * Ports: 4 LAN Ports, 1 WAN Port
>>
>> Co-developed-by: Tom Brautaset <[email protected]>
>> Signed-off-by: Tom Brautaset <[email protected]>
>> Signed-off-by: Arınç ÜNAL <[email protected]>
>> ---
>> arch/arm/boot/dts/broadcom/Makefile | 1 +
>> .../boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts | 164 +++++++++++++++++++++
>> 2 files changed, 165 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/broadcom/Makefile b/arch/arm/boot/dts/broadcom/Makefile
>> index 7099d9560033..c61fca514775 100644
>> --- a/arch/arm/boot/dts/broadcom/Makefile
>> +++ b/arch/arm/boot/dts/broadcom/Makefile
>> @@ -64,6 +64,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
>> bcm47081-luxul-xap-1410.dtb \
>> bcm47081-luxul-xwr-1200.dtb \
>> bcm47081-tplink-archer-c5-v2.dtb \
>> + bcm4709-asus-rt-ac3200.dtb \
>> bcm4709-asus-rt-ac87u.dtb \
>> bcm4709-buffalo-wxr-1900dhp.dtb \
>> bcm4709-linksys-ea9200.dtb \
>> diff --git a/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
>> new file mode 100644
>> index 000000000000..8640dda211ae
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts
>> @@ -0,0 +1,164 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>> +/*
>> + * Author: Tom Brautaset <[email protected]>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "bcm4709.dtsi"
>> +#include "bcm5301x-nand-cs0-bch8.dtsi"
>> +
>> +#include <dt-bindings/leds/common.h>
>> +
>> +/ {
>> + compatible = "asus,rt-ac3200", "brcm,bcm4709", "brcm,bcm4708";
>> + model = "ASUS RT-AC3200";
>> +
>> + chosen {
>> + bootargs = "console=ttyS0,115200 earlycon";
>
> 1. Use stdout.
> 2. Drop earlycon, it is for debugging, not regular mainline usage.

I see that bcm4708.dtsi which this device tree includes already describes
stdout-path with the same value so I'll just get rid of the chosen node
here.

>
>> + };
>> +
>> + memory@0 {
>> + device_type = "memory";
>> + reg = <0x00000000 0x08000000>,
>> + <0x88000000 0x08000000>;
>> + };
>> +
>> + nvram@1c080000 {
>> + compatible = "brcm,nvram";
>> + reg = <0x1c080000 0x00180000>;
>
> Why is this outside of soc? Both soc node and soc DTSI?

I don't maintain the SoC device tree files so I don't know. The nvram node
doesn't exist on any of the device tree files included by this device tree.

Arınç

2024-04-14 19:13:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 14/04/2024 18:59, Arınç ÜNAL wrote:
>>> + };
>>> +
>>> + memory@0 {
>>> + device_type = "memory";
>>> + reg = <0x00000000 0x08000000>,
>>> + <0x88000000 0x08000000>;
>>> + };
>>> +
>>> + nvram@1c080000 {
>>> + compatible = "brcm,nvram";
>>> + reg = <0x1c080000 0x00180000>;
>>
>> Why is this outside of soc? Both soc node and soc DTSI?
>
> I don't maintain the SoC device tree files so I don't know. The nvram node
> doesn't exist on any of the device tree files included by this device tree.

There are two problems here:
1. This looks like SoC component and such should not be in board DTS.
Regardless whether you maintain something or not, you should not add
incorrect code. Unless this is correct code, but then please share some
details.

2. You cannot have MMIO node outside of soc. That's a W=1 warning.

Best regards,
Krzysztof


2024-04-14 20:21:31

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 14.04.2024 22:12, Krzysztof Kozlowski wrote:
> On 14/04/2024 18:59, Arınç ÜNAL wrote:
>>>> + };
>>>> +
>>>> + memory@0 {
>>>> + device_type = "memory";
>>>> + reg = <0x00000000 0x08000000>,
>>>> + <0x88000000 0x08000000>;
>>>> + };
>>>> +
>>>> + nvram@1c080000 {
>>>> + compatible = "brcm,nvram";
>>>> + reg = <0x1c080000 0x00180000>;
>>>
>>> Why is this outside of soc? Both soc node and soc DTSI?
>>
>> I don't maintain the SoC device tree files so I don't know. The nvram node
>> doesn't exist on any of the device tree files included by this device tree.
>
> There are two problems here:
> 1. This looks like SoC component and such should not be in board DTS.
> Regardless whether you maintain something or not, you should not add
> incorrect code. Unless this is correct code, but then please share some
> details.

NVRAM is described as both flash device partition and memory mapped NVMEM.
This platform stores NVRAM on flash but makes it also memory accessible.

As device partitions are described in board DTS, the nvram node must also
be defined there as its address and size will be different by board. It has
been widely described on at least bcm4709 and bcm47094 SoC board DTS files
here.

>
> 2. You cannot have MMIO node outside of soc. That's a W=1 warning.

I was not able to spot a warning related to this with the command below.
The source code directory is checked out on a recent soc/soc.git for-next
tree. Please let me know the correct command to do this.

$ make W=1 dtbs
[...]
DTC arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dtb
arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0.dtsi:10.18-19.5: Warning (avoid_unnecessary_addr_size): /nand-controller@18028000/nand@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
also defined at arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0-bch8.dtsi:13.9-17.3
also defined at arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts:137.9-160.3
arch/arm/boot/dts/broadcom/bcm-ns.dtsi:24.28-47.4: Warning (unique_unit_address_if_enabled): /chipcommon-a-bus@18000000: duplicate unit-address (also used in node /axi@18000000)
arch/arm/boot/dts/broadcom/bcm-ns.dtsi:323.22-328.4: Warning (unique_unit_address_if_enabled): /mdio@18003000: duplicate unit-address (also used in node /mdio-mux@18003000)

Arınç

2024-04-15 07:57:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 14/04/2024 22:21, Arınç ÜNAL wrote:
> On 14.04.2024 22:12, Krzysztof Kozlowski wrote:
>> On 14/04/2024 18:59, Arınç ÜNAL wrote:
>>>>> + };
>>>>> +
>>>>> + memory@0 {
>>>>> + device_type = "memory";
>>>>> + reg = <0x00000000 0x08000000>,
>>>>> + <0x88000000 0x08000000>;
>>>>> + };
>>>>> +
>>>>> + nvram@1c080000 {
>>>>> + compatible = "brcm,nvram";
>>>>> + reg = <0x1c080000 0x00180000>;
>>>>
>>>> Why is this outside of soc? Both soc node and soc DTSI?
>>>
>>> I don't maintain the SoC device tree files so I don't know. The nvram node
>>> doesn't exist on any of the device tree files included by this device tree.
>>
>> There are two problems here:
>> 1. This looks like SoC component and such should not be in board DTS.
>> Regardless whether you maintain something or not, you should not add
>> incorrect code. Unless this is correct code, but then please share some
>> details.
>
> NVRAM is described as both flash device partition and memory mapped NVMEM.
> This platform stores NVRAM on flash but makes it also memory accessible.
>
> As device partitions are described in board DTS, the nvram node must also

Sorry, but we do not talk about partitions. Partitions are indeed board
property. But the piece of hardware, so NVMEM, is provided by SoC.

> be defined there as its address and size will be different by board. It has
> been widely described on at least bcm4709 and bcm47094 SoC board DTS files
> here.

These not proper arguments. What you are saying here is that SoC does no
have nvram at address 0x1c08000. Instead you are saying there some sort
of bus going out of SoC to the board and on the board physically there
is some NVRAM sort of memory attached to this bus.


>
>>
>> 2. You cannot have MMIO node outside of soc. That's a W=1 warning.
>
> I was not able to spot a warning related to this with the command below.
> The source code directory is checked out on a recent soc/soc.git for-next
> tree. Please let me know the correct command to do this.
>
> $ make W=1 dtbs
> [...]
> DTC arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dtb
> arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0.dtsi:10.18-19.5: Warning (avoid_unnecessary_addr_size): /nand-controller@18028000/nand@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> also defined at arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0-bch8.dtsi:13.9-17.3
> also defined at arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts:137.9-160.3
> arch/arm/boot/dts/broadcom/bcm-ns.dtsi:24.28-47.4: Warning (unique_unit_address_if_enabled): /chipcommon-a-bus@18000000: duplicate unit-address (also used in node /axi@18000000)
> arch/arm/boot/dts/broadcom/bcm-ns.dtsi:323.22-328.4: Warning (unique_unit_address_if_enabled): /mdio@18003000: duplicate unit-address (also used in node /mdio-mux@18003000)

Hm, indeed, that way it works. Actually should work if we allow soc@0
and memory@x, obviously.

Anyway, it is outside of soc node and soc dtsi, which leads to previous
point - you claim that it is not physically in the SoC package. How is
it connected? If it is on the board, why does it have brcm compatible,
not some "ti,whatever-eeprom-nvram"?

Best regards,
Krzysztof


2024-04-15 09:11:12

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>> NVRAM is described as both flash device partition and memory mapped NVMEM.
>> This platform stores NVRAM on flash but makes it also memory accessible.
>>
>> As device partitions are described in board DTS, the nvram node must also
>
> Sorry, but we do not talk about partitions. Partitions are indeed board
> property. But the piece of hardware, so NVMEM, is provided by SoC.
>
>> be defined there as its address and size will be different by board. It has
>> been widely described on at least bcm4709 and bcm47094 SoC board DTS files
>> here.
>
> These not proper arguments. What you are saying here is that SoC does no
> have nvram at address 0x1c08000. Instead you are saying there some sort
> of bus going out of SoC to the board and on the board physically there
> is some NVRAM sort of memory attached to this bus.

Yes that is the case. NVRAM is stored on a partition on the flash. On the
Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
flash base is 0x1e000000.

For the board in this patch, the flash is a NAND flash. The NVRAM partition
starts at address 0x00080000. Therefore, the NVRAM component's address is
0x1c080000.

>
>
>>
>>>
>>> 2. You cannot have MMIO node outside of soc. That's a W=1 warning.
>>
>> I was not able to spot a warning related to this with the command below.
>> The source code directory is checked out on a recent soc/soc.git for-next
>> tree. Please let me know the correct command to do this.
>>
>> $ make W=1 dtbs
>> [...]
>> DTC arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dtb
>> arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0.dtsi:10.18-19.5: Warning (avoid_unnecessary_addr_size): /nand-controller@18028000/nand@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>> also defined at arch/arm/boot/dts/broadcom/bcm5301x-nand-cs0-bch8.dtsi:13.9-17.3
>> also defined at arch/arm/boot/dts/broadcom/bcm4709-asus-rt-ac3200.dts:137.9-160.3
>> arch/arm/boot/dts/broadcom/bcm-ns.dtsi:24.28-47.4: Warning (unique_unit_address_if_enabled): /chipcommon-a-bus@18000000: duplicate unit-address (also used in node /axi@18000000)
>> arch/arm/boot/dts/broadcom/bcm-ns.dtsi:323.22-328.4: Warning (unique_unit_address_if_enabled): /mdio@18003000: duplicate unit-address (also used in node /mdio-mux@18003000)
>
> Hm, indeed, that way it works. Actually should work if we allow soc@0
> and memory@x, obviously.

I was a bit confused with memory@x too.

>
> Anyway, it is outside of soc node and soc dtsi, which leads to previous
> point - you claim that it is not physically in the SoC package. How is
> it connected? If it is on the board, why does it have brcm compatible,
> not some "ti,whatever-eeprom-nvram"?

I don't know what to tell you. I don't know this set of SoCs' dt-bindings.
I am merely submitting a board device tree source file. It would be great
if this didn't block this patch series and this conversation was further
discussed with Rafal who maintains this set of SoCs' dt-bindings, from what
I remember.

Arınç

2024-04-17 03:15:22

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200



On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>> NVRAM is described as both flash device partition and memory mapped
>>> NVMEM.
>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>
>>> As device partitions are described in board DTS, the nvram node must
>>> also
>>
>> Sorry, but we do not talk about partitions. Partitions are indeed board
>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>
>>> be defined there as its address and size will be different by board.
>>> It has
>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS
>>> files
>>> here.
>>
>> These not proper arguments. What you are saying here is that SoC does no
>> have nvram at address 0x1c08000. Instead you are saying there some sort
>> of bus going out of SoC to the board and on the board physically there
>> is some NVRAM sort of memory attached to this bus.
>
> Yes that is the case. NVRAM is stored on a partition on the flash. On the
> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
> flash base is 0x1e000000.
>
> For the board in this patch, the flash is a NAND flash. The NVRAM partition
> starts at address 0x00080000. Therefore, the NVRAM component's address is
> 0x1c080000.

Because the flash is memory mapped into the CPU's address space, a
separate node was defined since it is not part of the "soc" node which
describes the bridge that connects all of the peripherals.

Whether we should create an additional bus node which describes the
bridge being used to access the flash devices using the MMIO windows is
debatable. Rafal, what do you think?
--
Florian


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-04-17 08:24:25

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 17/04/2024 06:15, Florian Fainelli wrote:
>
>
> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>> NVRAM is described as both flash device partition and memory mapped NVMEM.
>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>
>>>> As device partitions are described in board DTS, the nvram node must also
>>>
>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>
>>>> be defined there as its address and size will be different by board. It has
>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS files
>>>> here.
>>>
>>> These not proper arguments. What you are saying here is that SoC does no
>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>> of bus going out of SoC to the board and on the board physically there
>>> is some NVRAM sort of memory attached to this bus.
>>
>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>> flash base is 0x1e000000.
>>
>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>> 0x1c080000.
>
> Because the flash is memory mapped into the CPU's address space, a separate node was defined since it is not part of the "soc" node which describes the bridge that connects all of the peripherals.
>
> Whether we should create an additional bus node which describes the bridge being used to access the flash devices using the MMIO windows is debatable. Rafal, what do you think?

Will this block this patch series? If not, I'd like to submit the next
version with Krzysztof's comments on earlycon and stdout-path addressed.

Arınç

2024-04-17 13:24:27

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 17/04/2024 05:15, Florian Fainelli wrote:
>
>
> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>> NVRAM is described as both flash device partition and memory mapped
>>>> NVMEM.
>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>
>>>> As device partitions are described in board DTS, the nvram node must
>>>> also
>>>
>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>
>>>> be defined there as its address and size will be different by board.
>>>> It has
>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS
>>>> files
>>>> here.
>>>
>>> These not proper arguments. What you are saying here is that SoC does no
>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>> of bus going out of SoC to the board and on the board physically there
>>> is some NVRAM sort of memory attached to this bus.
>>
>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>> flash base is 0x1e000000.
>>
>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>> 0x1c080000.
>
> Because the flash is memory mapped into the CPU's address space, a
> separate node was defined since it is not part of the "soc" node which
> describes the bridge that connects all of the peripherals.
>
> Whether we should create an additional bus node which describes the
> bridge being used to access the flash devices using the MMIO windows is
> debatable. Rafal, what do you think?

Sorry guys, I don't get. I don't know the addresses neither the names
like Broadcom Northstar, so this does not clarify me at all.

Please answer the simple questions:
1. Is NAND flash part of SoC?
2. If not, is NAND flash provided by Broadcom or anyone else?

Best regards,
Krzysztof


2024-04-17 13:25:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 17/04/2024 10:24, Arınç ÜNAL wrote:
> On 17/04/2024 06:15, Florian Fainelli wrote:
>>
>>
>> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>>> NVRAM is described as both flash device partition and memory mapped NVMEM.
>>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>>
>>>>> As device partitions are described in board DTS, the nvram node must also
>>>>
>>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>>
>>>>> be defined there as its address and size will be different by board. It has
>>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS files
>>>>> here.
>>>>
>>>> These not proper arguments. What you are saying here is that SoC does no
>>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>>> of bus going out of SoC to the board and on the board physically there
>>>> is some NVRAM sort of memory attached to this bus.
>>>
>>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>>> flash base is 0x1e000000.
>>>
>>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>>> 0x1c080000.
>>
>> Because the flash is memory mapped into the CPU's address space, a separate node was defined since it is not part of the "soc" node which describes the bridge that connects all of the peripherals.
>>
>> Whether we should create an additional bus node which describes the bridge being used to access the flash devices using the MMIO windows is debatable. Rafal, what do you think?
>
> Will this block this patch series? If not, I'd like to submit the next
> version with Krzysztof's comments on earlycon and stdout-path addressed.

Why are you so impatient? The review process takes time and your
reluctance to take responsibility for issues here are no helping.

Best regards,
Krzysztof


2024-04-17 14:46:43

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 17/04/2024 16:23, Krzysztof Kozlowski wrote:
> On 17/04/2024 10:24, Arınç ÜNAL wrote:
>> On 17/04/2024 06:15, Florian Fainelli wrote:
>>>
>>>
>>> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>>>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>>>> NVRAM is described as both flash device partition and memory mapped NVMEM.
>>>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>>>
>>>>>> As device partitions are described in board DTS, the nvram node must also
>>>>>
>>>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>>>
>>>>>> be defined there as its address and size will be different by board. It has
>>>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS files
>>>>>> here.
>>>>>
>>>>> These not proper arguments. What you are saying here is that SoC does no
>>>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>>>> of bus going out of SoC to the board and on the board physically there
>>>>> is some NVRAM sort of memory attached to this bus.
>>>>
>>>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>>>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>>>> flash base is 0x1e000000.
>>>>
>>>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>>>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>>>> 0x1c080000.
>>>
>>> Because the flash is memory mapped into the CPU's address space, a separate node was defined since it is not part of the "soc" node which describes the bridge that connects all of the peripherals.
>>>
>>> Whether we should create an additional bus node which describes the bridge being used to access the flash devices using the MMIO windows is debatable. Rafal, what do you think?
>>
>> Will this block this patch series? If not, I'd like to submit the next
>> version with Krzysztof's comments on earlycon and stdout-path addressed.
>
> Why are you so impatient? The review process takes time and your
> reluctance to take responsibility for issues here are no helping.

I have already stated that I don't maintain this architecture and I don't
know it very well, and called on at least Rafal to further discuss the
issue you've raised. I've already answered your questions to the best of my
knowledge. If I was impatient, I would declare that I have no
responsibility in the SoC dt-bindings and send the next version without a
care. What I am doing instead is confirming whether or not you or Florian
think that this SoC dt-bindings issue must be resolved before my patches
that add board DTS files go in.

Arınç

2024-04-17 16:48:24

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200



On 4/17/2024 6:23 AM, Krzysztof Kozlowski wrote:
> On 17/04/2024 05:15, Florian Fainelli wrote:
>>
>>
>> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>>> NVRAM is described as both flash device partition and memory mapped
>>>>> NVMEM.
>>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>>
>>>>> As device partitions are described in board DTS, the nvram node must
>>>>> also
>>>>
>>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>>
>>>>> be defined there as its address and size will be different by board.
>>>>> It has
>>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS
>>>>> files
>>>>> here.
>>>>
>>>> These not proper arguments. What you are saying here is that SoC does no
>>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>>> of bus going out of SoC to the board and on the board physically there
>>>> is some NVRAM sort of memory attached to this bus.
>>>
>>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>>> flash base is 0x1e000000.
>>>
>>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>>> 0x1c080000.
>>
>> Because the flash is memory mapped into the CPU's address space, a
>> separate node was defined since it is not part of the "soc" node which
>> describes the bridge that connects all of the peripherals.
>>
>> Whether we should create an additional bus node which describes the
>> bridge being used to access the flash devices using the MMIO windows is
>> debatable. Rafal, what do you think?
>
> Sorry guys, I don't get. I don't know the addresses neither the names
> like Broadcom Northstar, so this does not clarify me at all.

Northstar is just a code name for the BCM5301X SoC family. The SoC
memory map looks like this:

0x0000_0000 ~ 0x07FF_FFFF - DDR
0x0800_0000 ~ 0x0FFF_FFFF - PCIe0
0x1800_0000 ~ 0x180F_FFFF - Core registers (that is
chipcommon-a-bus@18000000 and axi@18000000 in DT)
0x1810_0000 ~ 0x181F_FFFF - IDM registers
0x1900_0000 ~ 0x190F_FFFF - ARMCore registers (that is
mpcore-bus@19000000 in DT)
0x1C00_0000 ~ 0x1DFF_FFFF - NAND flash
0x1E00_0000 ~ 0x1FFF_FFFF - SPI-NOR flash
0x4000_0000 ~ 0x47FF_FFFF - PCIe1
0x4800_0000 ~ 0x4FFF_FFFF - PCIe2
0x8000_0000 ~ 0xBFFF_FFFF - DDR

From the system diagram the CPU has 3 AXI ports to the NIC301 AXI
fabric, which itself has separate AXI ports to the NAND and SPI-NOR MMIO
interface and then different AXI and APB ports to various other peripherals.

This information was not accessible to Rafal at the time, so it would
not have been reasonable to expect from him to know such details.

>
> Please answer the simple questions:
> 1. Is NAND flash part of SoC?
> 2. If not, is NAND flash provided by Broadcom or anyone else?

The NAND flash is external to the SoC it is not manufactured by Broadcom
we have boards with Spansion, Micron, Macronix, Toshiba flashes etc.
--
Florian


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-04-17 19:03:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: BCM5301X: Add DT for ASUS RT-AC3200

On 17/04/2024 18:47, Florian Fainelli wrote:
>
>
> On 4/17/2024 6:23 AM, Krzysztof Kozlowski wrote:
>> On 17/04/2024 05:15, Florian Fainelli wrote:
>>>
>>>
>>> On 4/15/2024 2:10 AM, Arınç ÜNAL wrote:
>>>> On 15.04.2024 10:57, Krzysztof Kozlowski wrote:
>>>>> On 14/04/2024 22:21, Arınç ÜNAL wrote:
>>>>>> NVRAM is described as both flash device partition and memory mapped
>>>>>> NVMEM.
>>>>>> This platform stores NVRAM on flash but makes it also memory accessible.
>>>>>>
>>>>>> As device partitions are described in board DTS, the nvram node must
>>>>>> also
>>>>>
>>>>> Sorry, but we do not talk about partitions. Partitions are indeed board
>>>>> property. But the piece of hardware, so NVMEM, is provided by SoC.
>>>>>
>>>>>> be defined there as its address and size will be different by board.
>>>>>> It has
>>>>>> been widely described on at least bcm4709 and bcm47094 SoC board DTS
>>>>>> files
>>>>>> here.
>>>>>
>>>>> These not proper arguments. What you are saying here is that SoC does no
>>>>> have nvram at address 0x1c08000. Instead you are saying there some sort
>>>>> of bus going out of SoC to the board and on the board physically there
>>>>> is some NVRAM sort of memory attached to this bus.
>>>>
>>>> Yes that is the case. NVRAM is stored on a partition on the flash. On the
>>>> Broadcom NorthStar platform, the NAND flash base is 0x1c000000, the NOR
>>>> flash base is 0x1e000000.
>>>>
>>>> For the board in this patch, the flash is a NAND flash. The NVRAM partition
>>>> starts at address 0x00080000. Therefore, the NVRAM component's address is
>>>> 0x1c080000.
>>>
>>> Because the flash is memory mapped into the CPU's address space, a
>>> separate node was defined since it is not part of the "soc" node which
>>> describes the bridge that connects all of the peripherals.
>>>
>>> Whether we should create an additional bus node which describes the
>>> bridge being used to access the flash devices using the MMIO windows is
>>> debatable. Rafal, what do you think?
>>
>> Sorry guys, I don't get. I don't know the addresses neither the names
>> like Broadcom Northstar, so this does not clarify me at all.
>
> Northstar is just a code name for the BCM5301X SoC family. The SoC
> memory map looks like this:
>
> 0x0000_0000 ~ 0x07FF_FFFF - DDR
> 0x0800_0000 ~ 0x0FFF_FFFF - PCIe0
> 0x1800_0000 ~ 0x180F_FFFF - Core registers (that is
> chipcommon-a-bus@18000000 and axi@18000000 in DT)
> 0x1810_0000 ~ 0x181F_FFFF - IDM registers
> 0x1900_0000 ~ 0x190F_FFFF - ARMCore registers (that is
> mpcore-bus@19000000 in DT)
> 0x1C00_0000 ~ 0x1DFF_FFFF - NAND flash
> 0x1E00_0000 ~ 0x1FFF_FFFF - SPI-NOR flash
> 0x4000_0000 ~ 0x47FF_FFFF - PCIe1
> 0x4800_0000 ~ 0x4FFF_FFFF - PCIe2
> 0x8000_0000 ~ 0xBFFF_FFFF - DDR
>
> From the system diagram the CPU has 3 AXI ports to the NIC301 AXI
> fabric, which itself has separate AXI ports to the NAND and SPI-NOR MMIO
> interface and then different AXI and APB ports to various other peripherals.
>
> This information was not accessible to Rafal at the time, so it would
> not have been reasonable to expect from him to know such details.

Sure.

>
>>
>> Please answer the simple questions:
>> 1. Is NAND flash part of SoC?
>> 2. If not, is NAND flash provided by Broadcom or anyone else?
>
> The NAND flash is external to the SoC it is not manufactured by Broadcom
> we have boards with Spansion, Micron, Macronix, Toshiba flashes etc.

Thanks, then this explains why this is no part of SoC and my concerns
are addressed. I have only one remaining question - you use "brcm,nvram"
compatible, which kind of suggests the NVRAM controller or part itself
is coming from Broadcom. Driver looks like software construct, but it is
so widely used that I guess that ship has sailed.

Best regards,
Krzysztof