[PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Liviu Dudau]

This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
connected to the main SoC over PCIe.

The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
improve routing bandwidth.

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.dts     | 126 ++++++++++++++++++
 2 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts

diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
index 11732b8c8163a..c57a2b40876b4 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.dtb
 
 obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
new file mode 100644
index 0000000000000..83d15711907d0
--- /dev/null
+++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
@@ -0,0 +1,126 @@
+// 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", "mediatek,mt7621-soc";
+	model = "TP-Link HC220 G5 v1.0";
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x8000000>;
+	};
+
+	chosen {
+		/* bootargs = "earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0 kgdboc=ttyS0,115200 ip=192.168.88.1:192.168.88.2:::hc220:eth1:none kgdbcon console=ttyS0,115200"; */
+		/* bootargs = "console=ttyS0,115200 earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0"; */
+		bootargs = "console=ttyS0,115200 root=/dev/nfs ip=192.168.88.2:192.168.88.5::255.255.255.0:hc220_g5:eth1:none nfsroot=192.168.88.5:/mips,vers=4,sec=sys ro rootwait";
+	};
+
+	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 {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_FAULT;
+			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>;
+		};
+	};
+
+	resetc: reset-controller {
+		compatible = "ralink,rt2880-reset";
+		#reset-cells = <1>;
+	};
+
+	mtd {
+		compatible = "mediatek,mt7622-nfc";
+	};
+};
+
+&i2c {
+	status = "okay";
+};
+
+&pcie {
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+
+	flash@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+	};
+};
+
+/* gmac1 connected to MT7530's phy0 */
+&gmac1 {
+	status = "okay";
+	phy-handle = <&ethphy0>;
+};
+
+&mdio {
+	/* MT7530's phy0 */
+	ethphy0: ethernet-phy@0 {
+		reg = <0>;
+		phy-mode = "rgmii";
+	};
+};
+
+&switch0 {
+	/* #gpio-cells = <2>;
+	gpio-controller; */
+
+	ports {
+		/* phy0 is muxed to gmac1 */
+		/delete-node/ port@0;
+
+		port@1 {
+			status = "okay";
+			label = "lan1";
+		};
+
+		port@2 {
+			status = "okay";
+			label = "wan";
+		};
+	};
+};
-- 
2.40.0

Re: [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Arınç ÜNAL]

On 9.05.2023 22: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 MT7663 5GHz WiFi chips integrated on the board,
> connected to the main SoC over PCIe.
> 
> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
> improve routing bandwidth.
> 
> 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>

This is great to see. I'm going to mainline all the MT7621 devicetrees 
on OpenWrt at some point, this is a good step for this.

> ---
>   arch/mips/boot/dts/ralink/Makefile            |   3 +-
>   .../dts/ralink/mt7621-tplink-hc220_g5.dts     | 126 ++++++++++++++++++
>   2 files changed, 128 insertions(+), 1 deletion(-)
>   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> 
> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> index 11732b8c8163a..c57a2b40876b4 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.dtb
>   
>   obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> new file mode 100644
> index 0000000000000..83d15711907d0
> --- /dev/null
> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> @@ -0,0 +1,126 @@
> +// 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", "mediatek,mt7621-soc";

tplink,hc220-g5-v1 should fit better. Also please make another patch to 
add the compatible string under Boards with Mediatek/Ralink MT7621 SoC 
on Documentation/devicetree/bindings/mips/ralink.yaml.

> +	model = "TP-Link HC220 G5 v1.0";

"TP-Link HC220 G5 v1" should be enough.

> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x8000000>;
> +	};
> +
> +	chosen {
> +		/* bootargs = "earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0 kgdboc=ttyS0,115200 ip=192.168.88.1:192.168.88.2:::hc220:eth1:none kgdbcon console=ttyS0,115200"; */
> +		/* bootargs = "console=ttyS0,115200 earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0"; */
> +		bootargs = "console=ttyS0,115200 root=/dev/nfs ip=192.168.88.2:192.168.88.5::255.255.255.0:hc220_g5:eth1:none nfsroot=192.168.88.5:/mips,vers=4,sec=sys ro rootwait";
> +	};
> +
> +	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 {
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +			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>;
> +		};
> +	};
> +
> +	resetc: reset-controller {
> +		compatible = "ralink,rt2880-reset";
> +		#reset-cells = <1>;
> +	};
> +
> +	mtd {
> +		compatible = "mediatek,mt7622-nfc";
> +	};
> +};
> +
> +&i2c {
> +	status = "okay";
> +};
> +
> +&pcie {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	status = "okay";
> +
> +	flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <50000000>;
> +	};
> +};
> +
> +/* gmac1 connected to MT7530's phy0 */
> +&gmac1 {
> +	status = "okay";
> +	phy-handle = <&ethphy0>;
> +};
> +
> +&mdio {
> +	/* MT7530's phy0 */
> +	ethphy0: ethernet-phy@0 {
> +		reg = <0>;
> +		phy-mode = "rgmii";
> +	};
> +};

These are partially wrong, check the mt7621.dtsi on mainline. Or better, 
don't do it. I'm very close to adding support for changing the DSA 
conduit for user ports. I suggest you just add port@0 to the DSA switch 
node below.

> +
> +&switch0 {
> +	/* #gpio-cells = <2>;
> +	gpio-controller; */
> +
> +	ports {
> +		/* phy0 is muxed to gmac1 */
> +		/delete-node/ port@0;
> +
> +		port@1 {
> +			status = "okay";
> +			label = "lan1";
> +		};
> +
> +		port@2 {
> +			status = "okay";
> +			label = "wan";
> +		};
> +	};
> +};

Arınç

Re: [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Arınç ÜNAL]

On 9.05.2023 22: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 MT7663 5GHz WiFi chips integrated on the board,
> connected to the main SoC over PCIe.
> 
> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
> improve routing bandwidth.
> 
> 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.dts     | 126 ++++++++++++++++++
>   2 files changed, 128 insertions(+), 1 deletion(-)
>   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> 
> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> index 11732b8c8163a..c57a2b40876b4 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.dtb

No underscores please. And add v1 on the filename.

Arınç

Re: [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Arınç ÜNAL]

Please remove the dot from the subject. Also check the patches with 
scripts/checkpatch.pl. At least one of them has got 80 columns on a 
line, the limit is 75.

Arınç

Re: [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Liviu Dudau]

On Thu, May 11, 2023 at 03:41:30PM +0200, Arınç ÜNAL wrote:
> On 9.05.2023 22: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 MT7663 5GHz WiFi chips integrated on the board,
> > connected to the main SoC over PCIe.
> > 
> > The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
> > improve routing bandwidth.
> > 
> > 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>
> 
> This is great to see. I'm going to mainline all the MT7621 devicetrees on
> OpenWrt at some point, this is a good step for this.

AFAIK this board is not supported by OpenWrt at all. So the flow will be
the other way this time :)

> 
> > ---
> >   arch/mips/boot/dts/ralink/Makefile            |   3 +-
> >   .../dts/ralink/mt7621-tplink-hc220_g5.dts     | 126 ++++++++++++++++++
> >   2 files changed, 128 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> > 
> > diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> > index 11732b8c8163a..c57a2b40876b4 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.dtb
> >   obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> > diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> > new file mode 100644
> > index 0000000000000..83d15711907d0
> > --- /dev/null
> > +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
> > @@ -0,0 +1,126 @@
> > +// 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", "mediatek,mt7621-soc";
> 
> tplink,hc220-g5-v1 should fit better. Also please make another patch to add
> the compatible string under Boards with Mediatek/Ralink MT7621 SoC on
> Documentation/devicetree/bindings/mips/ralink.yaml.

Will do another patch with the documentation update. I will also send a v2
incorporating all your review comments, thanks for taking the time!

Regarding the full name: I'm quite tempted to drop the v1(.0) entirely as I'm
not really sure that TP-Link will release another board. Doing a search now
I fail to find on any TP-Link results the mention of the version.

> 
> > +	model = "TP-Link HC220 G5 v1.0";
> 
> "TP-Link HC220 G5 v1" should be enough.
> 
> > +
> > +	memory@0 {
> > +		device_type = "memory";
> > +		reg = <0x0 0x0 0x0 0x8000000>;
> > +	};
> > +
> > +	chosen {
> > +		/* bootargs = "earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0 kgdboc=ttyS0,115200 ip=192.168.88.1:192.168.88.2:::hc220:eth1:none kgdbcon console=ttyS0,115200"; */
> > +		/* bootargs = "console=ttyS0,115200 earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0"; */
> > +		bootargs = "console=ttyS0,115200 root=/dev/nfs ip=192.168.88.2:192.168.88.5::255.255.255.0:hc220_g5:eth1:none nfsroot=192.168.88.5:/mips,vers=4,sec=sys ro rootwait";
> > +	};
> > +
> > +	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 {
> > +			color = <LED_COLOR_ID_RED>;
> > +			function = LED_FUNCTION_FAULT;
> > +			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>;
> > +		};
> > +	};
> > +
> > +	resetc: reset-controller {
> > +		compatible = "ralink,rt2880-reset";
> > +		#reset-cells = <1>;
> > +	};
> > +
> > +	mtd {
> > +		compatible = "mediatek,mt7622-nfc";
> > +	};
> > +};
> > +
> > +&i2c {
> > +	status = "okay";
> > +};
> > +
> > +&pcie {
> > +	status = "okay";
> > +};
> > +
> > +&spi0 {
> > +	status = "okay";
> > +
> > +	flash@0 {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "jedec,spi-nor";
> > +		reg = <0>;
> > +		spi-max-frequency = <50000000>;
> > +	};
> > +};
> > +
> > +/* gmac1 connected to MT7530's phy0 */
> > +&gmac1 {
> > +	status = "okay";
> > +	phy-handle = <&ethphy0>;
> > +};
> > +
> > +&mdio {
> > +	/* MT7530's phy0 */
> > +	ethphy0: ethernet-phy@0 {
> > +		reg = <0>;
> > +		phy-mode = "rgmii";
> > +	};
> > +};
> 
> These are partially wrong, check the mt7621.dtsi on mainline. Or better,
> don't do it. I'm very close to adding support for changing the DSA conduit
> for user ports. I suggest you just add port@0 to the DSA switch node below.
>

I don't claim to be an expert on DSA or how MT7530 connects to MT7621, however
I do remember trying various combinations and failing to make all the ports
work. I blame TP-Link on having a weird setup, but to be sure I will try your
suggestion if I manage to understand exactly where I'm "partially wrong" :)

I will also try to remove the node to see what I get and report back here.

Best regards,
Liviu


> > +
> > +&switch0 {
> > +	/* #gpio-cells = <2>;
> > +	gpio-controller; */
> > +
> > +	ports {
> > +		/* phy0 is muxed to gmac1 */
> > +		/delete-node/ port@0;
> > +
> > +		port@1 {
> > +			status = "okay";
> > +			label = "lan1";
> > +		};
> > +
> > +		port@2 {
> > +			status = "okay";
> > +			label = "wan";
> > +		};
> > +	};
> > +};
> 
> Arınç

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

Re: [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

[Arınç ÜNAL]

On 11.05.2023 18:03, Liviu Dudau wrote:
> On Thu, May 11, 2023 at 03:41:30PM +0200, Arınç ÜNAL wrote:
>> On 9.05.2023 22: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 MT7663 5GHz WiFi chips integrated on the board,
>>> connected to the main SoC over PCIe.
>>>
>>> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
>>> improve routing bandwidth.
>>>
>>> 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>
>>
>> This is great to see. I'm going to mainline all the MT7621 devicetrees on
>> OpenWrt at some point, this is a good step for this.
> 
> AFAIK this board is not supported by OpenWrt at all. So the flow will be
> the other way this time :)
> 
>>
>>> ---
>>>    arch/mips/boot/dts/ralink/Makefile            |   3 +-
>>>    .../dts/ralink/mt7621-tplink-hc220_g5.dts     | 126 ++++++++++++++++++
>>>    2 files changed, 128 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
>>>
>>> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
>>> index 11732b8c8163a..c57a2b40876b4 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.dtb
>>>    obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
>>> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
>>> new file mode 100644
>>> index 0000000000000..83d15711907d0
>>> --- /dev/null
>>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220_g5.dts
>>> @@ -0,0 +1,126 @@
>>> +// 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", "mediatek,mt7621-soc";
>>
>> tplink,hc220-g5-v1 should fit better. Also please make another patch to add
>> the compatible string under Boards with Mediatek/Ralink MT7621 SoC on
>> Documentation/devicetree/bindings/mips/ralink.yaml.
> 
> Will do another patch with the documentation update. I will also send a v2
> incorporating all your review comments, thanks for taking the time!
> 
> Regarding the full name: I'm quite tempted to drop the v1(.0) entirely as I'm
> not really sure that TP-Link will release another board. Doing a search now
> I fail to find on any TP-Link results the mention of the version.

What I understand is TP-Link always call their first hardware release v1 
as they are notorious for introducing new versions to cut costs as much 
as possible. On the downloads website for the product, "v1" is there on 
the product name.

https://www.tp-link.com/us/support/download/hc220-g5/

> 
>>
>>> +	model = "TP-Link HC220 G5 v1.0";
>>
>> "TP-Link HC220 G5 v1" should be enough.
>>
>>> +
>>> +	memory@0 {
>>> +		device_type = "memory";
>>> +		reg = <0x0 0x0 0x0 0x8000000>;
>>> +	};
>>> +
>>> +	chosen {
>>> +		/* bootargs = "earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0 kgdboc=ttyS0,115200 ip=192.168.88.1:192.168.88.2:::hc220:eth1:none kgdbcon console=ttyS0,115200"; */
>>> +		/* bootargs = "console=ttyS0,115200 earlycon=uart8260,mmio32,0x1e000c00 root=/dev/ram0"; */
>>> +		bootargs = "console=ttyS0,115200 root=/dev/nfs ip=192.168.88.2:192.168.88.5::255.255.255.0:hc220_g5:eth1:none nfsroot=192.168.88.5:/mips,vers=4,sec=sys ro rootwait";
>>> +	};
>>> +
>>> +	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 {
>>> +			color = <LED_COLOR_ID_RED>;
>>> +			function = LED_FUNCTION_FAULT;
>>> +			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>;
>>> +		};
>>> +	};
>>> +
>>> +	resetc: reset-controller {
>>> +		compatible = "ralink,rt2880-reset";
>>> +		#reset-cells = <1>;
>>> +	};
>>> +
>>> +	mtd {
>>> +		compatible = "mediatek,mt7622-nfc";
>>> +	};
>>> +};
>>> +
>>> +&i2c {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&pcie {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&spi0 {
>>> +	status = "okay";
>>> +
>>> +	flash@0 {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +		compatible = "jedec,spi-nor";
>>> +		reg = <0>;
>>> +		spi-max-frequency = <50000000>;
>>> +	};
>>> +};
>>> +
>>> +/* gmac1 connected to MT7530's phy0 */
>>> +&gmac1 {
>>> +	status = "okay";
>>> +	phy-handle = <&ethphy0>;
>>> +};
>>> +
>>> +&mdio {
>>> +	/* MT7530's phy0 */
>>> +	ethphy0: ethernet-phy@0 {
>>> +		reg = <0>;
>>> +		phy-mode = "rgmii";
>>> +	};
>>> +};
>>
>> These are partially wrong, check the mt7621.dtsi on mainline. Or better,
>> don't do it. I'm very close to adding support for changing the DSA conduit
>> for user ports. I suggest you just add port@0 to the DSA switch node below.
>>
> 
> I don't claim to be an expert on DSA or how MT7530 connects to MT7621, however
> I do remember trying various combinations and failing to make all the ports
> work. I blame TP-Link on having a weird setup, but to be sure I will try your
> suggestion if I manage to understand exactly where I'm "partially wrong" :)

You're not wrong, the bindings are. You must disable fixed-link. The 
properties status okay and phy-mode rgmii on ethphy0 is not necessary so 
these can be dropped. Take a look at the GB-PC2 bindings, rgmii-rxid is 
specific to GB-PC2 so no need to do that:

https://github.com/torvalds/linux/blob/master/arch/mips/boot/dts/ralink/mt7621-gnubee-gb-pc2.dts#L114

If you define the port under the DSA switch node, it should just work.

Arınç

[PATCH v2 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board

[Liviu Dudau]

Add device tree and bindings for the TP-Link HC220 G5 v1 wireless AP,
a consumer product from TP-Link based on MT7621.

Changes since v1:
 - Changed compatible for the board to "tplink,hc220-g5-v1"
 - Updated the DSA switch nodes to better reflect actual usage.
 - Disabled the fixed-link in gmac1
 - Added device tree bindings in Documentation/

Liviu Dudau (2):
  mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

 .../devicetree/bindings/mips/ralink.yaml      |   1 +
 arch/mips/boot/dts/ralink/Makefile            |   3 +-
 .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 129 ++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts

-- 
2.40.1

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

[Liviu Dudau]

This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
connected to the main SoC over PCIe.

The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
improve routing bandwidth.

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  | 129 ++++++++++++++++++
 2 files changed, 131 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..f003ae615a58e
--- /dev/null
+++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
@@ -0,0 +1,129 @@
+// 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 = <0x0 0x0 0x0 0x8000000>;
+	};
+
+	chosen {
+		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to
+		 * bootargs for early boot messages
+		 */
+		bootargs = "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 {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_FAULT;
+			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>;
+		};
+	};
+
+	resetc: reset-controller {
+		compatible = "ralink,rt2880-reset";
+		#reset-cells = <1>;
+	};
+
+	mtd {
+		compatible = "mediatek,mt7622-nfc";
+	};
+};
+
+&i2c {
+	status = "okay";
+};
+
+&pcie {
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+
+	flash@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+	};
+};
+
+/* gmac1 connected to MT7530's phy0 */
+&gmac1 {
+	phy-handle = <&ethphy0>;
+
+	fixed-link {
+		status = "disabled";
+	};
+};
+
+&mdio {
+	/* MT7530's phy0 */
+	ethphy0: ethernet-phy@0 {
+		reg = <0>;
+	};
+};
+
+&switch0 {
+	ports {
+		/* phy0 is muxed to gmac1 */
+		port@0 {
+			status = "okay";
+			label = "lan2";
+		};
+
+		port@1 {
+			status = "okay";
+			label = "lan1";
+		};
+
+		port@2 {
+			status = "okay";
+			label = "wan";
+		};
+	};
+};
-- 
2.40.1

[PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

[Liviu Dudau]

Add bindings for the compatible string used for the TP-Link's
HC220 G5 V1 board, a wireless AP based on MT7621.

Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
---
 Documentation/devicetree/bindings/mips/ralink.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mips/ralink.yaml b/Documentation/devicetree/bindings/mips/ralink.yaml
index 704b5b5951271..53c1f66353770 100644
--- a/Documentation/devicetree/bindings/mips/ralink.yaml
+++ b/Documentation/devicetree/bindings/mips/ralink.yaml
@@ -80,6 +80,7 @@ properties:
           - enum:
               - gnubee,gb-pc1
               - gnubee,gb-pc2
+              - tplink,hc220-g5-v1
           - const: mediatek,mt7621-soc
 
 additionalProperties: true
-- 
2.40.1

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

[Liviu Dudau]

On Mon, May 29, 2023 at 04:08:32PM +0100, Liviu Dudau wrote:
> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
> connected to the main SoC over PCIe.
> 
> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
> improve routing bandwidth.
> 
> 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  | 129 ++++++++++++++++++
>  2 files changed, 131 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..f003ae615a58e
> --- /dev/null
> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> @@ -0,0 +1,129 @@
> +// 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 = <0x0 0x0 0x0 0x8000000>;
> +	};
> +
> +	chosen {
> +		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to
> +		 * bootargs for early boot messages
> +		 */
> +		bootargs = "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 {
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +			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>;
> +		};
> +	};
> +
> +	resetc: reset-controller {
> +		compatible = "ralink,rt2880-reset";
> +		#reset-cells = <1>;
> +	};
> +
> +	mtd {
> +		compatible = "mediatek,mt7622-nfc";
> +	};
> +};
> +
> +&i2c {
> +	status = "okay";
> +};
> +
> +&pcie {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	status = "okay";
> +
> +	flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <50000000>;
> +	};
> +};
> +
> +/* gmac1 connected to MT7530's phy0 */
> +&gmac1 {
> +	phy-handle = <&ethphy0>;
> +
> +	fixed-link {
> +		status = "disabled";
> +	};
> +};
> +
> +&mdio {
> +	/* MT7530's phy0 */
> +	ethphy0: ethernet-phy@0 {
> +		reg = <0>;
> +	};
> +};
> +
> +&switch0 {
> +	ports {
> +		/* phy0 is muxed to gmac1 */
> +		port@0 {
> +			status = "okay";
> +			label = "lan2";
> +		};

I've made the changes to look similar to the gnubee-gb-pc2, and things mostly
work, with the exception that I can mount an NFS root filesystem only on "lan2"
interface at boot time. All other interfaces (ports) hang forever waiting for
an DHCP response from my server. The only difference is where I plug in the
ethernet cable, no other change (not even a restart) on the server.

Best regards,
Liviu

> +
> +		port@1 {
> +			status = "okay";
> +			label = "lan1";
> +		};
> +
> +		port@2 {
> +			status = "okay";
> +			label = "wan";
> +		};
> +	};
> +};
> -- 
> 2.40.1
> 

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

Re: [PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

Hey,

On Mon, May 29, 2023 at 04:08:33PM +0100, Liviu Dudau wrote:
> Add bindings for the compatible string used for the TP-Link's
> HC220 G5 V1 board, a wireless AP based on MT7621.
> 
> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>

Two process things:
- Please run get_maintainer.pl against the current git tree - you seem
  to have missed 2 of the 3 dt-bindings maintainers. I don't mind my
  omission, but omitting Krzysztof means you're submitting patches based
  on a tree that is over a year old.
- Please don't send a v2 in reply to the v1.

> ---
>  Documentation/devicetree/bindings/mips/ralink.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mips/ralink.yaml b/Documentation/devicetree/bindings/mips/ralink.yaml
> index 704b5b5951271..53c1f66353770 100644
> --- a/Documentation/devicetree/bindings/mips/ralink.yaml
> +++ b/Documentation/devicetree/bindings/mips/ralink.yaml
> @@ -80,6 +80,7 @@ properties:
>            - enum:
>                - gnubee,gb-pc1
>                - gnubee,gb-pc2
> +              - tplink,hc220-g5-v1

For the actual change though,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

>            - const: mediatek,mt7621-soc
>  
>  additionalProperties: true
> -- 
> 2.40.1
> 

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

Re: [PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

[Liviu Dudau]

On Mon, May 29, 2023 at 10:00:20PM +0100, Conor Dooley wrote:
> Hey,

Hi Conor,

> 
> On Mon, May 29, 2023 at 04:08:33PM +0100, Liviu Dudau wrote:
> > Add bindings for the compatible string used for the TP-Link's
> > HC220 G5 V1 board, a wireless AP based on MT7621.
> > 
> > Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> 
> Two process things:
> - Please run get_maintainer.pl against the current git tree - you seem
>   to have missed 2 of the 3 dt-bindings maintainers. I don't mind my
>   omission, but omitting Krzysztof means you're submitting patches based
>   on a tree that is over a year old.

Appologies for that, I should have check get_maintainer.pl rather than looking
at the output of git log for the file.

> - Please don't send a v2 in reply to the v1.

Sorry, I'm not sure why that makes any difference. Reason for sending it as a
reply was the fact that last conversation was more than two weeks ago, people
might have forgot the context. This way it's all in one thread.

> 
> > ---
> >  Documentation/devicetree/bindings/mips/ralink.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mips/ralink.yaml b/Documentation/devicetree/bindings/mips/ralink.yaml
> > index 704b5b5951271..53c1f66353770 100644
> > --- a/Documentation/devicetree/bindings/mips/ralink.yaml
> > +++ b/Documentation/devicetree/bindings/mips/ralink.yaml
> > @@ -80,6 +80,7 @@ properties:
> >            - enum:
> >                - gnubee,gb-pc1
> >                - gnubee,gb-pc2
> > +              - tplink,hc220-g5-v1
> 
> For the actual change though,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Many thanks!

Best regards,
Liviu

> 
> Thanks,
> Conor.
> 
> >            - const: mediatek,mt7621-soc
> >  
> >  additionalProperties: true
> > -- 
> > 2.40.1
> > 



-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

Re: [PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]

On Mon, May 29, 2023 at 10:53:57PM +0100, Liviu Dudau wrote:
> On Mon, May 29, 2023 at 10:00:20PM +0100, Conor Dooley wrote:

> > Two process things:
> > - Please run get_maintainer.pl against the current git tree - you seem
> >   to have missed 2 of the 3 dt-bindings maintainers. I don't mind my
> >   omission, but omitting Krzysztof means you're submitting patches based
> >   on a tree that is over a year old.
> 
> Appologies for that, I should have check get_maintainer.pl rather than looking
> at the output of git log for the file.

Yeah, git log ain't the way to do things ;)

> 
> > - Please don't send a v2 in reply to the v1.
> 
> Sorry, I'm not sure why that makes any difference. Reason for sending it as a
> reply was the fact that last conversation was more than two weeks ago, people
> might have forgot the context. This way it's all in one thread.

That's fine, just include a link to v1 on lore for anyone that cares
about the context, like so:
https://lore.kernel.org/all/20230529150833.526084-1-liviu@dudau.co.uk/

I knew this mail had been sent today because I noticed at the time that
you'd only CCed Rob but I remembered nothing of the subject line since
I saw it hours ago. Look how far back in my mailbox it ended up because
of being sent as a reply to the v1, and that mailbox only contains
emails from the devicetree list!

4380     May 29 Liviu Dudau     (7.6K) │       ┌─>[PATCH v2 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
4381   T May 29 Liviu Dudau     (5.8K) │       │   ┌─>
4382  sF May 29 To Liviu Dudau  (6.8K) │       │ ┌─>
4383     May 29 Liviu Dudau     (4.6K) │       ├─>[PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board
4384     May 29 Liviu Dudau     (4.7K) │     ┌─>[PATCH v2 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
4385     May 12 Arınç ÜNAL      ( 12K) │   ┌─>
4386     May 11 Liviu Dudau     ( 10K) │ ┌─>
4387     May 11 Arınç ÜNAL      ( 10K) ├─>
4388     May 09 Liviu Dudau     (7.8K) [PATCH] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board.

Some patch application & CI tools apparently also get confused, but I
dunno what those tools are.

Cheers,
Conor.

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

Re: [PATCH v2 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

[Sergio Paracuellos]

On Mon, May 29, 2023 at 5:08 PM Liviu Dudau <liviu@dudau.co.uk> wrote:
>
> Add bindings for the compatible string used for the TP-Link's
> HC220 G5 V1 board, a wireless AP based on MT7621.
>
> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> ---
>  Documentation/devicetree/bindings/mips/ralink.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

Thanks,
    Sergio Paracuellos

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

[Arınç ÜNAL]

On 29.05.2023 18:55, Liviu Dudau wrote:
> On Mon, May 29, 2023 at 04:08:32PM +0100, Liviu Dudau wrote:
>> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
>> a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,

Do you mean MT7662 5GHz WiFi?

>> connected to the main SoC over PCIe.
>>
>> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
>> improve routing bandwidth.

This is not always true, I'd prefer you remove this sentence from the 
patch log.

>>
>> 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  | 129 ++++++++++++++++++
>>   2 files changed, 131 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..f003ae615a58e
>> --- /dev/null
>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>> @@ -0,0 +1,129 @@
>> +// 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 = <0x0 0x0 0x0 0x8000000>;

What's going on here? Just do 'reg = <0x00000000 0x08000000>;'.

>> +	};
>> +
>> +	chosen {
>> +		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to

8260?

>> +		 * bootargs for early boot messages

Isn't just adding "earlycon" to bootargs enough?

>> +		 */
>> +		bootargs = "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 {
>> +			color = <LED_COLOR_ID_RED>;
>> +			function = LED_FUNCTION_FAULT;
>> +			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>;
>> +		};
>> +	};
>> +
>> +	resetc: reset-controller {
>> +		compatible = "ralink,rt2880-reset";
>> +		#reset-cells = <1>;
>> +	};

We don't use this anymore.

>> +
>> +	mtd {
>> +		compatible = "mediatek,mt7622-nfc";
>> +	};

What's this got to do with this device?

>> +};
>> +
>> +&i2c {
>> +	status = "okay";
>> +};

Why does this device need i2c?

>> +
>> +&pcie {
>> +	status = "okay";
>> +};

Do both WiFi chips work by just enabling pcie? I was expecting 
'compatible = "mediatek,mt76";' on pcie@0,0 and pcie@1,0.

>> +
>> +&spi0 {
>> +	status = "okay";
>> +
>> +	flash@0 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		spi-max-frequency = <50000000>;
>> +	};
>> +};

I thought you said this device had NAND flash, not NOR.

>> +
>> +/* gmac1 connected to MT7530's phy0 */
>> +&gmac1 {
>> +	phy-handle = <&ethphy0>;
>> +
>> +	fixed-link {
>> +		status = "disabled";
>> +	};
>> +};
>> +
>> +&mdio {
>> +	/* MT7530's phy0 */
>> +	ethphy0: ethernet-phy@0 {
>> +		reg = <0>;
>> +	};
>> +};

Remove the two nodes above.

>> +
>> +&switch0 {
>> +	ports {
>> +		/* phy0 is muxed to gmac1 */
>> +		port@0 {
>> +			status = "okay";
>> +			label = "lan2";
>> +		};
> 
> I've made the changes to look similar to the gnubee-gb-pc2, and things mostly
> work, with the exception that I can mount an NFS root filesystem only on "lan2"
> interface at boot time. All other interfaces (ports) hang forever waiting for
> an DHCP response from my server. The only difference is where I plug in the
> ethernet cable, no other change (not even a restart) on the server.

This sounds like a userspace configuration issue.

Arınç

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

[Arınç ÜNAL]

On 29.05.2023 18:55, Liviu Dudau wrote:
> On Mon, May 29, 2023 at 04:08:32PM +0100, Liviu Dudau wrote:
>> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
>> a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
>> connected to the main SoC over PCIe.
>>
>> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
>> improve routing bandwidth.
>>
>> 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  | 129 ++++++++++++++++++
>>   2 files changed, 131 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..f003ae615a58e
>> --- /dev/null
>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>> @@ -0,0 +1,129 @@
>> +// 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 = <0x0 0x0 0x0 0x8000000>;
>> +	};
>> +
>> +	chosen {
>> +		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to
>> +		 * bootargs for early boot messages
>> +		 */
>> +		bootargs = "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 {
>> +			color = <LED_COLOR_ID_RED>;
>> +			function = LED_FUNCTION_FAULT;
>> +			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>;
>> +		};
>> +	};
>> +
>> +	resetc: reset-controller {
>> +		compatible = "ralink,rt2880-reset";
>> +		#reset-cells = <1>;
>> +	};
>> +
>> +	mtd {
>> +		compatible = "mediatek,mt7622-nfc";
>> +	};
>> +};
>> +
>> +&i2c {
>> +	status = "okay";
>> +};
>> +
>> +&pcie {
>> +	status = "okay";
>> +};
>> +
>> +&spi0 {
>> +	status = "okay";
>> +
>> +	flash@0 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		spi-max-frequency = <50000000>;
>> +	};
>> +};
>> +
>> +/* gmac1 connected to MT7530's phy0 */
>> +&gmac1 {
>> +	phy-handle = <&ethphy0>;
>> +
>> +	fixed-link {
>> +		status = "disabled";
>> +	};
>> +};
>> +
>> +&mdio {
>> +	/* MT7530's phy0 */
>> +	ethphy0: ethernet-phy@0 {
>> +		reg = <0>;
>> +	};
>> +};
>> +
>> +&switch0 {
>> +	ports {
>> +		/* phy0 is muxed to gmac1 */

I already suggested not doing this, please remove this line.

Arınç

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

[Liviu Dudau]

On Sat, Jun 03, 2023 at 03:06:08PM +0300, Arınç ÜNAL wrote:
> On 29.05.2023 18:55, Liviu Dudau wrote:
> > On Mon, May 29, 2023 at 04:08:32PM +0100, Liviu Dudau wrote:
> > > This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> > > a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
> 
> Do you mean MT7662 5GHz WiFi?

Actually it's MT76*1*3. Will fix for v3.

> 
> > > connected to the main SoC over PCIe.
> > > 
> > > The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
> > > improve routing bandwidth.
> 
> This is not always true, I'd prefer you remove this sentence from the patch
> log.

If your comment is about the statement not being always true in general, then
I agree. However for this device the port marked as WAN on HC220 v1 is the
second port on the switch and according to the DSA bindings docs only PHY0
or PHY4 can be connected to GMAC1, so I picked the one that worked (PHY4 did
not). More on that at the end of the email.

> 
> > > 
> > > 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  | 129 ++++++++++++++++++
> > >   2 files changed, 131 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..f003ae615a58e
> > > --- /dev/null
> > > +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > > @@ -0,0 +1,129 @@
> > > +// 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 = <0x0 0x0 0x0 0x8000000>;
> 
> What's going on here? Just do 'reg = <0x00000000 0x08000000>;'.
>

Right, hangover from 64bits world :) Will fix for v3.


> > > +	};
> > > +
> > > +	chosen {
> > > +		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to
> 
> 8260?

Duh, I feel stupid. It's 8250 and (as you mention bellow) not needed to
enable early boot messages.

> 
> > > +		 * bootargs for early boot messages
> 
> Isn't just adding "earlycon" to bootargs enough?
> 
> > > +		 */
> > > +		bootargs = "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 {
> > > +			color = <LED_COLOR_ID_RED>;
> > > +			function = LED_FUNCTION_FAULT;
> > > +			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>;
> > > +		};
> > > +	};
> > > +
> > > +	resetc: reset-controller {
> > > +		compatible = "ralink,rt2880-reset";
> > > +		#reset-cells = <1>;
> > > +	};
> 
> We don't use this anymore.
>

I've copied it from disassembled vendor device tree. Is this not needed
anymore for reset?

> > > +
> > > +	mtd {
> > > +		compatible = "mediatek,mt7622-nfc";
> > > +	};
> 
> What's this got to do with this device?

This is the NAND Flash Controller bindind. Left over from interrupted
development of the NAND support. Will remove for v3.

> 
> > > +};
> > > +
> > > +&i2c {
> > > +	status = "okay";
> > > +};
> 
> Why does this device need i2c?

It doesn't. I was curious if there was anything hanging on that bus so I've
enabled it to scan it. Will remove for v3.


> 
> > > +
> > > +&pcie {
> > > +	status = "okay";
> > > +};
> 
> Do both WiFi chips work by just enabling pcie? I was expecting 'compatible =
> "mediatek,mt76";' on pcie@0,0 and pcie@1,0.

Nothing else is needed and I can modprobe the mt7603e and mt7615e modules just
fine, with the devices becoming available one I put the appropriate firmware in
the right place.

> 
> > > +
> > > +&spi0 {
> > > +	status = "okay";
> > > +
> > > +	flash@0 {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <1>;
> > > +		compatible = "jedec,spi-nor";
> > > +		reg = <0>;
> > > +		spi-max-frequency = <50000000>;
> > > +	};
> > > +};
> 
> I thought you said this device had NAND flash, not NOR.

Appologies, I will remove this for now as I haven't finished looking into
storage problem yet.

> 
> > > +
> > > +/* gmac1 connected to MT7530's phy0 */
> > > +&gmac1 {
> > > +	phy-handle = <&ethphy0>;
> > > +
> > > +	fixed-link {
> > > +		status = "disabled";
> > > +	};
> > > +};
> > > +
> > > +&mdio {
> > > +	/* MT7530's phy0 */
> > > +	ethphy0: ethernet-phy@0 {
> > > +		reg = <0>;
> > > +	};
> > > +};
> 
> Remove the two nodes above.

It would be nice if you can provide some explanation to some of your requests
so that I can learn more about this. This is my first time dipping my toes
on MIPS and MT7621 world, so I don't have that much experience and would like
to know more.

> 
> > > +
> > > +&switch0 {
> > > +	ports {
> > > +		/* phy0 is muxed to gmac1 */
> > > +		port@0 {
> > > +			status = "okay";
> > > +			label = "lan2";
> > > +		};
> > 
> > I've made the changes to look similar to the gnubee-gb-pc2, and things mostly
> > work, with the exception that I can mount an NFS root filesystem only on "lan2"
> > interface at boot time. All other interfaces (ports) hang forever waiting for
> > an DHCP response from my server. The only difference is where I plug in the
> > ethernet cable, no other change (not even a restart) on the server.
> 
> This sounds like a userspace configuration issue.

Which userspace? On the server side? I don't touch anything there, only
change the ethernet port where the cable is connected on the device and
reboot. It only works for "lan2".

If you're talking about userspace on the device, I don't agree as there is nothing
loaded there but the kernel, and it is trying to mount the NFS root.

I have to admit that I don't really understand how the whole MT7530 DSA is wired
inside this device. I mostly did trial and error and it is very possible that what
I've got were just half results that are more confusing that they should be. The
vendor device tree uses the old bindings and drivers, with the gsw node outside
of the ethernet one and also a mtk-hnat_v1 node that seems to bundle all ports
under one single MAC address, so it's hard to understand from there how things work.
I was trying to both enable NFS root and squeeze the maximum speed from the switch,
so I thought I need to enable gmac1 and use the PHYs of the switch to enable 2Gbps
routing. When I've tried PHY4 things didn't work, while with PHY0 I've got all the
connections to work (but only if I've used LAN2 as the initial connection to the
NFS server).

Now I've tried removing the two nodes as suggested and I can mount the NFS root over
any of the three physical connections, but the iperf3 speed is only about 512Mbits/sec
vs the 730Mbits/sec that I was getting with the GMAC1 enabled.

Anyway, none of this is relevant for the upstreaming of the device tree and I will
make the requested changes. Thanks for spending time reviewing the patches.

Best regards,
Liviu

> 
> Arınç

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

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

[Arınç ÜNAL]

On 5.06.2023 00:30, Liviu Dudau wrote:
> On Sat, Jun 03, 2023 at 03:06:08PM +0300, Arınç ÜNAL wrote:
>> On 29.05.2023 18:55, Liviu Dudau wrote:
>>> On Mon, May 29, 2023 at 04:08:32PM +0100, Liviu Dudau wrote:
>>>> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
>>>> a MT7603 2.4GHz WiFi and a MT7663 5GHz WiFi chips integrated on the board,
>>
>> Do you mean MT7662 5GHz WiFi?
> 
> Actually it's MT76*1*3. Will fix for v3.
> 
>>
>>>> connected to the main SoC over PCIe.
>>>>
>>>> The GMAC1 on the SoC is connected to PHY0 on the GSW and can be used to
>>>> improve routing bandwidth.
>>
>> This is not always true, I'd prefer you remove this sentence from the patch
>> log.
> 
> If your comment is about the statement not being always true in general, then
> I agree. However for this device the port marked as WAN on HC220 v1 is the
> second port on the switch and according to the DSA bindings docs only PHY0
> or PHY4 can be connected to GMAC1, so I picked the one that worked (PHY4 did
> not). More on that at the end of the email.
> 
>>
>>>>
>>>> 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  | 129 ++++++++++++++++++
>>>>    2 files changed, 131 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..f003ae615a58e
>>>> --- /dev/null
>>>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>>> @@ -0,0 +1,129 @@
>>>> +// 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 = <0x0 0x0 0x0 0x8000000>;
>>
>> What's going on here? Just do 'reg = <0x00000000 0x08000000>;'.
>>
> 
> Right, hangover from 64bits world :) Will fix for v3.
> 
> 
>>>> +	};
>>>> +
>>>> +	chosen {
>>>> +		/* add 'earlycon=uart8260,mmio32,0x1e000c00' to
>>
>> 8260?
> 
> Duh, I feel stupid. It's 8250 and (as you mention bellow) not needed to
> enable early boot messages.
> 
>>
>>>> +		 * bootargs for early boot messages
>>
>> Isn't just adding "earlycon" to bootargs enough?
>>
>>>> +		 */
>>>> +		bootargs = "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 {
>>>> +			color = <LED_COLOR_ID_RED>;
>>>> +			function = LED_FUNCTION_FAULT;
>>>> +			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>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	resetc: reset-controller {
>>>> +		compatible = "ralink,rt2880-reset";
>>>> +		#reset-cells = <1>;
>>>> +	};
>>
>> We don't use this anymore.
>>
> 
> I've copied it from disassembled vendor device tree. Is this not needed
> anymore for reset?

No, you can just remove this node.

https://github.com/torvalds/linux/commit/64b2d6ffff862c0e7278198b4229e42e1abb3bb1

> 
>>>> +
>>>> +	mtd {
>>>> +		compatible = "mediatek,mt7622-nfc";
>>>> +	};
>>
>> What's this got to do with this device?
> 
> This is the NAND Flash Controller bindind. Left over from interrupted
> development of the NAND support. Will remove for v3.
> 
>>
>>>> +};
>>>> +
>>>> +&i2c {
>>>> +	status = "okay";
>>>> +};
>>
>> Why does this device need i2c?
> 
> It doesn't. I was curious if there was anything hanging on that bus so I've
> enabled it to scan it. Will remove for v3.
> 
> 
>>
>>>> +
>>>> +&pcie {
>>>> +	status = "okay";
>>>> +};
>>
>> Do both WiFi chips work by just enabling pcie? I was expecting 'compatible =
>> "mediatek,mt76";' on pcie@0,0 and pcie@1,0.
> 
> Nothing else is needed and I can modprobe the mt7603e and mt7615e modules just
> fine, with the devices becoming available one I put the appropriate firmware in
> the right place.

Ok I checked the mainline Linux source code, looks like the MT76 driver 
doesn't look for the compatible string at all. I suppose the compatible 
string is there for the purposes of describing the hardware and checking 
whether the devicetree bindings for the hardware are proper. Therefore, 
I would like you to add the compatible string regardless. Should look 
something like this:

&pcie {
	status = "okay";

	pcie@0,0 {
		compatible = "mediatek,mt76";
	};

	pcie@1,0 {
		compatible = "mediatek,mt76";
	};
};

See if both radios work with these bindings.

> 
>>
>>>> +
>>>> +&spi0 {
>>>> +	status = "okay";
>>>> +
>>>> +	flash@0 {
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <1>;
>>>> +		compatible = "jedec,spi-nor";
>>>> +		reg = <0>;
>>>> +		spi-max-frequency = <50000000>;
>>>> +	};
>>>> +};
>>
>> I thought you said this device had NAND flash, not NOR.
> 
> Appologies, I will remove this for now as I haven't finished looking into
> storage problem yet.
> 
>>
>>>> +
>>>> +/* gmac1 connected to MT7530's phy0 */
>>>> +&gmac1 {
>>>> +	phy-handle = <&ethphy0>;
>>>> +
>>>> +	fixed-link {
>>>> +		status = "disabled";
>>>> +	};
>>>> +};
>>>> +
>>>> +&mdio {
>>>> +	/* MT7530's phy0 */
>>>> +	ethphy0: ethernet-phy@0 {
>>>> +		reg = <0>;
>>>> +	};
>>>> +};
>>
>> Remove the two nodes above.
> 
> It would be nice if you can provide some explanation to some of your requests
> so that I can learn more about this. This is my first time dipping my toes
> on MIPS and MT7621 world, so I don't have that much experience and would like
> to know more.

The bindings above are used for the PHY muxing feature. Like I said on 
my previous mail, I would rather use the changing DSA conduit feature 
instead which makes the PHY muxing feature redundant. It is also better 
than PHY muxing as you'll be able to choose freely which DSA user port 
will be affine to which DSA conduit interface, from userspace. Like so:

ip link set lan1 type dsa conduit eth1

I see port@0 is defined on the DSA switch node which averts the PHY 
muxing feature so the bindings above are already unnecessary.

> 
>>
>>>> +
>>>> +&switch0 {
>>>> +	ports {
>>>> +		/* phy0 is muxed to gmac1 */
>>>> +		port@0 {
>>>> +			status = "okay";
>>>> +			label = "lan2";
>>>> +		};
>>>
>>> I've made the changes to look similar to the gnubee-gb-pc2, and things mostly
>>> work, with the exception that I can mount an NFS root filesystem only on "lan2"
>>> interface at boot time. All other interfaces (ports) hang forever waiting for
>>> an DHCP response from my server. The only difference is where I plug in the
>>> ethernet cable, no other change (not even a restart) on the server.
>>
>> This sounds like a userspace configuration issue.
> 
> Which userspace? On the server side? I don't touch anything there, only
> change the ethernet port where the cable is connected on the device and
> reboot. It only works for "lan2".
> 
> If you're talking about userspace on the device, I don't agree as there is nothing
> loaded there but the kernel, and it is trying to mount the NFS root.
> 
> I have to admit that I don't really understand how the whole MT7530 DSA is wired
> inside this device. I mostly did trial and error and it is very possible that what
> I've got were just half results that are more confusing that they should be. The
> vendor device tree uses the old bindings and drivers, with the gsw node outside
> of the ethernet one and also a mtk-hnat_v1 node that seems to bundle all ports
> under one single MAC address, so it's hard to understand from there how things work.
> I was trying to both enable NFS root and squeeze the maximum speed from the switch,
> so I thought I need to enable gmac1 and use the PHYs of the switch to enable 2Gbps
> routing. When I've tried PHY4 things didn't work, while with PHY0 I've got all the
> connections to work (but only if I've used LAN2 as the initial connection to the
> NFS server).
> 
> Now I've tried removing the two nodes as suggested and I can mount the NFS root over
> any of the three physical connections, but the iperf3 speed is only about 512Mbits/sec
> vs the 730Mbits/sec that I was getting with the GMAC1 enabled.

GMAC1 is enabled by default on mt7621.dtsi so your 512 Mbps test also 
had GMAC1 enabled, your problem is not there. What I did notice a while 
back with my MT7621 board is that I get better performance on the port 
that uses the PHY muxing feature. I assume this is what's causing the 
speed difference on your device.

Anyway, this SoC is not very efficient at processing packets at 
software. You'll get ~933 Mbps bandwidth whether you do PHY muxing or 
not when using the hardware flow offloading feature of the MediaTek 
ethernet driver. Here's the relevant documentation:

https://www.kernel.org/doc/html/latest/networking/nf_flowtable.html#hardware-offload

Arınç