linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] arm64: dts: initial NXP S32G2 support
@ 2021-08-05  6:54 Chester Lin
  2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
                   ` (8 more replies)
  0 siblings, 9 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, devicetree, linux-arm-kernel, s32
  Cc: linux-kernel, linux-serial, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Hello,

Here I'd like to propose a patchset, which is initial upstream support for NXP
S32G2. S32G is a processor family developed by NXP for automotive solutions,
such as vehicle networking and automotive high-performance processing. This
series focuses on S32G2, which is the latest generation we can find at the
moment. As the first round to support S32G2, this patchset only enables basic
components and interfaces the SoC must have while kernel booting, which aims
to have minimum hardware enablement for these two boards, S32G-VNP-EVB and
S32G-VNP-RDB2. The concepts of how these boards work are originated from the
downstream kernel tree[1] developed by NXP, which provides lots of details
about the SoC S32G274A and its integrated boards. This series has been
verified with downstream ATF[2] & U-Boot[3] based on the ATF boot flow.

Thanks,
Chester

[1] https://source.codeaurora.org/external/autobsps32/linux/
[2] https://source.codeaurora.org/external/autobsps32/arm-trusted-firmware/
[3] https://source.codeaurora.org/external/autobsps32/u-boot/

Chester Lin (8):
  dt-bindings: arm: fsl: add NXP S32G2 boards
  dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  arm64: dts: add NXP S32G2 support
  arm64: dts: s32g2: add serial/uart support
  arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support
  arm64: dts: s32g2: add memory nodes for evb and rdb2
  MAINTAINERS: Add an entry for NXP S32G2 boards

 .../devicetree/bindings/arm/fsl.yaml          |   7 +
 .../bindings/serial/fsl,s32-linflexuart.txt   |  22 ---
 .../bindings/serial/fsl,s32-linflexuart.yaml  |  66 +++++++++
 MAINTAINERS                                   |   6 +
 arch/arm64/boot/dts/freescale/Makefile        |   2 +
 arch/arm64/boot/dts/freescale/s32g2.dtsi      | 129 ++++++++++++++++++
 .../arm64/boot/dts/freescale/s32g274a-evb.dts |  29 ++++
 .../boot/dts/freescale/s32g274a-rdb2.dts      |  33 +++++
 8 files changed, 272 insertions(+), 22 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
 create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
 create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts

-- 
2.30.0


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 15:46   ` Andreas Färber
  2021-08-13 17:53   ` Rob Herring
  2021-08-05  6:54 ` [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format Chester Lin
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
design 2 board ( S32G-VNP-RDB2).

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index e2097011c4b0..3914aa09e503 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -983,6 +983,13 @@ properties:
           - const: solidrun,lx2160a-cex7
           - const: fsl,lx2160a
 
+      - description: S32G2 based Boards
+        items:
+          - enum:
+              - fsl,s32g274a-evb
+              - fsl,s32g274a-rdb2
+          - const: fsl,s32g2
+
       - description: S32V234 based Boards
         items:
           - enum:
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
  2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 16:04   ` Andreas Färber
  2021-08-13 18:07   ` Rob Herring
  2021-08-05  6:54 ` [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2 Chester Lin
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Convert the FSL LINFlexD UART binding to json-schema.

Signed-off-by: Chester Lin <clin@suse.com>
---
 .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
 .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
 2 files changed, 48 insertions(+), 22 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml

diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
deleted file mode 100644
index f1bbe0826be5..000000000000
--- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
+++ /dev/null
@@ -1,22 +0,0 @@
-* Freescale LINFlexD UART
-
-The LINFlexD controller implements several LIN protocol versions, as well as
-support for full-duplex UART communication through 8-bit and 9-bit frames.
-
-See chapter 47 ("LINFlexD") in the reference manual[1].
-
-Required properties:
-- compatible :
-  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
-    is compatible with the one integrated on S32V234 SoC
-- reg : Address and length of the register set for the device
-- interrupts : Should contain uart interrupt
-
-Example:
-uart0: serial@40053000 {
-	compatible = "fsl,s32v234-linflexuart";
-	reg = <0x0 0x40053000 0x0 0x1000>;
-	interrupts = <0 59 4>;
-};
-
-[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
new file mode 100644
index 000000000000..acfe34706ccb
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/fsl,s32-linflexuart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale LINFlexD UART
+
+description: |
+  The LINFlexD controller implements several LIN protocol versions, as well
+  as support for full-duplex UART communication through 8-bit and 9-bit
+  frames. See chapter 47 ("LINFlexD") in the reference manual
+  https://www.nxp.com/webapp/Download?colCode=S32V234RM.
+
+maintainers:
+  - Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+  - Rob Herring <robh@kernel.org>
+
+allOf:
+  - $ref: "serial.yaml"
+
+properties:
+  compatible:
+    description: The LINFlexD controller on S32V234 SoC, which can be
+      configured in UART mode.
+    items:
+      - const: fsl,s32v234-linflexuart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    serial@40053000 {
+        compatible = "fsl,s32v234-linflexuart";
+        reg = <0x40053000 0x1000>;
+        interrupts = <0 59 4>;
+    };
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
  2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
  2021-08-05  6:54 ` [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 16:27   ` Andreas Färber
  2021-08-13 18:09   ` Rob Herring
  2021-08-05  6:54 ` [PATCH 4/8] arm64: dts: add NXP S32G2 support Chester Lin
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add a compatible string for the uart binding of NXP S32G2 platforms. Here
we use "s32v234-linflexuart" as fallback since the current linflexuart
driver still works on S32G2.

Signed-off-by: Chester Lin <clin@suse.com>
---
 .../bindings/serial/fsl,s32-linflexuart.yaml  | 26 ++++++++++++++++---
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
index acfe34706ccb..e731f3f6cea4 100644
--- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
@@ -21,10 +21,20 @@ allOf:
 
 properties:
   compatible:
-    description: The LINFlexD controller on S32V234 SoC, which can be
-      configured in UART mode.
-    items:
-      - const: fsl,s32v234-linflexuart
+    minItems: 1
+    maxItems: 2
+    oneOf:
+      - description: The LINFlexD controller on S32G2 SoC, which can be
+          configured in UART mode.
+        items:
+          - enum:
+              - fsl,s32g2-linflexuart
+          - const: fsl,s32v234-linflexuart
+
+      - description: The LINFlexD controller on S32V234 SoC, which can be
+          configured in UART mode.
+        items:
+          - const: fsl,s32v234-linflexuart
 
   reg:
     maxItems: 1
@@ -41,8 +51,16 @@ unevaluatedProperties: false
 
 examples:
   - |
+    /* S32V234 */
     serial@40053000 {
         compatible = "fsl,s32v234-linflexuart";
         reg = <0x40053000 0x1000>;
         interrupts = <0 59 4>;
     };
+
+    /* S32G2 */
+    serial@401c8000 {
+        compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart";
+        reg = <0x401c8000 0x3000>;
+        interrupts = <0 82 1>;
+    };
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (2 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2 Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 17:26   ` Andreas Färber
  2021-08-05  6:54 ` [PATCH 5/8] arm64: dts: s32g2: add serial/uart support Chester Lin
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add an initial dtsi file for generic SoC features of NXP S32G2.

Signed-off-by: Chester Lin <clin@suse.com>
---
 arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
new file mode 100644
index 000000000000..3321819c1a2d
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright (c) 2021 SUSE LLC
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "fsl,s32g2";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0>;
+			enable-method = "psci";
+			next-level-cache = <&cluster0_l2>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x1>;
+			enable-method = "psci";
+			next-level-cache = <&cluster0_l2>;
+		};
+
+		cpu2: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x100>;
+			enable-method = "psci";
+			next-level-cache = <&cluster1_l2>;
+		};
+
+		cpu3: cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x101>;
+			enable-method = "psci";
+			next-level-cache = <&cluster1_l2>;
+		};
+
+		cluster0_l2: l2-cache0 {
+			compatible = "cache";
+		};
+
+		cluster1_l2: l2-cache1 {
+			compatible = "cache";
+		};
+	};
+
+	pmu {
+		compatible = "arm,cortex-a53-pmu";
+		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+
+		ranges;
+
+		gic: interrupt-controller@50800000 {
+			compatible = "arm,gic-v3";
+			#interrupt-cells = <3>;
+			interrupt-controller;
+			reg = <0 0x50800000 0 0x10000>,
+			      <0 0x50880000 0 0x200000>,
+			      <0 0x50400000 0 0x2000>,
+			      <0 0x50410000 0 0x2000>,
+			      <0 0x50420000 0 0x2000>;
+			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+						 IRQ_TYPE_LEVEL_HIGH)>;
+		};
+	};
+};
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 5/8] arm64: dts: s32g2: add serial/uart support
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (3 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 4/8] arm64: dts: add NXP S32G2 support Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 17:42   ` Andreas Färber
  2021-08-05  6:54 ` [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support Chester Lin
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add serial/uart support for NXP S32G2.

Signed-off-by: Chester Lin <clin@suse.com>
---
 arch/arm64/boot/dts/freescale/s32g2.dtsi | 31 ++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index 3321819c1a2d..0076eacad8a6 100644
--- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
 /*
  * Copyright (c) 2021 SUSE LLC
+ * Copyright 2017-2020 NXP
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -11,6 +12,12 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -82,6 +89,30 @@ soc {
 
 		ranges;
 
+		uart0: serial@401c8000 {
+			compatible = "fsl,s32g2-linflexuart",
+				     "fsl,s32v234-linflexuart";
+			reg = <0 0x401c8000 0 0x3000>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
+			status = "disabled";
+		};
+
+		uart1: serial@401cc000 {
+			compatible = "fsl,s32g2-linflexuart",
+				     "fsl,s32v234-linflexuart";
+			reg = <0 0x401cc000 0 0x3000>;
+			interrupts = <GIC_SPI 83 IRQ_TYPE_EDGE_RISING>;
+			status = "disabled";
+		};
+
+		uart2: serial@402bc000 {
+			compatible = "fsl,s32g2-linflexuart",
+				     "fsl,s32v234-linflexuart";
+			reg = <0 0x402bc000 0 0x3000>;
+			interrupts = <GIC_SPI 84 IRQ_TYPE_EDGE_RISING>;
+			status = "disabled";
+		};
+
 		gic: interrupt-controller@50800000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (4 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 5/8] arm64: dts: s32g2: add serial/uart support Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 18:00   ` Andreas Färber
  2021-08-05  6:54 ` [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2 Chester Lin
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add initial device-trees of NXP S32G2's Evaluation Board (S32G-VNP-EVB)
and Reference Design 2 Board (S32G-VNP-RDB2).

Signed-off-by: Chester Lin <clin@suse.com>
---
 arch/arm64/boot/dts/freescale/Makefile        |  2 ++
 .../arm64/boot/dts/freescale/s32g274a-evb.dts | 21 ++++++++++++++++
 .../boot/dts/freescale/s32g274a-rdb2.dts      | 25 +++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
 create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 4b4785d86324..2886ddd42894 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -67,4 +67,6 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
 
+dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
+dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb
 dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
new file mode 100644
index 000000000000..a1ae5031730a
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright (c) 2021 SUSE LLC
+ */
+
+/dts-v1/;
+
+#include "s32g2.dtsi"
+
+/ {
+	model = "NXP S32G2 Evaluation Board (S32G-VNP-EVB)";
+	compatible = "fsl,s32g274a-evb", "fsl,s32g2";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
new file mode 100644
index 000000000000..b2faae306b70
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright (c) 2021 SUSE LLC
+ */
+
+/dts-v1/;
+
+#include "s32g2.dtsi"
+
+/ {
+	model = "NXP S32G2 Reference Design 2 (S32G-VNP-RDB2)";
+	compatible = "fsl,s32g274a-rdb2", "fsl,s32g2";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (5 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-12 18:25   ` Andreas Färber
  2021-08-05  6:54 ` [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards Chester Lin
  2021-08-09  8:06 ` [PATCH 0/8] arm64: dts: initial NXP S32G2 support Shawn Guo
  8 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add memory nodes for S32G-VNP-EVB and S32G-VNP-RDB2 since they have fixed
RAM size.

Signed-off-by: Chester Lin <clin@suse.com>
---
 arch/arm64/boot/dts/freescale/s32g274a-evb.dts  | 8 ++++++++
 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
index a1ae5031730a..cd41f0af5dd8 100644
--- a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
+++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
 /*
  * Copyright (c) 2021 SUSE LLC
+ * Copyright 2019-2020 NXP
  */
 
 /dts-v1/;
@@ -14,6 +15,13 @@ / {
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	memory@80000000 {
+		device_type = "memory";
+		/* 4GB RAM */
+		reg = <0 0x80000000 0 0x80000000>,
+		      <8 0x80000000 0 0x80000000>;
+	};
 };
 
 &uart0 {
diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
index b2faae306b70..8fbbf3b45eb8 100644
--- a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
+++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
 /*
  * Copyright (c) 2021 SUSE LLC
+ * Copyright 2019-2020 NXP
  */
 
 /dts-v1/;
@@ -14,6 +15,13 @@ / {
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	memory@80000000 {
+		device_type = "memory";
+		/* 4GB RAM */
+		reg = <0 0x80000000 0 0x80000000>,
+		      <8 0x80000000 0 0x80000000>;
+	};
 };
 
 &uart0 {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (6 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2 Chester Lin
@ 2021-08-05  6:54 ` Chester Lin
  2021-08-05  7:49   ` Krzysztof Kozlowski
  2021-08-09  8:06 ` [PATCH 0/8] arm64: dts: initial NXP S32G2 support Shawn Guo
  8 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-05  6:54 UTC (permalink / raw)
  To: Rob Herring, devicetree
  Cc: linux-kernel, linux-arm-kernel, linux-serial, Greg Kroah-Hartman,
	Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi, Chester Lin

Add a new entry for the maintenance of NXP S32G2 DT files.

Signed-off-by: Chester Lin <clin@suse.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 36aee8517ab0..3c6ba6cefd8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2281,6 +2281,12 @@ F:	arch/arm/boot/dts/nuvoton-wpcm450*
 F:	arch/arm/mach-npcm/wpcm450.c
 F:	drivers/*/*wpcm*
 
+ARM/NXP S32G2 ARCHITECTURE
+M:	Chester Lin <clin@suse.com>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	arch/arm64/boot/dts/freescale/s32g2*
+
 ARM/OPENMOKO NEO FREERUNNER (GTA02) MACHINE SUPPORT
 L:	openmoko-kernel@lists.openmoko.org (subscribers-only)
 S:	Orphan
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards
  2021-08-05  6:54 ` [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards Chester Lin
@ 2021-08-05  7:49   ` Krzysztof Kozlowski
  2021-08-09  8:03     ` Shawn Guo
  0 siblings, 1 reply; 46+ messages in thread
From: Krzysztof Kozlowski @ 2021-08-05  7:49 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, devicetree
  Cc: linux-kernel, linux-arm-kernel, linux-serial, Greg Kroah-Hartman,
	Shawn Guo, Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer,
	Li Yang, Fabio Estevam, Matteo Lisi, Frieder Schrempf,
	Tim Harvey, Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc,
	bogdan.folea, ciprianmarian.costea, radu-nicolae.pirea,
	ghennadi.procopciuc, Matthias Brugger, Andreas Färber,
	Ivan T . Ivanov, Lee, Chun-Yi

On 05/08/2021 08:54, Chester Lin wrote:
> Add a new entry for the maintenance of NXP S32G2 DT files.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  MAINTAINERS | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36aee8517ab0..3c6ba6cefd8f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2281,6 +2281,12 @@ F:	arch/arm/boot/dts/nuvoton-wpcm450*
>  F:	arch/arm/mach-npcm/wpcm450.c
>  F:	drivers/*/*wpcm*
>  
> +ARM/NXP S32G2 ARCHITECTURE
> +M:	Chester Lin <clin@suse.com>
> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	arch/arm64/boot/dts/freescale/s32g2*

I support the idea of sub-sub-architecture maintainers but I think idea
of in-file addresses was preferred:
https://lore.kernel.org/lkml/20200830122922.3884-1-shawnguo@kernel.org/


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards
  2021-08-05  7:49   ` Krzysztof Kozlowski
@ 2021-08-09  8:03     ` Shawn Guo
  2021-08-12 15:30       ` Andreas Färber
  0 siblings, 1 reply; 46+ messages in thread
From: Shawn Guo @ 2021-08-09  8:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chester Lin, Rob Herring, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Thu, Aug 05, 2021 at 09:49:51AM +0200, Krzysztof Kozlowski wrote:
> On 05/08/2021 08:54, Chester Lin wrote:
> > Add a new entry for the maintenance of NXP S32G2 DT files.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  MAINTAINERS | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 36aee8517ab0..3c6ba6cefd8f 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2281,6 +2281,12 @@ F:	arch/arm/boot/dts/nuvoton-wpcm450*
> >  F:	arch/arm/mach-npcm/wpcm450.c
> >  F:	drivers/*/*wpcm*
> >  
> > +ARM/NXP S32G2 ARCHITECTURE
> > +M:	Chester Lin <clin@suse.com>
> > +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> > +S:	Maintained
> > +F:	arch/arm64/boot/dts/freescale/s32g2*
> 
> I support the idea of sub-sub-architecture maintainers but I think idea
> of in-file addresses was preferred:
> https://lore.kernel.org/lkml/20200830122922.3884-1-shawnguo@kernel.org/

Thanks for reminding that the patch didn't land.  I just resent it with
your Reviewed-by tag added.  Thanks!

Shawn

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 0/8] arm64: dts: initial NXP S32G2 support
  2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
                   ` (7 preceding siblings ...)
  2021-08-05  6:54 ` [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards Chester Lin
@ 2021-08-09  8:06 ` Shawn Guo
  8 siblings, 0 replies; 46+ messages in thread
From: Shawn Guo @ 2021-08-09  8:06 UTC (permalink / raw)
  To: Chester Lin
  Cc: Rob Herring, Greg Kroah-Hartman, devicetree, linux-arm-kernel,
	s32, linux-kernel, linux-serial, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Thu, Aug 05, 2021 at 02:54:21PM +0800, Chester Lin wrote:
> Hello,
> 
> Here I'd like to propose a patchset, which is initial upstream support for NXP
> S32G2. S32G is a processor family developed by NXP for automotive solutions,
> such as vehicle networking and automotive high-performance processing. This
> series focuses on S32G2, which is the latest generation we can find at the
> moment. As the first round to support S32G2, this patchset only enables basic
> components and interfaces the SoC must have while kernel booting, which aims
> to have minimum hardware enablement for these two boards, S32G-VNP-EVB and
> S32G-VNP-RDB2. The concepts of how these boards work are originated from the
> downstream kernel tree[1] developed by NXP, which provides lots of details
> about the SoC S32G274A and its integrated boards. This series has been
> verified with downstream ATF[2] & U-Boot[3] based on the ATF boot flow.
> 
> Thanks,
> Chester
> 
> [1] https://source.codeaurora.org/external/autobsps32/linux/
> [2] https://source.codeaurora.org/external/autobsps32/arm-trusted-firmware/
> [3] https://source.codeaurora.org/external/autobsps32/u-boot/
> 
> Chester Lin (8):
>   dt-bindings: arm: fsl: add NXP S32G2 boards
>   dt-bindings: serial: fsl-linflexuart: convert to json-schema format
>   dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
>   arm64: dts: add NXP S32G2 support
>   arm64: dts: s32g2: add serial/uart support
>   arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support
>   arm64: dts: s32g2: add memory nodes for evb and rdb2

The dts changes look good to me.  I will pick up the series once
bindings gets acked by Rob.

Shawn

>   MAINTAINERS: Add an entry for NXP S32G2 boards
> 
>  .../devicetree/bindings/arm/fsl.yaml          |   7 +
>  .../bindings/serial/fsl,s32-linflexuart.txt   |  22 ---
>  .../bindings/serial/fsl,s32-linflexuart.yaml  |  66 +++++++++
>  MAINTAINERS                                   |   6 +
>  arch/arm64/boot/dts/freescale/Makefile        |   2 +
>  arch/arm64/boot/dts/freescale/s32g2.dtsi      | 129 ++++++++++++++++++
>  .../arm64/boot/dts/freescale/s32g274a-evb.dts |  29 ++++
>  .../boot/dts/freescale/s32g274a-rdb2.dts      |  33 +++++
>  8 files changed, 272 insertions(+), 22 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> 
> -- 
> 2.30.0
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards
  2021-08-09  8:03     ` Shawn Guo
@ 2021-08-12 15:30       ` Andreas Färber
  2021-08-12 15:54         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 15:30 UTC (permalink / raw)
  To: Shawn Guo, Krzysztof Kozlowski
  Cc: Chester Lin, Rob Herring, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

Hello Shawn and Krzysztof,

On 09.08.21 10:03, Shawn Guo wrote:
> On Thu, Aug 05, 2021 at 09:49:51AM +0200, Krzysztof Kozlowski wrote:
>> On 05/08/2021 08:54, Chester Lin wrote:
>>> Add a new entry for the maintenance of NXP S32G2 DT files.
>>>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>  MAINTAINERS | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 36aee8517ab0..3c6ba6cefd8f 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -2281,6 +2281,12 @@ F:	arch/arm/boot/dts/nuvoton-wpcm450*
>>>  F:	arch/arm/mach-npcm/wpcm450.c
>>>  F:	drivers/*/*wpcm*
>>>  
>>> +ARM/NXP S32G2 ARCHITECTURE

Suggestion from NXP is to use the broader S32G name.

>>> +M:	Chester Lin <clin@suse.com>
>>> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>>> +S:	Maintained
>>> +F:	arch/arm64/boot/dts/freescale/s32g2*
>>
>> I support the idea of sub-sub-architecture maintainers but I think idea
>> of in-file addresses was preferred:
>> https://lore.kernel.org/lkml/20200830122922.3884-1-shawnguo@kernel.org/

I had specifically asked Chester to add a MAINTAINERS section.

Is your apparent suggestion of not accepting this MAINTAINERS patch
based on the assumption that we're dealing with only one email address
in three files? What do you see as the threshold to warrant a section?

From my point of view, above MAINTAINERS entry is incomplete, as we
should CC the full team working on S32G for patch review, not just
Chester himself.
So that would in my mind have been additional R: and L: entries in that
MAINTAINERS section.

> Thanks for reminding that the patch didn't land.  I just resent it with
> your Reviewed-by tag added.  Thanks!

Your above patch does not make clear to me what syntax we should use for
adding email addresses to .dts[i] files now:

https://lore.kernel.org/lkml/20210809081033.GQ30984@dragon/

Especially when not dealing with file authors.

I get the impression it is not a replacement for an F: wildcard used in
MAINTAINERS, but rather a complement?

Please understand that this is not about a single .dts file, as your
patch suggests, but about a complete SoC family consisting of s32g*.dts*
as well as in the future drivers specific to this platform. It seems way
easier to specify the list of maintainers/reviewers in MAINTAINERS once
with suitable wildcard paths, than copying them into each and every
.dtsi and .dts file and driver .c/.h and later needing to sync multiple
places. If a bot or user has fixes or cleanups for the S32G code, we
want to know about it, so that NXP can consider it for their BSP
branches and SUSE for our SLE branches, and obviously for follow-up
patch series that are already in the works and waiting on this one.

Once merged, I would expect Chester or someone from NXP to set up an
S32G tree on kernel.org that gets integrated into linux-next and sends
pull requests to the SoC tree maintainers without bothering i.MX and
Layerscape maintainers. Did you handle that differently for S32V?

Thanks,
Andreas

P.S. Have you checked or considered whether your script change might
start to CC non-existing email addresses, since we wouldn't be allowed
to remove them from copyright or authorship statements to prevent that?

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
@ 2021-08-12 15:46   ` Andreas Färber
  2021-08-13 17:49     ` Rob Herring
  2021-08-13 17:53   ` Rob Herring
  1 sibling, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 15:46 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

Hello Rob and NXP,

On 05.08.21 08:54, Chester Lin wrote:
> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> design 2 board ( S32G-VNP-RDB2).
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index e2097011c4b0..3914aa09e503 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -983,6 +983,13 @@ properties:
>            - const: solidrun,lx2160a-cex7
>            - const: fsl,lx2160a
>  
> +      - description: S32G2 based Boards
> +        items:
> +          - enum:
> +              - fsl,s32g274a-evb
> +              - fsl,s32g274a-rdb2

@Rob: Should for new boards the description: syntax be used also for
enums? Or just at SoC level, and for board enums still traditional #
comments?

> +          - const: fsl,s32g2

@NXP: Is it sufficient here to have s32g2, or should we call this
s32g274a and adjust the description above to S32G274A?

Related, is the trailing A for Arm, like for the Layerscape chips? I.e.,
not for Alpha or rev.A or something that will change for non-eval chips?

> +
>        - description: S32V234 based Boards
>          items:
>            - enum:

Otherwise,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards
  2021-08-12 15:30       ` Andreas Färber
@ 2021-08-12 15:54         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 46+ messages in thread
From: Krzysztof Kozlowski @ 2021-08-12 15:54 UTC (permalink / raw)
  To: Andreas Färber, Shawn Guo
  Cc: Chester Lin, Rob Herring, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On 12/08/2021 17:30, Andreas Färber wrote:
> Hello Shawn and Krzysztof,
> 
> On 09.08.21 10:03, Shawn Guo wrote:
>> On Thu, Aug 05, 2021 at 09:49:51AM +0200, Krzysztof Kozlowski wrote:
>>> On 05/08/2021 08:54, Chester Lin wrote:
>>>> Add a new entry for the maintenance of NXP S32G2 DT files.
>>>>
>>>> Signed-off-by: Chester Lin <clin@suse.com>
>>>> ---
>>>>  MAINTAINERS | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 36aee8517ab0..3c6ba6cefd8f 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -2281,6 +2281,12 @@ F:	arch/arm/boot/dts/nuvoton-wpcm450*
>>>>  F:	arch/arm/mach-npcm/wpcm450.c
>>>>  F:	drivers/*/*wpcm*
>>>>  
>>>> +ARM/NXP S32G2 ARCHITECTURE
> 
> Suggestion from NXP is to use the broader S32G name.
> 
>>>> +M:	Chester Lin <clin@suse.com>
>>>> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>>>> +S:	Maintained
>>>> +F:	arch/arm64/boot/dts/freescale/s32g2*
>>>
>>> I support the idea of sub-sub-architecture maintainers but I think idea
>>> of in-file addresses was preferred:
>>> https://lore.kernel.org/lkml/20200830122922.3884-1-shawnguo@kernel.org/
> 
> I had specifically asked Chester to add a MAINTAINERS section.
> 
> Is your apparent suggestion of not accepting this MAINTAINERS patch
> based on the assumption that we're dealing with only one email address
> in three files? What do you see as the threshold to warrant a section?
> 
> From my point of view, above MAINTAINERS entry is incomplete, as we
> should CC the full team working on S32G for patch review, not just
> Chester himself.
> So that would in my mind have been additional R: and L: entries in that
> MAINTAINERS section.

I assumed this is a sub-sub-architecture (something coming from
Layerscape or i.MX) but it seems it's not, therefore I don't mind having
separate entry in MAINTAINERS. The idea of in-file maintainers was for
specific boards and SoCs belonging to existing sub-architectures.

I agree with your following reasons.

> 
>> Thanks for reminding that the patch didn't land.  I just resent it with
>> your Reviewed-by tag added.  Thanks!
> 
> Your above patch does not make clear to me what syntax we should use for
> adding email addresses to .dts[i] files now:
> 
> https://lore.kernel.org/lkml/20210809081033.GQ30984@dragon/
> 
> Especially when not dealing with file authors.
> 
> I get the impression it is not a replacement for an F: wildcard used in
> MAINTAINERS, but rather a complement?
> 
> Please understand that this is not about a single .dts file, as your
> patch suggests, but about a complete SoC family consisting of s32g*.dts*
> as well as in the future drivers specific to this platform. It seems way
> easier to specify the list of maintainers/reviewers in MAINTAINERS once
> with suitable wildcard paths, than copying them into each and every
> .dtsi and .dts file and driver .c/.h and later needing to sync multiple
> places. If a bot or user has fixes or cleanups for the S32G code, we
> want to know about it, so that NXP can consider it for their BSP
> branches and SUSE for our SLE branches, and obviously for follow-up
> patch series that are already in the works and waiting on this one.
> 
> Once merged, I would expect Chester or someone from NXP to set up an
> S32G tree on kernel.org that gets integrated into linux-next and sends
> pull requests to the SoC tree maintainers without bothering i.MX and
> Layerscape maintainers. Did you handle that differently for S32V?
> 
> Thanks,
> Andreas
> 
> P.S. Have you checked or considered whether your script change might
> start to CC non-existing email addresses, since we wouldn't be allowed
> to remove them from copyright or authorship statements to prevent that?

The same can happen for DT bindings maintainers.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-05  6:54 ` [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format Chester Lin
@ 2021-08-12 16:04   ` Andreas Färber
  2021-08-13 11:11     ` Chester Lin
  2021-08-13 18:07   ` Rob Herring
  1 sibling, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 16:04 UTC (permalink / raw)
  To: Chester Lin, Greg Kroah-Hartman, Rob Herring, Shawn Guo
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, s32, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On 05.08.21 08:54, Chester Lin wrote:
> Convert the FSL LINFlexD UART binding to json-schema.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
>  2 files changed, 48 insertions(+), 22 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml

Thanks for your effort, Chester.

> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> deleted file mode 100644
> index f1bbe0826be5..000000000000
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -* Freescale LINFlexD UART
> -
> -The LINFlexD controller implements several LIN protocol versions, as well as
> -support for full-duplex UART communication through 8-bit and 9-bit frames.
> -
> -See chapter 47 ("LINFlexD") in the reference manual[1].
> -
> -Required properties:
> -- compatible :
> -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
> -    is compatible with the one integrated on S32V234 SoC
> -- reg : Address and length of the register set for the device
> -- interrupts : Should contain uart interrupt
> -
> -Example:
> -uart0: serial@40053000 {
> -	compatible = "fsl,s32v234-linflexuart";
> -	reg = <0x0 0x40053000 0x0 0x1000>;
> -	interrupts = <0 59 4>;
> -};
> -
> -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> new file mode 100644
> index 000000000000..acfe34706ccb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Since this is dual-licensed and BSD-2-Clause would seem compatible with
GPLv3, this could probably be s/GPL-2.0-only/GPL-2.0-or-later/ ? Not a
blocker.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/fsl,s32-linflexuart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale LINFlexD UART
> +
> +description: |
> +  The LINFlexD controller implements several LIN protocol versions, as well
> +  as support for full-duplex UART communication through 8-bit and 9-bit
> +  frames. See chapter 47 ("LINFlexD") in the reference manual
> +  https://www.nxp.com/webapp/Download?colCode=S32V234RM.
> +
> +maintainers:
> +  - Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> +  - Rob Herring <robh@kernel.org>

@Shawn: I assume we need both of them to ack this (or an S32V maintainer
to volunteer), since they were not named in the .txt file before?

> +
> +allOf:
> +  - $ref: "serial.yaml"
> +
> +properties:
> +  compatible:
> +    description: The LINFlexD controller on S32V234 SoC, which can be
> +      configured in UART mode.
> +    items:
> +      - const: fsl,s32v234-linflexuart
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    serial@40053000 {
> +        compatible = "fsl,s32v234-linflexuart";
> +        reg = <0x40053000 0x1000>;
> +        interrupts = <0 59 4>;
> +    };

Otherwise looking sane,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  2021-08-05  6:54 ` [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2 Chester Lin
@ 2021-08-12 16:27   ` Andreas Färber
  2021-08-13 14:27     ` Radu Nicolae Pirea (NXP OSS)
  2021-08-13 18:11     ` Rob Herring
  2021-08-13 18:09   ` Rob Herring
  1 sibling, 2 replies; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 16:27 UTC (permalink / raw)
  To: Chester Lin, Greg Kroah-Hartman, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On 05.08.21 08:54, Chester Lin wrote:
> Add a compatible string for the uart binding of NXP S32G2 platforms. Here
> we use "s32v234-linflexuart" as fallback since the current linflexuart
> driver still works on S32G2.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 26 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> index acfe34706ccb..e731f3f6cea4 100644
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -21,10 +21,20 @@ allOf:
>  
>  properties:
>    compatible:
> -    description: The LINFlexD controller on S32V234 SoC, which can be
> -      configured in UART mode.
> -    items:
> -      - const: fsl,s32v234-linflexuart
> +    minItems: 1
> +    maxItems: 2

Are these necessary for oneOf?

> +    oneOf:
> +      - description: The LINFlexD controller on S32G2 SoC, which can be
> +          configured in UART mode.
> +        items:
> +          - enum:
> +              - fsl,s32g2-linflexuart
> +          - const: fsl,s32v234-linflexuart

This reads inconsistent to me: Either this oneOf is for S32G2 only, then
please turn the enum into a const. Or change the description to "on SoCs
compatible with S32V234" if we expect the enum list to grow.

I believe the idea here was to avoid unnecessary driver compatible and
earlycon compatible additions, while preparing for eventual quirks
specific to S32G2.

@NXP: Should this be s32g2- like above or s32g274a- specifically? Do you
agree this is a useful thing to prepare here, as opposed to using only
s32v234- in the s32g2* DT?

I assume the ordering is done alphabetically as S32G < S32V;
alternatively we might order S32V234 first and then the compatible ones.

> +
> +      - description: The LINFlexD controller on S32V234 SoC, which can be
> +          configured in UART mode.
> +        items:
> +          - const: fsl,s32v234-linflexuart

To minimize this S32G2 patch, would it be valid to do oneOf for the
single S32V in the preceding patch already? Then we would avoid the text
movement and re-indentation above and more easily see the lines newly
getting added for S32G2.

>  
>    reg:
>      maxItems: 1
> @@ -41,8 +51,16 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    /* S32V234 */

Could this be:
  - description: S32V234
    |
?

>      serial@40053000 {
>          compatible = "fsl,s32v234-linflexuart";
>          reg = <0x40053000 0x1000>;
>          interrupts = <0 59 4>;
>      };
> +
> +    /* S32G2 */

This should not be part of the S32V example, but a new one:

  - |

(or with description, as discussed above)

> +    serial@401c8000 {
> +        compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart";

Potentially affected by naming discussions above.

> +        reg = <0x401c8000 0x3000>;
> +        interrupts = <0 82 1>;
> +    };

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-05  6:54 ` [PATCH 4/8] arm64: dts: add NXP S32G2 support Chester Lin
@ 2021-08-12 17:26   ` Andreas Färber
  2021-08-13  3:28     ` Chester Lin
  2021-08-20 13:12     ` Marc Zyngier
  0 siblings, 2 replies; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 17:26 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Marc Zyngier

Hi Chester et al.,

On 05.08.21 08:54, Chester Lin wrote:
> Add an initial dtsi file for generic SoC features of NXP S32G2.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> new file mode 100644
> index 000000000000..3321819c1a2d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi

Note: This DT is for running on the Cortex-A53 cores, but S32G2 also has
Cortex-M7 cores. For Vybrid SoCs, DTs later got contributed to also run
on its Cortex-M4 core:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610.dtsi
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf500.dtsi
vs.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610m4.dtsi

Should we plan for this in our file naming here and in following patches
(e.g., s32g2-a53* vs. s32g2-m7*)? To me, a later concatenation of
s32g274am7* would look awkward, and s32g274a-m7* would sort between -evb
and -rdb2.

> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*

 * NXP S32G2 SoC family
 *
?

@NXP: Are any models other than 274A in the queue that we should
distinguish between s32g2.dtsi and s32g274a.dtsi here already?

> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	compatible = "fsl,s32g2";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0>;
> +			enable-method = "psci";
> +			next-level-cache = <&cluster0_l2>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x1>;
> +			enable-method = "psci";
> +			next-level-cache = <&cluster0_l2>;
> +		};
> +
> +		cpu2: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x100>;
> +			enable-method = "psci";
> +			next-level-cache = <&cluster1_l2>;
> +		};
> +
> +		cpu3: cpu@101 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x101>;
> +			enable-method = "psci";
> +			next-level-cache = <&cluster1_l2>;
> +		};
> +
> +		cluster0_l2: l2-cache0 {
> +			compatible = "cache";
> +		};
> +
> +		cluster1_l2: l2-cache1 {
> +			compatible = "cache";
> +		};
> +	};
> +
> +	pmu {
> +		compatible = "arm,cortex-a53-pmu";
> +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;

interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;

> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0";
> +		method = "smc";
> +	};

Should we move this into a /firmware node, to group with future OP-TEE?

> +
> +	soc {
> +		compatible = "simple-bus";
> +		interrupt-parent = <&gic>;

Duplicate, already set on root node.

> +		#address-cells = <2>;
> +		#size-cells = <2>;

Why? Does it have any peripherals that go beyond 32-bit space?
For 64-bit Realtek platforms Rob had asked me to use 1, if possible.
I do understand that for /memory nodes we do have high-memory addresses,
so 2 for the root node looks correct.

> +

Please drop this white line.

> +		ranges;

According to Rob, the /soc ranges should exclude any RAM ranges for
safety reasons. Compare:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/realtek/rtd129x.dtsi

If you're lacking the maximum RAM areas to carve out, NXP is in CC to
help out :) and the EVB and RDB2 boards should give you starting numbers
that could be enlarged later if needed.

> +
> +		gic: interrupt-controller@50800000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			reg = <0 0x50800000 0 0x10000>,
> +			      <0 0x50880000 0 0x200000>,
> +			      <0 0x50400000 0 0x2000>,
> +			      <0 0x50410000 0 0x2000>,
> +			      <0 0x50420000 0 0x2000>;

Please order reg after compatible by convention, and sort
interrupt-controller or at least #interrupt-cells (applying to
consumers) last, after the below one applying to this device itself.

> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> +						 IRQ_TYPE_LEVEL_HIGH)>;
> +		};

CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.

> +	};
> +};

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 5/8] arm64: dts: s32g2: add serial/uart support
  2021-08-05  6:54 ` [PATCH 5/8] arm64: dts: s32g2: add serial/uart support Chester Lin
@ 2021-08-12 17:42   ` Andreas Färber
  2021-08-13  9:54     ` Radu Nicolae Pirea (NXP OSS)
  0 siblings, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 17:42 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

Hi Chester et al.,

On 05.08.21 08:54, Chester Lin wrote:
> Add serial/uart support for NXP S32G2.

You might mention here that (following our initial stub) this commit is
now apparently based on the CodeAurora BSP branch foo (and therefore
adding its last-year copyright below and separate from 4/8).

> 

@NXP: If there are downstream Signed-off-bys that you would like to see
included for this portion here, please speak up.

> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 31 ++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index 3321819c1a2d..0076eacad8a6 100644
> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>  /*
>   * Copyright (c) 2021 SUSE LLC
> + * Copyright 2017-2020 NXP

@NXP: Should this be updated to include 2021 from your latest BSP
releases? Do you want it visually aligned by adding the ASCII-art?

>   */
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -11,6 +12,12 @@ / {
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +	};

Note: In the past there had been controversies as to whether to define
aliases globally for a SoC or in a .dts specific to a board's usage.
In this case it does not seem to matter much, as uart0 is being used as
console on the reference boards.

> +
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -82,6 +89,30 @@ soc {
>  
>  		ranges;
>  
> +		uart0: serial@401c8000 {
> +			compatible = "fsl,s32g2-linflexuart",
> +				     "fsl,s32v234-linflexuart";
> +			reg = <0 0x401c8000 0 0x3000>;
> +			interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
> +			status = "disabled";
> +		};
> +
> +		uart1: serial@401cc000 {
> +			compatible = "fsl,s32g2-linflexuart",
> +				     "fsl,s32v234-linflexuart";
> +			reg = <0 0x401cc000 0 0x3000>;
> +			interrupts = <GIC_SPI 83 IRQ_TYPE_EDGE_RISING>;
> +			status = "disabled";
> +		};
> +
> +		uart2: serial@402bc000 {
> +			compatible = "fsl,s32g2-linflexuart",
> +				     "fsl,s32v234-linflexuart";
> +			reg = <0 0x402bc000 0 0x3000>;
> +			interrupts = <GIC_SPI 84 IRQ_TYPE_EDGE_RISING>;
> +			status = "disabled";
> +		};
> +
>  		gic: interrupt-controller@50800000 {
>  			compatible = "arm,gic-v3";
>  			#interrupt-cells = <3>;

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support
  2021-08-05  6:54 ` [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support Chester Lin
@ 2021-08-12 18:00   ` Andreas Färber
  2021-08-13  8:47     ` Chester Lin
  0 siblings, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 18:00 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

Hi Chester et al.,

On 05.08.21 08:54, Chester Lin wrote:
> Add initial device-trees of NXP S32G2's Evaluation Board (S32G-VNP-EVB)
> and Reference Design 2 Board (S32G-VNP-RDB2).

"Reference Design Board 2"?

> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |  2 ++
>  .../arm64/boot/dts/freescale/s32g274a-evb.dts | 21 ++++++++++++++++
>  .../boot/dts/freescale/s32g274a-rdb2.dts      | 25 +++++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 4b4785d86324..2886ddd42894 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -67,4 +67,6 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>  
> +dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
> +dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb

@NXP: Note that since there's no distinction between S32V and S32G on
the Kconfig level, we decided not to add a white line here. If you wish
to visually separate them, that could be changed.

For reference:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/Kconfig.platforms

speaks about S32 (without V), so did not need to get updated for S32G2.

>  dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> new file mode 100644
> index 000000000000..a1ae5031730a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g2.dtsi"
> +
> +/ {
> +	model = "NXP S32G2 Evaluation Board (S32G-VNP-EVB)";
> +	compatible = "fsl,s32g274a-evb", "fsl,s32g2";
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +

Is there any marking on the PCB that a /* ... */ comment could reference
here? Right now uart0 is close to stdout-path above, but that will
change once more device nodes get added and enabled alphabetically.

> +&uart0 {
> +	status = "okay";

No clock-frequency or clocks property needed?

> +};
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> new file mode 100644
> index 000000000000..b2faae306b70
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g2.dtsi"
> +
> +/ {
> +	model = "NXP S32G2 Reference Design 2 (S32G-VNP-RDB2)";

"Board" missing.

> +	compatible = "fsl,s32g274a-rdb2", "fsl,s32g2";
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +

Comment please.

> +&uart0 {
> +	status = "okay";

No clock-frequency or clocks property needed?

> +};
> +
> +&uart1 {
> +	status = "okay";
> +};

What is uart1? Please add a comment. Does it actually work without
clocks property?

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2
  2021-08-05  6:54 ` [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2 Chester Lin
@ 2021-08-12 18:25   ` Andreas Färber
  2021-08-13 14:58     ` Chester Lin
  0 siblings, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-08-12 18:25 UTC (permalink / raw)
  To: Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

Hi Chester et al.,

On 05.08.21 08:54, Chester Lin wrote:
> Add memory nodes for S32G-VNP-EVB and S32G-VNP-RDB2 since they have fixed
> RAM size.

You can drop "since they have fixed RAM size" - if they didn't, you
would simply choose the lowest size and rely on the bootloader (U-Boot)
to overwrite it with the actually detected size.

Please expand why this patch is separate - BSP based, I assume?

> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  arch/arm64/boot/dts/freescale/s32g274a-evb.dts  | 8 ++++++++
>  arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> index a1ae5031730a..cd41f0af5dd8 100644
> --- a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>  /*
>   * Copyright (c) 2021 SUSE LLC
> + * Copyright 2019-2020 NXP

@NXP: Please review year, alignment. Do any Signed-off-bys apply?

>   */
>  
>  /dts-v1/;
> @@ -14,6 +15,13 @@ / {
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		/* 4GB RAM */

This looks strange to me - either put /* 4 GiB RAM */ before the node,
three lines above, and/or append comment /* 2 GiB */ on each line below.
Note the space, and suggest to be precise about factor 1024 vs. 1000.

> +		reg = <0 0x80000000 0 0x80000000>,

Note that this gives you the range to use for the .dtsi /soc node:
Address 0x0 with size 0x80000000 gets mapped to 0x0 0x0, excluding the
upper 0x80000000 for the RAM here. Or address 0x0 0x0 for two /soc cells
if there are high-memory peripherals.

> +		      <8 0x80000000 0 0x80000000>;

Maybe use 0x8 here and 0x0 above? (second 0 stays same, so don't mind)

> +	};
>  };
>  
>  &uart0 {
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> index b2faae306b70..8fbbf3b45eb8 100644
> --- a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>  /*
>   * Copyright (c) 2021 SUSE LLC
> + * Copyright 2019-2020 NXP

@NXP: 2021?

>   */
>  
>  /dts-v1/;
> @@ -14,6 +15,13 @@ / {
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		/* 4GB RAM */
> +		reg = <0 0x80000000 0 0x80000000>,
> +		      <8 0x80000000 0 0x80000000>;
> +	};

Same comments as for EVB.

>  };
>  
>  &uart0 {

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-12 17:26   ` Andreas Färber
@ 2021-08-13  3:28     ` Chester Lin
  2021-08-13  7:05       ` Andreas Färber
  2021-08-20 13:12     ` Marc Zyngier
  1 sibling, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-13  3:28 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Rob Herring, s32, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Marc Zyngier

On Thu, Aug 12, 2021 at 07:26:28PM +0200, Andreas Färber wrote:
> Hi Chester et al.,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > new file mode 100644
> > index 000000000000..3321819c1a2d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> 
> Note: This DT is for running on the Cortex-A53 cores, but S32G2 also has
> Cortex-M7 cores. For Vybrid SoCs, DTs later got contributed to also run
> on its Cortex-M4 core:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610.dtsi
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf500.dtsi
> vs.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610m4.dtsi
> 
> Should we plan for this in our file naming here and in following patches
> (e.g., s32g2-a53* vs. s32g2-m7*)? To me, a later concatenation of
> s32g274am7* would look awkward, and s32g274a-m7* would sort between -evb
> and -rdb2.
> 
> > @@ -0,0 +1,98 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/*
> 
>  * NXP S32G2 SoC family
>  *
> ?

Will add it.

> 
> @NXP: Are any models other than 274A in the queue that we should
> distinguish between s32g2.dtsi and s32g274a.dtsi here already?
> 
> > + * Copyright (c) 2021 SUSE LLC
> > + */
> > +
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +/ {
> > +	compatible = "fsl,s32g2";
> > +	interrupt-parent = <&gic>;
> > +	#address-cells = <2>;
> > +	#size-cells = <2>;
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x0>;
> > +			enable-method = "psci";
> > +			next-level-cache = <&cluster0_l2>;
> > +		};
> > +
> > +		cpu1: cpu@1 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x1>;
> > +			enable-method = "psci";
> > +			next-level-cache = <&cluster0_l2>;
> > +		};
> > +
> > +		cpu2: cpu@100 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x100>;
> > +			enable-method = "psci";
> > +			next-level-cache = <&cluster1_l2>;
> > +		};
> > +
> > +		cpu3: cpu@101 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x101>;
> > +			enable-method = "psci";
> > +			next-level-cache = <&cluster1_l2>;
> > +		};
> > +
> > +		cluster0_l2: l2-cache0 {
> > +			compatible = "cache";
> > +		};
> > +
> > +		cluster1_l2: l2-cache1 {
> > +			compatible = "cache";
> > +		};
> > +	};
> > +
> > +	pmu {
> > +		compatible = "arm,cortex-a53-pmu";
> > +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
> 
> interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;

Actually I traced the pmu_parse_irqs() and found that this SoC never falls into
the pmu_has_irq_affinity() check because it's a percpu IRQ so the flow ends at
pmu_parse_percpu_irq(). But it looks good to me to have an interrupt-affinity
to indicate that each core has an associated PPI for PMU events as the binding
file has suggested.

> 
> > +	};
> > +
> > +	timer {
> > +		compatible = "arm,armv8-timer";
> > +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > +	};
> > +
> > +	psci {
> > +		compatible = "arm,psci-1.0";
> > +		method = "smc";
> > +	};
> 
> Should we move this into a /firmware node, to group with future OP-TEE?
> 

So far I can only see a few examples [e.g. ti/k3-*] which add the psci node
into /firmware but logically we should do it.

> > +
> > +	soc {
> > +		compatible = "simple-bus";
> > +		interrupt-parent = <&gic>;
> 
> Duplicate, already set on root node.

Will remove it.

> 
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> 
> Why? Does it have any peripherals that go beyond 32-bit space?
> For 64-bit Realtek platforms Rob had asked me to use 1, if possible.
> I do understand that for /memory nodes we do have high-memory addresses,
> so 2 for the root node looks correct.

Actually it's a limitation due to [PATCH 7/8] "arm64: dts: s32g2: add memory
nodes for evb and rdb2", which adds memory nodes to indicate maximum system
RAM size combined by two separated memory banks, and the second bank starts
from the offset 0x880000000 so that's why we need 64-bit address space here.
Please feel free to let me know if any suggestion.

> 
> > +
> 
> Please drop this white line.
> 

Will do.

> > +		ranges;
> 
> According to Rob, the /soc ranges should exclude any RAM ranges for
> safety reasons. Compare:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/realtek/rtd129x.dtsi
> 
> If you're lacking the maximum RAM areas to carve out, NXP is in CC to
> help out :) and the EVB and RDB2 boards should give you starting numbers
> that could be enlarged later if needed.

I added memory nodes in [PATCH 7/8] "arm64: dts: s32g2: add memory nodes for
evb and rdb2" to describe maximum RAM areas, which are based on the information
we found in NXP BSP and boards.

@NXP: Please feel free to correct me if anything wrong, thanks.

> 
> > +
> > +		gic: interrupt-controller@50800000 {
> > +			compatible = "arm,gic-v3";
> > +			#interrupt-cells = <3>;
> > +			interrupt-controller;
> > +			reg = <0 0x50800000 0 0x10000>,
> > +			      <0 0x50880000 0 0x200000>,
> > +			      <0 0x50400000 0 0x2000>,
> > +			      <0 0x50410000 0 0x2000>,
> > +			      <0 0x50420000 0 0x2000>;
> 
> Please order reg after compatible by convention, and sort
> interrupt-controller or at least #interrupt-cells (applying to
> consumers) last, after the below one applying to this device itself.
> 

Will do.

> > +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> > +						 IRQ_TYPE_LEVEL_HIGH)>;
> > +		};
> 
> CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.
> 

IIRC, gic-v3 shouldn't have this kind of interrupt specifier. It's my fault and
will fix it. Feel free to let me know if any suggestions.

> > +	};
> > +};
> 
> Thanks,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-13  3:28     ` Chester Lin
@ 2021-08-13  7:05       ` Andreas Färber
  0 siblings, 0 replies; 46+ messages in thread
From: Andreas Färber @ 2021-08-13  7:05 UTC (permalink / raw)
  To: Chester Lin
  Cc: Rob Herring, s32, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Marc Zyngier

Hi Chester,

On 13.08.21 05:28, Chester Lin wrote:
> On Thu, Aug 12, 2021 at 07:26:28PM +0200, Andreas Färber wrote:
>> On 05.08.21 08:54, Chester Lin wrote:
>>> Add an initial dtsi file for generic SoC features of NXP S32G2.
>>>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
>>>  1 file changed, 98 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>>> new file mode 100644
>>> index 000000000000..3321819c1a2d
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>>
>> Note: This DT is for running on the Cortex-A53 cores, but S32G2 also has
>> Cortex-M7 cores. For Vybrid SoCs, DTs later got contributed to also run
>> on its Cortex-M4 core:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610.dtsi
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf500.dtsi
>> vs.
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/vf610m4.dtsi
>>
>> Should we plan for this in our file naming here and in following patches
>> (e.g., s32g2-a53* vs. s32g2-m7*)? To me, a later concatenation of
>> s32g274am7* would look awkward, and s32g274a-m7* would sort between -evb
>> and -rdb2.
>>
>>> @@ -0,0 +1,98 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>> +/*
>>
>>  * NXP S32G2 SoC family
>>  *
>> ?
> 
> Will add it.
> 
>>
>> @NXP: Are any models other than 274A in the queue that we should
>> distinguish between s32g2.dtsi and s32g274a.dtsi here already?
>>
>>> + * Copyright (c) 2021 SUSE LLC
>>> + */
>>> +
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/ {
>>> +	compatible = "fsl,s32g2";
>>> +	interrupt-parent = <&gic>;
>>> +	#address-cells = <2>;
>>> +	#size-cells = <2>;
[...]
>>> +	pmu {
>>> +		compatible = "arm,cortex-a53-pmu";
>>> +		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
>>
>> interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> 
> Actually I traced the pmu_parse_irqs() and found that this SoC never falls into
> the pmu_has_irq_affinity() check because it's a percpu IRQ so the flow ends at
> pmu_parse_percpu_irq(). But it looks good to me to have an interrupt-affinity
> to indicate that each core has an associated PPI for PMU events as the binding
> file has suggested.

My comment was based on the DT binding requesting it also for PPI and
previous review requests for me to add it elsewhere, so it's quite
possible that your code analysis is correct.

FWIW also the cache nodes were not evaluated last time I checked,
although the specs do show those nodes and properties.

https://github.com/devicetree-org/devicetree-specification/blob/v0.3/source/devicenodes.rst

>>> +	};
[...]
>>> +
>>> +	soc {
>>> +		compatible = "simple-bus";
[...]
>>> +		#address-cells = <2>;
>>> +		#size-cells = <2>;
>>
>> Why? Does it have any peripherals that go beyond 32-bit space?
>> For 64-bit Realtek platforms Rob had asked me to use 1, if possible.
>> I do understand that for /memory nodes we do have high-memory addresses,
>> so 2 for the root node looks correct.
> 
> Actually it's a limitation due to [PATCH 7/8] "arm64: dts: s32g2: add memory
> nodes for evb and rdb2", which adds memory nodes to indicate maximum system
> RAM size combined by two separated memory banks, and the second bank starts
> from the offset 0x880000000 so that's why we need 64-bit address space here.
> Please feel free to let me know if any suggestion.

The /memory nodes are on the root node and thus only use #address-cells
and #size-cells from /, not from /soc here. Thus the only criterium is
whether something within /soc needs it.
PCI controllers may have 3 address cells, SPI/I2C controllers 1, so you
can have multiple sizes within one DT, you just need suitable ranges
wherever mappings to the parent address space are needed.
(I thought you had that distinction in an earlier draft I reviewed.)

Similarly, /reserved-memory would be outside the /soc node and use the
root node's #(address|size)-cells.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Note: I'm assuming that if someone were to add a DT for the Cortex-M7s,
the address space mapping of any shared peripherals will likely differ,
as would the interrupts obviously, so I assume no direct .dtsi sharing
of /soc sub-nodes could sensibly be applied here.

@NXP: Please correct me if I'm wrong in either M7 not having access to
DDR RAM or large enough SRAM for Linux or another DT-consuming OS (and
this line of thinking not being applicable then) or you having use cases
to factor out some of the /soc sub-nodes for sharing between the two
(which would then give another reason for #address-/size-cells = <1> for
32-bit Cortex-M7).

[...]
>>> +		ranges;
>>
>> According to Rob, the /soc ranges should exclude any RAM ranges for
>> safety reasons. Compare:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/realtek/rtd129x.dtsi
>>
>> If you're lacking the maximum RAM areas to carve out, NXP is in CC to
>> help out :) and the EVB and RDB2 boards should give you starting numbers
>> that could be enlarged later if needed.
> 
> I added memory nodes in [PATCH 7/8] "arm64: dts: s32g2: add memory nodes for
> evb and rdb2" to describe maximum RAM areas, which are based on the information
> we found in NXP BSP and boards.

Yes, as discussed in 7/8, you should be using here:

		ranges = <0x0 0 0x0 0x80000000>; /* excluding 4 GiB RAM */

https://github.com/devicetree-org/devicetree-specification/blob/v0.3/source/devicetree-basics.rst

> @NXP: Please feel free to correct me if anything wrong, thanks.
> 
>>
>>> +
>>> +		gic: interrupt-controller@50800000 {
>>> +			compatible = "arm,gic-v3";
>>> +			#interrupt-cells = <3>;
>>> +			interrupt-controller;
>>> +			reg = <0 0x50800000 0 0x10000>,
>>> +			      <0 0x50880000 0 0x200000>,
>>> +			      <0 0x50400000 0 0x2000>,
>>> +			      <0 0x50410000 0 0x2000>,
>>> +			      <0 0x50420000 0 0x2000>;
>>
>> Please order reg after compatible by convention, and sort
>> interrupt-controller or at least #interrupt-cells (applying to
>> consumers) last, after the below one applying to this device itself.
>>
> 
> Will do.
> 
>>> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
>>> +						 IRQ_TYPE_LEVEL_HIGH)>;
>>> +		};
>>
>> CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.
>>
> 
> IIRC, gic-v3 shouldn't have this kind of interrupt specifier. It's my fault and
> will fix it. Feel free to let me know if any suggestions.

You probably mean the GIC_CPU_MASK_SIMPLE()?

			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml

The interrupts property will be used by KVM (or possibly Xen) for vGIC.
Please ensure that the Kconfig you use for testing has KVM enabled.

>>> +	};
>>> +};

Cheers,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support
  2021-08-12 18:00   ` Andreas Färber
@ 2021-08-13  8:47     ` Chester Lin
  0 siblings, 0 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-13  8:47 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Rob Herring, s32, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On Thu, Aug 12, 2021 at 08:00:01PM +0200, Andreas Färber wrote:
> Hi Chester et al.,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add initial device-trees of NXP S32G2's Evaluation Board (S32G-VNP-EVB)
> > and Reference Design 2 Board (S32G-VNP-RDB2).
> 
> "Reference Design Board 2"?

It looks better. Will fix it.

> 
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile        |  2 ++
> >  .../arm64/boot/dts/freescale/s32g274a-evb.dts | 21 ++++++++++++++++
> >  .../boot/dts/freescale/s32g274a-rdb2.dts      | 25 +++++++++++++++++++
> >  3 files changed, 48 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> >  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 4b4785d86324..2886ddd42894 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -67,4 +67,6 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
> >  
> > +dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
> > +dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb
> 
> @NXP: Note that since there's no distinction between S32V and S32G on
> the Kconfig level, we decided not to add a white line here. If you wish
> to visually separate them, that could be changed.
> 
> For reference:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/Kconfig.platforms
> 
> speaks about S32 (without V), so did not need to get updated for S32G2.
> 
> >  dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> > new file mode 100644
> > index 000000000000..a1ae5031730a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/*
> > + * Copyright (c) 2021 SUSE LLC
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "s32g2.dtsi"
> > +
> > +/ {
> > +	model = "NXP S32G2 Evaluation Board (S32G-VNP-EVB)";
> > +	compatible = "fsl,s32g274a-evb", "fsl,s32g2";
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +};
> > +
> 
> Is there any marking on the PCB that a /* ... */ comment could reference
> here? Right now uart0 is close to stdout-path above, but that will
> change once more device nodes get added and enabled alphabetically.
> 

Yes, it's marked as "UART J58". Will add it in v2.

> > +&uart0 {
> > +	status = "okay";
> 
> No clock-frequency or clocks property needed?
> 

As a minimum support patch, it can still reply on TF-A's clock settings so no
clock property is required at the moment.

> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > new file mode 100644
> > index 000000000000..b2faae306b70
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > @@ -0,0 +1,25 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/*
> > + * Copyright (c) 2021 SUSE LLC
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "s32g2.dtsi"
> > +
> > +/ {
> > +	model = "NXP S32G2 Reference Design 2 (S32G-VNP-RDB2)";
> 
> "Board" missing.

This model name is based on what I can find on the official main page of RDB2,
which is shorter without having an additional "Board" string:

https://www.nxp.com/design/designs/s32g-reference-design-2-for-vehicle-network-processing:S32G-VNP-RDB2

But having a "Board" looks good to me as well.

> 
> > +	compatible = "fsl,s32g274a-rdb2", "fsl,s32g2";
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +};
> > +
> 
> Comment please.
> 
> > +&uart0 {
> > +	status = "okay";
> 
> No clock-frequency or clocks property needed?
> 

Same as what I have explained in the evb dts.

> > +};
> > +
> > +&uart1 {
> > +	status = "okay";
> > +};
> 
> What is uart1? Please add a comment. Does it actually work without
> clocks property?
> 

RDB2 has an additional serial port [uart1] on the board. Please see:
https://www.nxp.com/document/guide/get-started-with-the-s32g-reference-design-board-2-for-vehicle-network-processing:GS-S32G-VNP-RDB2

It's the same as the uart0, the uart1 is also driven by the same set of clks
[S32GEN1_CLK_LINFLEXD, S32GEN1_CLK_LIN_BAUD] according to the downstream TF-A:
https://source.codeaurora.org/external/autobsps32/arm-trusted-firmware/tree/drivers/nxp/s32g/clk/s32gen1_scmi_ids.c?h=release/bsp29.0-2.3#n44

> Thanks,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 5/8] arm64: dts: s32g2: add serial/uart support
  2021-08-12 17:42   ` Andreas Färber
@ 2021-08-13  9:54     ` Radu Nicolae Pirea (NXP OSS)
  0 siblings, 0 replies; 46+ messages in thread
From: Radu Nicolae Pirea (NXP OSS) @ 2021-08-13  9:54 UTC (permalink / raw)
  To: Andreas Färber, Chester Lin, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

Hi Andreas
On Thu, 2021-08-12 at 19:42 +0200, Andreas Färber wrote:
> Hi Chester et al.,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add serial/uart support for NXP S32G2.
> 
> You might mention here that (following our initial stub) this commit
> is
> now apparently based on the CodeAurora BSP branch foo (and therefore
> adding its last-year copyright below and separate from 4/8).
> 
> > 
> 
> @NXP: If there are downstream Signed-off-bys that you would like to
> see
> included for this portion here, please speak up.

Larisa signed-off should be added.
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>

> 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 31
> > ++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > index 3321819c1a2d..0076eacad8a6 100644
> > --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> >  /*
> >   * Copyright (c) 2021 SUSE LLC
> > + * Copyright 2017-2020 NXP
> 
> @NXP: Should this be updated to include 2021 from your latest BSP
> releases? Do you want it visually aligned by adding the ASCII-art?

Yes for both questions. The copyright year sould be updated to 2021 and
should be visually aligned.

> 
> >   */
> >  
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -11,6 +12,12 @@ / {
> >         #address-cells = <2>;
> >         #size-cells = <2>;
> >  
> > +       aliases {
> > +               serial0 = &uart0;
> > +               serial1 = &uart1;
> > +               serial2 = &uart2;
> > +       };
> 
> Note: In the past there had been controversies as to whether to
> define
> aliases globally for a SoC or in a .dts specific to a board's usage.
> In this case it does not seem to matter much, as uart0 is being used
> as
> console on the reference boards.
> 
> > +
> >         cpus {
> >                 #address-cells = <1>;
> >                 #size-cells = <0>;
> > @@ -82,6 +89,30 @@ soc {
> >  
> >                 ranges;
> >  
> > +               uart0: serial@401c8000 {
> > +                       compatible = "fsl,s32g2-linflexuart",
> > +                                    "fsl,s32v234-linflexuart";
> > +                       reg = <0 0x401c8000 0 0x3000>;
> > +                       interrupts = <GIC_SPI 82
> > IRQ_TYPE_EDGE_RISING>;
> > +                       status = "disabled";
> > +               };
> > +
> > +               uart1: serial@401cc000 {
> > +                       compatible = "fsl,s32g2-linflexuart",
> > +                                    "fsl,s32v234-linflexuart";
> > +                       reg = <0 0x401cc000 0 0x3000>;
> > +                       interrupts = <GIC_SPI 83
> > IRQ_TYPE_EDGE_RISING>;
> > +                       status = "disabled";
> > +               };
> > +
> > +               uart2: serial@402bc000 {
> > +                       compatible = "fsl,s32g2-linflexuart",
> > +                                    "fsl,s32v234-linflexuart";
> > +                       reg = <0 0x402bc000 0 0x3000>;
> > +                       interrupts = <GIC_SPI 84
> > IRQ_TYPE_EDGE_RISING>;
> > +                       status = "disabled";
> > +               };
> > +
> >                 gic: interrupt-controller@50800000 {
> >                         compatible = "arm,gic-v3";
> >                         #interrupt-cells = <3>;
> 
> Regards,
> Andreas
> 



^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-12 16:04   ` Andreas Färber
@ 2021-08-13 11:11     ` Chester Lin
  2021-08-13 11:28       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-13 11:11 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Greg Kroah-Hartman, Rob Herring, Shawn Guo, devicetree,
	linux-kernel, linux-arm-kernel, linux-serial,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, s32, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

Hi Andreas,

On Thu, Aug 12, 2021 at 06:04:44PM +0200, Andreas Färber wrote:
> On 05.08.21 08:54, Chester Lin wrote:
> > Convert the FSL LINFlexD UART binding to json-schema.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
> >  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
> >  2 files changed, 48 insertions(+), 22 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> >  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> 
> Thanks for your effort, Chester.
> 
> > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> > deleted file mode 100644
> > index f1bbe0826be5..000000000000
> > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> > +++ /dev/null
> > @@ -1,22 +0,0 @@
> > -* Freescale LINFlexD UART
> > -
> > -The LINFlexD controller implements several LIN protocol versions, as well as
> > -support for full-duplex UART communication through 8-bit and 9-bit frames.
> > -
> > -See chapter 47 ("LINFlexD") in the reference manual[1].
> > -
> > -Required properties:
> > -- compatible :
> > -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
> > -    is compatible with the one integrated on S32V234 SoC
> > -- reg : Address and length of the register set for the device
> > -- interrupts : Should contain uart interrupt
> > -
> > -Example:
> > -uart0: serial@40053000 {
> > -	compatible = "fsl,s32v234-linflexuart";
> > -	reg = <0x0 0x40053000 0x0 0x1000>;
> > -	interrupts = <0 59 4>;
> > -};
> > -
> > -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > new file mode 100644
> > index 000000000000..acfe34706ccb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> 
> Since this is dual-licensed and BSD-2-Clause would seem compatible with
> GPLv3, this could probably be s/GPL-2.0-only/GPL-2.0-or-later/ ? Not a
> blocker.

There's no license identifier in the original file so it's not a problem to
choose "GPL-2.0-or-later".

> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/serial/fsl,s32-linflexuart.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale LINFlexD UART
> > +
> > +description: |
> > +  The LINFlexD controller implements several LIN protocol versions, as well
> > +  as support for full-duplex UART communication through 8-bit and 9-bit
> > +  frames. See chapter 47 ("LINFlexD") in the reference manual
> > +  https://www.nxp.com/webapp/Download?colCode=S32V234RM.
> > +
> > +maintainers:
> > +  - Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > +  - Rob Herring <robh@kernel.org>
> 
> @Shawn: I assume we need both of them to ack this (or an S32V maintainer
> to volunteer), since they were not named in the .txt file before?
> 
> > +
> > +allOf:
> > +  - $ref: "serial.yaml"
> > +
> > +properties:
> > +  compatible:
> > +    description: The LINFlexD controller on S32V234 SoC, which can be
> > +      configured in UART mode.
> > +    items:
> > +      - const: fsl,s32v234-linflexuart
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    serial@40053000 {
> > +        compatible = "fsl,s32v234-linflexuart";
> > +        reg = <0x40053000 0x1000>;
> > +        interrupts = <0 59 4>;
> > +    };
> 
> Otherwise looking sane,
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> Thanks,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-13 11:11     ` Chester Lin
@ 2021-08-13 11:28       ` Krzysztof Kozlowski
  2021-08-13 11:43         ` Chester Lin
  0 siblings, 1 reply; 46+ messages in thread
From: Krzysztof Kozlowski @ 2021-08-13 11:28 UTC (permalink / raw)
  To: Chester Lin, Andreas Färber
  Cc: Greg Kroah-Hartman, Rob Herring, Shawn Guo, devicetree,
	linux-kernel, linux-arm-kernel, linux-serial, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On 13/08/2021 13:11, Chester Lin wrote:
> Hi Andreas,
> 
> On Thu, Aug 12, 2021 at 06:04:44PM +0200, Andreas Färber wrote:
>> On 05.08.21 08:54, Chester Lin wrote:
>>> Convert the FSL LINFlexD UART binding to json-schema.
>>>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
>>>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
>>>  2 files changed, 48 insertions(+), 22 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>>>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>
>> Thanks for your effort, Chester.
>>
>>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>>> deleted file mode 100644
>>> index f1bbe0826be5..000000000000
>>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>>> +++ /dev/null
>>> @@ -1,22 +0,0 @@
>>> -* Freescale LINFlexD UART
>>> -
>>> -The LINFlexD controller implements several LIN protocol versions, as well as
>>> -support for full-duplex UART communication through 8-bit and 9-bit frames.
>>> -
>>> -See chapter 47 ("LINFlexD") in the reference manual[1].
>>> -
>>> -Required properties:
>>> -- compatible :
>>> -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
>>> -    is compatible with the one integrated on S32V234 SoC
>>> -- reg : Address and length of the register set for the device
>>> -- interrupts : Should contain uart interrupt
>>> -
>>> -Example:
>>> -uart0: serial@40053000 {
>>> -	compatible = "fsl,s32v234-linflexuart";
>>> -	reg = <0x0 0x40053000 0x0 0x1000>;
>>> -	interrupts = <0 59 4>;
>>> -};
>>> -
>>> -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
>>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>> new file mode 100644
>>> index 000000000000..acfe34706ccb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
>>> @@ -0,0 +1,48 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>
>> Since this is dual-licensed and BSD-2-Clause would seem compatible with
>> GPLv3, this could probably be s/GPL-2.0-only/GPL-2.0-or-later/ ? Not a
>> blocker.
> 
> There's no license identifier in the original file so it's not a problem to
> choose "GPL-2.0-or-later".

That is not entirely correct. If there is no explicit license in the
file, it's kernel's default so GPL-2.0-only. You cannot relicense
derivative work without getting permission from authors and copyright
holders.

However if you did not copy the text/description from original bindings,
your work won't be derivative so you can relicense it.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-13 11:28       ` Krzysztof Kozlowski
@ 2021-08-13 11:43         ` Chester Lin
  2021-08-13 18:04           ` Rob Herring
  0 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-13 11:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andreas Färber, Greg Kroah-Hartman, Rob Herring, Shawn Guo,
	devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On Fri, Aug 13, 2021 at 01:28:40PM +0200, Krzysztof Kozlowski wrote:
> On 13/08/2021 13:11, Chester Lin wrote:
> > Hi Andreas,
> > 
> > On Thu, Aug 12, 2021 at 06:04:44PM +0200, Andreas Färber wrote:
> >> On 05.08.21 08:54, Chester Lin wrote:
> >>> Convert the FSL LINFlexD UART binding to json-schema.
> >>>
> >>> Signed-off-by: Chester Lin <clin@suse.com>
> >>> ---
> >>>  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
> >>>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
> >>>  2 files changed, 48 insertions(+), 22 deletions(-)
> >>>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> >>>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> >>
> >> Thanks for your effort, Chester.
> >>
> >>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> >>> deleted file mode 100644
> >>> index f1bbe0826be5..000000000000
> >>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> >>> +++ /dev/null
> >>> @@ -1,22 +0,0 @@
> >>> -* Freescale LINFlexD UART
> >>> -
> >>> -The LINFlexD controller implements several LIN protocol versions, as well as
> >>> -support for full-duplex UART communication through 8-bit and 9-bit frames.
> >>> -
> >>> -See chapter 47 ("LINFlexD") in the reference manual[1].
> >>> -
> >>> -Required properties:
> >>> -- compatible :
> >>> -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
> >>> -    is compatible with the one integrated on S32V234 SoC
> >>> -- reg : Address and length of the register set for the device
> >>> -- interrupts : Should contain uart interrupt
> >>> -
> >>> -Example:
> >>> -uart0: serial@40053000 {
> >>> -	compatible = "fsl,s32v234-linflexuart";
> >>> -	reg = <0x0 0x40053000 0x0 0x1000>;
> >>> -	interrupts = <0 59 4>;
> >>> -};
> >>> -
> >>> -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> >>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> >>> new file mode 100644
> >>> index 000000000000..acfe34706ccb
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> >>> @@ -0,0 +1,48 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>
> >> Since this is dual-licensed and BSD-2-Clause would seem compatible with
> >> GPLv3, this could probably be s/GPL-2.0-only/GPL-2.0-or-later/ ? Not a
> >> blocker.
> > 
> > There's no license identifier in the original file so it's not a problem to
> > choose "GPL-2.0-or-later".
> 
> That is not entirely correct. If there is no explicit license in the
> file, it's kernel's default so GPL-2.0-only. You cannot relicense
> derivative work without getting permission from authors and copyright
> holders.
> 
> However if you did not copy the text/description from original bindings,
> your work won't be derivative so you can relicense it.
> 

Sorry for my misunderstanding and thanks for the reminder.

Regards,
Chester


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  2021-08-12 16:27   ` Andreas Färber
@ 2021-08-13 14:27     ` Radu Nicolae Pirea (NXP OSS)
  2021-08-13 18:11     ` Rob Herring
  1 sibling, 0 replies; 46+ messages in thread
From: Radu Nicolae Pirea (NXP OSS) @ 2021-08-13 14:27 UTC (permalink / raw)
  To: Andreas Färber, Chester Lin, Greg Kroah-Hartman, Rob Herring, s32
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Thu, 2021-08-12 at 18:27 +0200, Andreas Färber wrote:
> On 05.08.21 08:54, Chester Lin wrote:
> > Add a compatible string for the uart binding of NXP S32G2
> > platforms. Here
> > we use "s32v234-linflexuart" as fallback since the current
> > linflexuart
> > driver still works on S32G2.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  .../bindings/serial/fsl,s32-linflexuart.yaml  | 26
> > ++++++++++++++++---
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-
> > linflexuart.yaml
> > b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > index acfe34706ccb..e731f3f6cea4 100644
> > --- a/Documentation/devicetree/bindings/serial/fsl,s32-
> > linflexuart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-
> > linflexuart.yaml
> > @@ -21,10 +21,20 @@ allOf:
> >  
> >  properties:
> >    compatible:
> > -    description: The LINFlexD controller on S32V234 SoC, which can
> > be
> > -      configured in UART mode.
> > -    items:
> > -      - const: fsl,s32v234-linflexuart
> > +    minItems: 1
> > +    maxItems: 2
> 
> Are these necessary for oneOf?
> 
> > +    oneOf:
> > +      - description: The LINFlexD controller on S32G2 SoC, which
> > can be
> > +          configured in UART mode.
> > +        items:
> > +          - enum:
> > +              - fsl,s32g2-linflexuart
> > +          - const: fsl,s32v234-linflexuart
> 
> This reads inconsistent to me: Either this oneOf is for S32G2 only,
> then
> please turn the enum into a const. Or change the description to "on
> SoCs
> compatible with S32V234" if we expect the enum list to grow.
> 
> I believe the idea here was to avoid unnecessary driver compatible
> and
> earlycon compatible additions, while preparing for eventual quirks
> specific to S32G2.
> 
> @NXP: Should this be s32g2- like above or s32g274a- specifically? Do
> you
> agree this is a useful thing to prepare here, as opposed to using
> only
> s32v234- in the s32g2* DT?

s32g2- is fine, but the vendor should be nxp, not fsl.
nxp,s32g2-linflexuart

> 
> I assume the ordering is done alphabetically as S32G < S32V;
> alternatively we might order S32V234 first and then the compatible
> ones.
> 
> > +
> > +      - description: The LINFlexD controller on S32V234 SoC, which
> > can be
> > +          configured in UART mode.
> > +        items:
> > +          - const: fsl,s32v234-linflexuart
> 
> To minimize this S32G2 patch, would it be valid to do oneOf for the
> single S32V in the preceding patch already? Then we would avoid the
> text
> movement and re-indentation above and more easily see the lines newly
> getting added for S32G2.
> 
> >  
> >    reg:
> >      maxItems: 1
> > @@ -41,8 +51,16 @@ unevaluatedProperties: false
> >  
> >  examples:
> >    - |
> > +    /* S32V234 */
> 
> Could this be:
>   - description: S32V234
>     |
> ?
> 
> >      serial@40053000 {
> >          compatible = "fsl,s32v234-linflexuart";
> >          reg = <0x40053000 0x1000>;
> >          interrupts = <0 59 4>;
> >      };
> > +
> > +    /* S32G2 */
> 
> This should not be part of the S32V example, but a new one:
> 
>   - |
> 
> (or with description, as discussed above)
> 
> > +    serial@401c8000 {
> > +        compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-
> > linflexuart";
> 
> Potentially affected by naming discussions above.
> 
> > +        reg = <0x401c8000 0x3000>;
> > +        interrupts = <0 82 1>;
> > +    };
> 
> Regards,
> Andreas
> 



^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2
  2021-08-12 18:25   ` Andreas Färber
@ 2021-08-13 14:58     ` Chester Lin
  0 siblings, 0 replies; 46+ messages in thread
From: Chester Lin @ 2021-08-13 14:58 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Rob Herring, s32, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On Thu, Aug 12, 2021 at 08:25:00PM +0200, Andreas Färber wrote:
> Hi Chester et al.,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add memory nodes for S32G-VNP-EVB and S32G-VNP-RDB2 since they have fixed
> > RAM size.
> 
> You can drop "since they have fixed RAM size" - if they didn't, you
> would simply choose the lowest size and rely on the bootloader (U-Boot)
> to overwrite it with the actually detected size.
> 
> Please expand why this patch is separate - BSP based, I assume?
> 

Yes, the information of memory banks is from s32 downstream kernel, which is
listed in the board DTs of older releases [bsp27] although newer releases
[bsp28 & bsp29] have moved it into s32 downstream u-boot as runtime fdt-fixup.
I think we should still reveal this information in kernel DTs in order to have
better understanding of system memory ranges that each board can have.

@NXP: Please don't hesitate to let me know if any better idea, thanks!

> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  arch/arm64/boot/dts/freescale/s32g274a-evb.dts  | 8 ++++++++
> >  arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts | 8 ++++++++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> > index a1ae5031730a..cd41f0af5dd8 100644
> > --- a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> > +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> >  /*
> >   * Copyright (c) 2021 SUSE LLC
> > + * Copyright 2019-2020 NXP
> 
> @NXP: Please review year, alignment. Do any Signed-off-bys apply?
> 
> >   */
> >  
> >  /dts-v1/;
> > @@ -14,6 +15,13 @@ / {
> >  	chosen {
> >  		stdout-path = "serial0:115200n8";
> >  	};
> > +
> > +	memory@80000000 {
> > +		device_type = "memory";
> > +		/* 4GB RAM */
> 
> This looks strange to me - either put /* 4 GiB RAM */ before the node,
> three lines above, and/or append comment /* 2 GiB */ on each line below.
> Note the space, and suggest to be precise about factor 1024 vs. 1000.
> 

Thank you for the suggestion.

> > +		reg = <0 0x80000000 0 0x80000000>,
> 
> Note that this gives you the range to use for the .dtsi /soc node:
> Address 0x0 with size 0x80000000 gets mapped to 0x0 0x0, excluding the
> upper 0x80000000 for the RAM here. Or address 0x0 0x0 for two /soc cells
> if there are high-memory peripherals.
> 

I don't know if memory ranges might be changed for new boards or CPU revisions
in the future, which means the first memory range might be expanded as well
[e.g. 0~4GB]. Based this assumption, I think the size should also be changed
accordingly. Not sure if overlays can still work with this case but overwriting
all reg properties under /soc could be awful.

However if we only have to think of current hardware spec, it's good to declare
"range = <0 0 0 0x80000000>".

Please feel free to let me know if any suggestions, thanks.

> > +		      <8 0x80000000 0 0x80000000>;
> 
> Maybe use 0x8 here and 0x0 above? (second 0 stays same, so don't mind)
> 

Will fix it.

> > +	};
> >  };
> >  
> >  &uart0 {
> > diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > index b2faae306b70..8fbbf3b45eb8 100644
> > --- a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> >  /*
> >   * Copyright (c) 2021 SUSE LLC
> > + * Copyright 2019-2020 NXP
> 
> @NXP: 2021?
> 
> >   */
> >  
> >  /dts-v1/;
> > @@ -14,6 +15,13 @@ / {
> >  	chosen {
> >  		stdout-path = "serial0:115200n8";
> >  	};
> > +
> > +	memory@80000000 {
> > +		device_type = "memory";
> > +		/* 4GB RAM */
> > +		reg = <0 0x80000000 0 0x80000000>,
> > +		      <8 0x80000000 0 0x80000000>;
> > +	};
> 
> Same comments as for EVB.
> 
> >  };
> >  
> >  &uart0 {
> 
> Regards,
> Andreas
> 
> -- 
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-12 15:46   ` Andreas Färber
@ 2021-08-13 17:49     ` Rob Herring
  0 siblings, 0 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 17:49 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Chester Lin, s32, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On Thu, Aug 12, 2021 at 05:46:16PM +0200, Andreas Färber wrote:
> Hello Rob and NXP,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> > design 2 board ( S32G-VNP-RDB2).
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index e2097011c4b0..3914aa09e503 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -983,6 +983,13 @@ properties:
> >            - const: solidrun,lx2160a-cex7
> >            - const: fsl,lx2160a
> >  
> > +      - description: S32G2 based Boards
> > +        items:
> > +          - enum:
> > +              - fsl,s32g274a-evb
> > +              - fsl,s32g274a-rdb2
> 
> @Rob: Should for new boards the description: syntax be used also for
> enums? Or just at SoC level, and for board enums still traditional #
> comments?

It's up to how the platform wants to do it.

Rob

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
  2021-08-12 15:46   ` Andreas Färber
@ 2021-08-13 17:53   ` Rob Herring
  2021-08-18 14:34     ` Chester Lin
  2021-09-06 19:35     ` Andreas Färber
  1 sibling, 2 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 17:53 UTC (permalink / raw)
  To: Chester Lin
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> design 2 board ( S32G-VNP-RDB2).
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index e2097011c4b0..3914aa09e503 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -983,6 +983,13 @@ properties:
>            - const: solidrun,lx2160a-cex7
>            - const: fsl,lx2160a
>  
> +      - description: S32G2 based Boards
> +        items:
> +          - enum:
> +              - fsl,s32g274a-evb
> +              - fsl,s32g274a-rdb2
> +          - const: fsl,s32g2

Given this is an entirely different family from i.MX and new?, shouldn't 
it use 'nxp' instead of 'fsl'? Either way,

Acked-by: Rob Herring <robh@kernel.org>

Rob

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-13 11:43         ` Chester Lin
@ 2021-08-13 18:04           ` Rob Herring
  0 siblings, 0 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 18:04 UTC (permalink / raw)
  To: Chester Lin
  Cc: Krzysztof Kozlowski, Andreas Färber, Greg Kroah-Hartman,
	Shawn Guo, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, s32, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Fri, Aug 13, 2021 at 07:43:17PM +0800, Chester Lin wrote:
> On Fri, Aug 13, 2021 at 01:28:40PM +0200, Krzysztof Kozlowski wrote:
> > On 13/08/2021 13:11, Chester Lin wrote:
> > > Hi Andreas,
> > > 
> > > On Thu, Aug 12, 2021 at 06:04:44PM +0200, Andreas Färber wrote:
> > >> On 05.08.21 08:54, Chester Lin wrote:
> > >>> Convert the FSL LINFlexD UART binding to json-schema.
> > >>>
> > >>> Signed-off-by: Chester Lin <clin@suse.com>
> > >>> ---
> > >>>  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
> > >>>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
> > >>>  2 files changed, 48 insertions(+), 22 deletions(-)
> > >>>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> > >>>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > >>
> > >> Thanks for your effort, Chester.
> > >>
> > >>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> > >>> deleted file mode 100644
> > >>> index f1bbe0826be5..000000000000
> > >>> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> > >>> +++ /dev/null
> > >>> @@ -1,22 +0,0 @@
> > >>> -* Freescale LINFlexD UART
> > >>> -
> > >>> -The LINFlexD controller implements several LIN protocol versions, as well as
> > >>> -support for full-duplex UART communication through 8-bit and 9-bit frames.
> > >>> -
> > >>> -See chapter 47 ("LINFlexD") in the reference manual[1].
> > >>> -
> > >>> -Required properties:
> > >>> -- compatible :
> > >>> -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
> > >>> -    is compatible with the one integrated on S32V234 SoC
> > >>> -- reg : Address and length of the register set for the device
> > >>> -- interrupts : Should contain uart interrupt
> > >>> -
> > >>> -Example:
> > >>> -uart0: serial@40053000 {
> > >>> -	compatible = "fsl,s32v234-linflexuart";
> > >>> -	reg = <0x0 0x40053000 0x0 0x1000>;
> > >>> -	interrupts = <0 59 4>;
> > >>> -};
> > >>> -
> > >>> -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> > >>> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > >>> new file mode 100644
> > >>> index 000000000000..acfe34706ccb
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > >>> @@ -0,0 +1,48 @@
> > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > >>
> > >> Since this is dual-licensed and BSD-2-Clause would seem compatible with
> > >> GPLv3, this could probably be s/GPL-2.0-only/GPL-2.0-or-later/ ? Not a
> > >> blocker.
> > > 
> > > There's no license identifier in the original file so it's not a problem to
> > > choose "GPL-2.0-or-later".
> > 
> > That is not entirely correct. If there is no explicit license in the
> > file, it's kernel's default so GPL-2.0-only. You cannot relicense
> > derivative work without getting permission from authors and copyright
> > holders.
> > 
> > However if you did not copy the text/description from original bindings,
> > your work won't be derivative so you can relicense it.
> > 
> 
> Sorry for my misunderstanding and thanks for the reminder.

TBC, please try and get permission to relicense. The single author was 
from NXP so changing the license here just needs the okay from any NXP 
employee.

But use 'GPL-2.0-only' not 'GPL-2.0-or-later' please. As gregkh says,
are you okay with GPLv4, v5, etc.?

Rob

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format
  2021-08-05  6:54 ` [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format Chester Lin
  2021-08-12 16:04   ` Andreas Färber
@ 2021-08-13 18:07   ` Rob Herring
  1 sibling, 0 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 18:07 UTC (permalink / raw)
  To: Chester Lin
  Cc: Greg Kroah-Hartman, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Thu, Aug 05, 2021 at 02:54:23PM +0800, Chester Lin wrote:
> Convert the FSL LINFlexD UART binding to json-schema.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.txt   | 22 ---------
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 48 +++++++++++++++++++
>  2 files changed, 48 insertions(+), 22 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> deleted file mode 100644
> index f1bbe0826be5..000000000000
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -* Freescale LINFlexD UART
> -
> -The LINFlexD controller implements several LIN protocol versions, as well as
> -support for full-duplex UART communication through 8-bit and 9-bit frames.
> -
> -See chapter 47 ("LINFlexD") in the reference manual[1].
> -
> -Required properties:
> -- compatible :
> -  - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
> -    is compatible with the one integrated on S32V234 SoC
> -- reg : Address and length of the register set for the device
> -- interrupts : Should contain uart interrupt
> -
> -Example:
> -uart0: serial@40053000 {
> -	compatible = "fsl,s32v234-linflexuart";
> -	reg = <0x0 0x40053000 0x0 0x1000>;
> -	interrupts = <0 59 4>;
> -};
> -
> -[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> new file mode 100644
> index 000000000000..acfe34706ccb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/fsl,s32-linflexuart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale LINFlexD UART
> +
> +description: |
> +  The LINFlexD controller implements several LIN protocol versions, as well
> +  as support for full-duplex UART communication through 8-bit and 9-bit
> +  frames. See chapter 47 ("LINFlexD") in the reference manual
> +  https://www.nxp.com/webapp/Download?colCode=S32V234RM.
> +
> +maintainers:
> +  - Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> +  - Rob Herring <robh@kernel.org>

Someone that cares about this h/w, not who applies patches.

> +
> +allOf:
> +  - $ref: "serial.yaml"
> +
> +properties:
> +  compatible:
> +    description: The LINFlexD controller on S32V234 SoC, which can be
> +      configured in UART mode.

Drop 'on S32V234 SoC' so we're not editting for every new SoC.

> +    items:
> +      - const: fsl,s32v234-linflexuart
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    serial@40053000 {
> +        compatible = "fsl,s32v234-linflexuart";
> +        reg = <0x40053000 0x1000>;
> +        interrupts = <0 59 4>;
> +    };
> -- 
> 2.30.0
> 
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  2021-08-05  6:54 ` [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2 Chester Lin
  2021-08-12 16:27   ` Andreas Färber
@ 2021-08-13 18:09   ` Rob Herring
  1 sibling, 0 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 18:09 UTC (permalink / raw)
  To: Chester Lin
  Cc: Greg Kroah-Hartman, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Shawn Guo, Krzysztof Kozlowski, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Thu, Aug 05, 2021 at 02:54:24PM +0800, Chester Lin wrote:
> Add a compatible string for the uart binding of NXP S32G2 platforms. Here
> we use "s32v234-linflexuart" as fallback since the current linflexuart
> driver still works on S32G2.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.yaml  | 26 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> index acfe34706ccb..e731f3f6cea4 100644
> --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> @@ -21,10 +21,20 @@ allOf:
>  
>  properties:
>    compatible:
> -    description: The LINFlexD controller on S32V234 SoC, which can be
> -      configured in UART mode.
> -    items:
> -      - const: fsl,s32v234-linflexuart
> +    minItems: 1
> +    maxItems: 2

minItems/maxItems not needed here.

> +    oneOf:
> +      - description: The LINFlexD controller on S32G2 SoC, which can be
> +          configured in UART mode.
> +        items:
> +          - enum:
> +              - fsl,s32g2-linflexuart
> +          - const: fsl,s32v234-linflexuart
> +
> +      - description: The LINFlexD controller on S32V234 SoC, which can be
> +          configured in UART mode.
> +        items:
> +          - const: fsl,s32v234-linflexuart
>  
>    reg:
>      maxItems: 1
> @@ -41,8 +51,16 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    /* S32V234 */
>      serial@40053000 {
>          compatible = "fsl,s32v234-linflexuart";
>          reg = <0x40053000 0x1000>;
>          interrupts = <0 59 4>;
>      };
> +
> +    /* S32G2 */
> +    serial@401c8000 {
> +        compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart";
> +        reg = <0x401c8000 0x3000>;
> +        interrupts = <0 82 1>;
> +    };

This doesn't really warrant another example just for a new compatible 
string.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2
  2021-08-12 16:27   ` Andreas Färber
  2021-08-13 14:27     ` Radu Nicolae Pirea (NXP OSS)
@ 2021-08-13 18:11     ` Rob Herring
  1 sibling, 0 replies; 46+ messages in thread
From: Rob Herring @ 2021-08-13 18:11 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Chester Lin, Greg Kroah-Hartman, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On Thu, Aug 12, 2021 at 06:27:57PM +0200, Andreas Färber wrote:
> On 05.08.21 08:54, Chester Lin wrote:
> > Add a compatible string for the uart binding of NXP S32G2 platforms. Here
> > we use "s32v234-linflexuart" as fallback since the current linflexuart
> > driver still works on S32G2.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  .../bindings/serial/fsl,s32-linflexuart.yaml  | 26 ++++++++++++++++---
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > index acfe34706ccb..e731f3f6cea4 100644
> > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml
> > @@ -21,10 +21,20 @@ allOf:
> >  
> >  properties:
> >    compatible:
> > -    description: The LINFlexD controller on S32V234 SoC, which can be
> > -      configured in UART mode.
> > -    items:
> > -      - const: fsl,s32v234-linflexuart
> > +    minItems: 1
> > +    maxItems: 2
> 
> Are these necessary for oneOf?
> 
> > +    oneOf:
> > +      - description: The LINFlexD controller on S32G2 SoC, which can be
> > +          configured in UART mode.
> > +        items:
> > +          - enum:
> > +              - fsl,s32g2-linflexuart
> > +          - const: fsl,s32v234-linflexuart
> 
> This reads inconsistent to me: Either this oneOf is for S32G2 only, then
> please turn the enum into a const. Or change the description to "on SoCs
> compatible with S32V234" if we expect the enum list to grow.
> 
> I believe the idea here was to avoid unnecessary driver compatible and
> earlycon compatible additions, while preparing for eventual quirks
> specific to S32G2.
> 
> @NXP: Should this be s32g2- like above or s32g274a- specifically? Do you
> agree this is a useful thing to prepare here, as opposed to using only
> s32v234- in the s32g2* DT?
> 
> I assume the ordering is done alphabetically as S32G < S32V;
> alternatively we might order S32V234 first and then the compatible ones.
> 
> > +
> > +      - description: The LINFlexD controller on S32V234 SoC, which can be
> > +          configured in UART mode.
> > +        items:
> > +          - const: fsl,s32v234-linflexuart
> 
> To minimize this S32G2 patch, would it be valid to do oneOf for the
> single S32V in the preceding patch already? Then we would avoid the text
> movement and re-indentation above and more easily see the lines newly
> getting added for S32G2.
> 
> >  
> >    reg:
> >      maxItems: 1
> > @@ -41,8 +51,16 @@ unevaluatedProperties: false
> >  
> >  examples:
> >    - |
> > +    /* S32V234 */
> 
> Could this be:
>   - description: S32V234
>     |
> ?

No, that would not be valid json-schema.

Rob

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-13 17:53   ` Rob Herring
@ 2021-08-18 14:34     ` Chester Lin
  2021-09-06 20:38       ` Andreas Färber
  2021-09-06 19:35     ` Andreas Färber
  1 sibling, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-18 14:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Andreas Färber, Ivan T . Ivanov, Lee,
	Chun-Yi

On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> > Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> > design 2 board ( S32G-VNP-RDB2).
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index e2097011c4b0..3914aa09e503 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -983,6 +983,13 @@ properties:
> >            - const: solidrun,lx2160a-cex7
> >            - const: fsl,lx2160a
> >  
> > +      - description: S32G2 based Boards
> > +        items:
> > +          - enum:
> > +              - fsl,s32g274a-evb
> > +              - fsl,s32g274a-rdb2
> > +          - const: fsl,s32g2
> 
> Given this is an entirely different family from i.MX and new?, shouldn't 
> it use 'nxp' instead of 'fsl'? Either way,

It sounds good and Radu from NXP has mentioned a similar idea for the
compatible string of linflexuart. To keep the naming consistency, should we
change all 'fsl' to 'nxp' as well? For example, we could rename the fsl.yaml
to nxp.yaml. However, changing all of them would cause some impacts, which will
need more verifications on new strings. Otherwise we would have to tolerate the
naming differences only used by s32g2.

Thanks,
Chester

> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> Rob
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-12 17:26   ` Andreas Färber
  2021-08-13  3:28     ` Chester Lin
@ 2021-08-20 13:12     ` Marc Zyngier
  2021-08-20 15:15       ` Chester Lin
  1 sibling, 1 reply; 46+ messages in thread
From: Marc Zyngier @ 2021-08-20 13:12 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Chester Lin, Rob Herring, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman, Shawn Guo,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Thu, 12 Aug 2021 18:26:28 +0100,
Andreas Färber <afaerber@suse.de> wrote:
> 
> Hi Chester et al.,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > new file mode 100644
> > index 000000000000..3321819c1a2d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi

[...]

> > +		gic: interrupt-controller@50800000 {
> > +			compatible = "arm,gic-v3";
> > +			#interrupt-cells = <3>;
> > +			interrupt-controller;
> > +			reg = <0 0x50800000 0 0x10000>,
> > +			      <0 0x50880000 0 0x200000>,

That's enough redistributor space for 16 CPUs. However, you only
describe 4. Either the number of CPUs is wrong, the size is wrong, or
the GIC has been configured for more cores than the SoC has.

> > +			      <0 0x50400000 0 0x2000>,
> > +			      <0 0x50410000 0 0x2000>,
> > +			      <0 0x50420000 0 0x2000>;
> 
> Please order reg after compatible by convention, and sort
> interrupt-controller or at least #interrupt-cells (applying to
> consumers) last, after the below one applying to this device itself.
> 
> > +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> > +						 IRQ_TYPE_LEVEL_HIGH)>;
> > +		};
> 
> CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.

There is more than just sizes. The interrupt specifier for the
maintenance interrupt is also wrong.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-20 13:12     ` Marc Zyngier
@ 2021-08-20 15:15       ` Chester Lin
  2021-08-20 15:29         ` Marc Zyngier
  0 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-20 15:15 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Andreas Färber, Rob Herring, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman, Shawn Guo,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Fri, Aug 20, 2021 at 02:12:13PM +0100, Marc Zyngier wrote:
> On Thu, 12 Aug 2021 18:26:28 +0100,
> Andreas Färber <afaerber@suse.de> wrote:
> > 
> > Hi Chester et al.,
> > 
> > On 05.08.21 08:54, Chester Lin wrote:
> > > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > > 
> > > Signed-off-by: Chester Lin <clin@suse.com>
> > > ---
> > >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> > >  1 file changed, 98 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > new file mode 100644
> > > index 000000000000..3321819c1a2d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> 
> [...]
> 
> > > +		gic: interrupt-controller@50800000 {
> > > +			compatible = "arm,gic-v3";
> > > +			#interrupt-cells = <3>;
> > > +			interrupt-controller;
> > > +			reg = <0 0x50800000 0 0x10000>,
> > > +			      <0 0x50880000 0 0x200000>,
> 
> That's enough redistributor space for 16 CPUs. However, you only
> describe 4. Either the number of CPUs is wrong, the size is wrong, or
> the GIC has been configured for more cores than the SoC has.

Confirmed the SoC can only find 4 redistributors:

localhost:~ # dmesg | grep CPU
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
[    0.000000] Detected VIPT I-cache on CPU0
[    0.000000] CPU features: detected: GIC system register CPU interface
[    0.000000] CPU features: detected: ARM erratum 845719
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=480 to nr_cpu_ids=4.
[    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000
[    0.063865] smp: Bringing up secondary CPUs ...
[    0.068852] Detected VIPT I-cache on CPU1
[    0.068894] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000
[    0.068963] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[    0.069809] Detected VIPT I-cache on CPU2
[    0.069851] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000
[    0.069903] CPU2: Booted secondary processor 0x0000000100 [0x410fd034]
[    0.070698] Detected VIPT I-cache on CPU3
[    0.070722] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000
[    0.070749] CPU3: Booted secondary processor 0x0000000101 [0x410fd034]
[    0.070847] smp: Brought up 1 node, 4 CPUs
<..snip..>

I will correct the size to 0x80000, thanks!

> 
> > > +			      <0 0x50400000 0 0x2000>,
> > > +			      <0 0x50410000 0 0x2000>,
> > > +			      <0 0x50420000 0 0x2000>;
> > 
> > Please order reg after compatible by convention, and sort
> > interrupt-controller or at least #interrupt-cells (applying to
> > consumers) last, after the below one applying to this device itself.
> > 
> > > +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> > > +						 IRQ_TYPE_LEVEL_HIGH)>;
> > > +		};
> > 
> > CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.
> 
> There is more than just sizes. The interrupt specifier for the
> maintenance interrupt is also wrong.
> 
> 	M.

I will remove the wrong interrupt specifier. Thanks!

Chester.

> 
> -- 
> Without deviation from the norm, progress is not possible.
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-20 15:15       ` Chester Lin
@ 2021-08-20 15:29         ` Marc Zyngier
  2021-08-21 12:39           ` Chester Lin
  0 siblings, 1 reply; 46+ messages in thread
From: Marc Zyngier @ 2021-08-20 15:29 UTC (permalink / raw)
  To: Chester Lin
  Cc: Andreas Färber, Rob Herring, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman, Shawn Guo,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Fri, 20 Aug 2021 16:15:49 +0100,
Chester Lin <clin@suse.com> wrote:
> 
> On Fri, Aug 20, 2021 at 02:12:13PM +0100, Marc Zyngier wrote:
> > On Thu, 12 Aug 2021 18:26:28 +0100,
> > Andreas Färber <afaerber@suse.de> wrote:
> > > 
> > > Hi Chester et al.,
> > > 
> > > On 05.08.21 08:54, Chester Lin wrote:
> > > > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > > > 
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> > > >  1 file changed, 98 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > > new file mode 100644
> > > > index 000000000000..3321819c1a2d
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > 
> > [...]
> > 
> > > > +		gic: interrupt-controller@50800000 {
> > > > +			compatible = "arm,gic-v3";
> > > > +			#interrupt-cells = <3>;
> > > > +			interrupt-controller;
> > > > +			reg = <0 0x50800000 0 0x10000>,
> > > > +			      <0 0x50880000 0 0x200000>,
> > 
> > That's enough redistributor space for 16 CPUs. However, you only
> > describe 4. Either the number of CPUs is wrong, the size is wrong, or
> > the GIC has been configured for more cores than the SoC has.
> 
> Confirmed the SoC can only find 4 redistributors:
> 
> localhost:~ # dmesg | grep CPU
> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
> [    0.000000] Detected VIPT I-cache on CPU0
> [    0.000000] CPU features: detected: GIC system register CPU interface
> [    0.000000] CPU features: detected: ARM erratum 845719
> [    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> [    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=480 to nr_cpu_ids=4.
> [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000
> [    0.063865] smp: Bringing up secondary CPUs ...
> [    0.068852] Detected VIPT I-cache on CPU1
> [    0.068894] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000
> [    0.068963] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
> [    0.069809] Detected VIPT I-cache on CPU2
> [    0.069851] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000
> [    0.069903] CPU2: Booted secondary processor 0x0000000100 [0x410fd034]
> [    0.070698] Detected VIPT I-cache on CPU3
> [    0.070722] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000
> [    0.070749] CPU3: Booted secondary processor 0x0000000101 [0x410fd034]
> [    0.070847] smp: Brought up 1 node, 4 CPUs
> <..snip..>

That's not the correct way to find out. Each CPU tries to find its
matching RD in the region. This doesn't mean there aren't more RDs
present in the GIC.

You need to iterate over all the RDs in the region until you find one
that has GICR_TYPER.Last == 1. This will give you the actual count.
Alternatively, you can check whether the RD at 508e0000 has that bit
set. If it doesn't, then you know there are more RDs than CPUs.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-20 15:29         ` Marc Zyngier
@ 2021-08-21 12:39           ` Chester Lin
  2021-08-21 14:20             ` Marc Zyngier
  0 siblings, 1 reply; 46+ messages in thread
From: Chester Lin @ 2021-08-21 12:39 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Andreas Färber, Rob Herring, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman, Shawn Guo,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

Hi Marc,

On Fri, Aug 20, 2021 at 04:29:00PM +0100, Marc Zyngier wrote:
> On Fri, 20 Aug 2021 16:15:49 +0100,
> Chester Lin <clin@suse.com> wrote:
> > 
> > On Fri, Aug 20, 2021 at 02:12:13PM +0100, Marc Zyngier wrote:
> > > On Thu, 12 Aug 2021 18:26:28 +0100,
> > > Andreas Färber <afaerber@suse.de> wrote:
> > > > 
> > > > Hi Chester et al.,
> > > > 
> > > > On 05.08.21 08:54, Chester Lin wrote:
> > > > > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > > > > 
> > > > > Signed-off-by: Chester Lin <clin@suse.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> > > > >  1 file changed, 98 insertions(+)
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > > > new file mode 100644
> > > > > index 000000000000..3321819c1a2d
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > 
> > > [...]
> > > 
> > > > > +		gic: interrupt-controller@50800000 {
> > > > > +			compatible = "arm,gic-v3";
> > > > > +			#interrupt-cells = <3>;
> > > > > +			interrupt-controller;
> > > > > +			reg = <0 0x50800000 0 0x10000>,
> > > > > +			      <0 0x50880000 0 0x200000>,
> > > 
> > > That's enough redistributor space for 16 CPUs. However, you only
> > > describe 4. Either the number of CPUs is wrong, the size is wrong, or
> > > the GIC has been configured for more cores than the SoC has.
> > 
> > Confirmed the SoC can only find 4 redistributors:
> > 
> > localhost:~ # dmesg | grep CPU
> > [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
> > [    0.000000] Detected VIPT I-cache on CPU0
> > [    0.000000] CPU features: detected: GIC system register CPU interface
> > [    0.000000] CPU features: detected: ARM erratum 845719
> > [    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> > [    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=480 to nr_cpu_ids=4.
> > [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000
> > [    0.063865] smp: Bringing up secondary CPUs ...
> > [    0.068852] Detected VIPT I-cache on CPU1
> > [    0.068894] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000
> > [    0.068963] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
> > [    0.069809] Detected VIPT I-cache on CPU2
> > [    0.069851] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000
> > [    0.069903] CPU2: Booted secondary processor 0x0000000100 [0x410fd034]
> > [    0.070698] Detected VIPT I-cache on CPU3
> > [    0.070722] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000
> > [    0.070749] CPU3: Booted secondary processor 0x0000000101 [0x410fd034]
> > [    0.070847] smp: Brought up 1 node, 4 CPUs
> > <..snip..>
> 
> That's not the correct way to find out. Each CPU tries to find its
> matching RD in the region. This doesn't mean there aren't more RDs
> present in the GIC.
> 
> You need to iterate over all the RDs in the region until you find one
> that has GICR_TYPER.Last == 1. This will give you the actual count.
> Alternatively, you can check whether the RD at 508e0000 has that bit
> set. If it doesn't, then you know there are more RDs than CPUs.
> 
> 	M.
> 

Thanks for your guidance. Not sure if any debug log can be enabled for this
check so I temporarily add an ugly message as below:


diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e0f4debe64e1..5998306fff39 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -866,10 +866,11 @@ static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr)
 		gic_data_rdist_rd_base() = ptr;
 		gic_data_rdist()->phys_base = region->phys_base + offset;
 
-		pr_info("CPU%d: found redistributor %lx region %d:%pa\n",
+		pr_info("CPU%d: found redistributor %lx region %d:%pa last: %d\n",
 			smp_processor_id(), mpidr,
 			(int)(region - gic_data.redist_regions),
-			&gic_data_rdist()->phys_base);
+			&gic_data_rdist()->phys_base,
+			(typer & GICR_TYPER_LAST) ? 1 : 0);
 		return 0;
 	}


The following log shows that the "Last" bit (GICR_TYPER[4]) of RD at
508e0000 has been set.

localhost:~ # dmesg | grep GIC
[    0.000000] CPU features: detected: GIC system register CPU interface
[    0.000000] GICv3: 544 SPIs implemented
[    0.000000] GICv3: 0 Extended SPIs implemented
[    0.000000] GICv3: Distributor has no Range Selector support
[    0.000000] GICv3: 16 PPIs implemented
[    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000 last: 0
[    0.078745] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000 last: 0
[    0.089598] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000 last: 0
[    0.100395] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000 last: 1


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support
  2021-08-21 12:39           ` Chester Lin
@ 2021-08-21 14:20             ` Marc Zyngier
  0 siblings, 0 replies; 46+ messages in thread
From: Marc Zyngier @ 2021-08-21 14:20 UTC (permalink / raw)
  To: Chester Lin
  Cc: Andreas Färber, Rob Herring, s32, devicetree, linux-kernel,
	linux-arm-kernel, linux-serial, Greg Kroah-Hartman, Shawn Guo,
	Krzysztof Kozlowski, Oleksij Rempel, Stefan Riedmueller,
	Matthias Schiffer, Li Yang, Fabio Estevam, Matteo Lisi,
	Frieder Schrempf, Tim Harvey, Jagan Teki, catalin-dan.udma,
	bogdan.hamciuc, bogdan.folea, ciprianmarian.costea,
	radu-nicolae.pirea, ghennadi.procopciuc, Matthias Brugger,
	Ivan T . Ivanov, Lee, Chun-Yi

On Sat, 21 Aug 2021 13:39:04 +0100,
Chester Lin <clin@suse.com> wrote:
> 
> The following log shows that the "Last" bit (GICR_TYPER[4]) of RD at
> 508e0000 has been set.
> 
> localhost:~ # dmesg | grep GIC
> [    0.000000] CPU features: detected: GIC system register CPU interface
> [    0.000000] GICv3: 544 SPIs implemented
> [    0.000000] GICv3: 0 Extended SPIs implemented
> [    0.000000] GICv3: Distributor has no Range Selector support
> [    0.000000] GICv3: 16 PPIs implemented
> [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000 last: 0
> [    0.078745] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000 last: 0
> [    0.089598] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000 last: 0
> [    0.100395] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000 last: 1

Looks convincing enough. Trimming the RD range to 512kB is then the
right thing to do.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-13 17:53   ` Rob Herring
  2021-08-18 14:34     ` Chester Lin
@ 2021-09-06 19:35     ` Andreas Färber
  1 sibling, 0 replies; 46+ messages in thread
From: Andreas Färber @ 2021-09-06 19:35 UTC (permalink / raw)
  To: Rob Herring, Chester Lin
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi

On 13.08.21 19:53, Rob Herring wrote:
> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>> design 2 board ( S32G-VNP-RDB2).
>>
>> Signed-off-by: Chester Lin <clin@suse.com>
>> ---
>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>> index e2097011c4b0..3914aa09e503 100644
>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>> @@ -983,6 +983,13 @@ properties:
>>            - const: solidrun,lx2160a-cex7
>>            - const: fsl,lx2160a
>>  
>> +      - description: S32G2 based Boards
>> +        items:
>> +          - enum:
>> +              - fsl,s32g274a-evb
>> +              - fsl,s32g274a-rdb2
>> +          - const: fsl,s32g2
> 
> Given this is an entirely different family from i.MX and new?, shouldn't 
> it use 'nxp' instead of 'fsl'?

S32V also still used fsl prefix, despite the company name long being NXP
(same for several Layerscape and i.MX models).

If, as Radu indicated on 3/8, NXP wants to make that switch now for S32G
then I see no reason against nxp. I verified that it's already defined:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/vendor-prefixes.yaml

However, should the matching .dts[i] files using nxp prefix (4-6/8) then
still go under dts/freescale/, or should they go to a new dts/nxp/ then?
That would separate it from S32V. Intel did do a switch from dts/altera/
to dts/intel/ at some point, so there's precedence for either, I guess.
No idea whether anything might break if we moved S32V alongside S32G.

Similarly, the easiest and most merge-friendly would be to leave
arm/fsl.yaml and add the nxp-prefixed S32G2 there, as done here. If NXP
want to rename fsl.yaml to nxp.yaml in a general housekeeping effort,
that could be done independently, outside Chester's patchset.

> Either way,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-08-18 14:34     ` Chester Lin
@ 2021-09-06 20:38       ` Andreas Färber
  2021-09-07  6:59         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 46+ messages in thread
From: Andreas Färber @ 2021-09-06 20:38 UTC (permalink / raw)
  To: Chester Lin
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-serial,
	Greg Kroah-Hartman, Shawn Guo, Krzysztof Kozlowski,
	Oleksij Rempel, Stefan Riedmueller, Matthias Schiffer, Li Yang,
	Fabio Estevam, Matteo Lisi, Frieder Schrempf, Tim Harvey,
	Jagan Teki, s32, catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Rob Herring

Hi Chester,

On 18.08.21 16:34, Chester Lin wrote:
> On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
>> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>>> design 2 board ( S32G-VNP-RDB2).
>>>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index e2097011c4b0..3914aa09e503 100644
>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> @@ -983,6 +983,13 @@ properties:
>>>            - const: solidrun,lx2160a-cex7
>>>            - const: fsl,lx2160a
>>>  
>>> +      - description: S32G2 based Boards
>>> +        items:
>>> +          - enum:
>>> +              - fsl,s32g274a-evb
>>> +              - fsl,s32g274a-rdb2
>>> +          - const: fsl,s32g2
>>
>> Given this is an entirely different family from i.MX and new?, shouldn't 
>> it use 'nxp' instead of 'fsl'? Either way,
> 
> It sounds good and Radu from NXP has mentioned a similar idea for the
> compatible string of linflexuart. To keep the naming consistency, should we
> change all 'fsl' to 'nxp' as well?

I assume that question was just unclearly phrased, so for the record:

ABI stability rules forbid us from changing "all 'fsl'" in compatible
strings or property names.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst

Deployed firmware providing mainline-merged platforms with DTBs using
fsl prefix (e.g., the quoted LX2160A) needs to continue working with
newer drivers, and deployed mainline Linux should continue working after
firmware updates that modify the DTB provided to Linux.

So, if NXP wants to use nxp prefix for new S32G bindings, you can do
that for your additions only, but for LINFlexD UART (3/8) you will still
need to use fsl for the "historical" S32V binding used as fallback.

Please keep S32G consistent with itself - so if we decide on nxp here,
we should apply it to SoC, boards, LINFlexD and any future peripherals.

> For example, we could rename the fsl.yaml
> to nxp.yaml.

Since other people might be contributing i.MX boards etc. to that file,
better not make your patch series conflict with other people's patches,
so that it can get merged and we can move on to the next patchsets.

The schema filename is not ABI, so it can be renamed later.

The .dtb path may become ABI (e.g., U-Boot $fdtfile), thus my comment
about consciously deciding between freescale/ vs. nxp/ subdirectory.

> However, changing all of them would cause some impacts, which will
> need more verifications on new strings. Otherwise we would have to tolerate the
> naming differences only used by s32g2.

I fear tolerating the mess one way or another is the only viable way.
Otherwise both bindings and drivers would need duplication for backwards
compatibility, for no good reason - Freescale was acquired back in 2015.

Cheers,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-09-06 20:38       ` Andreas Färber
@ 2021-09-07  6:59         ` Krzysztof Kozlowski
  2021-09-07  8:59           ` Andreas Färber
  0 siblings, 1 reply; 46+ messages in thread
From: Krzysztof Kozlowski @ 2021-09-07  6:59 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Chester Lin, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Rob Herring

On Mon, 6 Sept 2021 at 22:38, Andreas Färber <afaerber@suse.de> wrote:
>
> Hi Chester,
>
> On 18.08.21 16:34, Chester Lin wrote:
> > On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
> >> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> >>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> >>> design 2 board ( S32G-VNP-RDB2).
> >>>
> >>> Signed-off-by: Chester Lin <clin@suse.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> index e2097011c4b0..3914aa09e503 100644
> >>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> @@ -983,6 +983,13 @@ properties:
> >>>            - const: solidrun,lx2160a-cex7
> >>>            - const: fsl,lx2160a
> >>>
> >>> +      - description: S32G2 based Boards
> >>> +        items:
> >>> +          - enum:
> >>> +              - fsl,s32g274a-evb
> >>> +              - fsl,s32g274a-rdb2
> >>> +          - const: fsl,s32g2
> >>
> >> Given this is an entirely different family from i.MX and new?, shouldn't
> >> it use 'nxp' instead of 'fsl'? Either way,
> >
> > It sounds good and Radu from NXP has mentioned a similar idea for the
> > compatible string of linflexuart. To keep the naming consistency, should we
> > change all 'fsl' to 'nxp' as well?
>
> I assume that question was just unclearly phrased, so for the record:
>
> ABI stability rules forbid us from changing "all 'fsl'" in compatible
> strings or property names.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst
>
> Deployed firmware providing mainline-merged platforms with DTBs using
> fsl prefix (e.g., the quoted LX2160A) needs to continue working with
> newer drivers, and deployed mainline Linux should continue working after
> firmware updates that modify the DTB provided to Linux.

This is a new platform/SoC therefore there is no ABI. There is no
requirement in the kernel that a new ABI (which you define in this
patchset in the bindings) should be compatible with something
somewhere. It's some misunderstanding of stable ABI. Therefore all new
compatibles are allowed to be nxp, not fsl.

No one here proposed renaming existing compatibles from fsl tro nxp.
We talk about new ones.

Different question of course whether you want to be nice to some
existing out-of-tree users... but then have in mind that we don't care
about out of tree. :) Anyway being nice to out-of-tree is not part of
ABI. It's just being nice and useful.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards
  2021-09-07  6:59         ` Krzysztof Kozlowski
@ 2021-09-07  8:59           ` Andreas Färber
  0 siblings, 0 replies; 46+ messages in thread
From: Andreas Färber @ 2021-09-07  8:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chester Lin, devicetree, linux-kernel, linux-arm-kernel,
	linux-serial, Greg Kroah-Hartman, Shawn Guo, Oleksij Rempel,
	Stefan Riedmueller, Matthias Schiffer, Li Yang, Fabio Estevam,
	Matteo Lisi, Frieder Schrempf, Tim Harvey, Jagan Teki, s32,
	catalin-dan.udma, bogdan.hamciuc, bogdan.folea,
	ciprianmarian.costea, radu-nicolae.pirea, ghennadi.procopciuc,
	Matthias Brugger, Ivan T . Ivanov, Lee, Chun-Yi, Rob Herring

Hi Krzysztof,

On 07.09.21 08:59, Krzysztof Kozlowski wrote:
> On Mon, 6 Sept 2021 at 22:38, Andreas Färber <afaerber@suse.de> wrote:
>> On 18.08.21 16:34, Chester Lin wrote:
>>> On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
>>>> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>>>>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>>>>> design 2 board ( S32G-VNP-RDB2).
>>>>>
>>>>> Signed-off-by: Chester Lin <clin@suse.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>>>>  1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> index e2097011c4b0..3914aa09e503 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> @@ -983,6 +983,13 @@ properties:
>>>>>            - const: solidrun,lx2160a-cex7
>>>>>            - const: fsl,lx2160a
>>>>>
>>>>> +      - description: S32G2 based Boards
>>>>> +        items:
>>>>> +          - enum:
>>>>> +              - fsl,s32g274a-evb
>>>>> +              - fsl,s32g274a-rdb2
>>>>> +          - const: fsl,s32g2
>>>>
>>>> Given this is an entirely different family from i.MX and new?, shouldn't
>>>> it use 'nxp' instead of 'fsl'? Either way,
>>>
>>> It sounds good and Radu from NXP has mentioned a similar idea for the
>>> compatible string of linflexuart. To keep the naming consistency, should we
>>> change all 'fsl' to 'nxp' as well?
>>
>> I assume that question was just unclearly phrased, so for the record:
>>
>> ABI stability rules forbid us from changing "all 'fsl'" in compatible
>> strings or property names.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst
>>
>> Deployed firmware providing mainline-merged platforms with DTBs using
>> fsl prefix (e.g., the quoted LX2160A) needs to continue working with
>> newer drivers, and deployed mainline Linux should continue working after
>> firmware updates that modify the DTB provided to Linux.
> 
> This is a new platform/SoC therefore there is no ABI. There is no
> requirement in the kernel that a new ABI (which you define in this
> patchset in the bindings) should be compatible with something
> somewhere. It's some misunderstanding of stable ABI. Therefore all new
> compatibles are allowed to be nxp, not fsl.
> 
> No one here proposed renaming existing compatibles from fsl tro nxp.
> We talk about new ones.

Chester seemingly did: "all 'fsl' ... as well", not "all new 'fsl'"
ones, in the patch context of existing fsl.yaml. Like I said, it may
just have been unluckily worded.

Therefore my saying that it does contain tons of non-new SoC/platform
bindings that he's not allowed to break by changing them.

> Different question of course whether you want to be nice to some
> existing out-of-tree users... but then have in mind that we don't care
> about out of tree. :) Anyway being nice to out-of-tree is not part of
> ABI. It's just being nice and useful.

Nobody is suggesting new S32G ABI be compatible with downstream BSPs.
These patches and changes we're discussing already differ from the BSP.

My point was that as soon as we merge S32G into mainline, it will become
ABI and shouldn't be changed incompatibly anymore once in a release.

These automotive platforms don't run off-the-shelf distros yet and will
need to get their bootloaders upstreamed, too. In particular we'll need
mainline TF-A to merge the SCMI implementation before we can rely on it
here in the kernel for a clk driver; that's holding up MMC and Ethernet.

Best regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2021-09-09 12:52 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05  6:54 [PATCH 0/8] arm64: dts: initial NXP S32G2 support Chester Lin
2021-08-05  6:54 ` [PATCH 1/8] dt-bindings: arm: fsl: add NXP S32G2 boards Chester Lin
2021-08-12 15:46   ` Andreas Färber
2021-08-13 17:49     ` Rob Herring
2021-08-13 17:53   ` Rob Herring
2021-08-18 14:34     ` Chester Lin
2021-09-06 20:38       ` Andreas Färber
2021-09-07  6:59         ` Krzysztof Kozlowski
2021-09-07  8:59           ` Andreas Färber
2021-09-06 19:35     ` Andreas Färber
2021-08-05  6:54 ` [PATCH 2/8] dt-bindings: serial: fsl-linflexuart: convert to json-schema format Chester Lin
2021-08-12 16:04   ` Andreas Färber
2021-08-13 11:11     ` Chester Lin
2021-08-13 11:28       ` Krzysztof Kozlowski
2021-08-13 11:43         ` Chester Lin
2021-08-13 18:04           ` Rob Herring
2021-08-13 18:07   ` Rob Herring
2021-08-05  6:54 ` [PATCH 3/8] dt-bindings: serial: fsl-linflexuart: Add compatible for S32G2 Chester Lin
2021-08-12 16:27   ` Andreas Färber
2021-08-13 14:27     ` Radu Nicolae Pirea (NXP OSS)
2021-08-13 18:11     ` Rob Herring
2021-08-13 18:09   ` Rob Herring
2021-08-05  6:54 ` [PATCH 4/8] arm64: dts: add NXP S32G2 support Chester Lin
2021-08-12 17:26   ` Andreas Färber
2021-08-13  3:28     ` Chester Lin
2021-08-13  7:05       ` Andreas Färber
2021-08-20 13:12     ` Marc Zyngier
2021-08-20 15:15       ` Chester Lin
2021-08-20 15:29         ` Marc Zyngier
2021-08-21 12:39           ` Chester Lin
2021-08-21 14:20             ` Marc Zyngier
2021-08-05  6:54 ` [PATCH 5/8] arm64: dts: s32g2: add serial/uart support Chester Lin
2021-08-12 17:42   ` Andreas Färber
2021-08-13  9:54     ` Radu Nicolae Pirea (NXP OSS)
2021-08-05  6:54 ` [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support Chester Lin
2021-08-12 18:00   ` Andreas Färber
2021-08-13  8:47     ` Chester Lin
2021-08-05  6:54 ` [PATCH 7/8] arm64: dts: s32g2: add memory nodes for evb and rdb2 Chester Lin
2021-08-12 18:25   ` Andreas Färber
2021-08-13 14:58     ` Chester Lin
2021-08-05  6:54 ` [PATCH 8/8] MAINTAINERS: Add an entry for NXP S32G2 boards Chester Lin
2021-08-05  7:49   ` Krzysztof Kozlowski
2021-08-09  8:03     ` Shawn Guo
2021-08-12 15:30       ` Andreas Färber
2021-08-12 15:54         ` Krzysztof Kozlowski
2021-08-09  8:06 ` [PATCH 0/8] arm64: dts: initial NXP S32G2 support Shawn Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).