[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aIv6L30mf5bzUqup@lizhi-Precision-Tower-5810>
Date: Thu, 31 Jul 2025 19:20:15 -0400
From: Frank Li <Frank.li@....com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: mbrugger@...e.com, chester62515@...il.com,
ghennadi.procopciuc@....nxp.com, shawnguo@...nel.org,
s.hauer@...gutronix.de, s32@....com, kernel@...gutronix.de,
festevam@...il.com, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, linux-arm-kernel@...ts.infradead.org,
imx@...ts.linux.dev, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Ghennadi Procopciuc <ghennadi.procopciuc@....com>,
Thomas Fossati <thomas.fossati@...aro.org>
Subject: Re: [PATCH 1/8] arm64: dts: s32g2: Add the STM description
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@...aro.org>
> > > Cc: Ghennadi Procopciuc <ghennadi.procopciuc@....com>
> > > Cc: Thomas Fossati <thomas.fossati@...aro.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@...00000 {
> > > interrupt-controller;
> > > #interrupt-cells = <3>;
> > > };
> > > +
> > > + stm0: timer@...1c000 {
> >
> > 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@...20000 {
> > > + 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@...24000 {
> > > + 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@...28000 {
> > > + 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@...1c000 {
> > > + 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@...20000 {
> > > + 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@...24000 {
> > > + 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
Powered by blists - more mailing lists