2024-03-19 22:17:37

by Wadim Mueller

[permalink] [raw]
Subject: [PATCH v2 0/2] NXP S32G3 SoC initial bring-up


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

--
2.25.1



2024-03-19 22:17:54

by Wadim Mueller

[permalink] [raw]
Subject: [PATCH v2 1/2] arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3

This commit adds device tree support for the NXP S32G3-based
S32G-VNP-RDB3 Board [1].

The S32G3 features an 8-core ARM Cortex-A53 based SoC developed by NXP.

The device tree files are derived from the official NXP downstream
Linux tree [2].

This addition encompasses a limited selection of peripherals that
are upstream-supported. Apart from the ARM System Modules
(GIC, Generic Timer, etc.), the following IPs have been validated:

* UART: fsl-linflexuart
* SDHC: fsl-imx-esdhc

Clock settings for the chip rely on ATF Firmware [3].
Pin control integration into the device tree is pending and currently
relies on Firmware/U-Boot settings [4].

These changes were validated using BSP39 Firmware/U-Boot from NXP [5].

The modifications enable booting the official Ubuntu 22.04 from NXP on
the RDB3 with default settings from the SD card and eMMC.

[1] https://www.nxp.com/design/design-center/designs/s32g3-vehicle-networking-reference-design:S32G-VNP-RDB3
[2] https://github.com/nxp-auto-linux/linux
[3] https://github.com/nxp-auto-linux/arm-trusted-firmware
[4] https://github.com/nxp-auto-linux/u-boot
[5] https://github.com/nxp-auto-linux/auto_yocto_bsp

Signed-off-by: Wadim Mueller <[email protected]>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
arch/arm64/boot/dts/freescale/s32g3.dtsi | 236 ++++++++++++++++++
.../boot/dts/freescale/s32g399a-rdb3.dts | 43 ++++
3 files changed, 280 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/s32g3.dtsi
create mode 100644 arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 2cb0212b63c6..e701008dbc7b 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -252,3 +252,4 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-venice-gw74xx-rpidsi.dtb
dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb
dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
+dtb-$(CONFIG_ARCH_S32) += s32g399a-rdb3.dtb
diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
new file mode 100644
index 000000000000..6be638326142
--- /dev/null
+++ 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 <[email protected]>
+ * Ciprian Costea <[email protected]>
+ * Andra-Teodora Ilie <[email protected]>
+ *
+ * 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>
+/ {
+ 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 */
+ arm,no-tick-in-suspend;
+ };
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ scmi_shmem: shm@d0000000 {
+ compatible = "arm,scmi-shmem";
+ reg = <0x0 0xd0000000 0x0 0x80>;
+ no-map;
+ };
+ };
+
+ firmware {
+ scmi: scmi {
+ compatible = "arm,scmi-smc";
+ shmem = <&scmi_shmem>;
+ arm,smc-id = <0xc20000fe>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
+
+ dfs: protocol@13 {
+ reg = <0x13>;
+ #clock-cells = <1>;
+ };
+
+ clks: protocol@14 {
+ reg = <0x14>;
+ #clock-cells = <1>;
+ };
+ };
+
+ psci: psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+ };
+
+ soc@0 {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0 0x80000000>;
+
+ uart0: serial@401c8000 {
+ compatible = "nxp,s32g3-linflexuart",
+ "fsl,s32v234-linflexuart";
+ reg = <0x401c8000 0x3000>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+
+ uart1: serial@401cc000 {
+ compatible = "nxp,s32g3-linflexuart",
+ "fsl,s32v234-linflexuart";
+ reg = <0x401cc000 0x3000>;
+ interrupts = <GIC_SPI 83 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+
+ uart2: serial@402bc000 {
+ compatible = "nxp,s32g3-linflexuart",
+ "fsl,s32v234-linflexuart";
+ reg = <0x402bc000 0x3000>;
+ interrupts = <GIC_SPI 84 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+
+ gic: interrupt-controller@50800000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ reg = <0x50800000 0x10000>,
+ <0x50900000 0x200000>,
+ <0x50400000 0x2000>,
+ <0x50410000 0x2000>,
+ <0x50420000 0x2000>;
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ usdhc0: mmc@402f0000 {
+ compatible = "nxp,s32g2-usdhc";
+ reg = <0x402f0000 0x1000>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks 32>,
+ <&clks 31>,
+ <&clks 33>;
+ clock-names = "ipg", "ahb", "per";
+ status = "disabled";
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
new file mode 100644
index 000000000000..199605b28df3
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2021-2023 NXP
+ *
+ * NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)
+ */
+
+/dts-v1/;
+
+#include "s32g3.dtsi"
+
+/ {
+ model = "NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)";
+ compatible = "nxp,s32g399a-rdb3", "nxp,s32g3";
+
+ aliases {
+ serial0 = &uart0;
+ serial1 = &uart1;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ /* 4GiB RAM */
+ memory@80000000 {
+ device_type = "memory";
+ reg = <0x0 0x80000000 0 0x80000000>,
+ <0x8 0x80000000 0 0x80000000>;
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&uart1 {
+ status = "okay";
+};
+
+&usdhc0 {
+ status = "okay";
+};
--
2.25.1


2024-03-19 22:18:18

by Wadim Mueller

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: arm: fsl: Document missing s32g3 board

The nxp, s32g399a-rdb3 board is not documented.

* Add entry for S32G3 based boards with nxp,s32g399a-rdb3 item
* Add nxp,s32g3-linflexuart documentation

Signed-off-by: Wadim Mueller <[email protected]>
---
Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
.../devicetree/bindings/serial/fsl,s32-linflexuart.yaml | 3 +++
2 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 228dcc5c7d6f..23bf1d7f95b1 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -1503,6 +1503,12 @@ properties:
- nxp,s32g274a-rdb2
- const: nxp,s32g2

+ - description: S32G3 based Boards
+ items:
+ - enum:
+ - nxp,s32g399a-rdb3
+ - const: nxp,s32g3
+
- description: S32V234 based Boards
items:
- enum:
diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
index 7a105551fa6a..f8eb92c9a8d9 100644
--- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
@@ -25,6 +25,9 @@ properties:
- items:
- const: nxp,s32g2-linflexuart
- const: fsl,s32v234-linflexuart
+ - items:
+ - const: nxp,s32g3-linflexuart
+ - const: fsl,s32v234-linflexuart

reg:
maxItems: 1
--
2.25.1


2024-03-20 07:00:41

by Ghennadi Procopciuc

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3

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 <[email protected]>
> + * Ciprian Costea <[email protected]>
> + * Andra-Teodora Ilie <[email protected]>

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.
From drivers/clocksource/arm_arch_timer.c:

static const char *arch_timer_ppi_names[ARCH_TIMER_MAX_TIMER_PPI] = {
[ARCH_TIMER_PHYS_SECURE_PPI] = "sec-phys",
[ARCH_TIMER_PHYS_NONSECURE_PPI] = "phys",
[ARCH_TIMER_VIRT_PPI] = "virt",
[ARCH_TIMER_HYP_PPI] = "hyp-phys",
[ARCH_TIMER_HYP_VIRT_PPI] = "hyp-virt",
};

[...]

static int __init arch_timer_of_init(struct device_node *np)
{
[...]

for (i = ARCH_TIMER_PHYS_SECURE_PPI; i < ARCH_TIMER_MAX_TIMER_PPI;
i++) {
if (has_names)
irq = of_irq_get_byname(np, arch_timer_ppi_names[i]);
else
irq = of_irq_get(np, i);
if (irq > 0)
arch_timer_ppi[i] = irq;

}
[...]
}

As I understand it, if the names are not provided, then the interrupt
IDs must strictly follow the order specified in the arch_timer_ppi_names
list, which is: sec-phys, phys, virt, hyp-phys, hyp-virt. That is why I
personally prefer using interrupt names instead of the raw list of IDs.

> + arm,no-tick-in-suspend;
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + scmi_shmem: shm@d0000000 {
> + compatible = "arm,scmi-shmem";
> + reg = <0x0 0xd0000000 0x0 0x80>;
> + no-map;
> + };
> + };
> +
> + firmware {
> + scmi: scmi {
> + compatible = "arm,scmi-smc";
> + shmem = <&scmi_shmem>;
> + arm,smc-id = <0xc20000fe>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;

This interrupt is not needed if the RX is not used.

> +
> + dfs: protocol@13 {
> + reg = <0x13>;
> + #clock-cells = <1>;
> + };
> +
> + clks: protocol@14 {
> + reg = <0x14>;
> + #clock-cells = <1>;
> + };
> + };
> +
> + psci: psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> + };
> +
> + soc@0 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x80000000>;
> +
> + uart0: serial@401c8000 {
> + compatible = "nxp,s32g3-linflexuart",

From 'Submitting Devicetree (DT) binding patches' [0]:
5. The Documentation/ portion of the patch should come in the series
before the code implementing the binding.

> + "fsl,s32v234-linflexuart";
> + reg = <0x401c8000 0x3000>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };

Krzysztof, would it make sense to factor the common part of the s32g3
and s32g2 into a new dtsi file (e.g. s32g.dtsi) since both SoCs are part
of the same family and share the memory map with some exceptions?

> +
> + uart1: serial@401cc000 {
> + compatible = "nxp,s32g3-linflexuart",
> + "fsl,s32v234-linflexuart";
> + reg = <0x401cc000 0x3000>;
> + interrupts = <GIC_SPI 83 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };
> +
> + uart2: serial@402bc000 {
> + compatible = "nxp,s32g3-linflexuart",
> + "fsl,s32v234-linflexuart";
> + reg = <0x402bc000 0x3000>;
> + interrupts = <GIC_SPI 84 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };
> +
> + gic: interrupt-controller@50800000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + reg = <0x50800000 0x10000>,
> + <0x50900000 0x200000>,
> + <0x50400000 0x2000>,
> + <0x50410000 0x2000>,
> + <0x50420000 0x2000>;
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + usdhc0: mmc@402f0000 {
> + compatible = "nxp,s32g2-usdhc";

Is there a reason why the UART uses an S32G3 specific compatible string,
but the SD does not?

> + reg = <0x402f0000 0x1000>;
> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks 32>,
> + <&clks 31>,
> + <&clks 33>;
> + clock-names = "ipg", "ahb", "per";
> + status = "disabled";
> + };

Missing bus-width = <8> ?

> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
> new file mode 100644
> index 000000000000..199605b28df3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2021-2023 NXP
> + *
> + * NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g3.dtsi"
> +
> +/ {
> + model = "NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)";
> + compatible = "nxp,s32g399a-rdb3", "nxp,s32g3";
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + /* 4GiB RAM */
> + memory@80000000 {
> + device_type = "memory";
> + reg = <0x0 0x80000000 0 0x80000000>,
> + <0x8 0x80000000 0 0x80000000>;
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&uart1 {
> + status = "okay";
> +};
> +
> +&usdhc0 {
> + status = "okay";
> +}
[0]
https://docs.kernel.org/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Regards,
Ghennadi


2024-03-20 07:48:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: arm: fsl: Document missing s32g3 board

On 19/03/2024 23:16, Wadim Mueller wrote:
> The nxp, s32g399a-rdb3 board is not documented.

Use full name of the board.

Subject: How is it missing? There is no such board in the kernel, so
binding is not missing. Also, you touch here serial...

>
> * Add entry for S32G3 based boards with nxp,s32g399a-rdb3 item
> * Add nxp,s32g3-linflexuart documentation
>
> Signed-off-by: Wadim Mueller <[email protected]>

Bindings go before users.

> ---
> Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
> .../devicetree/bindings/serial/fsl,s32-linflexuart.yaml | 3 +++

Split the patches. They will go via different subsystems.

> 2 files changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 228dcc5c7d6f..23bf1d7f95b1 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -1503,6 +1503,12 @@ properties:
> - nxp,s32g274a-rdb2
> - const: nxp,s32g2
>
> + - description: S32G3 based Boards
> + items:
> + - enum:
> + - nxp,s32g399a-rdb3
> + - const: nxp,s32g3


Best regards,
Krzysztof


2024-03-20 07:50:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3

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 <[email protected]>
>> + * Ciprian Costea <[email protected]>
>> + * Andra-Teodora Ilie <[email protected]>
>
> 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


2024-03-20 08:24:27

by Ghennadi Procopciuc

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3

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 <[email protected]>
>>> + * Ciprian Costea <[email protected]>
>>> + * Andra-Teodora Ilie <[email protected]>
>>
>> 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


2024-03-20 09:24:29

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] arm64: dts: S32G3: Introduce device tree for S32G-VNP-RDB3

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 <[email protected]>
>>>> + * Ciprian Costea <[email protected]>
>>>> + * Andra-Teodora Ilie <[email protected]>
>>>
>>> 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