* [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU
2023-01-17 22:02 [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Bitterblue Smith
@ 2023-01-17 22:04 ` Bitterblue Smith
2023-01-18 1:10 ` Ping-Ke Shih
2023-01-17 22:05 ` [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU Bitterblue Smith
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Bitterblue Smith @ 2023-01-17 22:04 UTC (permalink / raw)
To: linux-wireless; +Cc: Jes Sorensen, Ping-Ke Shih
By default the LED will blink when there is some activity.
This was tested with a TP-Link TL-WN725N.
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
.../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 25 +++++++++++++++++++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 4 +++
2 files changed, 29 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index 08f3b93ad8d0..a99ddb41cd24 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1350,6 +1350,30 @@ static s8 rtl8188e_cck_rssi(struct rtl8xxxu_priv *priv, u8 cck_agc_rpt)
return rx_pwr_all;
}
+static int rtl8188eu_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct rtl8xxxu_priv *priv = container_of(led_cdev,
+ struct rtl8xxxu_priv,
+ led_cdev);
+ u8 ledcfg = rtl8xxxu_read8(priv, REG_LEDCFG2);
+
+ if (brightness == LED_OFF) {
+ ledcfg &= ~LEDCFG2_HW_LED_CONTROL;
+ ledcfg |= LEDCFG2_SW_LED_CONTROL | LEDCFG2_SW_LED_DISABLE;
+ } else if (brightness == LED_ON) {
+ ledcfg &= ~(LEDCFG2_HW_LED_CONTROL | LEDCFG2_SW_LED_DISABLE);
+ ledcfg |= LEDCFG2_SW_LED_CONTROL;
+ } else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
+ ledcfg &= ~LEDCFG2_SW_LED_DISABLE;
+ ledcfg |= LEDCFG2_HW_LED_CONTROL | LEDCFG2_HW_LED_ENABLE;
+ }
+
+ rtl8xxxu_write8(priv, REG_LEDCFG2, ledcfg);
+
+ return 0;
+}
+
static void rtl8188e_set_tx_rpt_timing(struct rtl8xxxu_ra_info *ra, u8 timing)
{
u8 idx;
@@ -1851,6 +1875,7 @@ struct rtl8xxxu_fileops rtl8188eu_fops = {
.fill_txdesc = rtl8xxxu_fill_txdesc_v3,
.set_crystal_cap = rtl8188f_set_crystal_cap,
.cck_rssi = rtl8188e_cck_rssi,
+ .led_classdev_brightness_set = rtl8188eu_led_brightness_set,
.writeN_block_size = 128,
.rx_desc_size = sizeof(struct rtl8xxxu_rxdesc16),
.tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
index 5818b2378bab..d510ce27b1b4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
@@ -148,6 +148,10 @@
#define LEDCFG0_DPDT_SELECT BIT(23)
#define REG_LEDCFG1 0x004d
#define REG_LEDCFG2 0x004e
+#define LEDCFG2_HW_LED_CONTROL BIT(1)
+#define LEDCFG2_HW_LED_ENABLE BIT(5)
+#define LEDCFG2_SW_LED_DISABLE BIT(3)
+#define LEDCFG2_SW_LED_CONTROL BIT(5)
#define LEDCFG2_DPDT_SELECT BIT(7)
#define REG_LEDCFG3 0x004f
#define REG_LEDCFG REG_LEDCFG2
--
2.38.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU
2023-01-17 22:04 ` [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU Bitterblue Smith
@ 2023-01-18 1:10 ` Ping-Ke Shih
0 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2023-01-18 1:10 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless; +Cc: Jes Sorensen
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, January 18, 2023 6:04 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU
>
> By default the LED will blink when there is some activity.
>
> This was tested with a TP-Link TL-WN725N.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU
2023-01-17 22:02 [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Bitterblue Smith
2023-01-17 22:04 ` [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU Bitterblue Smith
@ 2023-01-17 22:05 ` Bitterblue Smith
2023-01-18 1:11 ` Ping-Ke Shih
2023-01-17 22:05 ` [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU Bitterblue Smith
2023-01-18 0:52 ` [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Ping-Ke Shih
3 siblings, 1 reply; 11+ messages in thread
From: Bitterblue Smith @ 2023-01-17 22:05 UTC (permalink / raw)
To: linux-wireless; +Cc: Jes Sorensen, Ping-Ke Shih
By default the LED will blink when there is some activity.
This was tested with a cheap "HT-WR813" from Aliexpress.
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
.../realtek/rtl8xxxu/rtl8xxxu_8192e.c | 24 +++++++++++++++++++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 ++
2 files changed, 26 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 4a1c9bcafe31..5cfc00237f42 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1764,6 +1764,29 @@ static s8 rtl8192e_cck_rssi(struct rtl8xxxu_priv *priv, u8 cck_agc_rpt)
return rx_pwr_all;
}
+static int rtl8192eu_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct rtl8xxxu_priv *priv = container_of(led_cdev,
+ struct rtl8xxxu_priv,
+ led_cdev);
+ u8 ledcfg = rtl8xxxu_read8(priv, REG_LEDCFG1);
+
+ if (brightness == LED_OFF) {
+ ledcfg &= ~LEDCFG1_HW_LED_CONTROL;
+ ledcfg |= LEDCFG1_LED_DISABLE;
+ } else if (brightness == LED_ON) {
+ ledcfg &= ~(LEDCFG1_HW_LED_CONTROL | LEDCFG1_LED_DISABLE);
+ } else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
+ ledcfg &= ~LEDCFG1_LED_DISABLE;
+ ledcfg |= LEDCFG1_HW_LED_CONTROL;
+ }
+
+ rtl8xxxu_write8(priv, REG_LEDCFG1, ledcfg);
+
+ return 0;
+}
+
struct rtl8xxxu_fileops rtl8192eu_fops = {
.identify_chip = rtl8192eu_identify_chip,
.parse_efuse = rtl8192eu_parse_efuse,
@@ -1788,6 +1811,7 @@ struct rtl8xxxu_fileops rtl8192eu_fops = {
.fill_txdesc = rtl8xxxu_fill_txdesc_v2,
.set_crystal_cap = rtl8723a_set_crystal_cap,
.cck_rssi = rtl8192e_cck_rssi,
+ .led_classdev_brightness_set = rtl8192eu_led_brightness_set,
.writeN_block_size = 128,
.tx_desc_size = sizeof(struct rtl8xxxu_txdesc40),
.rx_desc_size = sizeof(struct rtl8xxxu_rxdesc24),
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
index d510ce27b1b4..5849fa4e1566 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
@@ -147,6 +147,8 @@
#define REG_LEDCFG0 0x004c
#define LEDCFG0_DPDT_SELECT BIT(23)
#define REG_LEDCFG1 0x004d
+#define LEDCFG1_HW_LED_CONTROL BIT(1)
+#define LEDCFG1_LED_DISABLE BIT(7)
#define REG_LEDCFG2 0x004e
#define LEDCFG2_HW_LED_CONTROL BIT(1)
#define LEDCFG2_HW_LED_ENABLE BIT(5)
--
2.38.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU
2023-01-17 22:05 ` [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU Bitterblue Smith
@ 2023-01-18 1:11 ` Ping-Ke Shih
0 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2023-01-18 1:11 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless; +Cc: Jes Sorensen
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, January 18, 2023 6:05 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU
>
> By default the LED will blink when there is some activity.
>
> This was tested with a cheap "HT-WR813" from Aliexpress.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU
2023-01-17 22:02 [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Bitterblue Smith
2023-01-17 22:04 ` [PATCH 2/4] wifi: rtl8xxxu: Add LED control code for RTL8188EU Bitterblue Smith
2023-01-17 22:05 ` [PATCH 3/4] wifi: rtl8xxxu: Add LED control code for RTL8192EU Bitterblue Smith
@ 2023-01-17 22:05 ` Bitterblue Smith
2023-01-18 1:11 ` Ping-Ke Shih
2023-01-18 0:52 ` [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Ping-Ke Shih
3 siblings, 1 reply; 11+ messages in thread
From: Bitterblue Smith @ 2023-01-17 22:05 UTC (permalink / raw)
To: linux-wireless; +Cc: Jes Sorensen, Ping-Ke Shih
By default the LED will blink when there is some activity.
This is only compile tested.
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
.../realtek/rtl8xxxu/rtl8xxxu_8723a.c | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
index 5ed523db2d87..5e7b58d395ba 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
@@ -457,6 +457,30 @@ s8 rtl8723a_cck_rssi(struct rtl8xxxu_priv *priv, u8 cck_agc_rpt)
return rx_pwr_all;
}
+static int rtl8723au_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct rtl8xxxu_priv *priv = container_of(led_cdev,
+ struct rtl8xxxu_priv,
+ led_cdev);
+ u8 ledcfg = rtl8xxxu_read8(priv, REG_LEDCFG2);
+
+ if (brightness == LED_OFF) {
+ ledcfg &= ~LEDCFG2_HW_LED_CONTROL;
+ ledcfg |= LEDCFG2_SW_LED_CONTROL | LEDCFG2_SW_LED_DISABLE;
+ } else if (brightness == LED_ON) {
+ ledcfg &= ~(LEDCFG2_HW_LED_CONTROL | LEDCFG2_SW_LED_DISABLE);
+ ledcfg |= LEDCFG2_SW_LED_CONTROL;
+ } else if (brightness == RTL8XXXU_HW_LED_CONTROL) {
+ ledcfg &= ~LEDCFG2_SW_LED_DISABLE;
+ ledcfg |= LEDCFG2_HW_LED_CONTROL | LEDCFG2_HW_LED_ENABLE;
+ }
+
+ rtl8xxxu_write8(priv, REG_LEDCFG2, ledcfg);
+
+ return 0;
+}
+
struct rtl8xxxu_fileops rtl8723au_fops = {
.identify_chip = rtl8723au_identify_chip,
.parse_efuse = rtl8723au_parse_efuse,
@@ -482,6 +506,7 @@ struct rtl8xxxu_fileops rtl8723au_fops = {
.fill_txdesc = rtl8xxxu_fill_txdesc_v1,
.set_crystal_cap = rtl8723a_set_crystal_cap,
.cck_rssi = rtl8723a_cck_rssi,
+ .led_classdev_brightness_set = rtl8723au_led_brightness_set,
.writeN_block_size = 1024,
.rx_agg_buf_size = 16000,
.tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
--
2.38.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU
2023-01-17 22:05 ` [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU Bitterblue Smith
@ 2023-01-18 1:11 ` Ping-Ke Shih
0 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2023-01-18 1:11 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless; +Cc: Jes Sorensen
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, January 18, 2023 6:06 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU
>
> By default the LED will blink when there is some activity.
>
> This is only compile tested.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
2023-01-17 22:02 [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Bitterblue Smith
` (2 preceding siblings ...)
2023-01-17 22:05 ` [PATCH 4/4] wifi: rtl8xxxu: Add LED control code for RTL8723AU Bitterblue Smith
@ 2023-01-18 0:52 ` Ping-Ke Shih
2023-01-18 13:52 ` Bitterblue Smith
3 siblings, 1 reply; 11+ messages in thread
From: Ping-Ke Shih @ 2023-01-18 0:52 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless; +Cc: Jes Sorensen
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, January 18, 2023 6:03 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
>
> If the chip can have an LED, register a struct led_classdev and enable
> hardware-controlled blinking. When the chip is not transmitting or
> receiving anything the LED is off. Otherwise the LED will blink
> faster or slower according to the throughput.
>
> The LED can be controlled from userspace by writing 0, 1, or 2 to
> /sys/class/leds/rtl8xxxu-usbX-Y/brightness:
> 0 - solid off.
> 1 - solid on.
> 2 - hardware-controlled blinking.
So, do you like
#define RTL8XXXU_HW_LED_BLINKING 2
>
> In this patch none of the chips advertise having an LED. That will be
> added in the next patches.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +++++
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 34 +++++++++++++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 90268479d3ad..c8cee4a24755 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1443,6 +1443,8 @@ struct rtl8xxxu_cfo_tracking {
> u32 packet_count_pre;
> };
>
> +#define RTL8XXXU_HW_LED_CONTROL 2
> +
> struct rtl8xxxu_priv {
> struct ieee80211_hw *hw;
> struct usb_device *udev;
> @@ -1564,6 +1566,10 @@ struct rtl8xxxu_priv {
> struct rtl8xxxu_ra_report ra_report;
> struct rtl8xxxu_cfo_tracking cfo_tracking;
> struct rtl8xxxu_ra_info ra_info;
> +
> + bool led_registered;
> + char led_name[32];
> + struct led_classdev led_cdev;
> };
>
> struct rtl8xxxu_rx_urb {
> @@ -1613,6 +1619,8 @@ struct rtl8xxxu_fileops {
> u32 rts_rate);
> void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
> s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, u8 cck_agc_rpt);
> + int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
> + enum led_brightness brightness);
> int writeN_block_size;
> int rx_agg_buf_size;
> char tx_desc_size;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 35dc777c1fba..b27edd503c23 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6955,6 +6955,34 @@ static int rtl8xxxu_parse_usb(struct rtl8xxxu_priv *priv,
> return ret;
> }
>
> +static void rtl8xxxu_init_led(struct rtl8xxxu_priv *priv)
> +{
> + struct led_classdev *led = &priv->led_cdev;
> +
> + led->brightness_set_blocking = priv->fops->led_classdev_brightness_set;
> +
> + snprintf(priv->led_name, sizeof(priv->led_name),
> + "rtl8xxxu-usb%s", dev_name(&priv->udev->dev));
> + led->name = priv->led_name;
> + led->max_brightness = RTL8XXXU_HW_LED_CONTROL;
> +
> + if (led_classdev_register(&priv->udev->dev, led))
> + return;
> +
> + priv->led_registered = true;
> +
> + led->brightness = led->max_brightness;
> + priv->fops->led_classdev_brightness_set(led, led->brightness);
> +}
> +
> +static void rtl8xxxu_deinit_led(struct rtl8xxxu_priv *priv)
> +{
> + struct led_classdev *led = &priv->led_cdev;
> +
> + priv->fops->led_classdev_brightness_set(led, LED_OFF);
> + led_classdev_unregister(led);
> +}
> +
> static int rtl8xxxu_probe(struct usb_interface *interface,
> const struct usb_device_id *id)
> {
> @@ -7135,6 +7163,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> goto err_set_intfdata;
> }
>
> + if (priv->fops->led_classdev_brightness_set)
I prefer to move this checking statement into rtl8xxxu_init_led(). Then,
the flow of rtl8xxxu_probe() looks clean. As well as rtl8xxxu_deinit_led().
Just soft suggestion.
> + rtl8xxxu_init_led(priv);
> +
> return 0;
>
> err_set_intfdata:
> @@ -7159,6 +7190,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
> hw = usb_get_intfdata(interface);
> priv = hw->priv;
>
> + if (priv->led_registered)
> + rtl8xxxu_deinit_led(priv);
> +
> ieee80211_unregister_hw(hw);
>
> priv->fops->power_off(priv);
> --
> 2.38.0
>
> ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
2023-01-18 0:52 ` [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink Ping-Ke Shih
@ 2023-01-18 13:52 ` Bitterblue Smith
2023-01-19 0:25 ` Ping-Ke Shih
0 siblings, 1 reply; 11+ messages in thread
From: Bitterblue Smith @ 2023-01-18 13:52 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless; +Cc: Jes Sorensen
On 18/01/2023 02:52, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Wednesday, January 18, 2023 6:03 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
>> Subject: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
>>
>> If the chip can have an LED, register a struct led_classdev and enable
>> hardware-controlled blinking. When the chip is not transmitting or
>> receiving anything the LED is off. Otherwise the LED will blink
>> faster or slower according to the throughput.
>>
>> The LED can be controlled from userspace by writing 0, 1, or 2 to
>> /sys/class/leds/rtl8xxxu-usbX-Y/brightness:
>> 0 - solid off.
>> 1 - solid on.
>> 2 - hardware-controlled blinking.
>
> So, do you like
>
> #define RTL8XXXU_HW_LED_BLINKING 2
>
I'm not sure what you mean. Can you elaborate?
>>
>> In this patch none of the chips advertise having an LED. That will be
>> added in the next patches.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +++++
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 34 +++++++++++++++++++
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index 90268479d3ad..c8cee4a24755 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -1443,6 +1443,8 @@ struct rtl8xxxu_cfo_tracking {
>> u32 packet_count_pre;
>> };
>>
>> +#define RTL8XXXU_HW_LED_CONTROL 2
>> +
>> struct rtl8xxxu_priv {
>> struct ieee80211_hw *hw;
>> struct usb_device *udev;
>> @@ -1564,6 +1566,10 @@ struct rtl8xxxu_priv {
>> struct rtl8xxxu_ra_report ra_report;
>> struct rtl8xxxu_cfo_tracking cfo_tracking;
>> struct rtl8xxxu_ra_info ra_info;
>> +
>> + bool led_registered;
>> + char led_name[32];
>> + struct led_classdev led_cdev;
>> };
>>
>> struct rtl8xxxu_rx_urb {
>> @@ -1613,6 +1619,8 @@ struct rtl8xxxu_fileops {
>> u32 rts_rate);
>> void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
>> s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, u8 cck_agc_rpt);
>> + int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
>> + enum led_brightness brightness);
>> int writeN_block_size;
>> int rx_agg_buf_size;
>> char tx_desc_size;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 35dc777c1fba..b27edd503c23 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6955,6 +6955,34 @@ static int rtl8xxxu_parse_usb(struct rtl8xxxu_priv *priv,
>> return ret;
>> }
>>
>> +static void rtl8xxxu_init_led(struct rtl8xxxu_priv *priv)
>> +{
>> + struct led_classdev *led = &priv->led_cdev;
>> +
>> + led->brightness_set_blocking = priv->fops->led_classdev_brightness_set;
>> +
>> + snprintf(priv->led_name, sizeof(priv->led_name),
>> + "rtl8xxxu-usb%s", dev_name(&priv->udev->dev));
>> + led->name = priv->led_name;
>> + led->max_brightness = RTL8XXXU_HW_LED_CONTROL;
>> +
>> + if (led_classdev_register(&priv->udev->dev, led))
>> + return;
>> +
>> + priv->led_registered = true;
>> +
>> + led->brightness = led->max_brightness;
>> + priv->fops->led_classdev_brightness_set(led, led->brightness);
>> +}
>> +
>> +static void rtl8xxxu_deinit_led(struct rtl8xxxu_priv *priv)
>> +{
>> + struct led_classdev *led = &priv->led_cdev;
>> +
>> + priv->fops->led_classdev_brightness_set(led, LED_OFF);
>> + led_classdev_unregister(led);
>> +}
>> +
>> static int rtl8xxxu_probe(struct usb_interface *interface,
>> const struct usb_device_id *id)
>> {
>> @@ -7135,6 +7163,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>> goto err_set_intfdata;
>> }
>>
>> + if (priv->fops->led_classdev_brightness_set)
>
> I prefer to move this checking statement into rtl8xxxu_init_led(). Then,
> the flow of rtl8xxxu_probe() looks clean. As well as rtl8xxxu_deinit_led().
> Just soft suggestion.
>
Okay. That works too.
>> + rtl8xxxu_init_led(priv);
>> +
>> return 0;
>>
>> err_set_intfdata:
>> @@ -7159,6 +7190,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
>> hw = usb_get_intfdata(interface);
>> priv = hw->priv;
>>
>> + if (priv->led_registered)
>> + rtl8xxxu_deinit_led(priv);
>> +
>> ieee80211_unregister_hw(hw);
>>
>> priv->fops->power_off(priv);
>> --
>> 2.38.0
>>
>> ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
2023-01-18 13:52 ` Bitterblue Smith
@ 2023-01-19 0:25 ` Ping-Ke Shih
2023-01-19 16:08 ` Bitterblue Smith
0 siblings, 1 reply; 11+ messages in thread
From: Ping-Ke Shih @ 2023-01-19 0:25 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless; +Cc: Jes Sorensen
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, January 18, 2023 9:53 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>
> Subject: Re: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
>
> On 18/01/2023 02:52, Ping-Ke Shih wrote:
> >
> >
> >> -----Original Message-----
> >> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> >> Sent: Wednesday, January 18, 2023 6:03 AM
> >> To: linux-wireless@vger.kernel.org
> >> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> >> Subject: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
> >>
> >> If the chip can have an LED, register a struct led_classdev and enable
> >> hardware-controlled blinking. When the chip is not transmitting or
> >> receiving anything the LED is off. Otherwise the LED will blink
> >> faster or slower according to the throughput.
> >>
> >> The LED can be controlled from userspace by writing 0, 1, or 2 to
> >> /sys/class/leds/rtl8xxxu-usbX-Y/brightness:
> >> 0 - solid off.
> >> 1 - solid on.
> >> 2 - hardware-controlled blinking.
> >
> > So, do you like
> >
> > #define RTL8XXXU_HW_LED_BLINKING 2
> >
> I'm not sure what you mean. Can you elaborate?
Originally, I would like to replace RTL8XXXU_HW_LED_CONTROL by
RTL8XXXU_HW_LED_BLINKING, because commit messages say "blinking".
After reviewing 2/4, 3/4 and 4/4, I aware that this blinking is
hardware behavior. Then, it becomes hard to me to decide which
name is clear. Anyway, original is fine to me.
>
> >>
> >> In this patch none of the chips advertise having an LED. That will be
> >> added in the next patches.
> >>
> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> >> ---
> >> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +++++
> >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 34 +++++++++++++++++++
> >> 2 files changed, 42 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> index 90268479d3ad..c8cee4a24755 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> @@ -1443,6 +1443,8 @@ struct rtl8xxxu_cfo_tracking {
> >> u32 packet_count_pre;
> >> };
> >>
> >> +#define RTL8XXXU_HW_LED_CONTROL 2
> >> +
> >> struct rtl8xxxu_priv {
> >> struct ieee80211_hw *hw;
> >> struct usb_device *udev;
> >> @@ -1564,6 +1566,10 @@ struct rtl8xxxu_priv {
> >> struct rtl8xxxu_ra_report ra_report;
> >> struct rtl8xxxu_cfo_tracking cfo_tracking;
> >> struct rtl8xxxu_ra_info ra_info;
> >> +
> >> + bool led_registered;
> >> + char led_name[32];
> >> + struct led_classdev led_cdev;
> >> };
> >>
> >> struct rtl8xxxu_rx_urb {
> >> @@ -1613,6 +1619,8 @@ struct rtl8xxxu_fileops {
> >> u32 rts_rate);
> >> void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
> >> s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, u8 cck_agc_rpt);
> >> + int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
> >> + enum led_brightness brightness);
> >> int writeN_block_size;
> >> int rx_agg_buf_size;
> >> char tx_desc_size;
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> index 35dc777c1fba..b27edd503c23 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> @@ -6955,6 +6955,34 @@ static int rtl8xxxu_parse_usb(struct rtl8xxxu_priv *priv,
> >> return ret;
> >> }
> >>
> >> +static void rtl8xxxu_init_led(struct rtl8xxxu_priv *priv)
> >> +{
> >> + struct led_classdev *led = &priv->led_cdev;
> >> +
> >> + led->brightness_set_blocking = priv->fops->led_classdev_brightness_set;
> >> +
> >> + snprintf(priv->led_name, sizeof(priv->led_name),
> >> + "rtl8xxxu-usb%s", dev_name(&priv->udev->dev));
> >> + led->name = priv->led_name;
> >> + led->max_brightness = RTL8XXXU_HW_LED_CONTROL;
> >> +
> >> + if (led_classdev_register(&priv->udev->dev, led))
> >> + return;
> >> +
> >> + priv->led_registered = true;
> >> +
> >> + led->brightness = led->max_brightness;
> >> + priv->fops->led_classdev_brightness_set(led, led->brightness);
> >> +}
> >> +
> >> +static void rtl8xxxu_deinit_led(struct rtl8xxxu_priv *priv)
> >> +{
> >> + struct led_classdev *led = &priv->led_cdev;
> >> +
> >> + priv->fops->led_classdev_brightness_set(led, LED_OFF);
> >> + led_classdev_unregister(led);
> >> +}
> >> +
> >> static int rtl8xxxu_probe(struct usb_interface *interface,
> >> const struct usb_device_id *id)
> >> {
> >> @@ -7135,6 +7163,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> >> goto err_set_intfdata;
> >> }
> >>
> >> + if (priv->fops->led_classdev_brightness_set)
> >
> > I prefer to move this checking statement into rtl8xxxu_init_led(). Then,
> > the flow of rtl8xxxu_probe() looks clean. As well as rtl8xxxu_deinit_led().
> > Just soft suggestion.
> >
> Okay. That works too.
>
> >> + rtl8xxxu_init_led(priv);
> >> +
> >> return 0;
> >>
> >> err_set_intfdata:
> >> @@ -7159,6 +7190,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
> >> hw = usb_get_intfdata(interface);
> >> priv = hw->priv;
> >>
> >> + if (priv->led_registered)
> >> + rtl8xxxu_deinit_led(priv);
> >> +
> >> ieee80211_unregister_hw(hw);
> >>
> >> priv->fops->power_off(priv);
> >> --
> >> 2.38.0
> >>
> >> ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
2023-01-19 0:25 ` Ping-Ke Shih
@ 2023-01-19 16:08 ` Bitterblue Smith
0 siblings, 0 replies; 11+ messages in thread
From: Bitterblue Smith @ 2023-01-19 16:08 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless; +Cc: Jes Sorensen
On 19/01/2023 02:25, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Wednesday, January 18, 2023 9:53 PM
>> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>
>> Subject: Re: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
>>
>> On 18/01/2023 02:52, Ping-Ke Shih wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>>>> Sent: Wednesday, January 18, 2023 6:03 AM
>>>> To: linux-wireless@vger.kernel.org
>>>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
>>>> Subject: [PATCH 1/4] wifi: rtl8xxxu: Register the LED and make it blink
>>>>
>>>> If the chip can have an LED, register a struct led_classdev and enable
>>>> hardware-controlled blinking. When the chip is not transmitting or
>>>> receiving anything the LED is off. Otherwise the LED will blink
>>>> faster or slower according to the throughput.
>>>>
>>>> The LED can be controlled from userspace by writing 0, 1, or 2 to
>>>> /sys/class/leds/rtl8xxxu-usbX-Y/brightness:
>>>> 0 - solid off.
>>>> 1 - solid on.
>>>> 2 - hardware-controlled blinking.
>>>
>>> So, do you like
>>>
>>> #define RTL8XXXU_HW_LED_BLINKING 2
>>>
>> I'm not sure what you mean. Can you elaborate?
>
> Originally, I would like to replace RTL8XXXU_HW_LED_CONTROL by
> RTL8XXXU_HW_LED_BLINKING, because commit messages say "blinking".
> After reviewing 2/4, 3/4 and 4/4, I aware that this blinking is
> hardware behavior. Then, it becomes hard to me to decide which
> name is clear. Anyway, original is fine to me.
>
Ah, okay. Now I get it. I will keep the original name.
^ permalink raw reply [flat|nested] 11+ messages in thread