[PATCH] mac80211: Add radio led trigger

[PATCH] mac80211: Add radio led trigger

[Ivo van Doorn]

Some devices have a seperate LED which indicates if the radio is
enabled or not. This adds a LED trigger to mac80211 where drivers
can hook into when they are interested in radio status changes.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>

---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1d3d4f3..42499f6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1173,6 +1173,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_tx_led_name(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_rx_led_name(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_assoc_led_name(struct ieee80211_hw *hw);
+extern char *__ieee80211_get_radio_led_name(struct ieee80211_hw *hw);
 #endif
 /**
  * ieee80211_get_tx_led_name - get name of TX LED
@@ -1212,6 +1213,16 @@ static inline char *ieee80211_get_rx_led_name(struct ieee80211_hw *hw)
 #endif
 }
 
+/**
+ * ieee80211_get_assoc_led_name - get name of association LED
+ *
+ * mac80211 creates a association LED trigger for each wireless hardware
+ * that can be used to drive LEDs if your driver registers a LED device.
+ * This function returns the name (or %NULL if not configured for LEDs)
+ * of the trigger so you can automatically link the LED device.
+ *
+ * @hw: the hardware to get the LED trigger name for
+ */
 static inline char *ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 {
 #ifdef CONFIG_MAC80211_LEDS
@@ -1221,6 +1232,24 @@ static inline char *ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 #endif
 }
 
+/**
+ * ieee80211_get_radio_led_name - get name of radio LED
+ *
+ * mac80211 creates a radio change LED trigger for each wireless hardware
+ * that can be used to drive LEDs if your driver registers a LED device.
+ * This function returns the name (or %NULL if not configured for LEDs)
+ * of the trigger so you can automatically link the LED device.
+ *
+ * @hw: the hardware to get the LED trigger name for
+ */
+static inline char *ieee80211_get_radio_led_name(struct ieee80211_hw *hw)
+{
+#ifdef CONFIG_MAC80211_LEDS
+	return __ieee80211_get_radio_led_name(hw);
+#else
+	return NULL;
+#endif
+}
 
 /* Register a new hardware PHYMODE capability to the stack. */
 int ieee80211_register_hwmode(struct ieee80211_hw *hw,
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 5aa9c2d..df64b6e 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -219,6 +219,7 @@ static int ieee80211_open(struct net_device *dev)
 		if (res)
 			return res;
 		ieee80211_hw_config(local);
+		ieee80211_led_radio(local, 1);
 	}
 
 	switch (sdata->vif.type) {
@@ -390,6 +391,8 @@ static int ieee80211_stop(struct net_device *dev)
 		if (local->ops->stop)
 			local->ops->stop(local_to_hw(local));
 
+		ieee80211_led_radio(local, 0);
+
 		tasklet_disable(&local->tx_pending_tasklet);
 		tasklet_disable(&local->tasklet);
 	}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 083c432..e4d2f66 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -513,8 +513,9 @@ struct ieee80211_local {
 
 #ifdef CONFIG_MAC80211_LEDS
 	int tx_led_counter, rx_led_counter;
-	struct led_trigger *tx_led, *rx_led, *assoc_led;
-	char tx_led_name[32], rx_led_name[32], assoc_led_name[32];
+	struct led_trigger *tx_led, *rx_led, *assoc_led, *radio_led;
+	char tx_led_name[32], rx_led_name[32],
+	     assoc_led_name[32], radio_led_name[32];
 #endif
 
 	u32 channel_use;
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index be80f44..b28eb8f 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -21,6 +21,7 @@
 
 #include <net/mac80211.h>
 #include "ieee80211_i.h"
+#include "ieee80211_led.h"
 #include "ieee80211_rate.h"
 #include "wpa.h"
 #include "aes_ccm.h"
@@ -651,6 +652,7 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
 	if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
 		local->hw.conf.radio_enabled = !(data->txpower.disabled);
 		need_reconfig = 1;
+		ieee80211_led_radio(local, local->hw.conf.radio_enabled);
 	}
 
 	if (need_reconfig) {
diff --git a/net/mac80211/ieee80211_led.c b/net/mac80211/ieee80211_led.c
index 4cf89af..f401484 100644
--- a/net/mac80211/ieee80211_led.c
+++ b/net/mac80211/ieee80211_led.c
@@ -43,6 +43,16 @@ void ieee80211_led_assoc(struct ieee80211_local *local, bool associated)
 		led_trigger_event(local->assoc_led, LED_OFF);
 }
 
+void ieee80211_led_radio(struct ieee80211_local *local, bool enabled)
+{
+	if (unlikely(!local->radio_led))
+		return;
+	if (enabled)
+		led_trigger_event(local->radio_led, LED_FULL);
+	else
+		led_trigger_event(local->radio_led, LED_OFF);
+}
+
 void ieee80211_led_init(struct ieee80211_local *local)
 {
 	local->rx_led = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
@@ -77,10 +87,25 @@ void ieee80211_led_init(struct ieee80211_local *local)
 			local->assoc_led = NULL;
 		}
 	}
+
+	local->radio_led = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
+	if (local->radio_led) {
+		snprintf(local->radio_led_name, sizeof(local->radio_led_name),
+			 "%sradio", wiphy_name(local->hw.wiphy));
+		local->radio_led->name = local->radio_led_name;
+		if (led_trigger_register(local->radio_led)) {
+			kfree(local->radio_led);
+			local->radio_led = NULL;
+		}
+	}
 }
 
 void ieee80211_led_exit(struct ieee80211_local *local)
 {
+	if (local->radio_led) {
+		led_trigger_unregister(local->radio_led);
+		kfree(local->radio_led);
+	}
 	if (local->assoc_led) {
 		led_trigger_unregister(local->assoc_led);
 		kfree(local->assoc_led);
@@ -95,6 +120,16 @@ void ieee80211_led_exit(struct ieee80211_local *local)
 	}
 }
 
+char *__ieee80211_get_radio_led_name(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+
+	if (local->radio_led)
+		return local->radio_led_name;
+	return NULL;
+}
+EXPORT_SYMBOL(__ieee80211_get_radio_led_name);
+
 char *__ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
diff --git a/net/mac80211/ieee80211_led.h b/net/mac80211/ieee80211_led.h
index 0feb226..77b1e1b 100644
--- a/net/mac80211/ieee80211_led.h
+++ b/net/mac80211/ieee80211_led.h
@@ -16,6 +16,8 @@ extern void ieee80211_led_rx(struct ieee80211_local *local);
 extern void ieee80211_led_tx(struct ieee80211_local *local, int q);
 extern void ieee80211_led_assoc(struct ieee80211_local *local,
 				bool associated);
+extern void ieee80211_led_radio(struct ieee80211_local *local,
+				bool enabled);
 extern void ieee80211_led_init(struct ieee80211_local *local);
 extern void ieee80211_led_exit(struct ieee80211_local *local);
 #else
@@ -29,6 +31,10 @@ static inline void ieee80211_led_assoc(struct ieee80211_local *local,
 				       bool associated)
 {
 }
+static inline void ieee80211_led_radio(struct ieee80211_local *local,
+				       bool enabled)
+{
+}
 static inline void ieee80211_led_init(struct ieee80211_local *local)
 {
 }

Re: [PATCH] mac80211: Add radio led trigger

[Tomas Winkler]

On Jan 6, 2008 3:10 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> Some devices have a seperate LED which indicates if the radio is
> enabled or not. This adds a LED trigger to mac80211 where drivers
> can hook into when they are interested in radio status changes.
>
>
I wonder how this go together with RF KILL led?

Tomas

Re: [PATCH] mac80211: Add radio led trigger

[Michael Buesch]

On Sunday 06 January 2008 19:19:54 Tomas Winkler wrote:
> On Jan 6, 2008 3:10 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > Some devices have a seperate LED which indicates if the radio is
> > enabled or not. This adds a LED trigger to mac80211 where drivers
> > can hook into when they are interested in radio status changes.
> >
> >
> I wonder how this go together with RF KILL led?

The rfkill LED is a seperate LED.
This LED is the software status of the radio. Not too useful, but hey. :)

-- 
Greetings Michael.

Re: [PATCH] mac80211: Add radio led trigger

[Ivo van Doorn]

On Sunday 06 January 2008, Tomas Winkler wrote:
> On Jan 6, 2008 3:10 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > Some devices have a seperate LED which indicates if the radio is
> > enabled or not. This adds a LED trigger to mac80211 where drivers
> > can hook into when they are interested in radio status changes.
> >
> >
> I wonder how this go together with RF KILL led?

They would be competing I fear, but not all devices have a rfkill button,
which makes the mac80211 radio led trigger more usefull I think.

Ivo

Re: [PATCH] mac80211: Add radio led trigger

[Tomas Winkler]

On Jan 6, 2008 8:25 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
>
> On Sunday 06 January 2008, Tomas Winkler wrote:
> > On Jan 6, 2008 3:10 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > > Some devices have a seperate LED which indicates if the radio is
> > > enabled or not. This adds a LED trigger to mac80211 where drivers
> > > can hook into when they are interested in radio status changes.
> > >
> > >
> > I wonder how this go together with RF KILL led?
>
> They would be competing I fear, but not all devices have a rfkill button,
> which makes the mac80211 radio led trigger more usefull I think.
>
What inquires me is  in the following snippet is  would you lit the
led if even if rfkill HW switch is on?
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -219,6 +219,7 @@ static int ieee80211_open(struct net_device *dev)
               if (res)
                       return res;
               ieee80211_hw_config(local);
+               ieee80211_led_radio(local, 1);
       }


Thanks
Tomas
> Ivo
>

Re: [PATCH] mac80211: Add radio led trigger

[Ivo van Doorn]

On Monday 07 January 2008, Tomas Winkler wrote:
> On Jan 6, 2008 8:25 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> >
> > On Sunday 06 January 2008, Tomas Winkler wrote:
> > > On Jan 6, 2008 3:10 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > > > Some devices have a seperate LED which indicates if the radio is
> > > > enabled or not. This adds a LED trigger to mac80211 where drivers
> > > > can hook into when they are interested in radio status changes.
> > > >
> > > >
> > > I wonder how this go together with RF KILL led?
> >
> > They would be competing I fear, but not all devices have a rfkill button,
> > which makes the mac80211 radio led trigger more usefull I think.
> >
> What inquires me is  in the following snippet is  would you lit the
> led if even if rfkill HW switch is on?
> --- a/net/mac80211/ieee80211.c
> +++ b/net/mac80211/ieee80211.c
> @@ -219,6 +219,7 @@ static int ieee80211_open(struct net_device *dev)
>                if (res)
>                        return res;
>                ieee80211_hw_config(local);
> +               ieee80211_led_radio(local, 1);
>        }
> 

Good point. I have updated the patch to make it check the radio_enabled
configuration parameter.

Ivo

[PATCH v2] mac80211: Add radio led trigger

[Ivo van Doorn]

Some devices have a seperate LED which indicates if the radio is
enabled or not. This adds a LED trigger to mac80211 where drivers
can hook into when they are interested in radio status changes.

v2: Check hw.conf.radio_enabled when calling start().

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>

---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1d3d4f3..42499f6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1173,6 +1173,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_tx_led_name(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_rx_led_name(struct ieee80211_hw *hw);
 extern char *__ieee80211_get_assoc_led_name(struct ieee80211_hw *hw);
+extern char *__ieee80211_get_radio_led_name(struct ieee80211_hw *hw);
 #endif
 /**
  * ieee80211_get_tx_led_name - get name of TX LED
@@ -1212,6 +1213,16 @@ static inline char *ieee80211_get_rx_led_name(struct ieee80211_hw *hw)
 #endif
 }
 
+/**
+ * ieee80211_get_assoc_led_name - get name of association LED
+ *
+ * mac80211 creates a association LED trigger for each wireless hardware
+ * that can be used to drive LEDs if your driver registers a LED device.
+ * This function returns the name (or %NULL if not configured for LEDs)
+ * of the trigger so you can automatically link the LED device.
+ *
+ * @hw: the hardware to get the LED trigger name for
+ */
 static inline char *ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 {
 #ifdef CONFIG_MAC80211_LEDS
@@ -1221,6 +1232,24 @@ static inline char *ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 #endif
 }
 
+/**
+ * ieee80211_get_radio_led_name - get name of radio LED
+ *
+ * mac80211 creates a radio change LED trigger for each wireless hardware
+ * that can be used to drive LEDs if your driver registers a LED device.
+ * This function returns the name (or %NULL if not configured for LEDs)
+ * of the trigger so you can automatically link the LED device.
+ *
+ * @hw: the hardware to get the LED trigger name for
+ */
+static inline char *ieee80211_get_radio_led_name(struct ieee80211_hw *hw)
+{
+#ifdef CONFIG_MAC80211_LEDS
+	return __ieee80211_get_radio_led_name(hw);
+#else
+	return NULL;
+#endif
+}
 
 /* Register a new hardware PHYMODE capability to the stack. */
 int ieee80211_register_hwmode(struct ieee80211_hw *hw,
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 5aa9c2d..df64b6e 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -219,6 +219,7 @@ static int ieee80211_open(struct net_device *dev)
 		if (res)
 			return res;
 		ieee80211_hw_config(local);
+		ieee80211_led_radio(local, local->hw.conf.radio_enabled);
 	}
 
 	switch (sdata->vif.type) {
@@ -390,6 +391,8 @@ static int ieee80211_stop(struct net_device *dev)
 		if (local->ops->stop)
 			local->ops->stop(local_to_hw(local));
 
+		ieee80211_led_radio(local, 0);
+
 		tasklet_disable(&local->tx_pending_tasklet);
 		tasklet_disable(&local->tasklet);
 	}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 083c432..e4d2f66 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -513,8 +513,9 @@ struct ieee80211_local {
 
 #ifdef CONFIG_MAC80211_LEDS
 	int tx_led_counter, rx_led_counter;
-	struct led_trigger *tx_led, *rx_led, *assoc_led;
-	char tx_led_name[32], rx_led_name[32], assoc_led_name[32];
+	struct led_trigger *tx_led, *rx_led, *assoc_led, *radio_led;
+	char tx_led_name[32], rx_led_name[32],
+	     assoc_led_name[32], radio_led_name[32];
 #endif
 
 	u32 channel_use;
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index be80f44..b28eb8f 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -21,6 +21,7 @@
 
 #include <net/mac80211.h>
 #include "ieee80211_i.h"
+#include "ieee80211_led.h"
 #include "ieee80211_rate.h"
 #include "wpa.h"
 #include "aes_ccm.h"
@@ -651,6 +652,7 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
 	if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
 		local->hw.conf.radio_enabled = !(data->txpower.disabled);
 		need_reconfig = 1;
+		ieee80211_led_radio(local, local->hw.conf.radio_enabled);
 	}
 
 	if (need_reconfig) {
diff --git a/net/mac80211/ieee80211_led.c b/net/mac80211/ieee80211_led.c
index 4cf89af..f401484 100644
--- a/net/mac80211/ieee80211_led.c
+++ b/net/mac80211/ieee80211_led.c
@@ -43,6 +43,16 @@ void ieee80211_led_assoc(struct ieee80211_local *local, bool associated)
 		led_trigger_event(local->assoc_led, LED_OFF);
 }
 
+void ieee80211_led_radio(struct ieee80211_local *local, bool enabled)
+{
+	if (unlikely(!local->radio_led))
+		return;
+	if (enabled)
+		led_trigger_event(local->radio_led, LED_FULL);
+	else
+		led_trigger_event(local->radio_led, LED_OFF);
+}
+
 void ieee80211_led_init(struct ieee80211_local *local)
 {
 	local->rx_led = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
@@ -77,10 +87,25 @@ void ieee80211_led_init(struct ieee80211_local *local)
 			local->assoc_led = NULL;
 		}
 	}
+
+	local->radio_led = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
+	if (local->radio_led) {
+		snprintf(local->radio_led_name, sizeof(local->radio_led_name),
+			 "%sradio", wiphy_name(local->hw.wiphy));
+		local->radio_led->name = local->radio_led_name;
+		if (led_trigger_register(local->radio_led)) {
+			kfree(local->radio_led);
+			local->radio_led = NULL;
+		}
+	}
 }
 
 void ieee80211_led_exit(struct ieee80211_local *local)
 {
+	if (local->radio_led) {
+		led_trigger_unregister(local->radio_led);
+		kfree(local->radio_led);
+	}
 	if (local->assoc_led) {
 		led_trigger_unregister(local->assoc_led);
 		kfree(local->assoc_led);
@@ -95,6 +120,16 @@ void ieee80211_led_exit(struct ieee80211_local *local)
 	}
 }
 
+char *__ieee80211_get_radio_led_name(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+
+	if (local->radio_led)
+		return local->radio_led_name;
+	return NULL;
+}
+EXPORT_SYMBOL(__ieee80211_get_radio_led_name);
+
 char *__ieee80211_get_assoc_led_name(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
diff --git a/net/mac80211/ieee80211_led.h b/net/mac80211/ieee80211_led.h
index 0feb226..77b1e1b 100644
--- a/net/mac80211/ieee80211_led.h
+++ b/net/mac80211/ieee80211_led.h
@@ -16,6 +16,8 @@ extern void ieee80211_led_rx(struct ieee80211_local *local);
 extern void ieee80211_led_tx(struct ieee80211_local *local, int q);
 extern void ieee80211_led_assoc(struct ieee80211_local *local,
 				bool associated);
+extern void ieee80211_led_radio(struct ieee80211_local *local,
+				bool enabled);
 extern void ieee80211_led_init(struct ieee80211_local *local);
 extern void ieee80211_led_exit(struct ieee80211_local *local);
 #else
@@ -29,6 +31,10 @@ static inline void ieee80211_led_assoc(struct ieee80211_local *local,
 				       bool associated)
 {
 }
+static inline void ieee80211_led_radio(struct ieee80211_local *local,
+				       bool enabled)
+{
+}
 static inline void ieee80211_led_init(struct ieee80211_local *local)
 {
 }

Re: [PATCH v2] mac80211: Add radio led trigger

[Tomas Winkler]

On Jan 7, 2008 8:45 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> Some devices have a seperate LED which indicates if the radio is
> enabled or not. This adds a LED trigger to mac80211 where drivers
> can hook into when they are interested in radio status changes.
>
> v2: Check hw.conf.radio_enabled when calling start().
>
>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
>
> ---
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 1d3d4f3..42499f6 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1173,6 +1173,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw);
>  extern char *__ieee80211_get_tx_led_name(struct ieee80211_hw *hw);
>  extern char *__ieee80211_get_rx_led_name(struct ieee80211_hw *hw);
>  extern char *__ieee80211_get_assoc_led_name(struct ieee80211_hw *hw);
> +extern char *__ieee80211_get_radio_led_name(struct ieee80211_hw *hw);
>  #endif
>  /**
>   * ieee80211_get_tx_led_name - get name of TX LED
> @@ -1212,6 +1213,16 @@ static inline char *ieee80211_get_rx_led_name(struct ieee80211_hw *hw)
>  #endif
>  }
>
> @@ -651,6 +652,7 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
>         if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
>                 local->hw.conf.radio_enabled = !(data->txpower.disabled);
>                 need_reconfig = 1;
> +               ieee80211_led_radio(local, local->hw.conf.radio_enabled);

Not sure but isn't it cleaner to push it to ieee80211_hw_config().
This will cover also ieee80211_open and maybe in the future also
cfg80211
Will it be so bad to call led_radio even the led is already lit?

Thanks
Tomas

Re: [PATCH v2] mac80211: Add radio led trigger

[Ivo van Doorn]

Hi,

> > @@ -651,6 +652,7 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
> >         if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
> >                 local->hw.conf.radio_enabled = !(data->txpower.disabled);
> >                 need_reconfig = 1;
> > +               ieee80211_led_radio(local, local->hw.conf.radio_enabled);
> 
> Not sure but isn't it cleaner to push it to ieee80211_hw_config().
> This will cover also ieee80211_open and maybe in the future also
> cfg80211
> Will it be so bad to call led_radio even the led is already lit?

Well that means it will be called quite often, and although it would perhaps
not matter that much, I don't consider it a clean solution.

We could move it to ieee80211_hw_config() eventually, but I think we should
first implement some kind of "what config option changed" handling. That
would benefit drivers as well since they should now handle that to prevent
configuring _everything_ when hw_config() is called.

Ivo

Re: [PATCH v2] mac80211: Add radio led trigger

[Tomas Winkler]

On Jan 9, 2008 6:50 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> Hi,
>
> > > @@ -651,6 +652,7 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
> > >         if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
> > >                 local->hw.conf.radio_enabled = !(data->txpower.disabled);
> > >                 need_reconfig = 1;
> > > +               ieee80211_led_radio(local, local->hw.conf.radio_enabled);
> >
> > Not sure but isn't it cleaner to push it to ieee80211_hw_config().
> > This will cover also ieee80211_open and maybe in the future also
> > cfg80211
> > Will it be so bad to call led_radio even the led is already lit?
>
> Well that means it will be called quite often, and although it would perhaps
> not matter that much, I don't consider it a clean solution.

If you have one physical  led mapped to few triggers as it is on
current  laptops it will be probably  complicate few things so after
all I'm fine with how it is now.

> We could move it to ieee80211_hw_config() eventually, but I think we should
> first implement some kind of "what config option changed" handling. That
> would benefit drivers as well since they should now handle that to prevent
> configuring _everything_ when hw_config() is called.

You are talking from my heart.

Tomas

> Ivo
>