[PATCH 1/8] arm64: dts: s32g2: Add the STM description

Daniel Lezcano posted 8 patches 2 months ago
[PATCH 1/8] arm64: dts: s32g2: Add the STM description
Posted by Daniel Lezcano 2 months ago
The s32g2 has a STM module containing 8 timers. Each timer has a
dedicated interrupt and share the same clock.

Add the timers STM0->STM6 description for the s32g2 SoC. The STM7 is
not added because it is slightly different and needs an extra property
which will be added later when supported by the driver.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
Cc: Thomas Fossati <thomas.fossati@linaro.org>
---
 arch/arm64/boot/dts/freescale/s32g2.dtsi | 63 ++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index ea1456d361a3..3e775d030e37 100644
--- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -503,5 +503,68 @@ gic: interrupt-controller@50800000 {
 			interrupt-controller;
 			#interrupt-cells = <3>;
 		};
+
+		stm0: timer@4011c000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x4011c000 0x3000>;
+			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			status = "disabled";
+		};
+
+		stm1: timer@40120000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x40120000 0x3000>;
+			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			status = "disabled";
+		};
+
+		stm2: timer@40124000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x40124000 0x3000>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			status = "disabled";
+		};
+
+		stm3: timer@40128000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x40128000 0x3000>;
+			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			status = "disabled";
+		};
+
+		stm4: timer@4021c000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x4021c000 0x3000>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		stm5: timer@40220000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x40220000 0x3000>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		stm6: timer@40224000 {
+			compatible = "nxp,s32g2-stm";
+			reg = <0x40224000 0x3000>;
+			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
+			clock-names = "counter", "module", "register";
+			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.43.0
Re: [PATCH 1/8] arm64: dts: s32g2: Add the STM description
Posted by Frank Li 2 months ago
On Wed, Jul 30, 2025 at 09:50:14PM +0200, Daniel Lezcano wrote:

I think replace all 'description' with 'node' is easy to read.

> The s32g2 has a STM module containing 8 timers. Each timer has a
> dedicated interrupt and share the same clock.
>
> Add the timers STM0->STM6 description for the s32g2 SoC. The STM7 is
> not added because it is slightly different and needs an extra property
> which will be added later when supported by the driver.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> Cc: Thomas Fossati <thomas.fossati@linaro.org>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 63 ++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index ea1456d361a3..3e775d030e37 100644
> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> @@ -503,5 +503,68 @@ gic: interrupt-controller@50800000 {
>  			interrupt-controller;
>  			#interrupt-cells = <3>;
>  		};
> +
> +		stm0: timer@4011c000 {

keep order according to address.

4011c000 should less than 50800000.

> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x4011c000 0x3000>;
> +			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			status = "disabled";

why not default enable.

Frank

> +		};
> +
> +		stm1: timer@40120000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x40120000 0x3000>;
> +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			status = "disabled";
> +		};
> +
> +		stm2: timer@40124000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x40124000 0x3000>;
> +			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			status = "disabled";
> +		};
> +
> +		stm3: timer@40128000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x40128000 0x3000>;
> +			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			status = "disabled";
> +		};
> +
> +		stm4: timer@4021c000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x4021c000 0x3000>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		stm5: timer@40220000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x40220000 0x3000>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		stm6: timer@40224000 {
> +			compatible = "nxp,s32g2-stm";
> +			reg = <0x40224000 0x3000>;
> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> +			clock-names = "counter", "module", "register";
> +			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
>  	};
>  };
> --
> 2.43.0
>
Re: [PATCH 1/8] arm64: dts: s32g2: Add the STM description
Posted by Daniel Lezcano 2 months ago
Hi Frank,

thanks for the reviews,

On 30/07/2025 22:19, Frank Li wrote:
> On Wed, Jul 30, 2025 at 09:50:14PM +0200, Daniel Lezcano wrote:
> 
> I think replace all 'description' with 'node' is easy to read.

Sure

>> The s32g2 has a STM module containing 8 timers. Each timer has a
>> dedicated interrupt and share the same clock.
>>
>> Add the timers STM0->STM6 description for the s32g2 SoC. The STM7 is
>> not added because it is slightly different and needs an extra property
>> which will be added later when supported by the driver.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>> Cc: Thomas Fossati <thomas.fossati@linaro.org>
>> ---
>>   arch/arm64/boot/dts/freescale/s32g2.dtsi | 63 ++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>> index ea1456d361a3..3e775d030e37 100644
>> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>> @@ -503,5 +503,68 @@ gic: interrupt-controller@50800000 {
>>   			interrupt-controller;
>>   			#interrupt-cells = <3>;
>>   		};
>> +
>> +		stm0: timer@4011c000 {
> 
> keep order according to address.
> 
> 4011c000 should less than 50800000.

Ah, sure. I'll fix that.

>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x4011c000 0x3000>;
>> +			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			status = "disabled";
> 
> why not default enable.

The S32G2 and S32G3 can have different variants with 2, 4, 8 Cortex-A53 
and 3 or 4 Cortex-M7. We enable the same number of CPUs present on the 
system.

AFAIU:
	S32G233A : 2 x Cortex-A53
	S32G274A : 4 x Cortex-A53

	S32G399A : 8 x Cortex-A53
	S32G379A : 4 x Cortex-A53

Otherwise we would have to do the opposite, that is disable the unused 
ones in the s32g274a-rdb2.dts, s32g399a-rdb3.dts and other dts which 
include the s32g2.dtsi and s32g3.dtsi.


>> +		};
>> +
>> +		stm1: timer@40120000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x40120000 0x3000>;
>> +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			status = "disabled";
>> +		};
>> +
>> +		stm2: timer@40124000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x40124000 0x3000>;
>> +			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			status = "disabled";
>> +		};
>> +
>> +		stm3: timer@40128000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x40128000 0x3000>;
>> +			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			status = "disabled";
>> +		};
>> +
>> +		stm4: timer@4021c000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x4021c000 0x3000>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
>> +			status = "disabled";
>> +		};
>> +
>> +		stm5: timer@40220000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x40220000 0x3000>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
>> +			status = "disabled";
>> +		};
>> +
>> +		stm6: timer@40224000 {
>> +			compatible = "nxp,s32g2-stm";
>> +			reg = <0x40224000 0x3000>;
>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>> +			clock-names = "counter", "module", "register";
>> +			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
>> +			status = "disabled";
>> +		};
>>   	};
>>   };
>> --
>> 2.43.0
>>


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Re: [PATCH 1/8] arm64: dts: s32g2: Add the STM description
Posted by Frank Li 2 months ago
On Wed, Jul 30, 2025 at 11:15:40PM +0200, Daniel Lezcano wrote:
>
> Hi Frank,
>
> thanks for the reviews,
>
> On 30/07/2025 22:19, Frank Li wrote:
> > On Wed, Jul 30, 2025 at 09:50:14PM +0200, Daniel Lezcano wrote:
> >
> > I think replace all 'description' with 'node' is easy to read.
>
> Sure
>
> > > The s32g2 has a STM module containing 8 timers. Each timer has a
> > > dedicated interrupt and share the same clock.
> > >
> > > Add the timers STM0->STM6 description for the s32g2 SoC. The STM7 is
> > > not added because it is slightly different and needs an extra property
> > > which will be added later when supported by the driver.
> > >
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> > > Cc: Thomas Fossati <thomas.fossati@linaro.org>
> > > ---
> > >   arch/arm64/boot/dts/freescale/s32g2.dtsi | 63 ++++++++++++++++++++++++
> > >   1 file changed, 63 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > index ea1456d361a3..3e775d030e37 100644
> > > --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > @@ -503,5 +503,68 @@ gic: interrupt-controller@50800000 {
> > >   			interrupt-controller;
> > >   			#interrupt-cells = <3>;
> > >   		};
> > > +
> > > +		stm0: timer@4011c000 {
> >
> > keep order according to address.
> >
> > 4011c000 should less than 50800000.
>
> Ah, sure. I'll fix that.
>
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x4011c000 0x3000>;
> > > +			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			status = "disabled";
> >
> > why not default enable.
>
> The S32G2 and S32G3 can have different variants with 2, 4, 8 Cortex-A53 and
> 3 or 4 Cortex-M7. We enable the same number of CPUs present on the system.
>
> AFAIU:
> 	S32G233A : 2 x Cortex-A53
> 	S32G274A : 4 x Cortex-A53
>
> 	S32G399A : 8 x Cortex-A53
> 	S32G379A : 4 x Cortex-A53
>
> Otherwise we would have to do the opposite, that is disable the unused ones
> in the s32g274a-rdb2.dts, s32g399a-rdb3.dts and other dts which include the
> s32g2.dtsi and s32g3.dtsi.
>

That's fine by default disabled. but what's impact if it is enable but no
one use it?

Frank

>
> > > +		};
> > > +
> > > +		stm1: timer@40120000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x40120000 0x3000>;
> > > +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		stm2: timer@40124000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x40124000 0x3000>;
> > > +			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		stm3: timer@40128000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x40128000 0x3000>;
> > > +			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		stm4: timer@4021c000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x4021c000 0x3000>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		stm5: timer@40220000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x40220000 0x3000>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		stm6: timer@40224000 {
> > > +			compatible = "nxp,s32g2-stm";
> > > +			reg = <0x40224000 0x3000>;
> > > +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
> > > +			clock-names = "counter", "module", "register";
> > > +			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> > > +			status = "disabled";
> > > +		};
> > >   	};
> > >   };
> > > --
> > > 2.43.0
> > >
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
Re: [PATCH 1/8] arm64: dts: s32g2: Add the STM description
Posted by Daniel Lezcano 2 months ago
On 01/08/2025 01:20, Frank Li wrote:
> On Wed, Jul 30, 2025 at 11:15:40PM +0200, Daniel Lezcano wrote:

[ ... ]

>>>> +			compatible = "nxp,s32g2-stm";
>>>> +			reg = <0x4011c000 0x3000>;
>>>> +			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>>>> +			clocks = <&clks 0x3b>, <&clks 0x3c>, <&clks 0x3c>;
>>>> +			clock-names = "counter", "module", "register";
>>>> +			status = "disabled";
>>>
>>> why not default enable.
>>
>> The S32G2 and S32G3 can have different variants with 2, 4, 8 Cortex-A53 and
>> 3 or 4 Cortex-M7. We enable the same number of CPUs present on the system.
>>
>> AFAIU:
>> 	S32G233A : 2 x Cortex-A53
>> 	S32G274A : 4 x Cortex-A53
>>
>> 	S32G399A : 8 x Cortex-A53
>> 	S32G379A : 4 x Cortex-A53
>>
>> Otherwise we would have to do the opposite, that is disable the unused ones
>> in the s32g274a-rdb2.dts, s32g399a-rdb3.dts and other dts which include the
>> s32g2.dtsi and s32g3.dtsi.
>>
> 
> That's fine by default disabled. but what's impact if it is enable but no
> one use it?

At the first glance I would say we call the probe function for nothing, 
so adding an extra overhead. When repeated into multiple drivers that 
increases the boot time significantly. It is certainly a good practice 
as a rule of thumb to enable only the ones we really need.

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog