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 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 (2):
dt-bindings: arm: amlogic: add Amlogic A311D2 bindings
arm64: dts: meson-t7-a311d2-khadas-vim4: add initial device-tree
.../devicetree/bindings/arm/amlogic.yaml | 7 +
arch/arm64/boot/dts/amlogic/Makefile | 1 +
.../amlogic/meson-t7-a311d2-khadas-vim4.dts | 52 ++++++
arch/arm64/boot/dts/amlogic/meson-t7.dtsi | 158 ++++++++++++++++++
4 files changed, 218 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
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 <[email protected]>
---
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..b268889bf8ea
--- /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
Add bindings for the Khadas Vim4 board, using A311D2 soc, a Meson T7
family chip.
Signed-off-by: Lucas Tanure <[email protected]>
Acked-by: Conor Dooley <[email protected]>
---
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
Hi Lucas:
On 14:48 Tue 20 Jun , Lucas Tanure wrote:
> 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 <[email protected]>
> ---
> 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..b268889bf8ea
> --- /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",
~~~~~~~~~~~~~~~~~
if you introduce new compatible string, then at least you need to document it
so Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml need to be updated
but my qeustion here, why bother introducing new compatible string if nothing
changed with the compatible data? given the uart is same IP with g12a, can't we just
use "amlogic,meson-g12-uart" for this? no only it will reduce the structure length of
meson_uart_dt_match[], but also relieve maintainer's review burden?
> + "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-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
--
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55
On 21/06/2023 00:09, Yixun Lan wrote:
>> + 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",
> ~~~~~~~~~~~~~~~~~
> if you introduce new compatible string, then at least you need to document it
> so Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml need to be updated
>
> but my qeustion here, why bother introducing new compatible string if nothing
> changed with the compatible data? given the uart is same IP with g12a, can't we just
> use "amlogic,meson-g12-uart" for this? no only it will reduce the structure length of
> meson_uart_dt_match[], but also relieve maintainer's review burden?
https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
Best regards,
Krzysztof
On Wed, Jun 21, 2023 at 7:02 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 21/06/2023 00:09, Yixun Lan wrote:
> >> + 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",
> > ~~~~~~~~~~~~~~~~~
> > if you introduce new compatible string, then at least you need to document it
> > so Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml need to be updated
> >
> > but my qeustion here, why bother introducing new compatible string if nothing
> > changed with the compatible data? given the uart is same IP with g12a, can't we just
> > use "amlogic,meson-g12-uart" for this? no only it will reduce the structure length of
> > meson_uart_dt_match[], but also relieve maintainer's review burden?
>
> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>
> Best regards,
> Krzysztof
>
Hi, I did not understand the recommendation here.
Can I add "amlogic,meson-t7-uart" without Documentation changes?
I think Yes, as I can see a few compatible strings in dts that don't
exist anywhere else.
My idea here is to add "amlogic,meson-t7-uart" for future use if ever
created, like if we find a bug in the future that is only relevant to
T7 soc.
But for now, fallback to s4 uart, as it seems to be the same controller.
From Krzysztof said in the writing-bindings.rst, I am following the rules.
So, what's the path forward here?
Thanks
Lucas
On Wed, Jun 21, 2023 at 08:37:02AM +0100, Lucas Tanure wrote:
> On Wed, Jun 21, 2023 at 7:02 AM Krzysztof Kozlowski
> <[email protected]> wrote:
> >
> > On 21/06/2023 00:09, Yixun Lan wrote:
> > >> + 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",
> > > ~~~~~~~~~~~~~~~~~
> > > if you introduce new compatible string, then at least you need to document it
> > > so Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml need to be updated
> > >
> > > but my qeustion here, why bother introducing new compatible string if nothing
> > > changed with the compatible data? given the uart is same IP with g12a, can't we just
> > > use "amlogic,meson-g12-uart" for this? no only it will reduce the structure length of
> > > meson_uart_dt_match[], but also relieve maintainer's review burden?
> >
> > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
> >
> > Best regards,
> > Krzysztof
> >
> Hi, I did not understand the recommendation here.
> Can I add "amlogic,meson-t7-uart" without Documentation changes?
No, you can't.
> I think Yes, as I can see a few compatible strings in dts that don't
> exist anywhere else.
Aye, but we do not want to propagate that. New stuff should not be
adding undocumented compatibles, and those that are undocumented should
be documented.
> My idea here is to add "amlogic,meson-t7-uart" for future use if ever
> created, like if we find a bug in the future that is only relevant to
> T7 soc.
> But for now, fallback to s4 uart, as it seems to be the same controller.
>
> >From Krzysztof said in the writing-bindings.rst, I am following the rules.
>
> So, what's the path forward here?
You are following the rules from the dts point of view, you just need a
3rd patch in which you document the pattern you have added here in
amlogic,meson-uart.yaml. It is probably something like:
+ - items:
+ - const: amlogic,meson-t7-uart
+ - const: amlogic,meson-s4-uart
But I have not tested that, I just wrote that in my mail client.
Cheers,
Conor.
On 21/06/2023 10:01, Conor Dooley wrote:
> On Wed, Jun 21, 2023 at 08:37:02AM +0100, Lucas Tanure wrote:
>> On Wed, Jun 21, 2023 at 7:02 AM Krzysztof Kozlowski
>> <[email protected]> wrote:
>>>
>>> On 21/06/2023 00:09, Yixun Lan wrote:
>>>>> + 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",
>>>> ~~~~~~~~~~~~~~~~~
>>>> if you introduce new compatible string, then at least you need to document it
>>>> so Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml need to be updated
>>>>
>>>> but my qeustion here, why bother introducing new compatible string if nothing
>>>> changed with the compatible data? given the uart is same IP with g12a, can't we just
>>>> use "amlogic,meson-g12-uart" for this? no only it will reduce the structure length of
>>>> meson_uart_dt_match[], but also relieve maintainer's review burden?
>>>
>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Hi, I did not understand the recommendation here.
>> Can I add "amlogic,meson-t7-uart" without Documentation changes?
>
> No, you can't.
>
>> I think Yes, as I can see a few compatible strings in dts that don't
>> exist anywhere else.
>
> Aye, but we do not want to propagate that. New stuff should not be
> adding undocumented compatibles, and those that are undocumented should
> be documented.
Documentation is ongoing, it takes time !
Neil
>
>> My idea here is to add "amlogic,meson-t7-uart" for future use if ever
>> created, like if we find a bug in the future that is only relevant to
>> T7 soc.
>> But for now, fallback to s4 uart, as it seems to be the same controller.
>>
>> >From Krzysztof said in the writing-bindings.rst, I am following the rules.
>>
>> So, what's the path forward here?
>
> You are following the rules from the dts point of view, you just need a
> 3rd patch in which you document the pattern you have added here in
> amlogic,meson-uart.yaml. It is probably something like:
> + - items:
> + - const: amlogic,meson-t7-uart
> + - const: amlogic,meson-s4-uart
> But I have not tested that, I just wrote that in my mail client.
>
> Cheers,
> Conor.
On 21/06/2023 09:37, Lucas Tanure wrote:
>>
>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>
>> Best regards,
>> Krzysztof
>>
> Hi, I did not understand the recommendation here.
> Can I add "amlogic,meson-t7-uart" without Documentation changes?
No, you cannot and checkpatch will ensure that.
Best regards,
Krzysztof
On Wed, Jun 21, 2023 at 10:04:28AM +0200, Neil Armstrong wrote:
> On 21/06/2023 10:01, Conor Dooley wrote:
> > Aye, but we do not want to propagate that. New stuff should not be
> > adding undocumented compatibles, and those that are undocumented should
> > be documented.
>
> Documentation is ongoing, it takes time !
Don't worry, I wasn't trying to criticise, just pointing out the ideal
situation.