Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.

PATCH In review Dmytro Milinevskyy 3 participants 9 messages Jun 9, 2010 – Jun 12, 2010

Subsystems (3)

Networking Network drivers Atheros ATH5K wireless driver

lore

Share & tools

lore

Dmytro Milinevskyy

author

2010-06-09 07:30 UTC #0 lore raw

PATCH 1/1 Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.

Hi!

Here is the patch to disable ath5k leds support on build stage. However if the leds support was enabled do not force selection of 802.11 leds layer. Depency on LEDS_CLASS is kept.

Suggestion given by Pavel Roskin and Bob Copeland applied.

Changes from the previos version of the patch:
- Use bool directive in Kconfig for switching LEDs support
- Move ath5k_hw_set_ledstate into led.c and change its semantic to align with other leds routines

Regards,

--Dima

Signed-off-by Dmytro Milinevskyy <[email protected]>

---
 drivers/net/wireless/ath/ath5k/Kconfig  |   12 +++++--
 drivers/net/wireless/ath/ath5k/Makefile |    2 +-
 drivers/net/wireless/ath/ath5k/ath5k.h  |   21 +++++++++++-
 drivers/net/wireless/ath/ath5k/attach.c |    2 +-
 drivers/net/wireless/ath/ath5k/base.c   |    6 ++--
 drivers/net/wireless/ath/ath5k/base.h   |   11 ++++--
 drivers/net/wireless/ath/ath5k/gpio.c   |   52 ------------------------------
 drivers/net/wireless/ath/ath5k/led.c    |   53 +++++++++++++++++++++++++++++++
 8 files changed, 94 insertions(+), 65 deletions(-)

drivers/net/wireless/ath/ath5k/Kconfig +9 -3

index eb83b7b..6b2a682 100644
@@ -1,9 +1,6 @@
 config ATH5K
 	tristate "Atheros 5xxx wireless cards support"
 	depends on PCI && MAC80211
-	select MAC80211_LEDS
-	select LEDS_CLASS
-	select NEW_LEDS
 	---help---
 	  This module adds support for wireless adapters based on
 	  Atheros 5xxx chipset.

@@ -18,6 +15,15 @@ config ATH5K
 	  If you choose to build a module, it'll be called ath5k. Say M if
 	  unsure.
 
+
+config ATH5K_LEDS
+	bool "Atheros 5xxx wireless cards LEDs support"
+	depends on ATH5K
+	select NEW_LEDS
+	select LEDS_CLASS
+	---help---
+	  Atheros 5xxx LED support.
+
 config ATH5K_DEBUG
 	bool "Atheros 5xxx debugging"
 	depends on ATH5K

drivers/net/wireless/ath/ath5k/Makefile +1 -1

index cc09595..6d552dd 100644
@@ -10,8 +10,8 @@ ath5k-y				+= phy.o
 ath5k-y				+= reset.o
 ath5k-y				+= attach.o
 ath5k-y				+= base.o
-ath5k-y				+= led.o
 ath5k-y				+= rfkill.o
 ath5k-y				+= ani.o
 ath5k-$(CONFIG_ATH5K_DEBUG)	+= debug.o
+ath5k-$(CONFIG_ATH5K_LEDS) += led.o
 obj-$(CONFIG_ATH5K)		+= ath5k.o

drivers/net/wireless/ath/ath5k/ath5k.h +20 -1

index 2785946..202d143 100644
@@ -1148,11 +1148,31 @@ struct ath5k_hw {
 int ath5k_hw_attach(struct ath5k_softc *sc);
 void ath5k_hw_detach(struct ath5k_hw *ah);
 
+#ifdef CONFIG_ATH5K_LEDS
 /* LED functions */
+void ath5k_hw_set_ledstate(struct ath5k_softc *sc, unsigned int state);
 int ath5k_init_leds(struct ath5k_softc *sc);
 void ath5k_led_enable(struct ath5k_softc *sc);
 void ath5k_led_off(struct ath5k_softc *sc);
 void ath5k_unregister_leds(struct ath5k_softc *sc);
+#else
+static inline void ath5k_hw_set_ledstate(struct ath5k_softc *sc, unsigned int state)
+{
+}
+static inline int ath5k_init_leds(struct ath5k_softc *sc)
+{
+	return 0;
+}
+static inline void ath5k_led_enable(struct ath5k_softc *sc)
+{
+}
+static inline void ath5k_led_off(struct ath5k_softc *sc)
+{
+}
+static inline void ath5k_unregister_leds(struct ath5k_softc *sc)
+{
+}
+#endif
 
 /* Reset Functions */
 int ath5k_hw_nic_wakeup(struct ath5k_hw *ah, int flags, bool initial);

@@ -1233,7 +1253,6 @@ int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time);
 int ath5k_hw_init_desc_functions(struct ath5k_hw *ah);
 
 /* GPIO Functions */
-void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state);
 int ath5k_hw_set_gpio_input(struct ath5k_hw *ah, u32 gpio);
 int ath5k_hw_set_gpio_output(struct ath5k_hw *ah, u32 gpio);
 u32 ath5k_hw_get_gpio(struct ath5k_hw *ah, u32 gpio);

drivers/net/wireless/ath/ath5k/attach.c +1 -1

index e0c244b..7930a39 100644
@@ -336,7 +336,7 @@ int ath5k_hw_attach(struct ath5k_softc *sc)
 	ath5k_hw_init_nfcal_hist(ah);
 
 	/* turn on HW LEDs */
-	ath5k_hw_set_ledstate(ah, AR5K_LED_INIT);
+	ath5k_hw_set_ledstate(sc, AR5K_LED_INIT);
 
 	return 0;
 err_free:

drivers/net/wireless/ath/ath5k/base.c +3 -3

index 2978359..64ea83d 100644
@@ -3411,7 +3411,7 @@ static void ath5k_bss_info_changed(struct ieee80211_hw *hw,
 		sc->assoc = bss_conf->assoc;
 		if (sc->opmode == NL80211_IFTYPE_STATION)
 			set_beacon_filter(hw, sc->assoc);
-		ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
+		ath5k_hw_set_ledstate(sc, sc->assoc ?
 			AR5K_LED_ASSOC : AR5K_LED_INIT);
 		if (bss_conf->assoc) {
 			ATH5K_DBG(sc, ATH5K_DEBUG_ANY,

@@ -3444,13 +3444,13 @@ static void ath5k_sw_scan_start(struct ieee80211_hw *hw)
 {
 	struct ath5k_softc *sc = hw->priv;
 	if (!sc->assoc)
-		ath5k_hw_set_ledstate(sc->ah, AR5K_LED_SCAN);
+		ath5k_hw_set_ledstate(sc, AR5K_LED_SCAN);
 }
 
 static void ath5k_sw_scan_complete(struct ieee80211_hw *hw)
 {
 	struct ath5k_softc *sc = hw->priv;
-	ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
+	ath5k_hw_set_ledstate(sc, sc->assoc ?
 		AR5K_LED_ASSOC : AR5K_LED_INIT);
 }

drivers/net/wireless/ath/ath5k/base.h +7 -4

index 56221bc..8d289a3 100644
@@ -186,9 +186,6 @@ struct ath5k_softc {
 
 	u8			bssidmask[ETH_ALEN];
 
-	unsigned int		led_pin,	/* GPIO pin for driving LED */
-				led_on;		/* pin setting for LED on */
-
 	struct tasklet_struct	restq;		/* reset tasklet */
 
 	unsigned int		rxbufsize;	/* rx size based on mtu */

@@ -196,7 +193,6 @@ struct ath5k_softc {
 	spinlock_t		rxbuflock;
 	u32			*rxlink;	/* link ptr in last RX desc */
 	struct tasklet_struct	rxtq;		/* rx intr tasklet */
-	struct ath5k_led	rx_led;		/* rx led */
 
 	struct list_head	txbuf;		/* transmit buffer */
 	spinlock_t		txbuflock;

@@ -204,7 +200,14 @@ struct ath5k_softc {
 	struct ath5k_txq	txqs[AR5K_NUM_TX_QUEUES];	/* tx queues */
 	struct ath5k_txq	*txq;		/* main tx queue */
 	struct tasklet_struct	txtq;		/* tx intr tasklet */
+
+
+#ifdef CONFIG_ATH5K_LEDS
+	unsigned int		led_pin,	/* GPIO pin for driving LED */
+				led_on;		/* pin setting for LED on */
+	struct ath5k_led	rx_led;		/* rx led */
 	struct ath5k_led	tx_led;		/* tx led */
+#endif
 
 	struct ath5k_rfkill	rf_kill;

drivers/net/wireless/ath/ath5k/gpio.c +0 ~~-52~~

58 lines not on this page · raw message

drivers/net/wireless/ath/ath5k/led.c +53 -0

65 lines not on this page · raw message

Signature · 1 line

1.7.1

Bob Copeland

2010-06-09 13:58 UTC #1 lore raw

Quoted 1 line · Dmytro Milinevskyy

However if the leds support was enabled do not force selection of 802.11 leds layer.

I don't understand this part. What's the point of enabling software LEDs if nothing triggers them?

Also, we can probably build hardware LEDs (hw_set_ledstate) in regardless. It's a pure register write and doesn't require the rest of the LED machinery.

I assume you are doing this to reduce the size of the module. If so, can you include size(1) output in the commit message?

Signature · 1 line

Bob Copeland %% www.bobcopeland.com

Dmytro Milinevskyy

author

2010-06-09 14:43 UTC #2 lore raw

Hi, Bob.

For instance I don't use 802.11 leds layer and trigger leds from userspace according to my purposes.

-- Dima

Quoted 16 lines · Bob Copeland

Quoted 1 line · Dmytro Milinevskyy
However if the leds support was enabled do not force selection of 802.11 leds layer.
I don't understand this part. What's the point of enabling software LEDs if nothing triggers them?
Also, we can probably build hardware LEDs (hw_set_ledstate) in regardless. It's a pure register write and doesn't require the rest of the LED machinery.
I assume you are doing this to reduce the size of the module. If so, can you include size(1) output in the commit message?
--
Bob Copeland %% www.bobcopeland.com

Bob Copeland

2010-06-11 17:07 UTC #3 lore raw

Quoted 4 lines · Dmytro Milinevskyy

Hi, Bob.
For instance I don't use 802.11 leds layer and trigger leds from userspace according to my purposes.

FWIW you can disable mac80211 triggers from userspace as well. But, I guess hooking up null triggers will work too.

Signature · 5 lines

Bob Copeland %% www.bobcopeland.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dmytro Milinevskyy

author

2010-06-11 19:52 UTC #4 lore raw

I didn't know. Thank you for noting that.
However I think it's better to give a chance to disable 802.11 leds.

-- Dima

Quoted 13 lines · Bob Copeland

Quoted 4 lines · Dmytro Milinevskyy
Hi, Bob.
For instance I don't use 802.11 leds layer and trigger leds from userspace according to my purposes.
FWIW you can disable mac80211 triggers from userspace as well. But, I guess hooking up null triggers will work too.
--
Bob Copeland %% www.bobcopeland.com

Johannes Berg

2010-06-11 20:09 UTC #5 lore raw

In reply to Dmytro Milinevskyy

Quoted 4 lines · Dmytro Milinevskyy

Hi, Bob.
For instance I don't use 802.11 leds layer and trigger leds from userspace according to my purposes.

Without the LED stuff in sysfs, how do you do that?

johannes

Dmytro Milinevskyy

author

2010-06-11 20:26 UTC #6 lore raw

Generic LED class is not disabled so it's possible to control leds in sysfs.

-- Dima

Quoted 11 lines · Johannes Berg

Quoted 4 lines · Dmytro Milinevskyy
Hi, Bob.
For instance I don't use 802.11 leds layer and trigger leds from userspace according to my purposes.
Without the LED stuff in sysfs, how do you do that?
johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Johannes Berg

2010-06-11 20:29 UTC #7 lore raw

Quoted 1 line · Dmytro Milinevskyy

Generic LED class is not disabled so it's possible to control leds in sysfs.

But if you turn off ATH5K_LEDS??

johannes

Dmytro Milinevskyy

author

2010-06-12 02:02 UTC #8 lore raw

In this case no way to control LEDs. If you turn LEDs support you can't tune them out, no?

I believe that user should be able to chose whether LEDs support is needed or not. This is how done in intel wireless drivers. The driver should be tolerant.

-- Dima

Quoted 8 lines · Johannes Berg

Quoted 1 line · Dmytro Milinevskyy
Generic LED class is not disabled so it's possible to control leds in sysfs.
But if you turn off ATH5K_LEDS??
johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html