Hi Chester et al., On 05.08.21 08:54, Chester Lin wrote: > Add an initial dtsi file for generic SoC features of NXP S32G2. > > Signed-off-by: Chester Lin <clin@xxxxxxxx> > --- > arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++ > 1 file changed, 98 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi > new file mode 100644 > index 000000000000..3321819c1a2d > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi Note: This DT is for running on the Cortex-A53 cores, but S32G2 also has Cortex-M7 cores. For Vybrid SoCs, DTs later got contributed to also run on its Cortex-M4 core: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610.dtsi https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf500.dtsi vs. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610m4.dtsi Should we plan for this in our file naming here and in following patches (e.g., s32g2-a53* vs. s32g2-m7*)? To me, a later concatenation of s32g274am7* would look awkward, and s32g274a-m7* would sort between -evb and -rdb2. > @@ -0,0 +1,98 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* * NXP S32G2 SoC family * ? @NXP: Are any models other than 274A in the queue that we should distinguish between s32g2.dtsi and s32g274a.dtsi here already? > + * Copyright (c) 2021 SUSE LLC > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > + > +/ { > + compatible = "fsl,s32g2"; > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x0>; > + enable-method = "psci"; > + next-level-cache = <&cluster0_l2>; > + }; > + > + cpu1: cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x1>; > + enable-method = "psci"; > + next-level-cache = <&cluster0_l2>; > + }; > + > + cpu2: cpu@100 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x100>; > + enable-method = "psci"; > + next-level-cache = <&cluster1_l2>; > + }; > + > + cpu3: cpu@101 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x101>; > + enable-method = "psci"; > + next-level-cache = <&cluster1_l2>; > + }; > + > + cluster0_l2: l2-cache0 { > + compatible = "cache"; > + }; > + > + cluster1_l2: l2-cache1 { > + compatible = "cache"; > + }; > + }; > + > + pmu { > + compatible = "arm,cortex-a53-pmu"; > + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>; > + }; > + > + 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>; > + }; > + > + psci { > + compatible = "arm,psci-1.0"; > + method = "smc"; > + }; Should we move this into a /firmware node, to group with future OP-TEE? > + > + soc { > + compatible = "simple-bus"; > + interrupt-parent = <&gic>; Duplicate, already set on root node. > + #address-cells = <2>; > + #size-cells = <2>; Why? Does it have any peripherals that go beyond 32-bit space? For 64-bit Realtek platforms Rob had asked me to use 1, if possible. I do understand that for /memory nodes we do have high-memory addresses, so 2 for the root node looks correct. > + Please drop this white line. > + ranges; According to Rob, the /soc ranges should exclude any RAM ranges for safety reasons. Compare: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/realtek/rtd129x.dtsi If you're lacking the maximum RAM areas to carve out, NXP is in CC to help out :) and the EVB and RDB2 boards should give you starting numbers that could be enlarged later if needed. > + > + gic: interrupt-controller@50800000 { > + compatible = "arm,gic-v3"; > + #interrupt-cells = <3>; > + interrupt-controller; > + reg = <0 0x50800000 0 0x10000>, > + <0 0x50880000 0 0x200000>, > + <0 0x50400000 0 0x2000>, > + <0 0x50410000 0 0x2000>, > + <0 0x50420000 0 0x2000>; Please order reg after compatible by convention, and sort interrupt-controller or at least #interrupt-cells (applying to consumers) last, after the below one applying to this device itself. > + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | > + IRQ_TYPE_LEVEL_HIGH)>; > + }; CC'ing Marc for additional GIC scrutiny, often the sizes are wrong. > + }; > +}; Thanks, Andreas -- SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
