Skip to contentSkip to conversation

wifi: rtl8xxxu: fix LED control code of RTL8192FU

PATCH Superseded Zenm Chen 3 participants 5 messages Sep 10, 2023 – Sep 12, 2023 RTL8XXXU wireless driver (rtl8xxxu)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Zenm Chen

author
#0 lore raw

PATCH 1/1 wifi: rtl8xxxu: fix LED control code of RTL8192FU

ASUS USB-N13 C1 is a wifi adapter based on a RTL8192FU chip and its LED doesn't blink with the rtl8xxxu driver. This patch fix this via writing the correct value to the register "REG_LEDCFG0".

This was tested with these two wifi adapters:
ASUS USB-N13 C1 (vid=0x0b05, pid=0x18f1, rfe_type=0x1)
MERCURY MW310UH (vid=0x0bda, pid=0xf192, rfe_type=0x5)

---
 .../realtek/rtl8xxxu/rtl8xxxu_8192f.c         | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +10 -14
index 28e93835e05a..ab6d4e4e228e 100644
@@ -2014,26 +2014,22 @@ static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev,
 	struct rtl8xxxu_priv *priv = container_of(led_cdev,
 						  struct rtl8xxxu_priv,
 						  led_cdev);
-	u16 ledcfg;
+	u32 ledcfg;
 
 	/* Values obtained by observing the USB traffic from the Windows driver. */
 	rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_0, 0x20080);
 	rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_1, 0x1b0000);
 
-	ledcfg = rtl8xxxu_read16(priv, REG_LEDCFG0);
-
-	if (brightness == LED_OFF) {
-		/* Value obtained like above. */
-		ledcfg = BIT(1) | BIT(7);
-	} else if (brightness == LED_ON) {
-		/* Value obtained like above. */
-		ledcfg = BIT(1) | BIT(7) | BIT(11);
-	} else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
-		/* Value obtained by brute force. */
-		ledcfg = BIT(8) | BIT(9);
-	}
+	/* Enable LED0 GPIO and turn LED0/LED1 off. */
+	ledcfg = (rtl8xxxu_read32(priv, REG_LEDCFG0) | BIT(21)) & ~0xffff;
+
+	/* Values obtained by brute force. */
+	if (brightness == LED_ON)
+		ledcfg |= BIT(3) | BIT(11);
+	else if (brightness == RTL8XXXU_HW_LED_CONTROL)
+		ledcfg |= BIT(0) | BIT(1) | BIT(8) | BIT(9);
 
-	rtl8xxxu_write16(priv, REG_LEDCFG0, ledcfg);
+	rtl8xxxu_write32(priv, REG_LEDCFG0, ledcfg);
 
 	return 0;
 }
Signature · 1 line
2.42.0
#1 lore raw
Quoted 10 lines

-----Original Message-----
From: Zenm Chen <[email protected]>
Sent: Sunday, September 10, 2023 8:21 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Ping-Ke Shih <[email protected]>; Zenm Chen <[email protected]>
Subject: [PATCH] wifi: rtl8xxxu: fix LED control code of RTL8192FU

Quoted 7 lines

ASUS USB-N13 C1 is a wifi adapter based on a RTL8192FU chip and its LED doesn't blink with the rtl8xxxu driver. This patch fix this via writing the correct value to the register "REG_LEDCFG0".

This was tested with these two wifi adapters:
ASUS USB-N13 C1 (vid=0x0b05, pid=0x18f1, rfe_type=0x1) MERCURY MW310UH (vid=0x0bda, pid=0xf192, rfe_type=0x5)

Did you ever try Linux vendor driver for software and hardware LED? Seemingly, the vendor driver is different from yours, but I'm pretty sure which one is correct because I don't have a RTL8192FU stick.

Quoted 24 lines
---
 .../realtek/rtl8xxxu/rtl8xxxu_8192f.c         | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +1 -3
index 28e93835e05a..ab6d4e4e228e 100644
@@ -2014,26 +2014,22 @@ static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev,
        struct rtl8xxxu_priv *priv = container_of(led_cdev,
                                                  struct rtl8xxxu_priv,
                                                  led_cdev);
-       u16 ledcfg;
+       u32 ledcfg;
 
        /* Values obtained by observing the USB traffic from the Windows driver. */
        rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_0, 0x20080);
        rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_1, 0x1b0000);
 
-       ledcfg = rtl8xxxu_read16(priv, REG_LEDCFG0);
-

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2034

Quoted 10 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +0 -10
-       if (brightness == LED_OFF) {
-               /* Value obtained like above. */
-               ledcfg = BIT(1) | BIT(7);
-       } else if (brightness == LED_ON) {
-               /* Value obtained like above. */
-               ledcfg = BIT(1) | BIT(7) | BIT(11);
-       } else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
-               /* Value obtained by brute force. */
-               ledcfg = BIT(8) | BIT(9);
-       }

The definitions are used by this driver:

#define BIT_LED2EN BIT(21)
#define BIT_LED1SV BIT(11) // the same definition with BIT_LED0SV #define BIT_LED1CM GENMASK(10, 8) // use the same value of BIT_LED0CM #define BIT_LED0SV BIT(3)
#define BIT_LED0CM GENMASK(2, 0)
#define LED_MODE_TX_OR_RX_EVENTS 3
#define LED_MODE_SW_CTRL 0

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2024

Quoted 2 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +2 -0
+       /* Enable LED0 GPIO and turn LED0/LED1 off. */
+       ledcfg = (rtl8xxxu_read32(priv, REG_LEDCFG0) | BIT(21)) & ~0xffff;

Split this statement into multiple lines and use definitions I provide above.

Quoted 14 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +7 -2
+
+       /* Values obtained by brute force. */
+       if (brightness == LED_ON)
+               ledcfg |= BIT(3) | BIT(11);
+       else if (brightness == RTL8XXXU_HW_LED_CONTROL)
+               ledcfg |= BIT(0) | BIT(1) | BIT(8) | BIT(9);
 
-       rtl8xxxu_write16(priv, REG_LEDCFG0, ledcfg);
+       rtl8xxxu_write32(priv, REG_LEDCFG0, ledcfg);
 
        return 0;
 }
--

2.42.0

author
#2 lore raw
In reply to Zenm Chen

Bitterblue Smith <[email protected]> wrote:

Quoted 5 lines

Can you explain in the commit message why you changed the LED_ON and LED_OFF branches? It's not obvious to me and they don't have anything to do with the hardware-controlled blinking.

Hi,

Sorry that I didn't explain this clearly.
After some researches these days, I came to some conclusions:

1. The LED of MERCURY MW310UH and COMFAST CF-826F seems to be controlled by REG_LEDCFG1, and currently rtl8xxxu controls the LED through writing some suitable values to this register, so MW310UH and CF-826F work fine with rtl8xxxu.

2. The LED of ASUS USB-N13 C1 seems to be controlled by another register "REG_LEDCFG0" for unknown reason, so when I write 1 or 2 to /sys/class/leds/rtl8xxxu-usbX-Y/brightness, the LED doesn't turn on or blink because the register "REG_LEDCFG0" is not filled with a suitable value, This is why I changed the LED_ON and LED_OFF branches.

I will modify the commit message in the next version of patch, thanks.

#3 lore raw
In reply to Zenm Chen
Quoted 36 lines · Zenm Chen

ASUS USB-N13 C1 is a wifi adapter based on a RTL8192FU chip and its LED doesn't blink with the rtl8xxxu driver. This patch fix this via writing the correct value to the register "REG_LEDCFG0".

This was tested with these two wifi adapters:
ASUS USB-N13 C1 (vid=0x0b05, pid=0x18f1, rfe_type=0x1)
MERCURY MW310UH (vid=0x0bda, pid=0xf192, rfe_type=0x5)

---
 .../realtek/rtl8xxxu/rtl8xxxu_8192f.c         | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +1 -9
index 28e93835e05a..ab6d4e4e228e 100644
@@ -2014,26 +2014,22 @@ static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev,
 	struct rtl8xxxu_priv *priv = container_of(led_cdev,
 						  struct rtl8xxxu_priv,
 						  led_cdev);
-	u16 ledcfg;
+	u32 ledcfg;
 
 	/* Values obtained by observing the USB traffic from the Windows driver. */
 	rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_0, 0x20080);
 	rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_1, 0x1b0000);
 
-	ledcfg = rtl8xxxu_read16(priv, REG_LEDCFG0);
-
-	if (brightness == LED_OFF) {
-		/* Value obtained like above. */
-		ledcfg = BIT(1) | BIT(7);
-	} else if (brightness == LED_ON) {
-		/* Value obtained like above. */
-		ledcfg = BIT(1) | BIT(7) | BIT(11);

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2028

Quoted 10 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +6 -4
-	} else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
-		/* Value obtained by brute force. */
-		ledcfg = BIT(8) | BIT(9);
-	}
+	/* Enable LED0 GPIO and turn LED0/LED1 off. */
+	ledcfg = (rtl8xxxu_read32(priv, REG_LEDCFG0) | BIT(21)) & ~0xffff;
+
+	/* Values obtained by brute force. */
+	if (brightness == LED_ON)
+		ledcfg |= BIT(3) | BIT(11);

Can you explain in the commit message why you changed the LED_ON and LED_OFF branches? It's not obvious to me and they don't have anything to do with the hardware-controlled blinking.

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2035

Quoted 8 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +3 -1
+	else if (brightness == RTL8XXXU_HW_LED_CONTROL)
+		ledcfg |= BIT(0) | BIT(1) | BIT(8) | BIT(9);
 
-	rtl8xxxu_write16(priv, REG_LEDCFG0, ledcfg);
+	rtl8xxxu_write32(priv, REG_LEDCFG0, ledcfg);
 
 	return 0;
 }

My Comfast CF-826F still works fine with this patch.

author
#4 lore raw
In reply to Zenm Chen

Ping-Ke Shih <[email protected]> wrote:

Quoted 69 lines
Quoted 11 lines

-----Original Message-----
From: Zenm Chen <[email protected]>
Sent: Sunday, September 10, 2023 8:21 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Ping-Ke Shih <[email protected]>; Zenm Chen <[email protected]>
Subject: [PATCH] wifi: rtl8xxxu: fix LED control code of RTL8192FU

Quoted 7 lines

ASUS USB-N13 C1 is a wifi adapter based on a RTL8192FU chip and its LED doesn't blink with the rtl8xxxu driver. This patch fix this via writing the correct value to the register "REG_LEDCFG0".

This was tested with these two wifi adapters:
ASUS USB-N13 C1 (vid=0x0b05, pid=0x18f1, rfe_type=0x1) MERCURY MW310UH (vid=0x0bda, pid=0xf192, rfe_type=0x5)

Did you ever try Linux vendor driver for software and hardware LED? Seemingly, the vendor driver is different from yours, but I'm pretty sure which one is correct because I don't have a RTL8192FU stick.

Quoted 24 lines
---
 .../realtek/rtl8xxxu/rtl8xxxu_8192f.c         | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +1 -3
index 28e93835e05a..ab6d4e4e228e 100644
@@ -2014,26 +2014,22 @@ static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev,
        struct rtl8xxxu_priv *priv = container_of(led_cdev,
                                                  struct rtl8xxxu_priv,
                                                  led_cdev);
-       u16 ledcfg;
+       u32 ledcfg;
 
        /* Values obtained by observing the USB traffic from the Windows driver. */
        rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_0, 0x20080);
        rtl8xxxu_write32(priv, REG_SW_GPIO_SHARE_CTRL_1, 0x1b0000);
 
-       ledcfg = rtl8xxxu_read16(priv, REG_LEDCFG0);
-

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2034

Quoted 10 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +0 -10
-       if (brightness == LED_OFF) {
-               /* Value obtained like above. */
-               ledcfg = BIT(1) | BIT(7);
-       } else if (brightness == LED_ON) {
-               /* Value obtained like above. */
-               ledcfg = BIT(1) | BIT(7) | BIT(11);
-       } else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
-               /* Value obtained by brute force. */
-               ledcfg = BIT(8) | BIT(9);
-       }

The definitions are used by this driver:

#define BIT_LED2EN BIT(21)
#define BIT_LED1SV BIT(11) // the same definition with BIT_LED0SV #define BIT_LED1CM GENMASK(10, 8) // use the same value of BIT_LED0CM #define BIT_LED0SV BIT(3)
#define BIT_LED0CM GENMASK(2, 0)
#define LED_MODE_TX_OR_RX_EVENTS 3
#define LED_MODE_SW_CTRL 0

Quoted 20 lines

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c static int rtl8192fu_led_brightness_set(struct led_classdev *led_cdev, · line 2024

Quoted 2 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +2 -0
+       /* Enable LED0 GPIO and turn LED0/LED1 off. */
+       ledcfg = (rtl8xxxu_read32(priv, REG_LEDCFG0) | BIT(21)) & ~0xffff;

Split this statement into multiple lines and use definitions I provide above.

Quoted 14 lines
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192f.c +7 -2
+
+       /* Values obtained by brute force. */
+       if (brightness == LED_ON)
+               ledcfg |= BIT(3) | BIT(11);
+       else if (brightness == RTL8XXXU_HW_LED_CONTROL)
+               ledcfg |= BIT(0) | BIT(1) | BIT(8) | BIT(9);
 
-       rtl8xxxu_write16(priv, REG_LEDCFG0, ledcfg);
+       rtl8xxxu_write32(priv, REG_LEDCFG0, ledcfg);
 
        return 0;
 }
--

2.42.0

Thanks for the help and suggestions, I will make these changes. :)