On 18/01/2023 02:52, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>> Sent: Wednesday, January 18, 2023 6:03 AM
>> To: linux-wireless@xxxxxxxxxxxxxxx
>> Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
>> 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@xxxxxxxxx>
>> ---
>> .../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.
