Subject: [PATCH] rtl8187: add radio led and fix warnings on suspend

Michael Buesch reports that his rtl8187 gives warnings on suspend
("queueing ieee80211 work while going to suspend" warnings), as rtl8187
can call ieee80211_queue_delayed_work after mac80211 is suspended.

This change enhances rtl8187 led code so we can avoid queuing work after
mac80211 is suspended: now we register a radio led and make additional
checks to ensure led is off/on properly as mac80211 wants.

Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
Tested-by: Larry Finger <[email protected]>
Cc: Stable <[email protected]>
---
drivers/net/wireless/rtl818x/rtl8187.h | 1 +
drivers/net/wireless/rtl818x/rtl8187_leds.c | 68 ++++++++++++++++++--------
drivers/net/wireless/rtl818x/rtl8187_leds.h | 2 +
3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8187.h b/drivers/net/wireless/rtl818x/rtl8187.h
index b1a24de..6af0f3f 100644
--- a/drivers/net/wireless/rtl818x/rtl8187.h
+++ b/drivers/net/wireless/rtl818x/rtl8187.h
@@ -108,6 +108,7 @@ struct rtl8187_priv {
struct delayed_work work;
struct ieee80211_hw *dev;
#ifdef CONFIG_RTL8187_LEDS
+ struct rtl8187_led led_radio;
struct rtl8187_led led_tx;
struct rtl8187_led led_rx;
struct delayed_work led_on;
diff --git a/drivers/net/wireless/rtl818x/rtl8187_leds.c b/drivers/net/wireless/rtl818x/rtl8187_leds.c
index cf8a4a4..ded44c0 100644
--- a/drivers/net/wireless/rtl818x/rtl8187_leds.c
+++ b/drivers/net/wireless/rtl818x/rtl8187_leds.c
@@ -105,19 +105,36 @@ static void rtl8187_led_brightness_set(struct led_classdev *led_dev,
struct rtl8187_led *led = container_of(led_dev, struct rtl8187_led,
led_dev);
struct ieee80211_hw *hw = led->dev;
- struct rtl8187_priv *priv = hw->priv;
+ struct rtl8187_priv *priv;
+ static bool radio_on;

- if (brightness == LED_OFF) {
- ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
- /* The LED is off for 1/20 sec so that it just blinks. */
- ieee80211_queue_delayed_work(hw, &priv->led_on, HZ / 20);
- } else
- ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
+ if (!hw)
+ return;
+ priv = hw->priv;
+ if (led->is_radio) {
+ if (brightness == LED_FULL) {
+ ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
+ radio_on = true;
+ } else if (radio_on) {
+ radio_on = false;
+ cancel_delayed_work_sync(&priv->led_on);
+ ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
+ }
+ } else if (radio_on) {
+ if (brightness == LED_OFF) {
+ ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
+ /* The LED is off for 1/20 sec - it just blinks. */
+ ieee80211_queue_delayed_work(hw, &priv->led_on,
+ HZ / 20);
+ } else
+ ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
+ }
}

static int rtl8187_register_led(struct ieee80211_hw *dev,
struct rtl8187_led *led, const char *name,
- const char *default_trigger, u8 ledpin)
+ const char *default_trigger, u8 ledpin,
+ bool is_radio)
{
int err;
struct rtl8187_priv *priv = dev->priv;
@@ -128,6 +145,7 @@ static int rtl8187_register_led(struct ieee80211_hw *dev,
return -EINVAL;
led->dev = dev;
led->ledpin = ledpin;
+ led->is_radio = is_radio;
strncpy(led->name, name, sizeof(led->name));

led->led_dev.name = led->name;
@@ -145,7 +163,11 @@ static int rtl8187_register_led(struct ieee80211_hw *dev,

static void rtl8187_unregister_led(struct rtl8187_led *led)
{
+ struct ieee80211_hw *hw = led->dev;
+ struct rtl8187_priv *priv = hw->priv;
+
led_classdev_unregister(&led->led_dev);
+ flush_delayed_work(&priv->led_off);
led->dev = NULL;
}

@@ -183,33 +205,37 @@ void rtl8187_leds_init(struct ieee80211_hw *dev, u16 custid)
INIT_DELAYED_WORK(&priv->led_off, led_turn_off);

snprintf(name, sizeof(name),
+ "rtl8187-%s::radio", wiphy_name(dev->wiphy));
+ err = rtl8187_register_led(dev, &priv->led_radio, name,
+ ieee80211_get_radio_led_name(dev), ledpin, true);
+ if (err)
+ return;
+
+ snprintf(name, sizeof(name),
"rtl8187-%s::tx", wiphy_name(dev->wiphy));
err = rtl8187_register_led(dev, &priv->led_tx, name,
- ieee80211_get_tx_led_name(dev), ledpin);
+ ieee80211_get_tx_led_name(dev), ledpin, false);
if (err)
- goto error;
+ goto err_tx;
+
snprintf(name, sizeof(name),
"rtl8187-%s::rx", wiphy_name(dev->wiphy));
err = rtl8187_register_led(dev, &priv->led_rx, name,
- ieee80211_get_rx_led_name(dev), ledpin);
- if (!err) {
- ieee80211_queue_delayed_work(dev, &priv->led_on, 0);
+ ieee80211_get_rx_led_name(dev), ledpin, false);
+ if (!err)
return;
- }
- /* registration of RX LED failed - unregister TX */
+
+ /* registration of RX LED failed - unregister */
rtl8187_unregister_led(&priv->led_tx);
-error:
- /* If registration of either failed, cancel delayed work */
- cancel_delayed_work_sync(&priv->led_off);
- cancel_delayed_work_sync(&priv->led_on);
+err_tx:
+ rtl8187_unregister_led(&priv->led_radio);
}

void rtl8187_leds_exit(struct ieee80211_hw *dev)
{
struct rtl8187_priv *priv = dev->priv;

- /* turn the LED off before exiting */
- ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
+ rtl8187_unregister_led(&priv->led_radio);
rtl8187_unregister_led(&priv->led_rx);
rtl8187_unregister_led(&priv->led_tx);
cancel_delayed_work_sync(&priv->led_off);
diff --git a/drivers/net/wireless/rtl818x/rtl8187_leds.h b/drivers/net/wireless/rtl818x/rtl8187_leds.h
index a033202..efe8041 100644
--- a/drivers/net/wireless/rtl818x/rtl8187_leds.h
+++ b/drivers/net/wireless/rtl818x/rtl8187_leds.h
@@ -47,6 +47,8 @@ struct rtl8187_led {
u8 ledpin;
/* The unique name string for this LED device. */
char name[RTL8187_LED_MAX_NAME_LEN + 1];
+ /* If the LED is radio or tx/rx */
+ bool is_radio;
};

void rtl8187_leds_init(struct ieee80211_hw *dev, u16 code);
--
1.6.5.5



Subject: Re: [PATCH] rtl8187: add radio led and fix warnings on suspend

Em Qua 09 Dez 2009, ?s 14:56:13, Herton Ronaldo Krzesinski escreveu:
> Michael Buesch reports that his rtl8187 gives warnings on suspend
> ("queueing ieee80211 work while going to suspend" warnings), as rtl8187
> can call ieee80211_queue_delayed_work after mac80211 is suspended.
>
> This change enhances rtl8187 led code so we can avoid queuing work after
> mac80211 is suspended: now we register a radio led and make additional
> checks to ensure led is off/on properly as mac80211 wants.
>
> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
> Tested-by: Larry Finger <[email protected]>
> Cc: Stable <[email protected]>

Sorry, forgot to note that stable is only about 2.6.32.x, which is
where warnings on suspend started to happen.

> ---
> drivers/net/wireless/rtl818x/rtl8187.h | 1 +
> drivers/net/wireless/rtl818x/rtl8187_leds.c | 68 ++++++++++++++++++--------
> drivers/net/wireless/rtl818x/rtl8187_leds.h | 2 +
> 3 files changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8187.h b/drivers/net/wireless/rtl818x/rtl8187.h
> index b1a24de..6af0f3f 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187.h
> +++ b/drivers/net/wireless/rtl818x/rtl8187.h
> @@ -108,6 +108,7 @@ struct rtl8187_priv {
> struct delayed_work work;
> struct ieee80211_hw *dev;
> #ifdef CONFIG_RTL8187_LEDS
> + struct rtl8187_led led_radio;
> struct rtl8187_led led_tx;
> struct rtl8187_led led_rx;
> struct delayed_work led_on;
> diff --git a/drivers/net/wireless/rtl818x/rtl8187_leds.c b/drivers/net/wireless/rtl818x/rtl8187_leds.c
> index cf8a4a4..ded44c0 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187_leds.c
> +++ b/drivers/net/wireless/rtl818x/rtl8187_leds.c
> @@ -105,19 +105,36 @@ static void rtl8187_led_brightness_set(struct led_classdev *led_dev,
> struct rtl8187_led *led = container_of(led_dev, struct rtl8187_led,
> led_dev);
> struct ieee80211_hw *hw = led->dev;
> - struct rtl8187_priv *priv = hw->priv;
> + struct rtl8187_priv *priv;
> + static bool radio_on;
>
> - if (brightness == LED_OFF) {
> - ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
> - /* The LED is off for 1/20 sec so that it just blinks. */
> - ieee80211_queue_delayed_work(hw, &priv->led_on, HZ / 20);
> - } else
> - ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
> + if (!hw)
> + return;
> + priv = hw->priv;
> + if (led->is_radio) {
> + if (brightness == LED_FULL) {
> + ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
> + radio_on = true;
> + } else if (radio_on) {
> + radio_on = false;
> + cancel_delayed_work_sync(&priv->led_on);
> + ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
> + }
> + } else if (radio_on) {
> + if (brightness == LED_OFF) {
> + ieee80211_queue_delayed_work(hw, &priv->led_off, 0);
> + /* The LED is off for 1/20 sec - it just blinks. */
> + ieee80211_queue_delayed_work(hw, &priv->led_on,
> + HZ / 20);
> + } else
> + ieee80211_queue_delayed_work(hw, &priv->led_on, 0);
> + }
> }
>
> static int rtl8187_register_led(struct ieee80211_hw *dev,
> struct rtl8187_led *led, const char *name,
> - const char *default_trigger, u8 ledpin)
> + const char *default_trigger, u8 ledpin,
> + bool is_radio)
> {
> int err;
> struct rtl8187_priv *priv = dev->priv;
> @@ -128,6 +145,7 @@ static int rtl8187_register_led(struct ieee80211_hw *dev,
> return -EINVAL;
> led->dev = dev;
> led->ledpin = ledpin;
> + led->is_radio = is_radio;
> strncpy(led->name, name, sizeof(led->name));
>
> led->led_dev.name = led->name;
> @@ -145,7 +163,11 @@ static int rtl8187_register_led(struct ieee80211_hw *dev,
>
> static void rtl8187_unregister_led(struct rtl8187_led *led)
> {
> + struct ieee80211_hw *hw = led->dev;
> + struct rtl8187_priv *priv = hw->priv;
> +
> led_classdev_unregister(&led->led_dev);
> + flush_delayed_work(&priv->led_off);
> led->dev = NULL;
> }
>
> @@ -183,33 +205,37 @@ void rtl8187_leds_init(struct ieee80211_hw *dev, u16 custid)
> INIT_DELAYED_WORK(&priv->led_off, led_turn_off);
>
> snprintf(name, sizeof(name),
> + "rtl8187-%s::radio", wiphy_name(dev->wiphy));
> + err = rtl8187_register_led(dev, &priv->led_radio, name,
> + ieee80211_get_radio_led_name(dev), ledpin, true);
> + if (err)
> + return;
> +
> + snprintf(name, sizeof(name),
> "rtl8187-%s::tx", wiphy_name(dev->wiphy));
> err = rtl8187_register_led(dev, &priv->led_tx, name,
> - ieee80211_get_tx_led_name(dev), ledpin);
> + ieee80211_get_tx_led_name(dev), ledpin, false);
> if (err)
> - goto error;
> + goto err_tx;
> +
> snprintf(name, sizeof(name),
> "rtl8187-%s::rx", wiphy_name(dev->wiphy));
> err = rtl8187_register_led(dev, &priv->led_rx, name,
> - ieee80211_get_rx_led_name(dev), ledpin);
> - if (!err) {
> - ieee80211_queue_delayed_work(dev, &priv->led_on, 0);
> + ieee80211_get_rx_led_name(dev), ledpin, false);
> + if (!err)
> return;
> - }
> - /* registration of RX LED failed - unregister TX */
> +
> + /* registration of RX LED failed - unregister */
> rtl8187_unregister_led(&priv->led_tx);
> -error:
> - /* If registration of either failed, cancel delayed work */
> - cancel_delayed_work_sync(&priv->led_off);
> - cancel_delayed_work_sync(&priv->led_on);
> +err_tx:
> + rtl8187_unregister_led(&priv->led_radio);
> }
>
> void rtl8187_leds_exit(struct ieee80211_hw *dev)
> {
> struct rtl8187_priv *priv = dev->priv;
>
> - /* turn the LED off before exiting */
> - ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
> + rtl8187_unregister_led(&priv->led_radio);
> rtl8187_unregister_led(&priv->led_rx);
> rtl8187_unregister_led(&priv->led_tx);
> cancel_delayed_work_sync(&priv->led_off);
> diff --git a/drivers/net/wireless/rtl818x/rtl8187_leds.h b/drivers/net/wireless/rtl818x/rtl8187_leds.h
> index a033202..efe8041 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187_leds.h
> +++ b/drivers/net/wireless/rtl818x/rtl8187_leds.h
> @@ -47,6 +47,8 @@ struct rtl8187_led {
> u8 ledpin;
> /* The unique name string for this LED device. */
> char name[RTL8187_LED_MAX_NAME_LEN + 1];
> + /* If the LED is radio or tx/rx */
> + bool is_radio;
> };
>
> void rtl8187_leds_init(struct ieee80211_hw *dev, u16 code);
>

--
[]'s
Herton