Re: [PATCH v2 1/1] arm64: dts: s32g: add PIT support for s32g2 and s32g3

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added device tree nodes `pit0` and `pit1` are named `pit@...` instead of the generic name `timer@...`.
--

commit 296fdf97d8d5f42b2426eda4d41b0132d74ccbbd
Author: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

arm64: dts: s32g: add PIT support for s32g2 and s32g3

This commit adds device tree nodes for the Periodic Interrupt Timer (PIT0 and
PIT1) blocks on the NXP S32G2 and S32G3 SoCs.

> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index 51d00dac12deb..57ff97e445075 100644
> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
[ ... ]
> @@ -417,6 +417,15 @@ edma0: dma-controller@40144000 {
>  			clock-names = "dmamux0", "dmamux1";
>  		};
>  
> +		pit0: pit@40188000 {

This isn't a bug, but should these nodes use the generic name timer instead of
pit? The Devicetree Specification recommends using generic names that reflect
the primary function of the device rather than specific IP block acronyms.

> +			compatible = "nxp,s32g2-pit";
> +			reg = <0x40188000 0x3000>;
> +			interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 61>;
> +			clock-names = "pit";
> +			status = "disabled";
> +		};
> +
[ ... ]
> @@ -622,6 +631,15 @@ edma1: dma-controller@40244000 {
>  			clock-names = "dmamux0", "dmamux1";
>  		};
>  
> +		pit1: pit@40288000 {

Could this node also be named timer@40288000 for the same reason?

> +			compatible = "nxp,s32g2-pit";
> +			reg = <0x40288000 0x3000>;
> +			interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 61>;
> +			clock-names = "pit";
> +			status = "disabled";
> +		};
[ ... ]
> diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> index e314f3c7d61d0..efe5398e12403 100644
> --- a/arch/arm64/boot/dts/freescale/s32g3.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
[ ... ]
> @@ -475,6 +475,15 @@ edma0: dma-controller@40144000 {
>  			clock-names = "dmamux0", "dmamux1";
>  		};
>  
> +		pit0: pit@40188000 {

And similarly, could this node be updated to timer@40188000?

> +			compatible = "nxp,s32g3-pit", "nxp,s32g2-pit";
> +			reg = <0x40188000 0x3000>;
> +			interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 61>;
> +			clock-names = "pit";
> +			status = "disabled";
> +		};
> +
[ ... ]
> @@ -693,6 +702,15 @@ edma1: dma-controller@40244000 {
>  			clock-names = "dmamux0", "dmamux1";
>  		};
>  
> +		pit1: pit@40288000 {

And timer@40288000 here?

> +			compatible = "nxp,s32g3-pit", "nxp,s32g2-pit";
> +			reg = <0x40288000 0x3000>;
> +			interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clks 61>;
> +			clock-names = "pit";
> +			status = "disabled";
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518063547.2890353-2-khristineandreea.barbulescu@oss.nxp.com?part=1