Messages in this thread | | | From | Andreas Färber <> | Subject | Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support | Date | Thu, 12 Aug 2021 19:26:28 +0200 |
| |
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@suse.com> > --- > 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)
| |