| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <86r03ip0kf.wl-maz@kernel.org>
Date: Fri, 28 Feb 2025 15:10:24 +0000
From: Marc Zyngier <maz@...nel.org>
To: Peter Chen <peter.chen@...tech.com>
Cc: robh@...nel.org,
krzk+dt@...nel.org,
conor+dt@...nel.org,
catalin.marinas@....com,
will@...nel.org,
arnd@...db.de,
linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
cix-kernel-upstream@...tech.com,
marcin@...zkiewicz.com.pl,
Fugang Duan <fugang.duan@...tech.com>
Subject: Re: [PATCH v3 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support
On Thu, 27 Feb 2025 12:06:19 +0000,
Peter Chen <peter.chen@...tech.com> 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.
>
> Acked-by: Fugang Duan <fugang.duan@...tech.com>
> Signed-off-by: Peter Chen <peter.chen@...tech.com>
> ---
> Changes for v3:
> - Fix two dts coding sytle issues
>
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/cix/Makefile | 2 +
> arch/arm64/boot/dts/cix/sky1-orion-o6.dts | 26 +++
> arch/arm64/boot/dts/cix/sky1.dtsi | 216 ++++++++++++++++++++++
> 4 files changed, 245 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..78f4fcd87216
> --- /dev/null
> +++ b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
> @@ -0,0 +1,26 @@
> +// 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", "cix,sky1";
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0x0 0x28000000>;
> + linux,cma-default;
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
> new file mode 100644
> index 000000000000..c6d7a48e9893
> --- /dev/null
> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
> @@ -0,0 +1,216 @@
> +// 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>;
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + compatible = "arm,cortex-a520";
> + enable-method = "psci";
> + reg = <0x0 0x0>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + cpu1: cpu@100 {
> + compatible = "arm,cortex-a520";
> + enable-method = "psci";
> + reg = <0x0 0x100>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + cpu2: cpu@200 {
> + compatible = "arm,cortex-a520";
> + enable-method = "psci";
> + reg = <0x0 0x200>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + cpu3: cpu@300 {
> + compatible = "arm,cortex-a520";
> + enable-method = "psci";
> + reg = <0x0 0x300>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <403>;
> + };
> +
> + cpu4: cpu@400 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x400>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu5: cpu@500 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x500>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu6: cpu@600 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x600>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu7: cpu@700 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x700>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu8: cpu@800 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x800>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu9: cpu@900 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0x900>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu10: cpu@a00 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0xa00>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
> +
> + cpu11: cpu@b00 {
> + compatible = "arm,cortex-a720";
> + enable-method = "psci";
> + reg = <0x0 0xb00>;
> + device_type = "cpu";
> + capacity-dmips-mhz = <1024>;
> + };
Given that half the A720s are advertised with lower clock speed, how
comes they all have the same capacity?
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> + 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>;
> + };
> + };
> + };
> + };
> +
> + pmu-a520 {
> + compatible = "arm,cortex-a520-pmu";
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> + };
> +
> + pmu-a720 {
> + compatible = "arm,cortex-a720-pmu";
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> + };
This is wrong. The default configuration for PPIs is to expose the
*same* device on all CPUs. You must use PPI affinities for your PMUs.
Please see the GICv3 binding for the details.
> +
> + pmu-spe {
> + compatible = "arm,statistical-profiling-extension-v1";
> + interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_LOW>;
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + soc@0 {
> + compatible = "simple-bus";
> + ranges = <0 0 0 0 0x20 0>;
> + dma-ranges;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + gic: interrupt-controller@...0000 {
> + compatible = "arm,gic-v3";
> + reg = <0x0 0x0e010000 0 0x10000>, /* GICD */
> + <0x0 0x0e090000 0 0x300000>; /* GICR * 12 */
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> + #interrupt-cells = <3>;
This will need to be bumped up to 4, and all the interrupt specifiers adjusted.
> + interrupt-controller;
> + #redistributor-regions = <1>;
Drop this, this is useless. It is pretty obvious that there is a
single RD region, and 1 is the default.
> + redistributor-stride = <0 0x40000>;
Drop this. This is a standard GIC700 that doesn't need any help
computing the stride as it obeys the architecture.
> + #address-cells = <2>;
> + #size-cells = <2>;
I don't understand why you repeat this on every sub-nodes.
> + ranges;
> +
> + gic_its: msi-controller@...0000 {
> + compatible = "arm,gic-v3-its";
> + reg = <0x0 0x0e050000 0x0 0x30000>;
> + msi-controller;
> + #msi-cells = <1>;
> + };
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupt-names = "sec-phys", "phys", "virt", "hyp-phys", "hyp-virt";
> + 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>,
> + <GIC_PPI 12 IRQ_TYPE_LEVEL_LOW>;
> + clock-frequency = <1000000000>;
Drop this. The firmware already sets CNTFRQ_EL0 to the correct value,
it seems. And if it doesn't, please fix the firmware.
> + arm,no-tick-in-suspend;
Why do you need this? Is the HW so broken that you have implemented
the global counter in a power domain that isn't always on?
As it stands, this DT is completely broken and needs major fixing.
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists