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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 17/11/23 11:43, Rafał Miłecki ha scritto:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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@milecki.pl>
> ---
>   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 = ...

> +			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

> +	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