Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board

["Arınç ÜNAL" <arinc.unal@arinc9.com>]

On 5.06.2023 18:01, Liviu Dudau wrote:
> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
> connected to the main SoC over PCIe.
> 
> The device uses NMBM over NAND, which is not currently supported in the
> mainline, so NAND node is skipped in this revision.
> 
> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> ---
>   arch/mips/boot/dts/ralink/Makefile            |  3 +-
>   .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
>   2 files changed, 94 insertions(+), 1 deletion(-)
>   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> 
> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> index 11732b8c8163a..d27d7e8c700fe 100644
> --- a/arch/mips/boot/dts/ralink/Makefile
> +++ b/arch/mips/boot/dts/ralink/Makefile
> @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
>   
>   dtb-$(CONFIG_SOC_MT7621) += \
>   	mt7621-gnubee-gb-pc1.dtb \
> -	mt7621-gnubee-gb-pc2.dtb
> +	mt7621-gnubee-gb-pc2.dtb \
> +	mt7621-tplink-hc220-g5-v1.dtb
>   
>   obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> new file mode 100644
> index 0000000000000..859aaa1c1bc2b
> --- /dev/null
> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/dts-v1/;
> +
> +#include "mt7621.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
> +	model = "TP-Link HC220 G5 v1";
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;

Please use 8 digit addressing for the memory start and size offsets:

0x00000000 0x08000000

> +	};
> +
> +	chosen {
> +		bootargs = "earlycon console=ttyS0,115200";
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		key-reset {
> +			label = "reset";
> +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_RESTART>;
> +		};
> +
> +		key-wps {
> +			label = "wps";
> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_WPS_BUTTON>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		red {

Usually the led name would point to the component the LED is used for.

> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;

Is there a specific reason you're using leds/common.h,
color & function instead of 'label = "red:ledname"'?

> +			gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		green {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_POWER;
> +			gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		blue {
> +			color = <LED_COLOR_ID_BLUE>;
> +			function = LED_FUNCTION_WPS;
> +			gpios = <&gpio 15 GPIO_ACTIVE_HIGH>;
> +		};

Every led node needs the "led-" prefix to satisfy the leds-gpio.yaml
schema. You can check for dt-schema warnings using this command:

ARCH=mips make clean dtbs_check

Ignore the warning for mediatek,mt7621-eth.

> +	};
> +};
> +
> +&pcie {
> +	status = "okay";
> +
> +	pcie@0,0 {
> +		compatible = "mediatek,mt76";
> +	};
> +
> +	pcie@1,0 {
> +		compatible = "mediatek,mt76";
> +	};

Both radios work with this then?

Also, I see a bunch of warnings now that the mediatek,mt76 compatible
string is added. The warning from schemas/pci/pci-bus.yaml is concerning.

/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0: Unevaluated properties are not allowed ('compatible' was unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'reset-gpios' were unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
	From schema: /usr/lib/python3/dist-packages/dtschema/schemas/pci/pci-bus.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: clocks: [[2, 23]] is too short
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'clocks', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'phy-names', 'phys', 'ranges' were unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml

Rob, Krzysztof any ideas what to do? The PCI child node is supposed to
be the properties of the wireless device. But the compatible string
doesn't match the schema on schemas/pci/pci-bus.yaml.

       compatible:
         contains:
           pattern: "^(pci[0-9a-f]{3,4},[0-9a-f]{1,4}|pciclass,[0-9a-f]{4,6})$"

Liviu, in the meantime, you should submit this patch without this
compatible string. I will handle this issue.

Arınç