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

From: Arınç ÜNAL
Date: Mon Jun 05 2023 - 03:29:56 EST

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@xxxxxxxxxxx>
---
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ç