[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b6grcxwvic33ojj3o46sjjvlyc2xwcy247n7ls4lyzkwhd73gs@pxi533kjm3yx>
Date: Fri, 3 Jan 2025 08:53:36 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Vincenzo Frascino <vincenzo.frascino@....com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, Linus Walleij <linus.walleij@...aro.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Liviu Dudau <liviu.dudau@....com>,
Sudeep Holla <sudeep.holla@....com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH v3 2/4] arm64: dts: morello: Add support for common
functionalities
On Thu, Jan 02, 2025 at 03:54:14PM +0000, Vincenzo Frascino wrote:
> The Morello architecture is an experimental extension to Armv8.2-A,
> which extends the AArch64 state with the principles proposed in
> version 7 of the Capability Hardware Enhanced RISC Instructions
> (CHERI) ISA.
>
> The Morello Platform (soc) and the Fixed Virtual Platfom (fvp) share
> some functionalities that have conveniently been included in
> morello.dtsi to avoid duplication.
>
> Introduce morello.dtsi.
>
> Note: Morello fvp will be introduced with a future patch series.
>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@....com>
> ---
> arch/arm64/boot/dts/arm/morello.dtsi | 341 +++++++++++++++++++++++++++
> 1 file changed, 341 insertions(+)
> create mode 100644 arch/arm64/boot/dts/arm/morello.dtsi
>
> diff --git a/arch/arm64/boot/dts/arm/morello.dtsi b/arch/arm64/boot/dts/arm/morello.dtsi
> new file mode 100644
> index 000000000000..67bc960f4596
> --- /dev/null
> +++ b/arch/arm64/boot/dts/arm/morello.dtsi
> @@ -0,0 +1,341 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +/*
> + * Copyright (c) 2020-2024, Arm Limited. All rights reserved.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + interrupt-parent = <&gic>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
Drop
> +
> + clocks {
> + soc_refclk50mhz: clock-50000000 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + clock-output-names = "apb_pclk";
> + };
> +
> + soc_refclk85mhz: clock-85000000 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <85000000>;
> + clock-output-names = "iofpga:aclk";
> + };
> +
> + soc_uartclk: clock-50000000-uart {
Keep nodes sorted by name.
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + clock-output-names = "uartclk";
> + };
> +
> + dpu_aclk: dpu-aclk {
Keep consistent naming. If others use "clock-" then why this has
different pattern?
> + /* 77.1 MHz derived from 24 MHz reference clock */
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <350000000>;
> + clock-output-names = "aclk";
> + };
> +
> + dpu_pixel_clk: dpu-pixel-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <148500000>;
> + clock-output-names = "pxclk";
> + };
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + cpu0: cpu0@0 {
cpu@0
> + compatible = "arm,neoverse-n1";
> + reg = <0x0 0x0>;
> + device_type = "cpu";
> + enable-method = "psci";
> + clocks = <&scmi_dvfs 0>;
> + };
> +
> + cpu1: cpu1@100 {
cpu@100
This applies to the entire patchset.
> + compatible = "arm,neoverse-n1";
> + reg = <0x0 0x100>;
> + device_type = "cpu";
> + enable-method = "psci";
> + clocks = <&scmi_dvfs 0>;
> + };
> +
> + cpu2: cpu2@...00 {
> + compatible = "arm,neoverse-n1";
> + reg = <0x0 0x10000>;
> + device_type = "cpu";
> + enable-method = "psci";
> + clocks = <&scmi_dvfs 1>;
> + };
> +
> + cpu3: cpu3@...00 {
> + compatible = "arm,neoverse-n1";
> + reg = <0x0 0x10100>;
> + device_type = "cpu";
> + enable-method = "psci";
> + clocks = <&scmi_dvfs 1>;
> + };
Missing cache nodes and properties.
> + };
> +
> + firmware {
> + interrupt-parent = <&gic>;
> +
> + scmi {
> + compatible = "arm,scmi";
> + mbox-names = "tx", "rx";
> + mboxes = <&mailbox 1 0>, <&mailbox 1 1>;
> + shmem = <&cpu_scp_hpri0>, <&cpu_scp_hpri1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + scmi_dvfs: protocol@13 {
> + reg = <0x13>;
> + #clock-cells = <1>;
> + };
> +
> + scmi_clk: protocol@14 {
> + reg = <0x14>;
> + #clock-cells = <1>;
> + };
> + };
> + };
> +
> + /* The first bank of memory, memory map is actually provided by UEFI. */
> + memory@...00000 {
> + device_type = "memory";
> + /* [0x80000000-0xffffffff] */
> + reg = <0x00000000 0x80000000 0x0 0x7F000000>;
> + };
> +
> + memory@...0000000 {
> + device_type = "memory";
> + /* [0x8080000000-0x83f7ffffff] */
> + reg = <0x00000080 0x80000000 0x3 0x78000000>;
> + };
> +
> + pmu {
> + compatible = "arm,armv8-pmuv3";
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + secure-firmware@...00000 {
> + reg = <0 0xff000000 0 0x01000000>;
> + no-map;
> + };
> + };
> +
> + spe-pmu {
> + compatible = "arm,statistical-profiling-extension-v1";
> + interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + soc: soc {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-parent = <&gic>;
> + ranges;
> +
> + dp0: display@...00000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "arm,mali-d32", "arm,mali-d71";
I am pretty sure I asked for this: order properties according to DTS
coding style, not random.
> + reg = <0 0x2cc00000 0 0x20000>;
> + interrupts = <0 69 4>;
> + clocks = <&dpu_aclk>;
> + clock-names = "aclk";
> + iommus = <&smmu_dp 0>, <&smmu_dp 1>, <&smmu_dp 2>, <&smmu_dp 3>,
> + <&smmu_dp 8>;
> +
> + pl0: pipeline@0 {
> + reg = <0>;
> + clocks = <&dpu_pixel_clk>;
> + clock-names = "pxclk";
> + port {
> + dp_pl0_out0: endpoint {
> + remote-endpoint = <&tda998x_0_input>;
> + };
> + };
> + };
> + };
> +
> + i2c: i2c@...f0000 {
> + compatible = "cdns,i2c-r1p14";
> + reg = <0x0 0x1c0f0000 0x0 0x1000>;
And here order is correct... why each node is formatted differently?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + clock-frequency = <100000>;
This is usually property of the board, unless this is somehow blurred
here.
> + interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&dpu_aclk>;
> +
> + hdmi_tx: hdmi-transmitter@70 {
> + compatible = "nxp,tda998x";
> + reg = <0x70>;
> + video-ports = <0x234501>;
> + port {
> + tda998x_0_input: endpoint {
> + remote-endpoint = <&dp_pl0_out0>;
> + };
> + };
> + };
> + };
> +
> + gic: interrupt-controller@...10000 {
> + compatible = "arm,gic-v3";
> + #address-cells = <2>;
> + #interrupt-cells = <3>;
> + #size-cells = <2>;
> + ranges;
Missing reg.... and why do you have ranges here? No children.
> + interrupt-controller;
> + };
> +
> + smmu_dp: iommu@...00000 {
> + compatible = "arm,smmu-v3";
> + reg = <0 0x2ce00000 0 0x40000>;
> + interrupts = <GIC_SPI 76 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 80 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 78 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "eventq", "gerror", "cmdq-sync";
> + #iommu-cells = <1>;
> + };
> +
> + smmu_ccix: iommu@...00000 {
> + compatible = "arm,smmu-v3";
> + reg = <0 0x4f000000 0 0x40000>;
No hex here in reg but...
> + interrupts = <GIC_SPI 228 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 230 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 41 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 229 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "eventq", "gerror", "priq", "cmdq-sync";
> + msi-parent = <&its1 0>;
> + #iommu-cells = <1>;
> + dma-coherent;
> + };
> +
> + smmu_pcie: iommu@...00000 {
> + compatible = "arm,smmu-v3";
> + reg = <0 0x4f400000 0 0x40000>;
> + interrupts = <GIC_SPI 235 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 237 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 40 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 236 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "eventq", "gerror", "priq", "cmdq-sync";
> + msi-parent = <&its2 0>;
> + #iommu-cells = <1>;
> + dma-coherent;
> + };
> +
> + mailbox: mhu@...00000 {
> + compatible = "arm,mhu-doorbell", "arm,primecell";
> + reg = <0x0 0x45000000 0x0 0x1000>;
here hex. Well, one more inconsistency in the same file. It's usually
hex everywhere in reg and ranges.
> + interrupts = <GIC_SPI 318 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>;
> + #mbox-cells = <2>;
> + clocks = <&soc_refclk50mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + pcie_ctlr: pcie@...0000000 {
> + compatible = "pci-host-ecam-generic";
> + device_type = "pci";
> + reg = <0x28 0xC0000000 0 0x10000000>;
> + ranges = <0x01000000 0x00 0x00000000 0x00 0x6F000000 0x00 0x00800000>,
> + <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x0F000000>,
> + <0x42000000 0x09 0x00000000 0x09 0x00000000 0x1F 0xC0000000>;
lowercase hex
> + bus-range = <0 255>;
> + linux,pci-domain = <0>;
> + #address-cells = <3>;
Best regards,
Krzysztof
Powered by blists - more mailing lists