mbox series

[v2,0/2] NXP S32G3 SoC initial bring-up

Message ID 20240319221614.56652-1-wafgo01@gmail.com
Headers show
Series NXP S32G3 SoC initial bring-up | expand

Message

Wadim Mueller March 19, 2024, 10:16 p.m. UTC 
This series brings up initial support for the NXP S32G3 SoC,
used on the S32G-VNP-RDB3 board [1].

The following features are supported in this initial port:

  * Devicetree for the S32G-VNP-RDB3 
  * UART (fsl-linflexuart) with earlycon support
  * SDHC: fsl-imx-esdhc (SD/eMMC)

== Changes since v1 ==:

  * fix the reported checkpatch.pl errors. Two warnings still available but can be ignored
  * clean up unneeded DT nodes and properties
  * fix 'make dt_binding_check dtbs_check' errors
  * remove the S32 STMMAC driver and DT node which will be introduced in a new patchset
  * add NXP authorship and copyright into the dtsi header
 

[1] https://www.nxp.com/design/design-center/designs/s32g3-vehicle-networking-reference-design:S32G-VNP-RDB3


Wadim Mueller (2):
  arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3
  dt-bindings: arm: fsl: Document missing s32g3 board and linflexuart

 .../devicetree/bindings/arm/fsl.yaml          |   6 +
 .../bindings/serial/fsl,s32-linflexuart.yaml  |   3 +
 arch/arm64/boot/dts/freescale/Makefile        |   1 +
 arch/arm64/boot/dts/freescale/s32g3.dtsi      | 236 ++++++++++++++++++
 .../boot/dts/freescale/s32g399a-rdb3.dts      |  43 ++++
 5 files changed, 289 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/s32g3.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts

Comments

Ghennadi Procopciuc March 20, 2024, 7 a.m. UTC | #1 
On 3/20/24 00:16, Wadim Mueller wrote:
[...]
> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2021-2023 NXP
> + *
> + * Authors: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> + *          Ciprian Costea <ciprianmarian.costea@nxp.com>
> + *          Andra-Teodora Ilie <andra.ilie@nxp.com>

This pollutes the content of the file. Please make them part of the
commit description using 'Signed-off-by' tags.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +#include <dt-bindings/interrupt-controller/arm-gic.h>

Missing empty line ?

> +/ {
> +	compatible = "nxp,s32g3";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <0x02>;
> +	#size-cells = <0x02>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +
> +				core2 {
> +					cpu = <&cpu2>;
> +				};
> +
> +				core3 {
> +					cpu = <&cpu3>;
> +				};
> +			};
> +
> +			cluster1 {
> +				core0 {
> +					cpu = <&cpu4>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu5>;
> +				};
> +
> +				core2 {
> +					cpu = <&cpu6>;
> +				};
> +
> +				core3 {
> +					cpu = <&cpu7>;
> +				};
> +			};
> +		};
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x1>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x2>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x3>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu4: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x100>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu5: cpu@101 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x101>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu6: cpu@102 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x102>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +
> +		cpu7: cpu@103 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x103>;
> +			enable-method = "psci";
> +			clocks = <&dfs 0>;
> +		};
> +	};
> +
> +	pmu {
> +		compatible = "arm,cortex-a53-pmu";
> +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>, /* sec-phys */
> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, /* virt */
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, /* hyp-virt */
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>, /* sec-phys */
> +			     <GIC_PPI 12 IRQ_TYPE_LEVEL_LOW>; /* phys */

The interrupt order does not seem right.
Krzysztof Kozlowski March 20, 2024, 7:49 a.m. UTC | #2 
On 20/03/2024 08:00, Ghennadi Procopciuc wrote:
> On 3/20/24 00:16, Wadim Mueller wrote:
> [...]
>> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
>> @@ -0,0 +1,236 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright 2021-2023 NXP
>> + *
>> + * Authors: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>> + *          Ciprian Costea <ciprianmarian.costea@nxp.com>
>> + *          Andra-Teodora Ilie <andra.ilie@nxp.com>
> 
> This pollutes the content of the file. Please make them part of the
> commit description using 'Signed-off-by' tags.
> 

No, that's not how process works. SoB are part of DCO and Ghennadi is
allowed to skip them. Just read the DCO. Don't add fake SoB tags just
because someone wants...

Nothing is polluted in the file. That's what this section you have (if
someone wants).


Best regards,
Krzysztof
Ghennadi Procopciuc March 20, 2024, 8:24 a.m. UTC | #3 
On 3/20/24 09:49, Krzysztof Kozlowski wrote:
> On 20/03/2024 08:00, Ghennadi Procopciuc wrote:
>> On 3/20/24 00:16, Wadim Mueller wrote:
>> [...]
>>> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
>>> @@ -0,0 +1,236 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright 2021-2023 NXP
>>> + *
>>> + * Authors: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>>> + *          Ciprian Costea <ciprianmarian.costea@nxp.com>
>>> + *          Andra-Teodora Ilie <andra.ilie@nxp.com>
>>
>> This pollutes the content of the file. Please make them part of the
>> commit description using 'Signed-off-by' tags.
>>
> 
> No, that's not how process works. SoB are part of DCO and Ghennadi is
> allowed to skip them. Just read the DCO. Don't add fake SoB tags just
> because someone wants...
> 
> Nothing is polluted in the file. That's what this section you have (if
> someone wants).
> 

I apologize for not getting it right earlier. After reviewing the
information available at [0], I noticed that it suggests using
Co-developed-by and Signed-off-by when a patch has multiple
contributors. However, I could not find any mention of the 'Authors'
section in the file although it seems to be a common practice.

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n491

Regards,
Ghennadi
Krzysztof Kozlowski March 20, 2024, 9:24 a.m. UTC | #4 
On 20/03/2024 09:24, Ghennadi Procopciuc wrote:
> On 3/20/24 09:49, Krzysztof Kozlowski wrote:
>> On 20/03/2024 08:00, Ghennadi Procopciuc wrote:
>>> On 3/20/24 00:16, Wadim Mueller wrote:
>>> [...]
>>>> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
>>>> @@ -0,0 +1,236 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>> +/*
>>>> + * Copyright 2021-2023 NXP
>>>> + *
>>>> + * Authors: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>>>> + *          Ciprian Costea <ciprianmarian.costea@nxp.com>
>>>> + *          Andra-Teodora Ilie <andra.ilie@nxp.com>
>>>
>>> This pollutes the content of the file. Please make them part of the
>>> commit description using 'Signed-off-by' tags.
>>>
>>
>> No, that's not how process works. SoB are part of DCO and Ghennadi is
>> allowed to skip them. Just read the DCO. Don't add fake SoB tags just
>> because someone wants...
>>
>> Nothing is polluted in the file. That's what this section you have (if
>> someone wants).
>>
> 
> I apologize for not getting it right earlier. After reviewing the
> information available at [0], I noticed that it suggests using
> Co-developed-by and Signed-off-by when a patch has multiple
> contributors. However, I could not find any mention of the 'Authors'
> section in the file although it seems to be a common practice.

submitting-patches describes the process, thus also patches, but not the
Linux code. Whatever is in the C/H/DTS/SH/COCCI code, is independent.

Best regards,
Krzysztof