Getting back full led control on e7350

Any hints on getting back full led control on linksys e7350 ?
The switch lights are disturbing during the night.
I've noticed @arinc9 committed a few month ago a change in the dts removing switch led control https://github.com/openwrt/openwrt/commit/fc33c41c21362b7186aa051a2140623943fa3143

1 Like

Just found this:

Please be aware that you're messing with the switch register values manually...

2 Likes

Thanks but it doesn't seem to do anything on e7350 :frowning:

Yes those numbers have to be changed for the E7350.

Stock firmware controls the LEDs from user space which is nogo for OpenWrt. Need a patch for mt7621’s Ethernet driver to gain support for this. Earliest that will happen is with kernel 6.1.

1 Like

Any idea what are the numbers for e7350?

I’ll check the stock firmware later.

1 Like

ok so I did the checking of the ofw and unfortunately they only have control for wan,wps and power - same as openwrt via gpio 13 and 14

#!/bin/sh

wps_led_blink()
{
	i=1
	while [ $i -le 30 ]
	do
        	echo 0 >/sys/class/gpio/gpio13/value
        	sleep 1
        	echo 1 >/sys/class/gpio/gpio13/value
        	sleep 1
        	let i++
	done
        power_led_on
}

wps_led_off()
{
	echo 1 >/sys/class/gpio/gpio13/value
}

wps_led_on()
{
	echo 0 >/sys/class/gpio/gpio13/value
}

power_led_blink()
{
	while [ "1"="1" ]
	do
        	echo 0 >/sys/class/gpio/gpio14/value
        	sleep 1
        	echo 1 >/sys/class/gpio/gpio14/value
        	sleep 1
	done
}

power_led_off()
{
	echo 1 >/sys/class/gpio/gpio14/value
}

power_led_on()
{
	echo 0 >/sys/class/gpio/gpio14/value
}

all_led_off()
{
	echo 1 >/sys/class/gpio/gpio13/value
	echo 1 >/sys/class/gpio/gpio14/value
}

all_led_on()
{
	echo 0 >/sys/class/gpio/gpio13/value
	echo 0 >/sys/class/gpio/gpio14/value
}

power_wps_led_init()
{
	#13:yellow 14:blue
	echo 13 >/sys/class/gpio/export
	echo out >/sys/class/gpio/gpio13/direction
	echo 14 >/sys/class/gpio/export
	echo out >/sys/class/gpio/gpio14/direction
	all_led_off
}

power_wps_led_init

case "$1" in
	"power_led_off")
		power_led_off
		;;
	"power_led_on")
		power_led_on
		;;
	"power_led_blink")
		power_led_blink
		;;
	"wps_led_blink")
		wps_led_blink
		;;
	"wps_led_on")
		wps_led_on
		;;
	"wps_led_off")
		wps_led_off
		;;
	"all_led_off")
		all_led_off
		;;
	"all_led_on")
		all_led_on
		;;
	*)
		;;
esac

LE: it seems there is some sort of control - maybe the values we're after - maybe @neheb can help:

switch_port0_led_recovery()
{
	switch reg w 7d00 77777
        switch reg w 7d04 77777
        switch reg w 7d10 77777
        switch reg w 7d14 0
        switch reg w 7d18 76777
}


switch_port0_led_off()
{
	switch reg w 7d00 77777
	switch reg w 7d04 77776
	switch reg w 7d10 77777
	switch reg w 7d14 1
	switch reg w 7d18 76777
}

switch_port0_led_on()
{
	switch reg w 7d00 77777
	switch reg w 7d04 77776
	switch reg w 7d10 77777
	switch reg w 7d14 1
	switch reg w 7d18 76776
}

I'm not interested in chasing some magic numbers. I also don't see how mdio-tools can be used to write 32-bit values to the switch registers. So I used debugfs to control enabling and disabling the switch LEDs from the userspace. This will work for all devices using the MT7530 switch.

# Check the state
cat /sys/kernel/debug/mt7530/led_on

# Disable the switch LEDs
echo 0 > /sys/kernel/debug/mt7530/led_on

# Enable the switch LEDs
echo 1 > /sys/kernel/debug/mt7530/led_on

I based this patch on kernel 5.10.176 which is what OpenWrt 22.03.5 uses.

diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 4a943ccc2ca4..04f7528fa3f4 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -6,7 +6,7 @@ ifdef CONFIG_NET_DSA_LOOP
 obj-$(CONFIG_FIXED_PHY)		+= dsa_loop_bdinfo.o
 endif
 obj-$(CONFIG_NET_DSA_LANTIQ_GSWIP) += lantiq_gswip.o
-obj-$(CONFIG_NET_DSA_MT7530)	+= mt7530.o
+obj-$(CONFIG_NET_DSA_MT7530)	+= mt7530.o mt7530-debugfs.o
 obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
 obj-$(CONFIG_NET_DSA_QCA8K)	+= qca8k.o
 obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o
diff --git a/drivers/net/dsa/mt7530-debugfs.c b/drivers/net/dsa/mt7530-debugfs.c
new file mode 100644
index 000000000000..aa1f20dda100
--- /dev/null
+++ b/drivers/net/dsa/mt7530-debugfs.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/debugfs.h>
+#include <linux/kernel.h>
+#include <net/dsa.h>
+
+#include "mt7530.h"
+
+static ssize_t led_on_read(struct file *filp, char __user *buffer,
+			   size_t count, loff_t *ppos)
+{
+	struct mt7530_priv *priv = filp->private_data;
+	u32 value = mt7530_read(priv, MT7530_LED_EN);
+	char result[2];
+
+	if (value)
+		result[0] = '1';
+	else
+		result[0] = '0';
+	result[1] = '\n';
+
+	return simple_read_from_buffer(buffer, count, ppos, result,
+				       sizeof(result));
+}
+
+static ssize_t led_on_write(struct file *filp, const char __user *buffer,
+			    size_t count, loff_t *ppos)
+{
+	struct mt7530_priv *priv = filp->private_data;
+
+	if (buffer[0] == '1')
+		mt7530_write(priv, MT7530_LED_EN, 0x77777);
+	else
+		mt7530_write(priv, MT7530_LED_EN, 0);
+
+	return count;
+}
+
+static const struct file_operations led_on_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = led_on_read,
+	.write = led_on_write
+};
+
+void mt7530_debugfs_init(struct mt7530_priv *priv)
+{
+	struct dentry *dir;
+
+	dir = debugfs_lookup("mt7530", 0);
+	if (dir == NULL)
+		dir = debugfs_create_dir("mt7530", 0);
+
+	debugfs_create_file("led_on", 0644, dir, priv, &led_on_fops);
+}
+
+MODULE_AUTHOR("Arınç ÜNAL");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 70155e996f7d..c5d16de386f9 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -222,7 +222,7 @@ mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
 	return (hi << 16) | (lo & 0xffff);
 }
 
-static void
+void
 mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
 {
 	struct mii_bus *bus = priv->bus;
@@ -255,7 +255,7 @@ _mt7530_read(struct mt7530_dummy_poll *p)
 	return val;
 }
 
-static u32
+u32
 mt7530_read(struct mt7530_priv *priv, u32 reg)
 {
 	struct mt7530_dummy_poll p;
@@ -1551,6 +1551,8 @@ mt7530_setup(struct dsa_switch *ds)
 	u32 id, val;
 	int ret, i;
 
+	mt7530_debugfs_init(priv);
+
 	/* The parent node of master netdev which holds the common system
 	 * controller also is the container for two GMACs nodes representing
 	 * as two netdev instances.
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 9278a8e3d04e..7e88e8fdc878 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -772,4 +772,8 @@ static inline void INIT_MT7530_DUMMY_POLL(struct mt7530_dummy_poll *p,
 	p->reg = reg;
 }
 
+void mt7530_debugfs_init(struct mt7530_priv *priv);
+u32 mt7530_read(struct mt7530_priv *priv, u32 reg);
+void mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val);
+
 #endif /* __MT7530_H */

Adding below to the devicetree source file would achieve the same result only without being able to enable the LEDs back easily.

gpio-controller;
#gpio-cells = <2>;
2 Likes

@arinc9 true but a patch like that will probably never be merged. I think he's looking for a solution that works with stock firmware.

1 Like

@maurer the linked mdio command is for mt7531, not mt7530. So the values are probably different. I have no idea where 0x21 0x000b comes from.

Thanks. What about the values I got from linksys fw?
Thank you very much @arinc9 I'll keep your solution as a last resort as indeed I would like a solution I can use on top of official openwrt builds

There's a lot to explain here. Let's start with this:

switch reg w 7d00 77777:

  • 0x7d00 is the register address which is defined with the MT7530_LED_EN constant on the MT7530 DSA subdriver. My patch does what the Linksys firmware does, only in a simpler fashion.
  • 0x77777 is the value that enables the input/output function of all LEDs. Consult page 30 of MT7621 Giga Switch Programming Guide v0.3.

Keep in mind switch registers and ethernet PHY registers are different. mdio-tools is used to control the PHY registers, and the switch registers of certain switches.

My testing shows that they're not, for the standalone MT7530 switch.

What I found is that the PHYs of the MT7530 switch on the MT7621 SoC does not have the registers for LED functions.

It can be seen below that the first 32 registers of the PHY are accessed with the register addresses after 0x1f.

mdio mt7530-0 phy 0 raw 0x0 (up to 0x1f)
mdio mt7530-0 mmd 0:0x1f raw 0x20 (up to 0x3f)

MT7621's MT7530 PHY0:
0x0: 0x1040
0x1: 0x796d
0x2: 0x03a2
0x3: 0x9412
0x4: 0x05e1
0x5: 0xc1e1
0x6: 0x000d
0x7: 0x2001
0x8: 0x6801
0x9: 0x0600
0xa: 0x7800
---
0x20: 0x1040
0x21: 0x796d
0x22: 0x03a2
0x23: 0x9412
0x24: 0x05e1
0x25: 0xc1e1
0x26: 0x000d
0x27: 0x2001
0x28: 0x6801
0x29: 0x0600
0x2a: 0x7800

This is not the case on MT7531 and the standalone MT7530 switch, therefore the command mdio mt7530-0 mmd 0:0x1f raw 0x21 0xb works for them.

Looking at page 805 of MT7531 Reference Manual for Development Board v1.0, 0x21 is the register address of "LED Basic Control Register". The 0xb value makes it so that the LED outputs become disabled while keeping the LED clock enabled.

The conclusion is MT7621's MT7530 PHYs don't have LED registers, therefore the LEDs can only be disabled by controlling the relevant switch registers.

mdio-tools cannot control the switch registers of MT7530. It looks like out of the ordinary effort is needed to support controlling the registers of certain switches. Don't know, don't care. I don't know of any other tool that can be used to control switch registers. My patch can be used for your personal use case, and that's about it.

I haven't seen any drivers that provide an interface to the userspace to control LED pins out of the generic purpose input output (GPIO) way, and don't think it would make sense to do so.

1 Like

thank you very much for the detailed post
what about future evolution like the one @neheb mentioned for 6.1 - is there any chance led control can be enabled ?

Firstly, implementing "controlling switch LED from userspace" on the MediaTek ethernet driver would make no sense as that driver doesn't control the switch.

Second, my last paragraph from my previous reply was in response to that. I would say, if a pin does not function as general purpose input/output, it's got no business being controlled from userspace.

1 Like

Clear. Thanks

I meant the mt7530 DSA driver. My bad. @Ansuel ’s LED patches for DSA are upstream and currently implemented in qca8k. The mt7530 driver needs a patch to use it.

@robimarko wait ipq40xx switch still supports leds from PHY? i tought that wasn't a thing anymore...

I am failing to see how ipq40xx is related here?

And no, there is no LED support on the built-in switch as there are no PHY-s built in nor are there pins on the SoC for them

(facepalm) i read 7530 and tought it was a fritzbox....

1 Like