Re: [PATCH 3/9] arm64: dts: rockchip: Prepare Rockchip RK1808

From: Marc Zyngier <maz@kernel.org>
To: "Andreas Färber" <afaerber@suse.de>
Cc: linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 3/9] arm64: dts: rockchip: Prepare Rockchip RK1808
Date: Mon, 17 May 2021 10:21:04 +0100	[thread overview]
Message-ID: <87h7j1vhq7.wl-maz@kernel.org> (raw)
In-Reply-To: <20210516230551.12469-4-afaerber@suse.de>

On Mon, 17 May 2021 00:05:45 +0100,
Andreas Färber <afaerber@suse.de> wrote:
> 
> Add an initial Device Tree for Rockchip RK1808 SoC.
> Based on shipping TB-RK1808M0 DTB.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  arch/arm64/boot/dts/rockchip/rk1808.dtsi | 203 +++++++++++++++++++++++
>  1 file changed, 203 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk1808.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk1808.dtsi b/arch/arm64/boot/dts/rockchip/rk1808.dtsi
> new file mode 100644
> index 000000000000..af2b51afda7d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk1808.dtsi
> @@ -0,0 +1,203 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
> +/*
> + * Copyright (c) 2021 Andreas Färber
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	compatible = "rockchip,rk1808";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +		serial4 = &uart4;
> +		serial5 = &uart5;
> +		serial6 = &uart6;
> +		serial7 = &uart7;
> +	};
> +
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x0>;
> +			enable-method = "psci";
> +			cpu-idle-states = <&CPU_SLEEP>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x1>;
> +			enable-method = "psci";
> +			cpu-idle-states = <&CPU_SLEEP>;
> +		};
> +
> +		idle-states {
> +			entry-method = "psci";
> +
> +			CPU_SLEEP: cpu-sleep {
> +				compatible = "arm,idle-state";
> +				local-timer-stop;
> +				arm,psci-suspend-param = <0x10000>;
> +				entry-latency-us = <120>;
> +				exit-latency-us = <250>;
> +				min-residency-us = <900>;
> +			};
> +		};
> +	};
> +
> +	arm-pmu {
> +		compatible = "arm,cortex-a35-pmu";
> +		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-affinity = <&cpu0>, <&cpu1>;
> +	};
> +
> +	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>;
> +		arm,no-tick-in-suspend;

Another facepalm moment...

> +	};
> +
> +	xin24m: xin24m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		#clock-cells = <0>;
> +		clock-output-names = "xin24m";
> +	};
> +
> +	firmware {
> +		psci {
> +			compatible = "arm,psci-1.0";
> +			method = "smc";
> +		};
> +
> +		tee {
> +			compatible = "linaro,optee-tz";
> +			method = "smc";
> +		};
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		system_sram: sram@fec00000 {
> +			compatible = "mmio-sram";
> +			reg = <0xfec00000 0x200000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0xfec00000 0x200000>;
> +		};
> +
> +		gic: interrupt-controller@ff100000 {
> +			compatible = "arm,gic-v3";
> +			reg = <0xff100000 0x10000>, /* GICD */
> +			      <0xff140000 0xc0000>, /* GICR */

This is obviously wrong. You have two CPUs, and yet describe a range
that spans 6. I guess this is a copy paste from rk3399 again?

> +			      <0xff300000 0x10000>, /* GICC */
> +			      <0xff310000 0x10000>, /* GICH */
> +			      <0xff320000 0x10000>; /* GICV */
> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			gic_its: msi-controller@ff120000 {
> +				compatible = "arm,gic-v3-its";
> +				reg = <0xff120000 0x20000>;
> +				msi-controller;
> +				#msi-cells = <1>;
> +			};

What uses the ITS?

	M.

-- 
Without deviation from the norm, progress is not possible.