> -----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
>
> 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.
> + 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.
