linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Add Amlogic A311D2 and Khadas Vim4 Board Support
@ 2023-06-21 13:32 Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 1/3] dt-bindings: arm: amlogic: add Amlogic A311D2 bindings Lucas Tanure
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Lucas Tanure @ 2023-06-21 13:32 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Jerome Brunet, Kevin Hilman
  Cc: Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel, Lucas Tanure

The Khadas VIM4 uses the Amlogic A311D2 SoC, based on the Amlogic
Meson T7 family. This chip is not the same as A311D used in Vim3
board.

Work based on Khadas 5.4 branch:
https://github.com/khadas/linux/tree/khadas-vims-5.4.y

The current status is Vim4 board booting to emergency shell via uart.

Board Features:
- 8GB LPDDR4X 2016MHz
- 32GB eMMC 5.1 storage
- 32MB SPI flash
- 10/100/1000 Base-T Ethernet
- AP6275S Wireless (802.11 a/b/g/n/ac/ax, BT5.1)
- HDMI 2.1 video
- HDMI Input
- 1x USB 2.0 + 1x USB 3.0 ports
- 1x USB-C (power) with USB 2.0 OTG
- 3x LED's (1x red, 1x blue, 1x white)
- 3x buttons (power, function, reset)
- M2 socket with PCIe, USB, ADC & I2C
- 40pin GPIO Header
- 1x micro SD card slot

Changes Since v2:
 - Add "amlogic,meson-t7-uart" documentation

Changes Since v1:
 - Drop the T7 clock driver as it is not needed for serial boot. It will
 later use the S4 clock
 driver as S4 and  T7 seems to be similar chips.
 - Use "arm,gic-400" for interrupt controller to fix dtb_check
 - Remove CPU node properties not needed for serial boot
 - Move UART node to apb4 node
 - Drop T7 UART compatible line and use S4 uart
 - Use psci V1 instead of 0.2, it works, but I can't verify is correct
 as the datasheet I have
 doesn't contain that information.
 - Remove compatible from meson-t7.dtsi, move it to vim4 board dts
 - Add memory node with 8GB. Not sure about this one, works without,
 but doesn't detect 8GB
 - Use defines for GIC_CPU_MASK_SIMPLE, IRQ_TYPE_LEVEL_LOW,
 IRQ_TYPE_LEVEL_HIGH instead of hardcoded values

Lucas Tanure (3):
  dt-bindings: arm: amlogic: add Amlogic A311D2 bindings
  dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree

 .../devicetree/bindings/arm/amlogic.yaml      |   7 +
 .../bindings/serial/amlogic,meson-uart.yaml   |   2 +
 arch/arm64/boot/dts/amlogic/Makefile          |   1 +
 .../amlogic/meson-t7-a311d2-khadas-vim4.dts   |  52 ++++++
 arch/arm64/boot/dts/amlogic/meson-t7.dtsi     | 158 ++++++++++++++++++
 5 files changed, 220 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-t7-a311d2-khadas-vim4.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-t7.dtsi

--
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 1/3] dt-bindings: arm: amlogic: add Amlogic A311D2 bindings
  2023-06-21 13:32 [PATCH v3 0/3] Add Amlogic A311D2 and Khadas Vim4 Board Support Lucas Tanure
@ 2023-06-21 13:32 ` Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7 Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 3/3] arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree Lucas Tanure
  2 siblings, 0 replies; 13+ messages in thread
From: Lucas Tanure @ 2023-06-21 13:32 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Jerome Brunet, Kevin Hilman
  Cc: Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel, Lucas Tanure, Conor Dooley

Add bindings for the Khadas Vim4 board, using A311D2 soc, a Meson T7
family chip.

Signed-off-by: Lucas Tanure <tanure@linux.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/arm/amlogic.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/amlogic.yaml b/Documentation/devicetree/bindings/arm/amlogic.yaml
index 274ee0890312..8dbd65170c50 100644
--- a/Documentation/devicetree/bindings/arm/amlogic.yaml
+++ b/Documentation/devicetree/bindings/arm/amlogic.yaml
@@ -211,6 +211,13 @@ properties:
               - amlogic,aq222
           - const: amlogic,s4
 
+      - description: Boards with the Amlogic Meson t7 A311D2 SoC
+        items:
+          - enum:
+              - khadas,vim4
+          - const: amlogic,a311d2
+          - const: amlogic,t7
+
 additionalProperties: true
 
 ...
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-21 13:32 [PATCH v3 0/3] Add Amlogic A311D2 and Khadas Vim4 Board Support Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 1/3] dt-bindings: arm: amlogic: add Amlogic A311D2 bindings Lucas Tanure
@ 2023-06-21 13:32 ` Lucas Tanure
  2023-06-21 13:53   ` Krzysztof Kozlowski
  2023-06-21 13:32 ` [PATCH v3 3/3] arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree Lucas Tanure
  2 siblings, 1 reply; 13+ messages in thread
From: Lucas Tanure @ 2023-06-21 13:32 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Jerome Brunet, Kevin Hilman
  Cc: Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel, Lucas Tanure

Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
There is no need for an extra compatible line in the driver, but
add T7 compatible line for documentation.

Signed-off-by: Lucas Tanure <tanure@linux.com>
---
 .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 01ec45b3b406..860ab58d87b0 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -33,6 +33,7 @@ properties:
               - amlogic,meson8b-uart
               - amlogic,meson-gx-uart
               - amlogic,meson-s4-uart
+              - amlogic,meson-t7-uart
           - const: amlogic,meson-ao-uart
       - description: Always-on power domain UART controller on G12A SoCs
         items:
@@ -46,6 +47,7 @@ properties:
           - amlogic,meson8b-uart
           - amlogic,meson-gx-uart
           - amlogic,meson-s4-uart
+          - amlogic,meson-t7-uart
       - description: Everything-Else power domain UART controller on G12A SoCs
         items:
           - const: amlogic,meson-g12a-uart
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 3/3] arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree
  2023-06-21 13:32 [PATCH v3 0/3] Add Amlogic A311D2 and Khadas Vim4 Board Support Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 1/3] dt-bindings: arm: amlogic: add Amlogic A311D2 bindings Lucas Tanure
  2023-06-21 13:32 ` [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7 Lucas Tanure
@ 2023-06-21 13:32 ` Lucas Tanure
  2 siblings, 0 replies; 13+ messages in thread
From: Lucas Tanure @ 2023-06-21 13:32 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Jerome Brunet, Kevin Hilman
  Cc: Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel, Lucas Tanure

The Khadas VIM4 uses the Amlogic A311D2 SoC, based on the Amlogic T7 SoC
family, on a board with the same form factor as the VIM3 models.

- 8GB LPDDR4X 2016MHz
- 32GB eMMC 5.1 storage
- 32MB SPI flash
- 10/100/1000 Base-T Ethernet
- AP6275S Wireless (802.11 a/b/g/n/ac/ax, BT5.1)
- HDMI 2.1 video
- HDMI Input
- 1x USB 2.0 + 1x USB 3.0 ports
- 1x USB-C (power) with USB 2.0 OTG
- 3x LED's (1x red, 1x blue, 1x white)
- 3x buttons (power, function, reset)
- M2 socket with PCIe, USB, ADC & I2C
- 40pin GPIO Header
- 1x micro SD card slot

Signed-off-by: Lucas Tanure <tanure@linux.com>
---
 arch/arm64/boot/dts/amlogic/Makefile          |   1 +
 .../amlogic/meson-t7-a311d2-khadas-vim4.dts   |  52 ++++++
 arch/arm64/boot/dts/amlogic/meson-t7.dtsi     | 158 ++++++++++++++++++
 3 files changed, 211 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-t7-a311d2-khadas-vim4.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-t7.dtsi

diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
index cd1c5b04890a..1c5846bd1ca0 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-t7-a311d2-khadas-vim4.dtb
diff --git a/arch/arm64/boot/dts/amlogic/meson-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/meson-t7-a311d2-khadas-vim4.dts
new file mode 100644
index 000000000000..04cc8b0dfd8c
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-t7-a311d2-khadas-vim4.dts
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Wesion, Inc. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "meson-t7.dtsi"
+
+/ {
+	model = "Khadas vim4";
+	compatible = "khadas,vim4", "amlogic,a311d2", "amlogic,t7";
+
+	aliases {
+		serial0 = &uart_A;
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x2 0x0>; /* 8 GB */
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* 3 MiB reserved for ARM Trusted Firmware (BL31) */
+		secmon_reserved: secmon@5000000 {
+			reg = <0x0 0x05000000 0x0 0x300000>;
+			no-map;
+		};
+
+		/* 32 MiB reserved for ARM Trusted Firmware (BL32) */
+		secmon_reserved_bl32: secmon@5300000 {
+			reg = <0x0 0x05300000 0x0 0x2000000>;
+			no-map;
+		};
+	};
+
+	xtal: xtal-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+
+};
+
+&uart_A {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-t7.dtsi b/arch/arm64/boot/dts/amlogic/meson-t7.dtsi
new file mode 100644
index 000000000000..9b8c33708ecd
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-t7.dtsi
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <0x2>;
+		#size-cells = <0x0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu100>;
+				};
+				core1 {
+					cpu = <&cpu101>;
+				};
+				core2 {
+					cpu = <&cpu102>;
+				};
+				core3 {
+					cpu = <&cpu103>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+				core1 {
+					cpu = <&cpu1>;
+				};
+				core2 {
+					cpu = <&cpu2>;
+				};
+				core3 {
+					cpu = <&cpu3>;
+				};
+			};
+		};
+
+		cpu100: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+		};
+
+		cpu101: cpu@101{
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0 0x101>;
+			enable-method = "psci";
+		};
+
+		cpu102: cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0 0x102>;
+			enable-method = "psci";
+		};
+
+		cpu103: cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0 0x103>;
+			enable-method = "psci";
+		};
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x0 0x2>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a73";
+			reg = <0x0 0x3>;
+			enable-method = "psci";
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+	};
+
+	sm: secure-monitor {
+		compatible = "amlogic,meson-gxbb-sm";
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		gic: interrupt-controller@fff01000 {
+			compatible = "arm,gic-400";
+			#interrupt-cells = <3>;
+			#address-cells = <0>;
+			interrupt-controller;
+			reg = <0x0 0xfff01000 0 0x1000>,
+			      <0x0 0xfff02000 0 0x0100>;
+			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
+		apb4: bus@fe000000 {
+			compatible = "simple-bus";
+			reg = <0x0 0xfe000000 0x0 0x480000>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
+
+			uart_A: serial@78000 {
+				compatible = "amlogic,meson-t7-uart",
+					     "amlogic,meson-s4-uart";
+				reg = <0x0 0x78000 0x0 0x18>;
+				interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
+				status = "disabled";
+				clocks = <&xtal>, <&xtal>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
+			};
+		};
+
+	};
+};
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-21 13:32 ` [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7 Lucas Tanure
@ 2023-06-21 13:53   ` Krzysztof Kozlowski
  2023-06-21 18:11     ` Conor Dooley
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-21 13:53 UTC (permalink / raw)
  To: Lucas Tanure, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Neil Armstrong, Jerome Brunet, Kevin Hilman
  Cc: Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel

On 21/06/2023 15:32, Lucas Tanure wrote:
> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> There is no need for an extra compatible line in the driver, but
> add T7 compatible line for documentation.
> 
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> index 01ec45b3b406..860ab58d87b0 100644
> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> @@ -33,6 +33,7 @@ properties:
>                - amlogic,meson8b-uart
>                - amlogic,meson-gx-uart
>                - amlogic,meson-s4-uart
> +              - amlogic,meson-t7-uart
>            - const: amlogic,meson-ao-uart
>        - description: Always-on power domain UART controller on G12A SoCs
>          items:
> @@ -46,6 +47,7 @@ properties:
>            - amlogic,meson8b-uart
>            - amlogic,meson-gx-uart
>            - amlogic,meson-s4-uart
> +          - amlogic,meson-t7-uart

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-21 13:53   ` Krzysztof Kozlowski
@ 2023-06-21 18:11     ` Conor Dooley
  2023-06-22  5:32       ` Lucas Tanure
  0 siblings, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2023-06-21 18:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lucas Tanure, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Neil Armstrong, Jerome Brunet, Kevin Hilman, Nick, Artem,
	devicetree, linux-kernel, linux-amlogic, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1815 bytes --]

Lucas,

On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> On 21/06/2023 15:32, Lucas Tanure wrote:
> > Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> > There is no need for an extra compatible line in the driver, but
> > add T7 compatible line for documentation.
> > 
> > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > ---
> >  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > index 01ec45b3b406..860ab58d87b0 100644
> > --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > @@ -33,6 +33,7 @@ properties:
> >                - amlogic,meson8b-uart
> >                - amlogic,meson-gx-uart
> >                - amlogic,meson-s4-uart
> > +              - amlogic,meson-t7-uart
> >            - const: amlogic,meson-ao-uart
> >        - description: Always-on power domain UART controller on G12A SoCs
> >          items:
> > @@ -46,6 +47,7 @@ properties:
> >            - amlogic,meson8b-uart
> >            - amlogic,meson-gx-uart
> >            - amlogic,meson-s4-uart
> > +          - amlogic,meson-t7-uart
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).

Check back on the previous version, I should've posted an untested
version of what you need to add.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-21 18:11     ` Conor Dooley
@ 2023-06-22  5:32       ` Lucas Tanure
  2023-06-22  6:05         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Lucas Tanure @ 2023-06-22  5:32 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Jerome Brunet, Kevin Hilman, Nick,
	Artem, devicetree, linux-kernel, linux-amlogic, linux-arm-kernel

On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
>
> Lucas,
>
> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> > On 21/06/2023 15:32, Lucas Tanure wrote:
> > > Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> > > There is no need for an extra compatible line in the driver, but
> > > add T7 compatible line for documentation.
> > >
> > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > ---
> > >  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > index 01ec45b3b406..860ab58d87b0 100644
> > > --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > @@ -33,6 +33,7 @@ properties:
> > >                - amlogic,meson8b-uart
> > >                - amlogic,meson-gx-uart
> > >                - amlogic,meson-s4-uart
> > > +              - amlogic,meson-t7-uart
> > >            - const: amlogic,meson-ao-uart
> > >        - description: Always-on power domain UART controller on G12A SoCs
> > >          items:
> > > @@ -46,6 +47,7 @@ properties:
> > >            - amlogic,meson8b-uart
> > >            - amlogic,meson-gx-uart
> > >            - amlogic,meson-s4-uart
> > > +          - amlogic,meson-t7-uart
> >
> > It does not look like you tested the DTS against bindings. Please run
> > `make dtbs_check` (see
> > Documentation/devicetree/bindings/writing-schema.rst or
> > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> > for instructions).
>
> Check back on the previous version, I should've posted an untested
> version of what you need to add.
I saw that, but adding a S4 doesn't make sense to me. And you didn't
show the entire change, so I can't understand what you want there.
>
> Cheers,
> Conor.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  5:32       ` Lucas Tanure
@ 2023-06-22  6:05         ` Krzysztof Kozlowski
  2023-06-22  6:43           ` Lucas Tanure
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-22  6:05 UTC (permalink / raw)
  To: tanure, Conor Dooley
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Jerome Brunet, Kevin Hilman, Nick, Artem, devicetree,
	linux-kernel, linux-amlogic, linux-arm-kernel

On 22/06/2023 07:32, Lucas Tanure wrote:
> On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> Lucas,
>>
>> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
>>> On 21/06/2023 15:32, Lucas Tanure wrote:
>>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
>>>> There is no need for an extra compatible line in the driver, but
>>>> add T7 compatible line for documentation.
>>>>
>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>> ---
>>>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>> index 01ec45b3b406..860ab58d87b0 100644
>>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>> @@ -33,6 +33,7 @@ properties:
>>>>                - amlogic,meson8b-uart
>>>>                - amlogic,meson-gx-uart
>>>>                - amlogic,meson-s4-uart
>>>> +              - amlogic,meson-t7-uart
>>>>            - const: amlogic,meson-ao-uart
>>>>        - description: Always-on power domain UART controller on G12A SoCs
>>>>          items:
>>>> @@ -46,6 +47,7 @@ properties:
>>>>            - amlogic,meson8b-uart
>>>>            - amlogic,meson-gx-uart
>>>>            - amlogic,meson-s4-uart
>>>> +          - amlogic,meson-t7-uart
>>>
>>> It does not look like you tested the DTS against bindings. Please run
>>> `make dtbs_check` (see
>>> Documentation/devicetree/bindings/writing-schema.rst or
>>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
>>> for instructions).
>>
>> Check back on the previous version, I should've posted an untested
>> version of what you need to add.
> I saw that, but adding a S4 doesn't make sense to me. And you didn't
> show the entire change, so I can't understand what you want there.

For sure you need something which does not trigger errors. If you claim
adding S4 as fallback does not make sense, then why did you use it?
Sending a code which is clearly incorrect does not make sense.


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  6:05         ` Krzysztof Kozlowski
@ 2023-06-22  6:43           ` Lucas Tanure
  2023-06-22  7:11             ` Conor Dooley
  0 siblings, 1 reply; 13+ messages in thread
From: Lucas Tanure @ 2023-06-22  6:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Neil Armstrong, Jerome Brunet, Kevin Hilman, Nick, Artem,
	devicetree, linux-kernel, linux-amlogic, linux-arm-kernel

On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/06/2023 07:32, Lucas Tanure wrote:
> > On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> Lucas,
> >>
> >> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> >>> On 21/06/2023 15:32, Lucas Tanure wrote:
> >>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> >>>> There is no need for an extra compatible line in the driver, but
> >>>> add T7 compatible line for documentation.
> >>>>
> >>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
> >>>> ---
> >>>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>> index 01ec45b3b406..860ab58d87b0 100644
> >>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>> @@ -33,6 +33,7 @@ properties:
> >>>>                - amlogic,meson8b-uart
> >>>>                - amlogic,meson-gx-uart
> >>>>                - amlogic,meson-s4-uart
> >>>> +              - amlogic,meson-t7-uart
> >>>>            - const: amlogic,meson-ao-uart
> >>>>        - description: Always-on power domain UART controller on G12A SoCs
> >>>>          items:
> >>>> @@ -46,6 +47,7 @@ properties:
> >>>>            - amlogic,meson8b-uart
> >>>>            - amlogic,meson-gx-uart
> >>>>            - amlogic,meson-s4-uart
> >>>> +          - amlogic,meson-t7-uart
> >>>
> >>> It does not look like you tested the DTS against bindings. Please run
> >>> `make dtbs_check` (see
> >>> Documentation/devicetree/bindings/writing-schema.rst or
> >>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> >>> for instructions).
> >>
> >> Check back on the previous version, I should've posted an untested
> >> version of what you need to add.
> > I saw that, but adding a S4 doesn't make sense to me. And you didn't
> > show the entire change, so I can't understand what you want there.
>
> For sure you need something which does not trigger errors. If you claim
> adding S4 as fallback does not make sense, then why did you use it?
> Sending a code which is clearly incorrect does not make sense.
>
Sorry, I think we are talking about different things. It does not make
sense to me to add an S4 line in the documentation when it is already
there. So I could not understand or make sense of the patch Conor sent
in reply to my V2.

Krzysztof, I will check again with dtbs_check and re-send.

>
> Best regards,
> Krzysztof
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  6:43           ` Lucas Tanure
@ 2023-06-22  7:11             ` Conor Dooley
  2023-06-22  7:36               ` Lucas Tanure
  0 siblings, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2023-06-22  7:11 UTC (permalink / raw)
  To: Lucas Tanure
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Jerome Brunet,
	Kevin Hilman, Nick, Artem, devicetree, linux-kernel,
	linux-amlogic, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4588 bytes --]

On Thu, Jun 22, 2023 at 07:43:31AM +0100, Lucas Tanure wrote:
> On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 22/06/2023 07:32, Lucas Tanure wrote:
> > > On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
> > >> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> > >>> On 21/06/2023 15:32, Lucas Tanure wrote:
> > >>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> > >>>> There is no need for an extra compatible line in the driver, but
> > >>>> add T7 compatible line for documentation.
> > >>>>
> > >>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
> > >>>> ---
> > >>>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> > >>>>  1 file changed, 2 insertions(+)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > >>>> index 01ec45b3b406..860ab58d87b0 100644
> > >>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > >>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > >>>> @@ -33,6 +33,7 @@ properties:
> > >>>>                - amlogic,meson8b-uart
> > >>>>                - amlogic,meson-gx-uart
> > >>>>                - amlogic,meson-s4-uart
> > >>>> +              - amlogic,meson-t7-uart
> > >>>>            - const: amlogic,meson-ao-uart
> > >>>>        - description: Always-on power domain UART controller on G12A SoCs
> > >>>>          items:
> > >>>> @@ -46,6 +47,7 @@ properties:
> > >>>>            - amlogic,meson8b-uart
> > >>>>            - amlogic,meson-gx-uart
> > >>>>            - amlogic,meson-s4-uart
> > >>>> +          - amlogic,meson-t7-uart
> > >>>
> > >>> It does not look like you tested the DTS against bindings. Please run
> > >>> `make dtbs_check` (see
> > >>> Documentation/devicetree/bindings/writing-schema.rst or
> > >>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> > >>> for instructions).
> > >>
> > >> Check back on the previous version, I should've posted an untested
> > >> version of what you need to add.
> > > I saw that, but adding a S4 doesn't make sense to me. And you didn't
> > > show the entire change, so I can't understand what you want there.
> >
> > For sure you need something which does not trigger errors. If you claim
> > adding S4 as fallback does not make sense, then why did you use it?
> > Sending a code which is clearly incorrect does not make sense.
> >
> Sorry, I think we are talking about different things. It does not make
> sense to me to add an S4 line in the documentation when it is already
> there. So I could not understand or make sense of the patch Conor sent
> in reply to my V2.

That is just how it works. You need to spell out exactly which
combinations are permitted. The current entry for s4 says that s4 is
only permitted in isolation.
Since you are adding "amlogic,meson-t7-uart", "amlogic,meson-s4-uart"
you need to explicitly allow that combination. You'll notice if you look
at the file that the gx uart appears more than once.

Given the g12a was the most recently added compatible, it might make
sense to follow the pattern that it had set, given the thing your
original patch copied the match data from was the g12a. That change to
the dt-binding would look like:
diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 01ec45b3b406..eae11e87b88a 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -50,6 +50,13 @@ properties:
         items:
           - const: amlogic,meson-g12a-uart
           - const: amlogic,meson-gx-uart
+      - description:
+          Everything-Else power domain UART controller on G12A compatible SoCs
+        items:
+          - enum:
+              - amlogic,meson-t7-uart
+          - const: amlogic,meson-g12a-uart
+          - const: amlogic,meson-gx-uart
 
   reg:
     maxItems: 1

/I/ don't really care whether you do that, or do the s4 version of it,
but following the most recent pattern might make more sense. When I
suggested s4, it was because I only looked at the driver patch rather
than the code itself.

> Krzysztof, I will check again with dtbs_check and re-send.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  7:11             ` Conor Dooley
@ 2023-06-22  7:36               ` Lucas Tanure
  2023-06-22  8:13                 ` Neil Armstrong
  0 siblings, 1 reply; 13+ messages in thread
From: Lucas Tanure @ 2023-06-22  7:36 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Jerome Brunet,
	Kevin Hilman, Nick, Artem, devicetree, linux-kernel,
	linux-amlogic, linux-arm-kernel

On Thu, Jun 22, 2023 at 8:12 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Thu, Jun 22, 2023 at 07:43:31AM +0100, Lucas Tanure wrote:
> > On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > > On 22/06/2023 07:32, Lucas Tanure wrote:
> > > > On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
> > > >> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> > > >>> On 21/06/2023 15:32, Lucas Tanure wrote:
> > > >>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> > > >>>> There is no need for an extra compatible line in the driver, but
> > > >>>> add T7 compatible line for documentation.
> > > >>>>
> > > >>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > >>>> ---
> > > >>>>  .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> > > >>>>  1 file changed, 2 insertions(+)
> > > >>>>
> > > >>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > >>>> index 01ec45b3b406..860ab58d87b0 100644
> > > >>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > >>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> > > >>>> @@ -33,6 +33,7 @@ properties:
> > > >>>>                - amlogic,meson8b-uart
> > > >>>>                - amlogic,meson-gx-uart
> > > >>>>                - amlogic,meson-s4-uart
> > > >>>> +              - amlogic,meson-t7-uart
> > > >>>>            - const: amlogic,meson-ao-uart
> > > >>>>        - description: Always-on power domain UART controller on G12A SoCs
> > > >>>>          items:
> > > >>>> @@ -46,6 +47,7 @@ properties:
> > > >>>>            - amlogic,meson8b-uart
> > > >>>>            - amlogic,meson-gx-uart
> > > >>>>            - amlogic,meson-s4-uart
> > > >>>> +          - amlogic,meson-t7-uart
> > > >>>
> > > >>> It does not look like you tested the DTS against bindings. Please run
> > > >>> `make dtbs_check` (see
> > > >>> Documentation/devicetree/bindings/writing-schema.rst or
> > > >>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> > > >>> for instructions).
> > > >>
> > > >> Check back on the previous version, I should've posted an untested
> > > >> version of what you need to add.
> > > > I saw that, but adding a S4 doesn't make sense to me. And you didn't
> > > > show the entire change, so I can't understand what you want there.
> > >
> > > For sure you need something which does not trigger errors. If you claim
> > > adding S4 as fallback does not make sense, then why did you use it?
> > > Sending a code which is clearly incorrect does not make sense.
> > >
> > Sorry, I think we are talking about different things. It does not make
> > sense to me to add an S4 line in the documentation when it is already
> > there. So I could not understand or make sense of the patch Conor sent
> > in reply to my V2.
>
> That is just how it works. You need to spell out exactly which
> combinations are permitted. The current entry for s4 says that s4 is
> only permitted in isolation.
> Since you are adding "amlogic,meson-t7-uart", "amlogic,meson-s4-uart"
> you need to explicitly allow that combination. You'll notice if you look
> at the file that the gx uart appears more than once.
>
> Given the g12a was the most recently added compatible, it might make
> sense to follow the pattern that it had set, given the thing your
> original patch copied the match data from was the g12a. That change to
> the dt-binding would look like:
> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> index 01ec45b3b406..eae11e87b88a 100644
> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> @@ -50,6 +50,13 @@ properties:
>          items:
>            - const: amlogic,meson-g12a-uart
>            - const: amlogic,meson-gx-uart
> +      - description:
> +          Everything-Else power domain UART controller on G12A compatible SoCs
> +        items:
> +          - enum:
> +              - amlogic,meson-t7-uart
> +          - const: amlogic,meson-g12a-uart
> +          - const: amlogic,meson-gx-uart
>
>    reg:
>      maxItems: 1
>
> /I/ don't really care whether you do that, or do the s4 version of it,
> but following the most recent pattern might make more sense. When I
> suggested s4, it was because I only looked at the driver patch rather
> than the code itself.
>
> > Krzysztof, I will check again with dtbs_check and re-send.
>
> Cheers,
> Conor.
I am struggling to understand this. Everything I try fails the check.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  7:36               ` Lucas Tanure
@ 2023-06-22  8:13                 ` Neil Armstrong
  2023-06-22  8:26                   ` Lucas Tanure
  0 siblings, 1 reply; 13+ messages in thread
From: Neil Armstrong @ 2023-06-22  8:13 UTC (permalink / raw)
  To: tanure, Conor Dooley
  Cc: Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jerome Brunet, Kevin Hilman,
	Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel

On 22/06/2023 09:36, Lucas Tanure wrote:
> On Thu, Jun 22, 2023 at 8:12 AM Conor Dooley <conor.dooley@microchip.com> wrote:
>>
>> On Thu, Jun 22, 2023 at 07:43:31AM +0100, Lucas Tanure wrote:
>>> On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>> On 22/06/2023 07:32, Lucas Tanure wrote:
>>>>> On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
>>>>>> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
>>>>>>> On 21/06/2023 15:32, Lucas Tanure wrote:
>>>>>>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
>>>>>>>> There is no need for an extra compatible line in the driver, but
>>>>>>>> add T7 compatible line for documentation.
>>>>>>>>
>>>>>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>>>>>>>> ---
>>>>>>>>   .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
>>>>>>>>   1 file changed, 2 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>>>>>> index 01ec45b3b406..860ab58d87b0 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>>>>>>>> @@ -33,6 +33,7 @@ properties:
>>>>>>>>                 - amlogic,meson8b-uart
>>>>>>>>                 - amlogic,meson-gx-uart
>>>>>>>>                 - amlogic,meson-s4-uart
>>>>>>>> +              - amlogic,meson-t7-uart
>>>>>>>>             - const: amlogic,meson-ao-uart
>>>>>>>>         - description: Always-on power domain UART controller on G12A SoCs
>>>>>>>>           items:
>>>>>>>> @@ -46,6 +47,7 @@ properties:
>>>>>>>>             - amlogic,meson8b-uart
>>>>>>>>             - amlogic,meson-gx-uart
>>>>>>>>             - amlogic,meson-s4-uart
>>>>>>>> +          - amlogic,meson-t7-uart
>>>>>>>
>>>>>>> It does not look like you tested the DTS against bindings. Please run
>>>>>>> `make dtbs_check` (see
>>>>>>> Documentation/devicetree/bindings/writing-schema.rst or
>>>>>>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
>>>>>>> for instructions).
>>>>>>
>>>>>> Check back on the previous version, I should've posted an untested
>>>>>> version of what you need to add.
>>>>> I saw that, but adding a S4 doesn't make sense to me. And you didn't
>>>>> show the entire change, so I can't understand what you want there.
>>>>
>>>> For sure you need something which does not trigger errors. If you claim
>>>> adding S4 as fallback does not make sense, then why did you use it?
>>>> Sending a code which is clearly incorrect does not make sense.
>>>>
>>> Sorry, I think we are talking about different things. It does not make
>>> sense to me to add an S4 line in the documentation when it is already
>>> there. So I could not understand or make sense of the patch Conor sent
>>> in reply to my V2.
>>
>> That is just how it works. You need to spell out exactly which
>> combinations are permitted. The current entry for s4 says that s4 is
>> only permitted in isolation.
>> Since you are adding "amlogic,meson-t7-uart", "amlogic,meson-s4-uart"
>> you need to explicitly allow that combination. You'll notice if you look
>> at the file that the gx uart appears more than once.
>>
>> Given the g12a was the most recently added compatible, it might make
>> sense to follow the pattern that it had set, given the thing your
>> original patch copied the match data from was the g12a. That change to
>> the dt-binding would look like:
>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> index 01ec45b3b406..eae11e87b88a 100644
>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>> @@ -50,6 +50,13 @@ properties:
>>           items:
>>             - const: amlogic,meson-g12a-uart
>>             - const: amlogic,meson-gx-uart
>> +      - description:
>> +          Everything-Else power domain UART controller on G12A compatible SoCs
>> +        items:
>> +          - enum:
>> +              - amlogic,meson-t7-uart
>> +          - const: amlogic,meson-g12a-uart
>> +          - const: amlogic,meson-gx-uart
>>
>>     reg:
>>       maxItems: 1
>>
>> /I/ don't really care whether you do that, or do the s4 version of it,
>> but following the most recent pattern might make more sense. When I
>> suggested s4, it was because I only looked at the driver patch rather
>> than the code itself.
>>
>>> Krzysztof, I will check again with dtbs_check and re-send.
>>
>> Cheers,
>> Conor.
> I am struggling to understand this. Everything I try fails the check.

I just applied Conor's change on top of v6.4-rc1 and ran:
make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml

and the check was successful.

Neil



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7
  2023-06-22  8:13                 ` Neil Armstrong
@ 2023-06-22  8:26                   ` Lucas Tanure
  0 siblings, 0 replies; 13+ messages in thread
From: Lucas Tanure @ 2023-06-22  8:26 UTC (permalink / raw)
  To: neil.armstrong
  Cc: Conor Dooley, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jerome Brunet, Kevin Hilman,
	Nick, Artem, devicetree, linux-kernel, linux-amlogic,
	linux-arm-kernel

On Thu, Jun 22, 2023 at 9:13 AM Neil Armstrong
<neil.armstrong@linaro.org> wrote:
>
> On 22/06/2023 09:36, Lucas Tanure wrote:
> > On Thu, Jun 22, 2023 at 8:12 AM Conor Dooley <conor.dooley@microchip.com> wrote:
> >>
> >> On Thu, Jun 22, 2023 at 07:43:31AM +0100, Lucas Tanure wrote:
> >>> On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>> On 22/06/2023 07:32, Lucas Tanure wrote:
> >>>>> On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@kernel.org> wrote:
> >>>>>> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote:
> >>>>>>> On 21/06/2023 15:32, Lucas Tanure wrote:
> >>>>>>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A.
> >>>>>>>> There is no need for an extra compatible line in the driver, but
> >>>>>>>> add T7 compatible line for documentation.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Lucas Tanure <tanure@linux.com>
> >>>>>>>> ---
> >>>>>>>>   .../devicetree/bindings/serial/amlogic,meson-uart.yaml          | 2 ++
> >>>>>>>>   1 file changed, 2 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>>>>>> index 01ec45b3b406..860ab58d87b0 100644
> >>>>>>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>>>>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >>>>>>>> @@ -33,6 +33,7 @@ properties:
> >>>>>>>>                 - amlogic,meson8b-uart
> >>>>>>>>                 - amlogic,meson-gx-uart
> >>>>>>>>                 - amlogic,meson-s4-uart
> >>>>>>>> +              - amlogic,meson-t7-uart
> >>>>>>>>             - const: amlogic,meson-ao-uart
> >>>>>>>>         - description: Always-on power domain UART controller on G12A SoCs
> >>>>>>>>           items:
> >>>>>>>> @@ -46,6 +47,7 @@ properties:
> >>>>>>>>             - amlogic,meson8b-uart
> >>>>>>>>             - amlogic,meson-gx-uart
> >>>>>>>>             - amlogic,meson-s4-uart
> >>>>>>>> +          - amlogic,meson-t7-uart
> >>>>>>>
> >>>>>>> It does not look like you tested the DTS against bindings. Please run
> >>>>>>> `make dtbs_check` (see
> >>>>>>> Documentation/devicetree/bindings/writing-schema.rst or
> >>>>>>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> >>>>>>> for instructions).
> >>>>>>
> >>>>>> Check back on the previous version, I should've posted an untested
> >>>>>> version of what you need to add.
> >>>>> I saw that, but adding a S4 doesn't make sense to me. And you didn't
> >>>>> show the entire change, so I can't understand what you want there.
> >>>>
> >>>> For sure you need something which does not trigger errors. If you claim
> >>>> adding S4 as fallback does not make sense, then why did you use it?
> >>>> Sending a code which is clearly incorrect does not make sense.
> >>>>
> >>> Sorry, I think we are talking about different things. It does not make
> >>> sense to me to add an S4 line in the documentation when it is already
> >>> there. So I could not understand or make sense of the patch Conor sent
> >>> in reply to my V2.
> >>
> >> That is just how it works. You need to spell out exactly which
> >> combinations are permitted. The current entry for s4 says that s4 is
> >> only permitted in isolation.
> >> Since you are adding "amlogic,meson-t7-uart", "amlogic,meson-s4-uart"
> >> you need to explicitly allow that combination. You'll notice if you look
> >> at the file that the gx uart appears more than once.
> >>
> >> Given the g12a was the most recently added compatible, it might make
> >> sense to follow the pattern that it had set, given the thing your
> >> original patch copied the match data from was the g12a. That change to
> >> the dt-binding would look like:
> >> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> index 01ec45b3b406..eae11e87b88a 100644
> >> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> >> @@ -50,6 +50,13 @@ properties:
> >>           items:
> >>             - const: amlogic,meson-g12a-uart
> >>             - const: amlogic,meson-gx-uart
> >> +      - description:
> >> +          Everything-Else power domain UART controller on G12A compatible SoCs
> >> +        items:
> >> +          - enum:
> >> +              - amlogic,meson-t7-uart
> >> +          - const: amlogic,meson-g12a-uart
> >> +          - const: amlogic,meson-gx-uart
> >>
> >>     reg:
> >>       maxItems: 1
> >>
> >> /I/ don't really care whether you do that, or do the s4 version of it,
> >> but following the most recent pattern might make more sense. When I
> >> suggested s4, it was because I only looked at the driver patch rather
> >> than the code itself.
> >>
> >>> Krzysztof, I will check again with dtbs_check and re-send.
> >>
> >> Cheers,
> >> Conor.
> > I am struggling to understand this. Everything I try fails the check.
>
> I just applied Conor's change on top of v6.4-rc1 and ran:
> make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
>
> and the check was successful.
>
> Neil
>
>
I am sending v4 in a few minutes

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2023-06-22  8:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 13:32 [PATCH v3 0/3] Add Amlogic A311D2 and Khadas Vim4 Board Support Lucas Tanure
2023-06-21 13:32 ` [PATCH v3 1/3] dt-bindings: arm: amlogic: add Amlogic A311D2 bindings Lucas Tanure
2023-06-21 13:32 ` [PATCH v3 2/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for T7 Lucas Tanure
2023-06-21 13:53   ` Krzysztof Kozlowski
2023-06-21 18:11     ` Conor Dooley
2023-06-22  5:32       ` Lucas Tanure
2023-06-22  6:05         ` Krzysztof Kozlowski
2023-06-22  6:43           ` Lucas Tanure
2023-06-22  7:11             ` Conor Dooley
2023-06-22  7:36               ` Lucas Tanure
2023-06-22  8:13                 ` Neil Armstrong
2023-06-22  8:26                   ` Lucas Tanure
2023-06-21 13:32 ` [PATCH v3 3/3] arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree Lucas Tanure

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).