Re: [PATCH 3/3] arm64: dts: mediatek: Add Acelink EW-7886CAX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20.11.2023 15:17, AngeloGioacchino Del Regno wrote:
Il 17/11/23 11:43, Rafał Miłecki ha scritto:
From: Rafał Miłecki <rafal@xxxxxxxxxx>

Acelink EW-7886CAX is an MT7986A (AKA Filogic 830) based access point.
It has 512 MiB of RAM, one 2.5 Gbps PoE (802.3at) Ethernet port and
on-SoC Wi-Fi.

Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
---
  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
  .../mediatek/mt7986a-acelink-ew-7886cax.dts   | 175 ++++++++++++++++++
  2 files changed, 176 insertions(+)
  create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-acelink-ew-7886cax.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index e6e7592a3645..9ff2ab6c5e4d 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-acelink-ew-7886cax.dts b/arch/arm64/boot/dts/mediatek/mt7986a-acelink-ew-7886cax.dts
new file mode 100644
index 000000000000..18d19281dfdb
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-acelink-ew-7886cax.dts
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+
+/dts-v1/;
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+#include "mt7986a.dtsi"
+
+/ {
+    model = "Acelink EW-7886CAX";
+    compatible = "acelink,ew-7886cax", "mediatek,mt7986a";
+
+    aliases {
+        serial0 = &uart0;
+    };
+
+    chosen {
+        stdout-path = "serial0:115200n8";
+    };
+
+    memory@40000000 {
+        reg = <0 0x40000000 0 0x20000000>;
+        device_type = "memory";
+    };
+
+    keys {
+        compatible = "gpio-keys";
+
+        key-restart {
+            label = "Reset";
+            gpios = <&pio 7 GPIO_ACTIVE_LOW>;
+            linux,code = <KEY_RESTART>;
+        };
+    };
+
+    leds {
+        compatible = "gpio-leds";
+
+        led-0 {

Please, reorder by name

             color =    ...
             function = ...
             gpios = ...

Can you explain why and if there is a place I can find rules to follow
regarding such aspects? I really would like to just be aware of all
rules and don't waste anyone's time for such details.

FWIW I checked Documentation/devicetree/bindings/*.rst (after few years
I admit) but I couldn't find anything there about properties order.

If we currently don't have rules I don't really think we should enforce
following per-maintainer preferences. I really don't object your
suggestions but there is simply no way to remember each maintainer's
rules. We simply have too many subsystems and architectures boards.


+            function = LED_FUNCTION_STATUS;
+            color = <LED_COLOR_ID_RED>;
+            gpios = <&pio 18 GPIO_ACTIVE_HIGH>;
+        };
+
+        led-1 {
+            function = LED_FUNCTION_STATUS;
+            color = <LED_COLOR_ID_GREEN>;
+            gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
+        };
+
+        led-2 {
+            function = LED_FUNCTION_STATUS;
+            color = <LED_COLOR_ID_BLUE>;
+            gpios = <&pio 20 GPIO_ACTIVE_HIGH>;
+        };
+    };
+};
+
+&watchdog {

Ordering again, watchdog goes before wifi

Actually I ordered those in what I believe to be the most natural
numeric order. All those blocks references are ordered by the order
they appear in the included file and those follow numeric order I
believe.

I'd go as far as to claim using alhapbetical labels order is pretty
weak. Labels can be renamed and that would require reordering a lot of
.dts entries. On the other hand SoC's MMIO accessible hardware blocks
are quite unlikely to get reordered.


+    status = "okay";
+};
+
+&trng {
+    status = "okay";
+};
+
+&crypto {

crypto goes first.

crypto, eth, pcie_phy, spi0, trng, watchdog, wifi.

+    status = "okay";
+};
+
+&uart0 {
+    status = "okay";
+};
+
+&spi0 {
+    status = "okay";
+
+    flash@0 {
+        compatible = "spi-nand";
+        reg = <0>;

compatible
reg
#address/size cells
spi-max-frequency
spi-rx-bus-width
spi-tx-bus-width

+        spi-max-frequency = <52000000>;
+        spi-tx-bus-width = <4>;
+        spi-rx-bus-width = <4>;
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partitions: partitions {
+            compatible = "fixed-partitions";
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            partition@0 {
+                label = "bootloader";
+                reg = <0x0 0x100000>;

label, reg, read-only

+                read-only;
+            };
+
+            partition@100000 {
+                label = "u-boot-env";

reg first, label last

+                reg = <0x100000 0x80000>;
+            };
+
+            factory: partition@180000 {

Why do you have a phandle here? This has no use, please remove.

+                label = "factory";
+                reg = <0x180000 0x200000>;
+                read-only;
+                compatible = "nvmem-cells";

compatible
reg
label
read-only;

+
+                nvmem-layout {
+                    compatible = "fixed-layout";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    eeprom: eeprom@0 {
+                        reg = <0x0 0x1000>;
+                    };
+
+                    macaddr: macaddr@4 {
+                        reg = <0x4 0x6>;
+                    };
+                };
+            };
+
+            partition@380000 {
+                label = "fip";
+                reg = <0x380000 0x200000>;

reg first, label last

+            };
+
+            partition@580000 {
+                label = "ubi";
+                reg = <0x580000 0x4000000>;
+            };
+        };
+    };
+};
+

Regards,
Angelo





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux