[PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[Yao Zi]

Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for
multimedia application. This series add a basic device tree with CPU,
interrupts and UART nodes for it and is able to boot into a kernel with
only UART console.

Has been tested on Radxa E20C board[1] with vendor U-boot, successfully
booted into initramfs with this log[2].

[1]: https://docs.radxa.com/en/e/e20c
[2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319

Yao Zi (4):
  dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528
  dt-bindings: arm: rockchip: Add Radxa E20C board
  arm64: dts: rockchip: Add base DT for rk3528 SoC
  arm64: dts: rockchip: Add Radxa e20c board

 .../devicetree/bindings/arm/rockchip.yaml     |   5 +
 .../bindings/serial/snps-dw-apb-uart.yaml     |   1 +
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3528-radxa-e20c.dts   |  22 +++
 arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 182 ++++++++++++++++++
 5 files changed, 211 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi


base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d
-- 
2.45.2

[PATCH 1/4] dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528

[Yao Zi]

Rockchip RK3528 comes with a snps-dw-apb-uart compatible UART. Document
it in dt-bindings.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 4cdb0dcaccf3..4573044be189 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -48,6 +48,7 @@ properties:
               - rockchip,rk3328-uart
               - rockchip,rk3368-uart
               - rockchip,rk3399-uart
+              - rockchip,rk3528-uart
               - rockchip,rk3568-uart
               - rockchip,rk3588-uart
               - rockchip,rv1108-uart
-- 
2.45.2

Re: [PATCH 1/4] dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528

[Krzysztof Kozlowski]

On 03/08/2024 14:55, Yao Zi wrote:
> Rockchip RK3528 comes with a snps-dw-apb-uart compatible UART. Document
> it in dt-bindings.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

<form letter>
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528

[Heiko Stübner]

Am Samstag, 3. August 2024, 14:55:08 CEST schrieb Yao Zi:
> Rockchip RK3528 comes with a snps-dw-apb-uart compatible UART. Document
> it in dt-bindings.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> index 4cdb0dcaccf3..4573044be189 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> @@ -48,6 +48,7 @@ properties:
>                - rockchip,rk3328-uart
>                - rockchip,rk3368-uart
>                - rockchip,rk3399-uart
> +              - rockchip,rk3528-uart
>                - rockchip,rk3568-uart
>                - rockchip,rk3588-uart
>                - rockchip,rv1108-uart
> 

[PATCH 2/4] dt-bindings: arm: rockchip: Add Radxa E20C board

[Yao Zi]

Add device tree documentation for Radxa E20C board.

Link: https://docs.radxa.com/en/e/e20c
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index 1ef09fbfdfaf..2b8bfc7892cb 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -753,6 +753,11 @@ properties:
           - const: radxa,cm3i
           - const: rockchip,rk3568
 
+      - description: Radxa E20C
+        items:
+          - const: radxa,e20c
+          - const: rockchip,rk3528
+
       - description: Radxa Rock
         items:
           - const: radxa,rock
-- 
2.45.2

Re: [PATCH 2/4] dt-bindings: arm: rockchip: Add Radxa E20C board

[Krzysztof Kozlowski]

On 03/08/2024 14:55, Yao Zi wrote:
> Add device tree documentation for Radxa E20C board.
> 

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

This initial device tree describes CPU, interrupts and UART on the chip
and is able to boot into basic kernel with only UART. Cache information
is omitted for now as there is no precise documentation. Support for
other features will be added later.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
 1 file changed, 182 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi

diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
new file mode 100644
index 000000000000..77687d9e7e80
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
+ * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	compatible = "rockchip,rk3528";
+
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		serial4 = &uart4;
+		serial5 = &uart5;
+		serial6 = &uart6;
+		serial7 = &uart7;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+				core1 {
+					cpu = <&cpu1>;
+				};
+				core2 {
+					cpu = <&cpu2>;
+				};
+				core3 {
+					cpu = <&cpu3>;
+				};
+			};
+		};
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x1>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x2>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x3>;
+			enable-method = "psci";
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-1.0", "arm,psci-0.2";
+		method = "smc";
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	xin24m: xin24m {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24000000>;
+		clock-output-names = "xin24m";
+	};
+
+	gic: interrupt-controller@fed01000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x0 0xfed01000 0 0x1000>,
+		      <0x0 0xfed02000 0 0x2000>,
+		      <0x0 0xfed04000 0 0x2000>,
+		      <0x0 0xfed06000 0 0x2000>;
+		interrupts = <GIC_PPI 9
+			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	uart0: serial@ff9f0000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xff9f0000 0x0 0x100>;
+		interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		clock-frequency = <24000000>;
+		status = "disabled";
+	};
+
+	uart1: serial@ff9f8000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xff9f8000 0x0 0x100>;
+		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart2: serial@ffa00000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa00000 0x0 0x100>;
+		interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart3: serial@ffa08000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa08000 0x0 0x100>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart4: serial@ffa10000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa10000 0x0 0x100>;
+		interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart5: serial@ffa18000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa18000 0x0 0x100>;
+		interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart6: serial@ffa20000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa20000 0x0 0x100>;
+		interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
+	uart7: serial@ffa28000 {
+		compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xffa28000 0x0 0x100>;
+		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+};
-- 
2.45.2

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 03/08/2024 14:55, Yao Zi wrote:
> This initial device tree describes CPU, interrupts and UART on the chip
> and is able to boot into basic kernel with only UART. Cache information
> is omitted for now as there is no precise documentation. Support for
> other features will be added later.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
>  1 file changed, 182 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> new file mode 100644
> index 000000000000..77687d9e7e80
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
> + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	compatible = "rockchip,rk3528";
> +
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +		serial4 = &uart4;
> +		serial5 = &uart5;
> +		serial6 = &uart6;
> +		serial7 = &uart7;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +				core2 {
> +					cpu = <&cpu2>;
> +				};
> +				core3 {
> +					cpu = <&cpu3>;
> +				};
> +			};
> +		};
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x3>;
> +			enable-method = "psci";
> +		};
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0", "arm,psci-0.2";
> +		method = "smc";
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	xin24m: xin24m {

Please use name for all fixed clocks which matches current format
recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "xin24m";
> +	};
> +
> +	gic: interrupt-controller@fed01000 {

Why this all is outside of SoC?

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Hi Krzysztof,

Am Sonntag, 4. August 2024, 12:05:11 CEST schrieb Krzysztof Kozlowski:
> On 03/08/2024 14:55, Yao Zi wrote:
> > This initial device tree describes CPU, interrupts and UART on the chip
> > and is able to boot into basic kernel with only UART. Cache information
> > is omitted for now as there is no precise documentation. Support for
> > other features will be added later.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---

> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <24000000>;
> > +		clock-output-names = "xin24m";
> > +	};
> > +
> > +	gic: interrupt-controller@fed01000 {
> 
> Why this all is outside of SoC?

I guess you mean outside of a "soc {}" node?

Here the rk3528 simply follows all other Rockchip SoCs :-) .

Digging into the history, the first rk3066a and initial rk3288 submission
did use a soc {} node, which later got removed as suggested by arm-soc
maintainers at the time [0].

I guess that changed since then?


Heiko


[0] https://lore.kernel.org/linux-arm-kernel/20140716005528.GA26207@quad.lixom.net/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c3030d30d9c99c057b5ddfa289cffa637a2775f5

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 04/08/2024 14:49, Heiko Stübner wrote:
> 
>>> +		compatible = "fixed-clock";
>>> +		#clock-cells = <0>;
>>> +		clock-frequency = <24000000>;
>>> +		clock-output-names = "xin24m";
>>> +	};
>>> +
>>> +	gic: interrupt-controller@fed01000 {
>>
>> Why this all is outside of SoC?
> 
> I guess you mean outside of a "soc {}" node?
> 
> Here the rk3528 simply follows all other Rockchip SoCs :-) .
> 
> Digging into the history, the first rk3066a and initial rk3288 submission
> did use a soc {} node, which later got removed as suggested by arm-soc
> maintainers at the time [0].
> 
> I guess that changed since then?

Well, referenced patch was mixing MMIO with non-MMIO, so Olof's comment
could be understood that this is not correct approach. Even though DT
spec shows examples of "soc", it is not required. But then how do you
implement any ordering? By name or by unit address?

IOW, I think this is the only platform not using "soc" nodes.

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
> On 03/08/2024 14:55, Yao Zi wrote:
> > This initial device tree describes CPU, interrupts and UART on the chip
> > and is able to boot into basic kernel with only UART. Cache information
> > is omitted for now as there is no precise documentation. Support for
> > other features will be added later.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
> >  1 file changed, 182 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > new file mode 100644
> > index 000000000000..77687d9e7e80
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > @@ -0,0 +1,182 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> > + */
> > +
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +/ {
> > +	compatible = "rockchip,rk3528";
> > +
> > +	interrupt-parent = <&gic>;
> > +	#address-cells = <2>;
> > +	#size-cells = <2>;
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +		serial1 = &uart1;
> > +		serial2 = &uart2;
> > +		serial3 = &uart3;
> > +		serial4 = &uart4;
> > +		serial5 = &uart5;
> > +		serial6 = &uart6;
> > +		serial7 = &uart7;
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu-map {
> > +			cluster0 {
> > +				core0 {
> > +					cpu = <&cpu0>;
> > +				};
> > +				core1 {
> > +					cpu = <&cpu1>;
> > +				};
> > +				core2 {
> > +					cpu = <&cpu2>;
> > +				};
> > +				core3 {
> > +					cpu = <&cpu3>;
> > +				};
> > +			};
> > +		};
> > +
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x0>;
> > +			enable-method = "psci";
> > +		};
> > +
> > +		cpu1: cpu@1 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x1>;
> > +			enable-method = "psci";
> > +		};
> > +
> > +		cpu2: cpu@2 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x2>;
> > +			enable-method = "psci";
> > +		};
> > +
> > +		cpu3: cpu@3 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x3>;
> > +			enable-method = "psci";
> > +		};
> > +	};
> > +
> > +	psci {
> > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
> > +		method = "smc";
> > +	};
> > +
> > +	timer {
> > +		compatible = "arm,armv8-timer";
> > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> > +	};
> > +
> > +	xin24m: xin24m {
> 
> Please use name for all fixed clocks which matches current format
> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'

Will be fixed in next revision.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
> 
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <24000000>;
> > +		clock-output-names = "xin24m";
> > +	};
> > +
> > +	gic: interrupt-controller@fed01000 {
> 
> Why this all is outside of SoC?

Just as Heiko says, device tree for all other Rockchip SoCs don't have
a "soc" node. I didn't know why before but just follow the style.

If you prefer add a soc node, I am willing to.

Best regards,
Yao Zi

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Dragan Simic]

On 2024-08-04 15:20, Yao Zi wrote:
> On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
>> On 03/08/2024 14:55, Yao Zi wrote:
>> > This initial device tree describes CPU, interrupts and UART on the chip
>> > and is able to boot into basic kernel with only UART. Cache information
>> > is omitted for now as there is no precise documentation. Support for
>> > other features will be added later.
>> >
>> > Signed-off-by: Yao Zi <ziyao@disroot.org>
>> > ---
>> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
>> >  1 file changed, 182 insertions(+)
>> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> >
>> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> > new file mode 100644
>> > index 000000000000..77687d9e7e80
>> > --- /dev/null
>> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> > @@ -0,0 +1,182 @@
>> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> > +/*
>> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
>> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
>> > + */
>> > +
>> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> > +#include <dt-bindings/interrupt-controller/irq.h>
>> > +
>> > +/ {
>> > +	compatible = "rockchip,rk3528";
>> > +
>> > +	interrupt-parent = <&gic>;
>> > +	#address-cells = <2>;
>> > +	#size-cells = <2>;
>> > +
>> > +	aliases {
>> > +		serial0 = &uart0;
>> > +		serial1 = &uart1;
>> > +		serial2 = &uart2;
>> > +		serial3 = &uart3;
>> > +		serial4 = &uart4;
>> > +		serial5 = &uart5;
>> > +		serial6 = &uart6;
>> > +		serial7 = &uart7;
>> > +	};
>> > +
>> > +	cpus {
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		cpu-map {
>> > +			cluster0 {
>> > +				core0 {
>> > +					cpu = <&cpu0>;
>> > +				};
>> > +				core1 {
>> > +					cpu = <&cpu1>;
>> > +				};
>> > +				core2 {
>> > +					cpu = <&cpu2>;
>> > +				};
>> > +				core3 {
>> > +					cpu = <&cpu3>;
>> > +				};
>> > +			};
>> > +		};
>> > +
>> > +		cpu0: cpu@0 {
>> > +			device_type = "cpu";
>> > +			compatible = "arm,cortex-a53";
>> > +			reg = <0x0>;
>> > +			enable-method = "psci";
>> > +		};
>> > +
>> > +		cpu1: cpu@1 {
>> > +			device_type = "cpu";
>> > +			compatible = "arm,cortex-a53";
>> > +			reg = <0x1>;
>> > +			enable-method = "psci";
>> > +		};
>> > +
>> > +		cpu2: cpu@2 {
>> > +			device_type = "cpu";
>> > +			compatible = "arm,cortex-a53";
>> > +			reg = <0x2>;
>> > +			enable-method = "psci";
>> > +		};
>> > +
>> > +		cpu3: cpu@3 {
>> > +			device_type = "cpu";
>> > +			compatible = "arm,cortex-a53";
>> > +			reg = <0x3>;
>> > +			enable-method = "psci";
>> > +		};
>> > +	};
>> > +
>> > +	psci {
>> > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
>> > +		method = "smc";
>> > +	};
>> > +
>> > +	timer {
>> > +		compatible = "arm,armv8-timer";
>> > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> > +	};
>> > +
>> > +	xin24m: xin24m {
>> 
>> Please use name for all fixed clocks which matches current format
>> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
> 
> Will be fixed in next revision.

Hmm, why should we apply that rule to the xin24m clock, which is
named exactly like that everywhere else in Rockchip SoC dtsi files?
It's much better to remain consistent.

>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
>> 
>> > +		compatible = "fixed-clock";
>> > +		#clock-cells = <0>;
>> > +		clock-frequency = <24000000>;
>> > +		clock-output-names = "xin24m";
>> > +	};
>> > +
>> > +	gic: interrupt-controller@fed01000 {
>> 
>> Why this all is outside of SoC?
> 
> Just as Heiko says, device tree for all other Rockchip SoCs don't have
> a "soc" node. I didn't know why before but just follow the style.
> 
> If you prefer add a soc node, I am willing to.

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
> On 2024-08-04 15:20, Yao Zi wrote:
> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
> >> On 03/08/2024 14:55, Yao Zi wrote:
> >> > This initial device tree describes CPU, interrupts and UART on the chip
> >> > and is able to boot into basic kernel with only UART. Cache information
> >> > is omitted for now as there is no precise documentation. Support for
> >> > other features will be added later.
> >> >
> >> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> >> > ---
> >> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
> >> >  1 file changed, 182 insertions(+)
> >> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> >
> >> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> > new file mode 100644
> >> > index 000000000000..77687d9e7e80
> >> > --- /dev/null
> >> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> > @@ -0,0 +1,182 @@
> >> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> > +/*
> >> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
> >> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> >> > + */
> >> > +
> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >> > +#include <dt-bindings/interrupt-controller/irq.h>
> >> > +
> >> > +/ {
> >> > +	compatible = "rockchip,rk3528";
> >> > +
> >> > +	interrupt-parent = <&gic>;
> >> > +	#address-cells = <2>;
> >> > +	#size-cells = <2>;
> >> > +
> >> > +	aliases {
> >> > +		serial0 = &uart0;
> >> > +		serial1 = &uart1;
> >> > +		serial2 = &uart2;
> >> > +		serial3 = &uart3;
> >> > +		serial4 = &uart4;
> >> > +		serial5 = &uart5;
> >> > +		serial6 = &uart6;
> >> > +		serial7 = &uart7;
> >> > +	};
> >> > +
> >> > +	cpus {
> >> > +		#address-cells = <1>;
> >> > +		#size-cells = <0>;
> >> > +
> >> > +		cpu-map {
> >> > +			cluster0 {
> >> > +				core0 {
> >> > +					cpu = <&cpu0>;
> >> > +				};
> >> > +				core1 {
> >> > +					cpu = <&cpu1>;
> >> > +				};
> >> > +				core2 {
> >> > +					cpu = <&cpu2>;
> >> > +				};
> >> > +				core3 {
> >> > +					cpu = <&cpu3>;
> >> > +				};
> >> > +			};
> >> > +		};
> >> > +
> >> > +		cpu0: cpu@0 {
> >> > +			device_type = "cpu";
> >> > +			compatible = "arm,cortex-a53";
> >> > +			reg = <0x0>;
> >> > +			enable-method = "psci";
> >> > +		};
> >> > +
> >> > +		cpu1: cpu@1 {
> >> > +			device_type = "cpu";
> >> > +			compatible = "arm,cortex-a53";
> >> > +			reg = <0x1>;
> >> > +			enable-method = "psci";
> >> > +		};
> >> > +
> >> > +		cpu2: cpu@2 {
> >> > +			device_type = "cpu";
> >> > +			compatible = "arm,cortex-a53";
> >> > +			reg = <0x2>;
> >> > +			enable-method = "psci";
> >> > +		};
> >> > +
> >> > +		cpu3: cpu@3 {
> >> > +			device_type = "cpu";
> >> > +			compatible = "arm,cortex-a53";
> >> > +			reg = <0x3>;
> >> > +			enable-method = "psci";
> >> > +		};
> >> > +	};
> >> > +
> >> > +	psci {
> >> > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
> >> > +		method = "smc";
> >> > +	};
> >> > +
> >> > +	timer {
> >> > +		compatible = "arm,armv8-timer";
> >> > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> >> > +	};
> >> > +
> >> > +	xin24m: xin24m {
> >> 
> >> Please use name for all fixed clocks which matches current format
> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
> > 
> > Will be fixed in next revision.
> 
> Hmm, why should we apply that rule to the xin24m clock, which is
> named exactly like that everywhere else in Rockchip SoC dtsi files?
> It's much better to remain consistent.

bindings or how we write devicetrees evolve over time ... similarly the
xin24m name comes from more than 10 years ago.

We also name all those regulator nodes regulator-foo now, which in turn
automatically does enforce a nice sorting rule to keep all the regulators
around the same area ;-)

So I don't see a problem of going with xin24m: clock-xin24m {}


Heiko

> 
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
> >> 
> >> > +		compatible = "fixed-clock";
> >> > +		#clock-cells = <0>;
> >> > +		clock-frequency = <24000000>;
> >> > +		clock-output-names = "xin24m";
> >> > +	};
> >> > +
> >> > +	gic: interrupt-controller@fed01000 {
> >> 
> >> Why this all is outside of SoC?
> > 
> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> > a "soc" node. I didn't know why before but just follow the style.
> > 
> > If you prefer add a soc node, I am willing to.
> 

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Dragan Simic]

On 2024-08-04 15:44, Heiko Stübner wrote:
> Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
>> On 2024-08-04 15:20, Yao Zi wrote:
>> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
>> >> On 03/08/2024 14:55, Yao Zi wrote:
>> >> > This initial device tree describes CPU, interrupts and UART on the chip
>> >> > and is able to boot into basic kernel with only UART. Cache information
>> >> > is omitted for now as there is no precise documentation. Support for
>> >> > other features will be added later.
>> >> >
>> >> > Signed-off-by: Yao Zi <ziyao@disroot.org>
>> >> > ---
>> >> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
>> >> >  1 file changed, 182 insertions(+)
>> >> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> >> >
>> >> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> >> > new file mode 100644
>> >> > index 000000000000..77687d9e7e80
>> >> > --- /dev/null
>> >> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>> >> > @@ -0,0 +1,182 @@
>> >> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> >> > +/*
>> >> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
>> >> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
>> >> > + */
>> >> > +
>> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> >> > +#include <dt-bindings/interrupt-controller/irq.h>
>> >> > +
>> >> > +/ {
>> >> > +	compatible = "rockchip,rk3528";
>> >> > +
>> >> > +	interrupt-parent = <&gic>;
>> >> > +	#address-cells = <2>;
>> >> > +	#size-cells = <2>;
>> >> > +
>> >> > +	aliases {
>> >> > +		serial0 = &uart0;
>> >> > +		serial1 = &uart1;
>> >> > +		serial2 = &uart2;
>> >> > +		serial3 = &uart3;
>> >> > +		serial4 = &uart4;
>> >> > +		serial5 = &uart5;
>> >> > +		serial6 = &uart6;
>> >> > +		serial7 = &uart7;
>> >> > +	};
>> >> > +
>> >> > +	cpus {
>> >> > +		#address-cells = <1>;
>> >> > +		#size-cells = <0>;
>> >> > +
>> >> > +		cpu-map {
>> >> > +			cluster0 {
>> >> > +				core0 {
>> >> > +					cpu = <&cpu0>;
>> >> > +				};
>> >> > +				core1 {
>> >> > +					cpu = <&cpu1>;
>> >> > +				};
>> >> > +				core2 {
>> >> > +					cpu = <&cpu2>;
>> >> > +				};
>> >> > +				core3 {
>> >> > +					cpu = <&cpu3>;
>> >> > +				};
>> >> > +			};
>> >> > +		};
>> >> > +
>> >> > +		cpu0: cpu@0 {
>> >> > +			device_type = "cpu";
>> >> > +			compatible = "arm,cortex-a53";
>> >> > +			reg = <0x0>;
>> >> > +			enable-method = "psci";
>> >> > +		};
>> >> > +
>> >> > +		cpu1: cpu@1 {
>> >> > +			device_type = "cpu";
>> >> > +			compatible = "arm,cortex-a53";
>> >> > +			reg = <0x1>;
>> >> > +			enable-method = "psci";
>> >> > +		};
>> >> > +
>> >> > +		cpu2: cpu@2 {
>> >> > +			device_type = "cpu";
>> >> > +			compatible = "arm,cortex-a53";
>> >> > +			reg = <0x2>;
>> >> > +			enable-method = "psci";
>> >> > +		};
>> >> > +
>> >> > +		cpu3: cpu@3 {
>> >> > +			device_type = "cpu";
>> >> > +			compatible = "arm,cortex-a53";
>> >> > +			reg = <0x3>;
>> >> > +			enable-method = "psci";
>> >> > +		};
>> >> > +	};
>> >> > +
>> >> > +	psci {
>> >> > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
>> >> > +		method = "smc";
>> >> > +	};
>> >> > +
>> >> > +	timer {
>> >> > +		compatible = "arm,armv8-timer";
>> >> > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> >> > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> >> > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> >> > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> >> > +	};
>> >> > +
>> >> > +	xin24m: xin24m {
>> >>
>> >> Please use name for all fixed clocks which matches current format
>> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
>> >
>> > Will be fixed in next revision.
>> 
>> Hmm, why should we apply that rule to the xin24m clock, which is
>> named exactly like that everywhere else in Rockchip SoC dtsi files?
>> It's much better to remain consistent.
> 
> bindings or how we write devicetrees evolve over time ... similarly the
> xin24m name comes from more than 10 years ago.
> 
> We also name all those regulator nodes regulator-foo now, which in turn
> automatically does enforce a nice sorting rule to keep all the 
> regulators
> around the same area ;-)
> 
> So I don't see a problem of going with xin24m: clock-xin24m {}

I agree that using "clock-xin24m" makes more sense in general, but the
trouble is that we can't rename the already existing instances of 
"xin24m",
because that has become part of the ABI.  Thus, I'm not sure that 
breaking
away from the legacy brings benefits in this particular case.

>> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
>> >>
>> >> > +		compatible = "fixed-clock";
>> >> > +		#clock-cells = <0>;
>> >> > +		clock-frequency = <24000000>;
>> >> > +		clock-output-names = "xin24m";
>> >> > +	};
>> >> > +
>> >> > +	gic: interrupt-controller@fed01000 {
>> >>
>> >> Why this all is outside of SoC?
>> >
>> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
>> > a "soc" node. I didn't know why before but just follow the style.
>> >
>> > If you prefer add a soc node, I am willing to.
>> 
> 
> 
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Am Sonntag, 4. August 2024, 15:59:19 CEST schrieb Dragan Simic:
> On 2024-08-04 15:44, Heiko Stübner wrote:
> > Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
> >> On 2024-08-04 15:20, Yao Zi wrote:
> >> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
> >> >> On 03/08/2024 14:55, Yao Zi wrote:
> >> >> > This initial device tree describes CPU, interrupts and UART on the chip
> >> >> > and is able to boot into basic kernel with only UART. Cache information
> >> >> > is omitted for now as there is no precise documentation. Support for
> >> >> > other features will be added later.
> >> >> >
> >> >> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> >> >> > ---
> >> >> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
> >> >> >  1 file changed, 182 insertions(+)
> >> >> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> >> >
> >> >> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> >> > new file mode 100644
> >> >> > index 000000000000..77687d9e7e80
> >> >> > --- /dev/null
> >> >> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> >> > @@ -0,0 +1,182 @@
> >> >> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> >> > +/*
> >> >> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
> >> >> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> >> >> > + */
> >> >> > +
> >> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >> >> > +#include <dt-bindings/interrupt-controller/irq.h>
> >> >> > +
> >> >> > +/ {
> >> >> > +	compatible = "rockchip,rk3528";
> >> >> > +
> >> >> > +	interrupt-parent = <&gic>;
> >> >> > +	#address-cells = <2>;
> >> >> > +	#size-cells = <2>;
> >> >> > +
> >> >> > +	aliases {
> >> >> > +		serial0 = &uart0;
> >> >> > +		serial1 = &uart1;
> >> >> > +		serial2 = &uart2;
> >> >> > +		serial3 = &uart3;
> >> >> > +		serial4 = &uart4;
> >> >> > +		serial5 = &uart5;
> >> >> > +		serial6 = &uart6;
> >> >> > +		serial7 = &uart7;
> >> >> > +	};
> >> >> > +
> >> >> > +	cpus {
> >> >> > +		#address-cells = <1>;
> >> >> > +		#size-cells = <0>;
> >> >> > +
> >> >> > +		cpu-map {
> >> >> > +			cluster0 {
> >> >> > +				core0 {
> >> >> > +					cpu = <&cpu0>;
> >> >> > +				};
> >> >> > +				core1 {
> >> >> > +					cpu = <&cpu1>;
> >> >> > +				};
> >> >> > +				core2 {
> >> >> > +					cpu = <&cpu2>;
> >> >> > +				};
> >> >> > +				core3 {
> >> >> > +					cpu = <&cpu3>;
> >> >> > +				};
> >> >> > +			};
> >> >> > +		};
> >> >> > +
> >> >> > +		cpu0: cpu@0 {
> >> >> > +			device_type = "cpu";
> >> >> > +			compatible = "arm,cortex-a53";
> >> >> > +			reg = <0x0>;
> >> >> > +			enable-method = "psci";
> >> >> > +		};
> >> >> > +
> >> >> > +		cpu1: cpu@1 {
> >> >> > +			device_type = "cpu";
> >> >> > +			compatible = "arm,cortex-a53";
> >> >> > +			reg = <0x1>;
> >> >> > +			enable-method = "psci";
> >> >> > +		};
> >> >> > +
> >> >> > +		cpu2: cpu@2 {
> >> >> > +			device_type = "cpu";
> >> >> > +			compatible = "arm,cortex-a53";
> >> >> > +			reg = <0x2>;
> >> >> > +			enable-method = "psci";
> >> >> > +		};
> >> >> > +
> >> >> > +		cpu3: cpu@3 {
> >> >> > +			device_type = "cpu";
> >> >> > +			compatible = "arm,cortex-a53";
> >> >> > +			reg = <0x3>;
> >> >> > +			enable-method = "psci";
> >> >> > +		};
> >> >> > +	};
> >> >> > +
> >> >> > +	psci {
> >> >> > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
> >> >> > +		method = "smc";
> >> >> > +	};
> >> >> > +
> >> >> > +	timer {
> >> >> > +		compatible = "arm,armv8-timer";
> >> >> > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> >> > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> >> > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> >> >> > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> >> >> > +	};
> >> >> > +
> >> >> > +	xin24m: xin24m {
> >> >>
> >> >> Please use name for all fixed clocks which matches current format
> >> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
> >> >
> >> > Will be fixed in next revision.
> >> 
> >> Hmm, why should we apply that rule to the xin24m clock, which is
> >> named exactly like that everywhere else in Rockchip SoC dtsi files?
> >> It's much better to remain consistent.
> > 
> > bindings or how we write devicetrees evolve over time ... similarly the
> > xin24m name comes from more than 10 years ago.
> > 
> > We also name all those regulator nodes regulator-foo now, which in turn
> > automatically does enforce a nice sorting rule to keep all the 
> > regulators
> > around the same area ;-)
> > 
> > So I don't see a problem of going with xin24m: clock-xin24m {}
> 
> I agree that using "clock-xin24m" makes more sense in general, but the
> trouble is that we can't rename the already existing instances of 
> "xin24m",
> because that has become part of the ABI.  Thus, I'm not sure that 
> breaking
> away from the legacy brings benefits in this particular case.

In the regulator case, we have _new_ boards using the new _node_-names
but I don't see any renaming of old boards and also don't think we should.

But that still does not keep us from using the nicer naming convention in
new boards ;-) .


Same with xin24m. We're talking only about the node-name here. The
phandle stays the same and also the actual clock name stays the same and
really only the actual node name you need to look for in /proc/device-tree
changes ;-) .

So I don't see the need to go about changing all the old socs, but new
additions should use improved naming conventions.

xin24m: clock-xin24m {
	compatible = "fixed-clock";
	#clock-cells = <0>;
	clock-frequency = <24000000>;
	clock-output-names = "xin24m";
};


Heiko



> >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
> >> >>
> >> >> > +		compatible = "fixed-clock";
> >> >> > +		#clock-cells = <0>;
> >> >> > +		clock-frequency = <24000000>;
> >> >> > +		clock-output-names = "xin24m";
> >> >> > +	};
> >> >> > +
> >> >> > +	gic: interrupt-controller@fed01000 {
> >> >>
> >> >> Why this all is outside of SoC?
> >> >
> >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> >> > a "soc" node. I didn't know why before but just follow the style.
> >> >
> >> > If you prefer add a soc node, I am willing to.
> >> 
> > 
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 04/08/2024 17:51, Heiko Stübner wrote:
> Am Sonntag, 4. August 2024, 15:59:19 CEST schrieb Dragan Simic:
>> On 2024-08-04 15:44, Heiko Stübner wrote:
>>> Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
>>>> On 2024-08-04 15:20, Yao Zi wrote:
>>>>> On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 03/08/2024 14:55, Yao Zi wrote:
>>>>>>> This initial device tree describes CPU, interrupts and UART on the chip
>>>>>>> and is able to boot into basic kernel with only UART. Cache information
>>>>>>> is omitted for now as there is no precise documentation. Support for
>>>>>>> other features will be added later.
>>>>>>>
>>>>>>> Signed-off-by: Yao Zi <ziyao@disroot.org>
>>>>>>> ---
>>>>>>>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
>>>>>>>  1 file changed, 182 insertions(+)
>>>>>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..77687d9e7e80
>>>>>>> --- /dev/null
>>>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>>>>>> @@ -0,0 +1,182 @@
>>>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>>> +/*
>>>>>>> + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
>>>>>>> + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>>>>>> +
>>>>>>> +/ {
>>>>>>> +	compatible = "rockchip,rk3528";
>>>>>>> +
>>>>>>> +	interrupt-parent = <&gic>;
>>>>>>> +	#address-cells = <2>;
>>>>>>> +	#size-cells = <2>;
>>>>>>> +
>>>>>>> +	aliases {
>>>>>>> +		serial0 = &uart0;
>>>>>>> +		serial1 = &uart1;
>>>>>>> +		serial2 = &uart2;
>>>>>>> +		serial3 = &uart3;
>>>>>>> +		serial4 = &uart4;
>>>>>>> +		serial5 = &uart5;
>>>>>>> +		serial6 = &uart6;
>>>>>>> +		serial7 = &uart7;
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	cpus {
>>>>>>> +		#address-cells = <1>;
>>>>>>> +		#size-cells = <0>;
>>>>>>> +
>>>>>>> +		cpu-map {
>>>>>>> +			cluster0 {
>>>>>>> +				core0 {
>>>>>>> +					cpu = <&cpu0>;
>>>>>>> +				};
>>>>>>> +				core1 {
>>>>>>> +					cpu = <&cpu1>;
>>>>>>> +				};
>>>>>>> +				core2 {
>>>>>>> +					cpu = <&cpu2>;
>>>>>>> +				};
>>>>>>> +				core3 {
>>>>>>> +					cpu = <&cpu3>;
>>>>>>> +				};
>>>>>>> +			};
>>>>>>> +		};
>>>>>>> +
>>>>>>> +		cpu0: cpu@0 {
>>>>>>> +			device_type = "cpu";
>>>>>>> +			compatible = "arm,cortex-a53";
>>>>>>> +			reg = <0x0>;
>>>>>>> +			enable-method = "psci";
>>>>>>> +		};
>>>>>>> +
>>>>>>> +		cpu1: cpu@1 {
>>>>>>> +			device_type = "cpu";
>>>>>>> +			compatible = "arm,cortex-a53";
>>>>>>> +			reg = <0x1>;
>>>>>>> +			enable-method = "psci";
>>>>>>> +		};
>>>>>>> +
>>>>>>> +		cpu2: cpu@2 {
>>>>>>> +			device_type = "cpu";
>>>>>>> +			compatible = "arm,cortex-a53";
>>>>>>> +			reg = <0x2>;
>>>>>>> +			enable-method = "psci";
>>>>>>> +		};
>>>>>>> +
>>>>>>> +		cpu3: cpu@3 {
>>>>>>> +			device_type = "cpu";
>>>>>>> +			compatible = "arm,cortex-a53";
>>>>>>> +			reg = <0x3>;
>>>>>>> +			enable-method = "psci";
>>>>>>> +		};
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	psci {
>>>>>>> +		compatible = "arm,psci-1.0", "arm,psci-0.2";
>>>>>>> +		method = "smc";
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	timer {
>>>>>>> +		compatible = "arm,armv8-timer";
>>>>>>> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	xin24m: xin24m {
>>>>>>
>>>>>> Please use name for all fixed clocks which matches current format
>>>>>> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
>>>>>
>>>>> Will be fixed in next revision.
>>>>
>>>> Hmm, why should we apply that rule to the xin24m clock, which is
>>>> named exactly like that everywhere else in Rockchip SoC dtsi files?
>>>> It's much better to remain consistent.
>>>
>>> bindings or how we write devicetrees evolve over time ... similarly the
>>> xin24m name comes from more than 10 years ago.
>>>
>>> We also name all those regulator nodes regulator-foo now, which in turn
>>> automatically does enforce a nice sorting rule to keep all the 
>>> regulators
>>> around the same area ;-)
>>>
>>> So I don't see a problem of going with xin24m: clock-xin24m {}
>>
>> I agree that using "clock-xin24m" makes more sense in general, but the
>> trouble is that we can't rename the already existing instances of 
>> "xin24m",
>> because that has become part of the ABI.  Thus, I'm not sure that 
>> breaking
>> away from the legacy brings benefits in this particular case.
> 
> In the regulator case, we have _new_ boards using the new _node_-names
> but I don't see any renaming of old boards and also don't think we should.
> 
> But that still does not keep us from using the nicer naming convention in
> new boards ;-) .
> 
> 
> Same with xin24m. We're talking only about the node-name here. The
> phandle stays the same and also the actual clock name stays the same and
> really only the actual node name you need to look for in /proc/device-tree
> changes ;-) .
> 
> So I don't see the need to go about changing all the old socs, but new
> additions should use improved naming conventions.
> 
> xin24m: clock-xin24m {
> 	compatible = "fixed-clock";
> 	#clock-cells = <0>;
> 	clock-frequency = <24000000>;
> 	clock-output-names = "xin24m";

Just to make it clear - doc clearly says the preferred name is:
"clock-frequency".

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Dragan Simic]

On 2024-08-04 17:51, Heiko Stübner wrote:
> Am Sonntag, 4. August 2024, 15:59:19 CEST schrieb Dragan Simic:
>> On 2024-08-04 15:44, Heiko Stübner wrote:
>> > Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
>> >> On 2024-08-04 15:20, Yao Zi wrote:
>> >> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
>> >> >> On 03/08/2024 14:55, Yao Zi wrote:
>> >> >> > +	xin24m: xin24m {
>> >> >>
>> >> >> Please use name for all fixed clocks which matches current format
>> >> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
>> >> >
>> >> > Will be fixed in next revision.
>> >>
>> >> Hmm, why should we apply that rule to the xin24m clock, which is
>> >> named exactly like that everywhere else in Rockchip SoC dtsi files?
>> >> It's much better to remain consistent.
>> >
>> > bindings or how we write devicetrees evolve over time ... similarly the
>> > xin24m name comes from more than 10 years ago.
>> >
>> > We also name all those regulator nodes regulator-foo now, which in turn
>> > automatically does enforce a nice sorting rule to keep all the
>> > regulators
>> > around the same area ;-)
>> >
>> > So I don't see a problem of going with xin24m: clock-xin24m {}
>> 
>> I agree that using "clock-xin24m" makes more sense in general, but the
>> trouble is that we can't rename the already existing instances of
>> "xin24m",
>> because that has become part of the ABI.  Thus, I'm not sure that
>> breaking
>> away from the legacy brings benefits in this particular case.
> 
> In the regulator case, we have _new_ boards using the new _node_-names
> but I don't see any renaming of old boards and also don't think we 
> should.
> 
> But that still does not keep us from using the nicer naming convention 
> in
> new boards ;-) .
> 
> Same with xin24m. We're talking only about the node-name here. The
> phandle stays the same and also the actual clock name stays the same 
> and
> really only the actual node name you need to look for in 
> /proc/device-tree
> changes ;-) .
> 
> So I don't see the need to go about changing all the old socs, but new
> additions should use improved naming conventions.
> 
> xin24m: clock-xin24m {
> 	compatible = "fixed-clock";
> 	#clock-cells = <0>;
> 	clock-frequency = <24000000>;
> 	clock-output-names = "xin24m";
> };

Makes sense.  Though, updating the dtsi files for older SoCs to follow
the new rules, if possible, still remains an itch to me. :)

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 03:25:47PM +0200, Dragan Simic wrote:
> On 2024-08-04 15:20, Yao Zi wrote:
> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
> > > On 03/08/2024 14:55, Yao Zi wrote:
> > > > This initial device tree describes CPU, interrupts and UART on the chip
> > > > and is able to boot into basic kernel with only UART. Cache information
> > > > is omitted for now as there is no precise documentation. Support for
> > > > other features will be added later.
> > > >
> > > > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > > > ---
> > > >  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++
> > > >  1 file changed, 182 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > > >
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > > > new file mode 100644
> > > > index 000000000000..77687d9e7e80
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > > > @@ -0,0 +1,182 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > +/*
> > > > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd.
> > > > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > > +
> > > > +/ {
> > > > +	compatible = "rockchip,rk3528";
> > > > +
> > > > +	interrupt-parent = <&gic>;
> > > > +	#address-cells = <2>;
> > > > +	#size-cells = <2>;
> > > > +
> > > > +	aliases {
> > > > +		serial0 = &uart0;
> > > > +		serial1 = &uart1;
> > > > +		serial2 = &uart2;
> > > > +		serial3 = &uart3;
> > > > +		serial4 = &uart4;
> > > > +		serial5 = &uart5;
> > > > +		serial6 = &uart6;
> > > > +		serial7 = &uart7;
> > > > +	};
> > > > +
> > > > +	cpus {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		cpu-map {
> > > > +			cluster0 {
> > > > +				core0 {
> > > > +					cpu = <&cpu0>;
> > > > +				};
> > > > +				core1 {
> > > > +					cpu = <&cpu1>;
> > > > +				};
> > > > +				core2 {
> > > > +					cpu = <&cpu2>;
> > > > +				};
> > > > +				core3 {
> > > > +					cpu = <&cpu3>;
> > > > +				};
> > > > +			};
> > > > +		};
> > > > +
> > > > +		cpu0: cpu@0 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			reg = <0x0>;
> > > > +			enable-method = "psci";
> > > > +		};
> > > > +
> > > > +		cpu1: cpu@1 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			reg = <0x1>;
> > > > +			enable-method = "psci";
> > > > +		};
> > > > +
> > > > +		cpu2: cpu@2 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			reg = <0x2>;
> > > > +			enable-method = "psci";
> > > > +		};
> > > > +
> > > > +		cpu3: cpu@3 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			reg = <0x3>;
> > > > +			enable-method = "psci";
> > > > +		};
> > > > +	};
> > > > +
> > > > +	psci {
> > > > +		compatible = "arm,psci-1.0", "arm,psci-0.2";
> > > > +		method = "smc";
> > > > +	};
> > > > +
> > > > +	timer {
> > > > +		compatible = "arm,armv8-timer";
> > > > +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > > > +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > > > +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > > > +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> > > > +	};
> > > > +
> > > > +	xin24m: xin24m {
> > > 
> > > Please use name for all fixed clocks which matches current format
> > > recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
> > 
> > Will be fixed in next revision.
> 
> Hmm, why should we apply that rule to the xin24m clock, which is
> named exactly like that everywhere else in Rockchip SoC dtsi files?
> It's much better to remain consistent.

Historical reasons should not affect new code. And if we don't follow
the new rules now, they may never get followed.

Best regards,
Yao Zi

> 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1
> > > 
> > > > +		compatible = "fixed-clock";
> > > > +		#clock-cells = <0>;
> > > > +		clock-frequency = <24000000>;
> > > > +		clock-output-names = "xin24m";
> > > > +	};
> > > > +
> > > > +	gic: interrupt-controller@fed01000 {
> > > 
> > > Why this all is outside of SoC?
> > 
> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> > a "soc" node. I didn't know why before but just follow the style.
> > 
> > If you prefer add a soc node, I am willing to.

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 04/08/2024 15:20, Yao Zi wrote:
>>
>>> +		compatible = "fixed-clock";
>>> +		#clock-cells = <0>;
>>> +		clock-frequency = <24000000>;
>>> +		clock-output-names = "xin24m";
>>> +	};
>>> +
>>> +	gic: interrupt-controller@fed01000 {
>>
>> Why this all is outside of SoC?
> 
> Just as Heiko says, device tree for all other Rockchip SoCs don't have
> a "soc" node. I didn't know why before but just follow the style.
> 
> If you prefer add a soc node, I am willing to.

Surprising as usually we expect MMIO nodes being part of SoC to be under
soc@, but if that's Rockchip preference then fine.

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
> On 04/08/2024 15:20, Yao Zi wrote:
> >>
> >>> +		compatible = "fixed-clock";
> >>> +		#clock-cells = <0>;
> >>> +		clock-frequency = <24000000>;
> >>> +		clock-output-names = "xin24m";
> >>> +	};
> >>> +
> >>> +	gic: interrupt-controller@fed01000 {
> >>
> >> Why this all is outside of SoC?
> > 
> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> > a "soc" node. I didn't know why before but just follow the style.
> > 
> > If you prefer add a soc node, I am willing to.
> 
> Surprising as usually we expect MMIO nodes being part of SoC to be under
> soc@, but if that's Rockchip preference then fine.
> 

Okay, then I would leave it as is.

For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
follows the new rule?

Best regards,
Yao Zi

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Dragan Simic]

On 2024-08-05 12:59, Yao Zi wrote:
> On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
>> On 04/08/2024 15:20, Yao Zi wrote:
>> >>
>> >>> +		compatible = "fixed-clock";
>> >>> +		#clock-cells = <0>;
>> >>> +		clock-frequency = <24000000>;
>> >>> +		clock-output-names = "xin24m";
>> >>> +	};
>> >>> +
>> >>> +	gic: interrupt-controller@fed01000 {
>> >>
>> >> Why this all is outside of SoC?
>> >
>> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
>> > a "soc" node. I didn't know why before but just follow the style.
>> >
>> > If you prefer add a soc node, I am willing to.
>> 
>> Surprising as usually we expect MMIO nodes being part of SoC to be 
>> under
>> soc@, but if that's Rockchip preference then fine.
>> 
> 
> Okay, then I would leave it as is.
> 
> For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
> follows the new rule?

I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m"
part in /sys listing(s), for example, can only be helpful.

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Am Montag, 5. August 2024, 13:37:11 CEST schrieb Dragan Simic:
> On 2024-08-05 12:59, Yao Zi wrote:
> > On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
> >> On 04/08/2024 15:20, Yao Zi wrote:
> >> >>
> >> >>> +		compatible = "fixed-clock";
> >> >>> +		#clock-cells = <0>;
> >> >>> +		clock-frequency = <24000000>;
> >> >>> +		clock-output-names = "xin24m";
> >> >>> +	};
> >> >>> +
> >> >>> +	gic: interrupt-controller@fed01000 {
> >> >>
> >> >> Why this all is outside of SoC?
> >> >
> >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> >> > a "soc" node. I didn't know why before but just follow the style.
> >> >
> >> > If you prefer add a soc node, I am willing to.
> >> 
> >> Surprising as usually we expect MMIO nodes being part of SoC to be 
> >> under
> >> soc@, but if that's Rockchip preference then fine.
> >> 
> > 
> > Okay, then I would leave it as is.
> > 
> > For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
> > follows the new rule?
> 
> I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m"
> part in /sys listing(s), for example, can only be helpful.

I would second that :-) . Like on a number of boards we have for example
125MHz gmac clock generators ... with 2 gmacs, there are 2 of them.

I'm not sure the preferred name accounts for that?

Similarly we also keep the naming in the regulator node,
it's regulator-vcc3v3-somename {} instead of just regulator-3v3 {}.

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

On Mon, Aug 05, 2024 at 01:47:45PM +0200, Heiko Stübner wrote:
> Am Montag, 5. August 2024, 13:37:11 CEST schrieb Dragan Simic:
> > On 2024-08-05 12:59, Yao Zi wrote:
> > > On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
> > >> On 04/08/2024 15:20, Yao Zi wrote:
> > >> >>
> > >> >>> +		compatible = "fixed-clock";
> > >> >>> +		#clock-cells = <0>;
> > >> >>> +		clock-frequency = <24000000>;
> > >> >>> +		clock-output-names = "xin24m";
> > >> >>> +	};
> > >> >>> +
> > >> >>> +	gic: interrupt-controller@fed01000 {
> > >> >>
> > >> >> Why this all is outside of SoC?
> > >> >
> > >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
> > >> > a "soc" node. I didn't know why before but just follow the style.
> > >> >
> > >> > If you prefer add a soc node, I am willing to.
> > >> 
> > >> Surprising as usually we expect MMIO nodes being part of SoC to be 
> > >> under
> > >> soc@, but if that's Rockchip preference then fine.
> > >> 
> > > 
> > > Okay, then I would leave it as is.
> > > 
> > > For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
> > > follows the new rule?
> > 
> > I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m"
> > part in /sys listing(s), for example, can only be helpful.
> 
> I would second that :-) . Like on a number of boards we have for example
> 125MHz gmac clock generators ... with 2 gmacs, there are 2 of them.
> 
> I'm not sure the preferred name accounts for that?
> 
> Similarly we also keep the naming in the regulator node,
> it's regulator-vcc3v3-somename {} instead of just regulator-3v3 {}.
> 

"clock-xin24m" wouldn't be more descriptive than "clock-24m" and there
are usually only a few fixed clocks in dt, thus finding corresponding
definition isn't a problem I think.

For the gmac case, Krzysztof, do you think something like
"clock-125m-gmac1" is acceptable, just like what has been done for
regulators?

Best regards,
Yao Zi

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 05/08/2024 18:22, Yao Zi wrote:
> On Mon, Aug 05, 2024 at 01:47:45PM +0200, Heiko Stübner wrote:
>> Am Montag, 5. August 2024, 13:37:11 CEST schrieb Dragan Simic:
>>> On 2024-08-05 12:59, Yao Zi wrote:
>>>> On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
>>>>> On 04/08/2024 15:20, Yao Zi wrote:
>>>>>>>
>>>>>>>> +		compatible = "fixed-clock";
>>>>>>>> +		#clock-cells = <0>;
>>>>>>>> +		clock-frequency = <24000000>;
>>>>>>>> +		clock-output-names = "xin24m";
>>>>>>>> +	};
>>>>>>>> +
>>>>>>>> +	gic: interrupt-controller@fed01000 {
>>>>>>>
>>>>>>> Why this all is outside of SoC?
>>>>>>
>>>>>> Just as Heiko says, device tree for all other Rockchip SoCs don't have
>>>>>> a "soc" node. I didn't know why before but just follow the style.
>>>>>>
>>>>>> If you prefer add a soc node, I am willing to.
>>>>>
>>>>> Surprising as usually we expect MMIO nodes being part of SoC to be 
>>>>> under
>>>>> soc@, but if that's Rockchip preference then fine.
>>>>>
>>>>
>>>> Okay, then I would leave it as is.
>>>>
>>>> For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
>>>> follows the new rule?
>>>
>>> I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m"
>>> part in /sys listing(s), for example, can only be helpful.
>>
>> I would second that :-) . Like on a number of boards we have for example
>> 125MHz gmac clock generators ... with 2 gmacs, there are 2 of them.
>>
>> I'm not sure the preferred name accounts for that?
>>
>> Similarly we also keep the naming in the regulator node,
>> it's regulator-vcc3v3-somename {} instead of just regulator-3v3 {}.
>>
> 
> "clock-xin24m" wouldn't be more descriptive than "clock-24m" and there
> are usually only a few fixed clocks in dt, thus finding corresponding
> definition isn't a problem I think.
> 
> For the gmac case, Krzysztof, do you think something like
> "clock-125m-gmac1" is acceptable, just like what has been done for
> regulators?
>

How both above fit the generic node naming rule? You add now specific
part - purpose of the clock. The purpose is obvious from
clock-output-names or label. Anyway, not important topic to nak these
patches.

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Dragan Simic]

Hello Yao,

On 2024-08-05 18:22, Yao Zi wrote:
> On Mon, Aug 05, 2024 at 01:47:45PM +0200, Heiko Stübner wrote:
>> Am Montag, 5. August 2024, 13:37:11 CEST schrieb Dragan Simic:
>> > On 2024-08-05 12:59, Yao Zi wrote:
>> > > On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote:
>> > >> On 04/08/2024 15:20, Yao Zi wrote:
>> > >> >>
>> > >> >>> +		compatible = "fixed-clock";
>> > >> >>> +		#clock-cells = <0>;
>> > >> >>> +		clock-frequency = <24000000>;
>> > >> >>> +		clock-output-names = "xin24m";
>> > >> >>> +	};
>> > >> >>> +
>> > >> >>> +	gic: interrupt-controller@fed01000 {
>> > >> >>
>> > >> >> Why this all is outside of SoC?
>> > >> >
>> > >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have
>> > >> > a "soc" node. I didn't know why before but just follow the style.
>> > >> >
>> > >> > If you prefer add a soc node, I am willing to.
>> > >>
>> > >> Surprising as usually we expect MMIO nodes being part of SoC to be
>> > >> under
>> > >> soc@, but if that's Rockchip preference then fine.
>> > >>
>> > >
>> > > Okay, then I would leave it as is.
>> > >
>> > > For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and
>> > > follows the new rule?
>> >
>> > I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m"
>> > part in /sys listing(s), for example, can only be helpful.
>> 
>> I would second that :-) . Like on a number of boards we have for 
>> example
>> 125MHz gmac clock generators ... with 2 gmacs, there are 2 of them.
>> 
>> I'm not sure the preferred name accounts for that?
>> 
>> Similarly we also keep the naming in the regulator node,
>> it's regulator-vcc3v3-somename {} instead of just regulator-3v3 {}.
> 
> "clock-xin24m" wouldn't be more descriptive than "clock-24m" and there
> are usually only a few fixed clocks in dt, thus finding corresponding
> definition isn't a problem I think.

Well, using "clock-xin24m" comes with another benefit, which is using
the same "xin24m" as in the actual clock name.  That way, the same clock
name gets used in various /sys listings and in the debug clock summary
in /sys.  Having that kind of consistency can only be beneficial.

> For the gmac case, Krzysztof, do you think something like
> "clock-125m-gmac1" is acceptable, just like what has been done for
> regulators?

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 04/08/2024 16:05, Krzysztof Kozlowski wrote:
> On 04/08/2024 15:20, Yao Zi wrote:
>>>
>>>> +		compatible = "fixed-clock";
>>>> +		#clock-cells = <0>;
>>>> +		clock-frequency = <24000000>;
>>>> +		clock-output-names = "xin24m";
>>>> +	};
>>>> +
>>>> +	gic: interrupt-controller@fed01000 {
>>>
>>> Why this all is outside of SoC?
>>
>> Just as Heiko says, device tree for all other Rockchip SoCs don't have
>> a "soc" node. I didn't know why before but just follow the style.
>>
>> If you prefer add a soc node, I am willing to.
> 
> Surprising as usually we expect MMIO nodes being part of SoC to be under
> soc@, but if that's Rockchip preference then fine.

One more comment, I forgot we actually have it documented long time ago:

https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Am Dienstag, 13. August 2024, 18:38:58 CEST schrieb Krzysztof Kozlowski:
> On 04/08/2024 16:05, Krzysztof Kozlowski wrote:
> > On 04/08/2024 15:20, Yao Zi wrote:
> >>>
> >>>> +		compatible = "fixed-clock";
> >>>> +		#clock-cells = <0>;
> >>>> +		clock-frequency = <24000000>;
> >>>> +		clock-output-names = "xin24m";
> >>>> +	};
> >>>> +
> >>>> +	gic: interrupt-controller@fed01000 {
> >>>
> >>> Why this all is outside of SoC?
> >>
> >> Just as Heiko says, device tree for all other Rockchip SoCs don't have
> >> a "soc" node. I didn't know why before but just follow the style.
> >>
> >> If you prefer add a soc node, I am willing to.
> > 
> > Surprising as usually we expect MMIO nodes being part of SoC to be under
> > soc@, but if that's Rockchip preference then fine.
> 
> One more comment, I forgot we actually have it documented long time ago:
> 
> https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90

Thanks for finding that block.

I guess we'll follow that advice then and go with a soc node :-) .


Heiko

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Heiko Stübner]

Am Dienstag, 13. August 2024, 18:38:58 CEST schrieb Krzysztof Kozlowski:
> On 04/08/2024 16:05, Krzysztof Kozlowski wrote:
> > On 04/08/2024 15:20, Yao Zi wrote:
> >>>
> >>>> +		compatible = "fixed-clock";
> >>>> +		#clock-cells = <0>;
> >>>> +		clock-frequency = <24000000>;
> >>>> +		clock-output-names = "xin24m";
> >>>> +	};
> >>>> +
> >>>> +	gic: interrupt-controller@fed01000 {
> >>>
> >>> Why this all is outside of SoC?
> >>
> >> Just as Heiko says, device tree for all other Rockchip SoCs don't have
> >> a "soc" node. I didn't know why before but just follow the style.
> >>
> >> If you prefer add a soc node, I am willing to.
> > 
> > Surprising as usually we expect MMIO nodes being part of SoC to be under
> > soc@, but if that's Rockchip preference then fine.
> 
> One more comment, I forgot we actually have it documented long time ago:
> 
> https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90

I guess that piece of documentation should move to the dts style
guide though? Because it's not about writing bindings but how
to structure a dts/dtsi.

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Krzysztof Kozlowski]

On 15/08/2024 18:44, Heiko Stübner wrote:
>>
>> One more comment, I forgot we actually have it documented long time ago:
>>
>> https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90
> 
> I guess that piece of documentation should move to the dts style
> guide though? Because it's not about writing bindings but how
> to structure a dts/dtsi.

Yes, it should.

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Diederik de Haas]

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

On Saturday, 3 August 2024 14:55:10 CEST Yao Zi wrote:
> +       gic: interrupt-controller@fed01000 {
> +               compatible = "arm,gic-400";
> +               #interrupt-cells = <3>;
> +               #address-cells = <0>;
> +               interrupt-controller;
> +               reg = <0x0 0xfed01000 0 0x1000>,
> +                     <0x0 0xfed02000 0 0x2000>,
> +                     <0x0 0xfed04000 0 0x2000>,
> +                     <0x0 0xfed06000 0 0x2000>;
> +               interrupts = <GIC_PPI 9
> +                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +       };
> +
> +       uart0: serial@ff9f0000 {
> +               compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
> +               reg = <0x0 0xff9f0000 0x0 0x100>;
> +               interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> +               reg-shift = <2>;
> +               reg-io-width = <4>;
> +               clock-frequency = <24000000>;
> +               status = "disabled";
> +       };

The properties should be sorted as follows:
- compatible
- reg
- <other properties sorted alphabetically>
- status

This also applies to the other blocks which I didn't quote.

Cheers,
  Diederik

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 01:27:35PM +0200, Diederik de Haas wrote:
> On Saturday, 3 August 2024 14:55:10 CEST Yao Zi wrote:
> > +       gic: interrupt-controller@fed01000 {
> > +               compatible = "arm,gic-400";
> > +               #interrupt-cells = <3>;
> > +               #address-cells = <0>;
> > +               interrupt-controller;
> > +               reg = <0x0 0xfed01000 0 0x1000>,
> > +                     <0x0 0xfed02000 0 0x2000>,
> > +                     <0x0 0xfed04000 0 0x2000>,
> > +                     <0x0 0xfed06000 0 0x2000>;
> > +               interrupts = <GIC_PPI 9
> > +                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> > +       };
> > +
> > +       uart0: serial@ff9f0000 {
> > +               compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
> > +               reg = <0x0 0xff9f0000 0x0 0x100>;
> > +               interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > +               reg-shift = <2>;
> > +               reg-io-width = <4>;
> > +               clock-frequency = <24000000>;
> > +               status = "disabled";
> > +       };
> 
> The properties should be sorted as follows:
> - compatible
> - reg
> - <other properties sorted alphabetically>
> - status
> 
> This also applies to the other blocks which I didn't quote.

Thanks, will be fixed in next revision.

Best regards,
Yao Zi

[PATCH 4/4] arm64: dts: rockchip: Add Radxa e20c board

[Yao Zi]

Add board-level device tree for Radxa e20c board[1]. This basic
implementation supports boot into a kernel with only UART console.
Other features will be added later.

[1]: https://docs.radxa.com/en/e/e20c

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/arm64/boot/dts/rockchip/Makefile         |  1 +
 .../boot/dts/rockchip/rk3528-radxa-e20c.dts   | 22 +++++++++++++++++++
 2 files changed, 23 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index fda1b980eb4b..ecdd767d0323 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rockpro64.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire-excavator.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399pro-rock-pi-n10.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3528-radxa-e20c.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-d.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-s.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg353p.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
new file mode 100644
index 000000000000..534bd47e9971
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Rockchip Electronics Co., Ltd
+ * Copyright (c) 2024 Radxa Limited
+ * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
+ */
+
+/dts-v1/;
+#include "rk3528.dtsi"
+
+/ {
+	model = "Radxa E20C";
+	compatible = "radxa,e20c", "rockchip,rk3528";
+
+	chosen {
+		stdout-path = "serial5:115200n8";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
-- 
2.45.2

Re: [PATCH 4/4] arm64: dts: rockchip: Add Radxa e20c board

[Yao Zi]

On Sat, Aug 03, 2024 at 12:55:11PM +0000, Yao Zi wrote:
> Add board-level device tree for Radxa e20c board[1]. This basic
> implementation supports boot into a kernel with only UART console.
> Other features will be added later.
> 
> [1]: https://docs.radxa.com/en/e/e20c
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
>  .../boot/dts/rockchip/rk3528-radxa-e20c.dts   | 22 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index fda1b980eb4b..ecdd767d0323 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rockpro64.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire-excavator.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399pro-rock-pi-n10.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3528-radxa-e20c.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-d.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg-arc-s.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg353p.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> new file mode 100644
> index 000000000000..534bd47e9971
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020 Rockchip Electronics Co., Ltd
> + * Copyright (c) 2024 Radxa Limited
> + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> + */
> +
> +/dts-v1/;
> +#include "rk3528.dtsi"
> +
> +/ {
> +	model = "Radxa E20C";
> +	compatible = "radxa,e20c", "rockchip,rk3528";
> +
> +	chosen {
> +		stdout-path = "serial5:115200n8";

There is an error, stdout-path should be "serial0:1500000n8".
Sorry for my mistake, will be corrected in next revision.

Best regards,
Yao Zi

Re: [PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[Dragan Simic]

Hello all,

On 2024-08-03 14:55, Yao Zi wrote:
> Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for
> multimedia application. This series add a basic device tree with CPU,
> interrupts and UART nodes for it and is able to boot into a kernel with
> only UART console.
> 
> Has been tested on Radxa E20C board[1] with vendor U-boot, successfully
> booted into initramfs with this log[2].

I wonder will at least the RK3528 datasheet become available publicly?

> [1]: https://docs.radxa.com/en/e/e20c
> [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319
> 
> Yao Zi (4):
>   dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528
>   dt-bindings: arm: rockchip: Add Radxa E20C board
>   arm64: dts: rockchip: Add base DT for rk3528 SoC
>   arm64: dts: rockchip: Add Radxa e20c board
> 
>  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
>  .../bindings/serial/snps-dw-apb-uart.yaml     |   1 +
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3528-radxa-e20c.dts   |  22 +++
>  arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 182 ++++++++++++++++++
>  5 files changed, 211 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> 
> 
> base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d

Re: [PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote:
> Hello all,
> 
> On 2024-08-03 14:55, Yao Zi wrote:
> > Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for
> > multimedia application. This series add a basic device tree with CPU,
> > interrupts and UART nodes for it and is able to boot into a kernel with
> > only UART console.
> > 
> > Has been tested on Radxa E20C board[1] with vendor U-boot, successfully
> > booted into initramfs with this log[2].
> 
> I wonder will at least the RK3528 datasheet become available publicly?

I found none for now, and I am not someone from Rockchip, thus don't
know whether they have a plan to make it public, either. But there has
been some devices shipping it already and getting them mainlined will
be a neat thing.

Just FYI, the vendor kernel is available here[1] on the "develop-5.10"
branch.

Best regards,
Yao Zi

[1]: https://github.com/rockchip-linux/kernel

> 
> > [1]: https://docs.radxa.com/en/e/e20c
> > [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319
> > 
> > Yao Zi (4):
> >   dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528
> >   dt-bindings: arm: rockchip: Add Radxa E20C board
> >   arm64: dts: rockchip: Add base DT for rk3528 SoC
> >   arm64: dts: rockchip: Add Radxa e20c board
> > 
> >  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
> >  .../bindings/serial/snps-dw-apb-uart.yaml     |   1 +
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3528-radxa-e20c.dts   |  22 +++
> >  arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 182 ++++++++++++++++++
> >  5 files changed, 211 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> > 
> > 
> > base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d

Re: [PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[Piotr Oniszczuk]

> Wiadomość napisana przez Yao Zi <ziyao@disroot.org> w dniu 04.08.2024, o godz. 08:22:
> 
> On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote:
>> Hello all,
>> 
>> On 2024-08-03 14:55, Yao Zi wrote:
>>> Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for
>>> multimedia application. This series add a basic device tree with CPU,
>>> interrupts and UART nodes for it and is able to boot into a kernel with
>>> only UART console.
>>> 
>>> Has been tested on Radxa E20C board[1] with vendor U-boot, successfully
>>> booted into initramfs with this log[2].
>> 
>> I wonder will at least the RK3528 datasheet become available publicly?
> 
> I found none for now, and I am not someone from Rockchip, thus don't
> know whether they have a plan to make it public, either. But there has
> been some devices shipping it already and getting them mainlined will
> be a neat thing.
> 

Maybe this hight be useful: at some point in time I started to hack with rk3528 support in mainline kernel.
HW I’m using is vontar rk3528 tvbox.

So far - on mainline 6.10 kernel - I got working: clocks, pin control, uart, sdcard, eth, usb.

I started to work on dw hdmi and got to stage with correct mode sets by dw-hdmi but I stuck with vop3 support.

Generally - I was back porting code from bsp but rockchip bsp is too divergent from mainline to my back porting skills :-(

If anybody can help with vop3 - then we can have quite good initial support of rk3528 in mainline (the, sdcard, usb, eth, hdmi)
gpu, hdmi audio should be probably easy…

My rk3528 enablements:

Bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1100-dt-bindings-clock-add-rk3528-clock-definitions.patch

Clocks: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1101-clk-rockchip-add-clock-controller-for-the-RK3528.patch

Pincontrol: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1102-pinctrl-rockchip-add-rk3528-support.patch

Power domain bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1103-dt-bindings-power-add-RK3528-SoCs-header-for-idle.patch

Ethernet support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1104-ethernet-stmmac-dwmac-rk3528-add-GMAC-support.patch

Power domains support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1105-soc-rockchip-power-domain-add-rk3528-support.patch

usb: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1106-phy-rockchip-inno-usb2-add-phy-support-for-rk3528.patch

SoC power domains: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-soc-rockchip-power-domain-add-rk3528-support.patch

Thermal: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-thermal-rockchip-add-support-for-rk3528.patch

Naneng phy: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1109-phy-rockchip-naneng-combphy-add-support-for-rk3528.patch

dw hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1110-phy-rockchip-inno-hdmi-add-support-for-rk3528.patch

Inno hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1111-drm-rockchip-dw_hdmi-add-support-for-rk3528.patch       Nvmem: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1113-nvmem-rockchip-otp-add-support-for-rk3528.patch

Sound soc: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1114-sound-soc-codecs-add-rk3528-support.patch

rk3528 dtsi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1150-arm64-dtsi-rockchip-add-3528.dtsi.patch: 

Vontar tvbox dts: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1151-arm64-dts-rockchip-add-dts-for-vontar_r3.patch

It may happen that above patches will not apply clean on vanilla 6.10 as I’m patching kernel for s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399//3566/3568/3588/3528 and rk3528 patches are after applied s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399/3566/3568/3588 patches.

If anybody is interested in adding vop3 support - pls give me sign - we can work together…
 

> Just FYI, the vendor kernel is available here[1] on the "develop-5.10"
> branch.
> 
> Best regards,
> Yao Zi
> 
> [1]: https://github.com/rockchip-linux/kernel
> 
>> 
>>> [1]: https://docs.radxa.com/en/e/e20c
>>> [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319
>>> 
>>> Yao Zi (4):
>>>  dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528
>>>  dt-bindings: arm: rockchip: Add Radxa E20C board
>>>  arm64: dts: rockchip: Add base DT for rk3528 SoC
>>>  arm64: dts: rockchip: Add Radxa e20c board
>>> 
>>> .../devicetree/bindings/arm/rockchip.yaml     |   5 +
>>> .../bindings/serial/snps-dw-apb-uart.yaml     |   1 +
>>> arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>> .../boot/dts/rockchip/rk3528-radxa-e20c.dts   |  22 +++
>>> arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 182 ++++++++++++++++++
>>> 5 files changed, 211 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
>>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
>>> 
>>> 
>>> base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/4] Add initial support for Rockchip RK3528 SoC

[Yao Zi]

On Sun, Aug 04, 2024 at 11:13:59AM +0200, Piotr Oniszczuk wrote:
> 
> 
> > Wiadomość napisana przez Yao Zi <ziyao@disroot.org> w dniu 04.08.2024, o godz. 08:22:
> > 
> > On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote:
> >> Hello all,
> >> 
> >> On 2024-08-03 14:55, Yao Zi wrote:
> >>> Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for
> >>> multimedia application. This series add a basic device tree with CPU,
> >>> interrupts and UART nodes for it and is able to boot into a kernel with
> >>> only UART console.
> >>> 
> >>> Has been tested on Radxa E20C board[1] with vendor U-boot, successfully
> >>> booted into initramfs with this log[2].
> >> 
> >> I wonder will at least the RK3528 datasheet become available publicly?
> > 
> > I found none for now, and I am not someone from Rockchip, thus don't
> > know whether they have a plan to make it public, either. But there has
> > been some devices shipping it already and getting them mainlined will
> > be a neat thing.
> > 
> 
> Maybe this hight be useful: at some point in time I started to hack with rk3528 support in mainline kernel.
> HW I’m using is vontar rk3528 tvbox.
> 
> So far - on mainline 6.10 kernel - I got working: clocks, pin control, uart, sdcard, eth, usb.
> 
> I started to work on dw hdmi and got to stage with correct mode sets by dw-hdmi but I stuck with vop3 support.
> 
> Generally - I was back porting code from bsp but rockchip bsp is too divergent from mainline to my back porting skills :-(
> 
> If anybody can help with vop3 - then we can have quite good initial support of rk3528 in mainline (the, sdcard, usb, eth, hdmi)
> gpu, hdmi audio should be probably easy…
> 
> My rk3528 enablements:
> 
> Bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1100-dt-bindings-clock-add-rk3528-clock-definitions.patch
> 
> Clocks: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1101-clk-rockchip-add-clock-controller-for-the-RK3528.patch
> 
> Pincontrol: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1102-pinctrl-rockchip-add-rk3528-support.patch
> 
> Power domain bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1103-dt-bindings-power-add-RK3528-SoCs-header-for-idle.patch
> 
> Ethernet support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1104-ethernet-stmmac-dwmac-rk3528-add-GMAC-support.patch
> 
> Power domains support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1105-soc-rockchip-power-domain-add-rk3528-support.patch
> 
> usb: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1106-phy-rockchip-inno-usb2-add-phy-support-for-rk3528.patch
> 
> SoC power domains: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-soc-rockchip-power-domain-add-rk3528-support.patch
> 
> Thermal: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-thermal-rockchip-add-support-for-rk3528.patch
> 
> Naneng phy: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1109-phy-rockchip-naneng-combphy-add-support-for-rk3528.patch
> 
> dw hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1110-phy-rockchip-inno-hdmi-add-support-for-rk3528.patch
> 
> Inno hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1111-drm-rockchip-dw_hdmi-add-support-for-rk3528.patch       Nvmem: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1113-nvmem-rockchip-otp-add-support-for-rk3528.patch
> 
> Sound soc: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1114-sound-soc-codecs-add-rk3528-support.patch
> 
> rk3528 dtsi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1150-arm64-dtsi-rockchip-add-3528.dtsi.patch: 

FYI, This link is dead.

> Vontar tvbox dts: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1151-arm64-dts-rockchip-add-dts-for-vontar_r3.patch
> 
> It may happen that above patches will not apply clean on vanilla 6.10 as I’m patching kernel for s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399//3566/3568/3588/3528 and rk3528 patches are after applied s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399/3566/3568/3588 patches.
> 
> If anybody is interested in adding vop3 support - pls give me sign - we can work together…
> 
> > Just FYI, the vendor kernel is available here[1] on the "develop-5.10"
> > branch.
> > 
> > Best regards,
> > Yao Zi
> > 
> > [1]: https://github.com/rockchip-linux/kernel
> > 
> >> 
> >>> [1]: https://docs.radxa.com/en/e/e20c
> >>> [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319
> >>> 
> >>> Yao Zi (4):
> >>>  dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528
> >>>  dt-bindings: arm: rockchip: Add Radxa E20C board
> >>>  arm64: dts: rockchip: Add base DT for rk3528 SoC
> >>>  arm64: dts: rockchip: Add Radxa e20c board
> >>> 
> >>> .../devicetree/bindings/arm/rockchip.yaml     |   5 +
> >>> .../bindings/serial/snps-dw-apb-uart.yaml     |   1 +
> >>> arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >>> .../boot/dts/rockchip/rk3528-radxa-e20c.dts   |  22 +++
> >>> arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 182 ++++++++++++++++++
> >>> 5 files changed, 211 insertions(+)
> >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
> >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >>> 
> >>> 
> >>> base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

Best regards,
Yao Zi