[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f56e9c68-0745-43c1-ae80-e2dc0942ea07@kernel.org>
Date: Thu, 20 Feb 2025 13:23:03 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Peter Chen <peter.chen@...tech.com>, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, catalin.marinas@....com, will@...nel.org, arnd@...db.de
Cc: linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, cix-kernel-upstream@...tech.com,
"Fugang . duan" <fugang.duan@...tech.com>
Subject: Re: [PATCH 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support
On 20/02/2025 09:40, Peter Chen wrote:
> CIX SKY1 SoC is high performance Armv9 SoC designed by Cixtech,
> and Orion O6 is open source motherboard launched by Radxa.
> See below for detail:
> https://docs.radxa.com/en/orion/o6/getting-started/introduction
>
> In this commit, it only adds limited components for running initramfs
> at Orion O6.
>
Test the patches before posting.
> Acked-by: Fugang.duan <fugang.duan@...tech.com>
Again, difference in names...
> Signed-off-by: Peter Chen <peter.chen@...tech.com>
> ---
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/cix/Makefile | 2 +
> arch/arm64/boot/dts/cix/sky1-orion-o6.dts | 21 ++
> arch/arm64/boot/dts/cix/sky1.dtsi | 264 ++++++++++++++++++++++
> 4 files changed, 288 insertions(+)
> create mode 100644 arch/arm64/boot/dts/cix/Makefile
> create mode 100644 arch/arm64/boot/dts/cix/sky1-orion-o6.dts
> create mode 100644 arch/arm64/boot/dts/cix/sky1.dtsi
>
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 79b73a21ddc2..8e7ccd0027bd 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -13,6 +13,7 @@ subdir-y += bitmain
> subdir-y += blaize
> subdir-y += broadcom
> subdir-y += cavium
> +subdir-y += cix
> subdir-y += exynos
> subdir-y += freescale
> subdir-y += hisilicon
> diff --git a/arch/arm64/boot/dts/cix/Makefile b/arch/arm64/boot/dts/cix/Makefile
> new file mode 100644
> index 000000000000..ed3713982012
> --- /dev/null
> +++ b/arch/arm64/boot/dts/cix/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_CIX) += sky1-orion-o6.dtb
> diff --git a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
> new file mode 100644
> index 000000000000..dbee1616076d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright 2025 Cix Technology Group Co., Ltd.
> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "sky1.dtsi"
> +/ {
> + model = "Radxa Orion O6";
> + compatible = "radxa,orion-o6";
Never tested.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.
> +
> + chosen {
> + stdout-path = &uart2;
> + };
> +};
> +
> +&uart2 {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
> new file mode 100644
> index 000000000000..d98735f782e0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright 2025 Cix Technology Group Co., Ltd.
> + *
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + aliases {
> + serial2 = &uart2;
This is not part of the SoC, but board.
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&CPU0>;
Labels are lowercase.
> + };
> + core1 {
> + cpu = <&CPU1>;
> + };
> + core2 {
> + cpu = <&CPU2>;
> + };
> + core3 {
> + cpu = <&CPU3>;
> + };
> + core4 {
> + cpu = <&CPU4>;
> + };
> + core5 {
> + cpu = <&CPU5>;
> + };
> + core6 {
> + cpu = <&CPU6>;
> + };
> + core7 {
> + cpu = <&CPU7>;
> + };
> + core8 {
> + cpu = <&CPU8>;
> + };
> + core9 {
> + cpu = <&CPU9>;
> + };
> + core10 {
> + cpu = <&CPU10>;
> + };
> + core11 {
> + cpu = <&CPU11>;
> + };
> + };
> + };
> +
> + CPU0: cpu0@0 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
cpu@
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x0>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + CPU1: cpu1@100 {
cpu@
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x100>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + CPU2: cpu2@200 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x200>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + CPU3: cpu3@300 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x300>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + CPU4: cpu4@400 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x400>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU5: cpu5@500 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x500>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU6: cpu6@600 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x600>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU7: cpu7@700 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x700>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU8: cpu8@800 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x800>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU9: cpu9@900 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0x900>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU10: cpu10@a00 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0xa00>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + CPU11: cpu11@b00 {
> + compatible = "arm,armv8";
> + enable-method = "psci";
> + reg = <0x0 0xb00>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> + };
> +
> + arch_timer: timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> + clock-frequency = <1000000000>;
> + interrupt-parent = <&gic>;
> + arm,no-tick-in-suspend;
> + };
> +
> + memory@...00000 {
This does not look like part of SoC.
> + #address-cells = <2>;
> + #size-cells = <2>;
> + device_type = "memory";
> + reg = <0x00000000 0x80000000 0x1 0x00000000>;
Order properties according to DTS coding style.
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + pmu: pmu {
Keep some sort of order. See DTS coding style.
> + compatible = "arm,armv8-pmuv3";
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-parent = <&gic>;
> + status = "okay";
Where did you disable it? Why only this has status=okay but other do not?
> + };
> +
> + pmu_spe: pmu_spe {
DTS coding style. No underscores.
> + compatible = "arm,statistical-profiling-extension-v1";
> + interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-parent = <&gic>;
> + status = "okay";
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + linux,cma {
This does not look like part of SoC.
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0x0 0x28000000>;
> + linux,cma-default;
> + };
> +
> + };
> +
> + sky1_fixed_clocks: fixed-clocks {
> + uartclk: uartclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + clock-output-names = "uartclk";
> + };
> +
> + uart_apb_pclk: uart_apb_pclk {
This code is not getting better...
Where are these clocks physically?
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <200000000>;
> + clock-output-names = "apb_pclk";
> + };
> + };
> +
> + soc@0 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + dma-ranges;
> +
> + uart2: uart@...d0000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x0 0x040d0000 0x0 0x1000>;
> + interrupts = <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "uartclk", "apb_pclk";
> + clocks = <&uartclk>, <&uart_apb_pclk>;
> + status = "disabled";
> + };
> +
> + gic: interrupt-controller@...01000 {
> + compatible = "arm,gic-v3";
> + #address-cells = <2>;
> + #interrupt-cells = <3>;
> + #size-cells = <2>;
> + ranges;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + reg = <0x0 0x0e010000 0 0x10000>, /* GICD */
> + <0x0 0x0e090000 0 0x300000>; /* GICR * 12 */
> + redistributor-stride = <0x40000>;
DTS coding style.
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-parent = <&gic>;
> +
> + its_pcie: its@...0000 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + reg = <0x0 0x0e050000 0x0 0x30000>;
> + };
> + };
> + };
> +};
Best regards,
Krzysztof
Powered by blists - more mailing lists