Skip to contentSkip to conversation

ath5k: convert LED code to use mac80211 triggers

PATCH In review Bob Copeland 3 participants 9 messages Jun 22, 2008 – Jul 9, 2008 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ath5k: convert LED code to use mac80211 triggers

Clean up the ath5k LED code and convert it to use the standard LED device class along with the rx/tx LED triggers provided by mac80211. This moves ath5k in line with other wireless drivers that have software LEDs.

---

I tested this with printk but I don't have the IBM 5212/5211/Compaq hardware to fully test. If anyone can give it a spin or has comments please let me know.

 drivers/net/wireless/ath5k/Kconfig |    2 +
 drivers/net/wireless/ath5k/base.c  |  255 ++++++++++++++++--------------------
 drivers/net/wireless/ath5k/base.h  |   32 +++--
 3 files changed, 133 insertions(+), 156 deletions(-)
drivers/net/wireless/ath5k/Kconfig +2 -0
index f1f2aea..314e89f 100644
@@ -1,6 +1,8 @@
 config ATH5K
 	tristate "Atheros 5xxx wireless cards support"
 	depends on PCI && MAC80211 && WLAN_80211 && EXPERIMENTAL
+	select MAC80211_LEDS
+	select LEDS_CLASS
 	---help---
 	  This module adds support for wireless adapters based on
 	  Atheros 5xxx chipset.
drivers/net/wireless/ath5k/base.c +113 -142

353 lines not on this page · raw message

drivers/net/wireless/ath5k/base.h +18 -14
index bb4b26d..47f414b 100644
@@ -45,6 +45,7 @@
 #include <linux/list.h>
 #include <linux/wireless.h>
 #include <linux/if_ether.h>
+#include <linux/leds.h>
 
 #include "ath5k.h"
 #include "debug.h"
@@ -79,6 +80,19 @@ struct ath5k_txq {
 	bool			setup;
 };
 
+#define ATH5K_LED_MAX_NAME_LEN 31
+
+/*
+ * State for LED triggers
+ */
+struct ath5k_led
+{
+	char name[ATH5K_LED_MAX_NAME_LEN + 1];	/* name of the LED in sysfs */
+	struct ath5k_softc *sc;			/* driver state */
+	struct led_classdev led_dev;		/* led classdev */
+};
+
+
 #if CHAN_DEBUG
 #define ATH_CHAN_MAX	(26+26+26+200+200)
 #else
@@ -118,13 +132,11 @@ struct ath5k_softc {
 	size_t			desc_len;	/* size of TX/RX descriptors */
 	u16			cachelsz;	/* cache line size */
 
-	DECLARE_BITMAP(status, 6);
+	DECLARE_BITMAP(status, 4);
 #define ATH_STAT_INVALID	0		/* disable hardware accesses */
 #define ATH_STAT_MRRETRY	1		/* multi-rate retry support */
 #define ATH_STAT_PROMISC	2
-#define ATH_STAT_LEDBLINKING	3		/* LED blink operation active */
-#define ATH_STAT_LEDENDBLINK	4		/* finish LED blink operation */
-#define ATH_STAT_LEDSOFT	5		/* enable LED gpio status */
+#define ATH_STAT_LEDSOFT	3		/* enable LED gpio status */
 
 	unsigned int		filter_flags;	/* HW flags, AR5K_RX_FILTER_* */
 	unsigned int		curmode;	/* current phy mode */
@@ -132,13 +144,6 @@ struct ath5k_softc {
 
 	struct ieee80211_vif *vif;
 
-	struct {
-		u8	rxflags;	/* radiotap rx flags */
-		u8	txflags;	/* radiotap tx flags */
-		u16	ledon;		/* softled on time */
-		u16	ledoff;		/* softled off time */
-	} hwmap[32];				/* h/w rate ix mappings */
-
 	enum ath5k_int		imask;		/* interrupt mask copy */
 
 	DECLARE_BITMAP(keymap, AR5K_KEYCACHE_SIZE); /* key use bit map */
@@ -148,9 +153,6 @@ struct ath5k_softc {
 	unsigned int		led_pin,	/* GPIO pin for driving LED */
 				led_on,		/* pin setting for LED on */
 				led_off;	/* off time for current blink */
-	struct timer_list	led_tim;	/* led off timer */
-	u8			led_rxrate;	/* current rx rate for LED */
-	u8			led_txrate;	/* current tx rate for LED */
 
 	struct tasklet_struct	restq;		/* reset tasklet */
 
@@ -159,6 +161,7 @@ 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;
@@ -167,6 +170,7 @@ struct ath5k_softc {
 
 	struct ath5k_txq	*txq;		/* beacon and tx*/
 	struct tasklet_struct	txtq;		/* tx intr tasklet */
+	struct ath5k_led	tx_led;		/* tx led */
 
 	struct ath5k_buf	*bbuf;		/* beacon buffer */
 	unsigned int		bhalq,		/* SW q for outgoing beacons */
Signature · 4 lines
1.5.4.2.182.gb3092

-- 
Bob Copeland %% www.bobcopeland.com 
Acked-by on v1 1/1
#1 lore raw

2008/6/22 Bob Copeland <[email protected]>:

Quoted 491 lines

Clean up the ath5k LED code and convert it to use the standard LED device class along with the rx/tx LED triggers provided by mac80211. This moves ath5k in line with other wireless drivers that have software LEDs.

---

I tested this with printk but I don't have the IBM 5212/5211/Compaq hardware to fully test. If anyone can give it a spin or has comments please let me know.

 drivers/net/wireless/ath5k/Kconfig |    2 +
 drivers/net/wireless/ath5k/base.c  |  255 ++++++++++++++++--------------------
 drivers/net/wireless/ath5k/base.h  |   32 +++--
 3 files changed, 133 insertions(+), 156 deletions(-)
drivers/net/wireless/ath5k/Kconfig +2 -0
index f1f2aea..314e89f 100644
@@ -1,6 +1,8 @@
 config ATH5K
       tristate "Atheros 5xxx wireless cards support"
       depends on PCI && MAC80211 && WLAN_80211 && EXPERIMENTAL
+       select MAC80211_LEDS
+       select LEDS_CLASS
       ---help---
         This module adds support for wireless adapters based on
         Atheros 5xxx chipset.
drivers/net/wireless/ath5k/base.c +113 -142
index 85045af..a66d8b4 100644
@@ -58,11 +58,6 @@
 #include "reg.h"
 #include "debug.h"
 
-enum {
-       ATH_LED_TX,
-       ATH_LED_RX,
-};
-
 static int ath5k_calinterval = 10; /* Calibrate PHY every 10 secs (TODO: Fixme) */
 
 
@@ -309,13 +304,10 @@ static void       ath5k_tasklet_reset(unsigned long data);
 
 static void    ath5k_calibrate(unsigned long data);
 /* LED functions */
-static void    ath5k_led_off(unsigned long data);
-static void    ath5k_led_blink(struct ath5k_softc *sc,
-                               unsigned int on,
-                               unsigned int off);
-static void    ath5k_led_event(struct ath5k_softc *sc,
-                               int event);
-
+static int     ath5k_init_leds(struct ath5k_softc *sc);
+static void    ath5k_led_enable(struct ath5k_softc *sc);
+static void    ath5k_led_off(struct ath5k_softc *sc);
+static void    ath5k_unregister_leds(struct ath5k_softc *sc);
 
 /*
 * Module init/exit functions
@@ -596,8 +588,7 @@ ath5k_pci_suspend(struct pci_dev *pdev, pm_message_t state)
       struct ieee80211_hw *hw = pci_get_drvdata(pdev);
       struct ath5k_softc *sc = hw->priv;
 
-       if (test_bit(ATH_STAT_LEDSOFT, sc->status))
-               ath5k_hw_set_gpio(sc->ah, sc->led_pin, 1);
+       ath5k_led_off(sc);
 
       ath5k_stop_hw(sc);
       pci_save_state(pdev);
@@ -632,10 +623,7 @@ ath5k_pci_resume(struct pci_dev *pdev)
       pci_write_config_byte(pdev, 0x41, 0);
 
       ath5k_init(sc);
-       if (test_bit(ATH_STAT_LEDSOFT, sc->status)) {
-               ath5k_hw_set_gpio_output(ah, sc->led_pin);
-               ath5k_hw_set_gpio(ah, sc->led_pin, 0);
-       }
+       ath5k_led_enable(sc);
 
       /*
        * Reset the key cache since some parts do not
@@ -742,27 +730,6 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)
       tasklet_init(&sc->txtq, ath5k_tasklet_tx, (unsigned long)sc);
       tasklet_init(&sc->restq, ath5k_tasklet_reset, (unsigned long)sc);
       setup_timer(&sc->calib_tim, ath5k_calibrate, (unsigned long)sc);
-       setup_timer(&sc->led_tim, ath5k_led_off, (unsigned long)sc);
-
-       sc->led_on = 0; /* low true */
-       /*
-        * Auto-enable soft led processing for IBM cards and for
-        * 5211 minipci cards.
-        */
-       if (pdev->device == PCI_DEVICE_ID_ATHEROS_AR5212_IBM ||
-                       pdev->device == PCI_DEVICE_ID_ATHEROS_AR5211) {
-               __set_bit(ATH_STAT_LEDSOFT, sc->status);
-               sc->led_pin = 0;
-       }
-       /* Enable softled on PIN1 on HP Compaq nc6xx, nc4000 & nx5000 laptops */
-       if (pdev->subsystem_vendor == PCI_VENDOR_ID_COMPAQ) {
-               __set_bit(ATH_STAT_LEDSOFT, sc->status);
-               sc->led_pin = 0;
-       }
-       if (test_bit(ATH_STAT_LEDSOFT, sc->status)) {
-               ath5k_hw_set_gpio_output(ah, sc->led_pin);
-               ath5k_hw_set_gpio(ah, sc->led_pin, !sc->led_on);
-       }
 
       ath5k_hw_get_lladdr(ah, mac);
       SET_IEEE80211_PERM_ADDR(hw, mac);
@@ -776,6 +743,8 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)
               goto err_queues;
       }
 
+       ath5k_init_leds(sc);
+
       return 0;
 err_queues:
       ath5k_txq_release(sc);
@@ -809,6 +778,7 @@ ath5k_detach(struct pci_dev *pdev, struct ieee80211_hw *hw)
       ath5k_desc_free(sc, pdev);
       ath5k_txq_release(sc);
       ath5k_hw_release_tx_queue(sc->ah, sc->bhalq);
+       ath5k_unregister_leds(sc);
 
       /*
        * NB: can't reclaim these until after ieee80211_ifdetach
@@ -1060,65 +1030,9 @@ ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan)
       return 0;
 }
 
-/*
- * TODO: CLEAN THIS !!!
- */
 static void
 ath5k_setcurmode(struct ath5k_softc *sc, unsigned int mode)
 {
-       if (unlikely(test_bit(ATH_STAT_LEDSOFT, sc->status))) {
-               /* from Atheros NDIS driver, w/ permission */
-               static const struct {
-                       u16 rate;       /* tx/rx 802.11 rate */
-                       u16 timeOn;     /* LED on time (ms) */
-                       u16 timeOff;    /* LED off time (ms) */
-               } blinkrates[] = {
-                       { 108,  40,  10 },
-                       {  96,  44,  11 },
-                       {  72,  50,  13 },
-                       {  48,  57,  14 },
-                       {  36,  67,  16 },
-                       {  24,  80,  20 },
-                       {  22, 100,  25 },
-                       {  18, 133,  34 },
-                       {  12, 160,  40 },
-                       {  10, 200,  50 },
-                       {   6, 240,  58 },
-                       {   4, 267,  66 },
-                       {   2, 400, 100 },
-                       {   0, 500, 130 }
-               };
-               const struct ath5k_rate_table *rt =
-                               ath5k_hw_get_rate_table(sc->ah, mode);
-               unsigned int i, j;
-
-               BUG_ON(rt == NULL);
-
-               memset(sc->hwmap, 0, sizeof(sc->hwmap));
-               for (i = 0; i < 32; i++) {
-                       u8 ix = rt->rate_code_to_index[i];
-                       if (ix == 0xff) {
-                               sc->hwmap[i].ledon = msecs_to_jiffies(500);
-                               sc->hwmap[i].ledoff = msecs_to_jiffies(130);
-                               continue;
-                       }
-                       sc->hwmap[i].txflags = IEEE80211_RADIOTAP_F_DATAPAD;
-                       /* receive frames include FCS */
-                       sc->hwmap[i].rxflags = sc->hwmap[i].txflags |
-                                       IEEE80211_RADIOTAP_F_FCS;
-                       /* setup blink rate table to avoid per-packet lookup */
-                       for (j = 0; j < ARRAY_SIZE(blinkrates) - 1; j++)
-                               if (blinkrates[j].rate == /* XXX why 7f? */
-                                               (rt->rates[ix].dot11_rate&0x7f))
-                                       break;
-
-                       sc->hwmap[i].ledon = msecs_to_jiffies(blinkrates[j].
-                                       timeOn);
-                       sc->hwmap[i].ledoff = msecs_to_jiffies(blinkrates[j].
-                                       timeOff);
-               }
-       }
-
       sc->curmode = mode;
 
       if (mode == AR5K_MODE_11A) {
@@ -1903,8 +1817,6 @@ accept:
                       ath5k_check_ibss_tsf(sc, skb, &rxs);
 
               __ieee80211_rx(sc->hw, skb, &rxs);
-               sc->led_rxrate = rs.rs_rate;
-               ath5k_led_event(sc, ATH_LED_RX);
 next:
               list_move_tail(&bf->list, &sc->rxbuf);
       } while (ath5k_rxbuf_setup(sc, bf) == 0);
@@ -1985,13 +1897,9 @@ ath5k_tasklet_tx(unsigned long data)
       struct ath5k_softc *sc = (void *)data;
 
       ath5k_tx_processq(sc, sc->txq);
-
-       ath5k_led_event(sc, ATH_LED_TX);
 }
 
 
-
-
 /*****************\
 * Beacon handling *
 \*****************/
@@ -2366,11 +2274,7 @@ ath5k_stop_locked(struct ath5k_softc *sc)
       ieee80211_stop_queues(sc->hw);
 
       if (!test_bit(ATH_STAT_INVALID, sc->status)) {
-               if (test_bit(ATH_STAT_LEDSOFT, sc->status)) {
-                       del_timer_sync(&sc->led_tim);
-                       ath5k_hw_set_gpio(ah, sc->led_pin, !sc->led_on);
-                       __clear_bit(ATH_STAT_LEDBLINKING, sc->status);
-               }
+               ath5k_led_off(sc);
               ath5k_hw_set_intr(ah, 0);
       }
       ath5k_txq_cleanup(sc);
@@ -2566,54 +2470,123 @@ ath5k_calibrate(unsigned long data)
 \***************/
 
 static void
-ath5k_led_off(unsigned long data)
+ath5k_led_enable(struct ath5k_softc *sc)
 {
-       struct ath5k_softc *sc = (void *)data;
-
-       if (test_bit(ATH_STAT_LEDENDBLINK, sc->status))
-               __clear_bit(ATH_STAT_LEDBLINKING, sc->status);
-       else {
-               __set_bit(ATH_STAT_LEDENDBLINK, sc->status);
-               ath5k_hw_set_gpio(sc->ah, sc->led_pin, !sc->led_on);
-               mod_timer(&sc->led_tim, jiffies + sc->led_off);
+       if (test_bit(ATH_STAT_LEDSOFT, sc->status)) {
+               ath5k_hw_set_gpio_output(sc->ah, sc->led_pin);
+               ath5k_hw_set_gpio(sc->ah, sc->led_pin, 0);
       }
 }
 
-/*
- * Blink the LED according to the specified on/off times.
- */
-static void
-ath5k_led_blink(struct ath5k_softc *sc, unsigned int on,
-               unsigned int off)
+static void
+ath5k_led_on(struct ath5k_softc *sc)
 {
-       ATH5K_DBG(sc, ATH5K_DEBUG_LED, "on %u off %u\n", on, off);
+       if (!test_bit(ATH_STAT_LEDSOFT, sc->status))
+               return;
       ath5k_hw_set_gpio(sc->ah, sc->led_pin, sc->led_on);
-       __set_bit(ATH_STAT_LEDBLINKING, sc->status);
-       __clear_bit(ATH_STAT_LEDENDBLINK, sc->status);
-       sc->led_off = off;
-       mod_timer(&sc->led_tim, jiffies + on);
 }
 
-static void
-ath5k_led_event(struct ath5k_softc *sc, int event)
+static void
+ath5k_led_off(struct ath5k_softc *sc)
 {
-       if (likely(!test_bit(ATH_STAT_LEDSOFT, sc->status)))
+       if (!test_bit(ATH_STAT_LEDSOFT, sc->status))
               return;
-       if (unlikely(test_bit(ATH_STAT_LEDBLINKING, sc->status)))
-               return; /* don't interrupt active blink */
-       switch (event) {
-       case ATH_LED_TX:
-               ath5k_led_blink(sc, sc->hwmap[sc->led_txrate].ledon,
-                       sc->hwmap[sc->led_txrate].ledoff);
-               break;
-       case ATH_LED_RX:
-               ath5k_led_blink(sc, sc->hwmap[sc->led_rxrate].ledon,
-                       sc->hwmap[sc->led_rxrate].ledoff);
-               break;
+       ath5k_hw_set_gpio(sc->ah, sc->led_pin, !sc->led_on);
+}
+
+static void
+ath5k_led_brightness_set(struct led_classdev *led_dev,
+       enum led_brightness brightness)
+{
+       struct ath5k_led *led = container_of(led_dev, struct ath5k_led,
+               led_dev);
+
+       if (brightness == LED_OFF)
+               ath5k_led_off(led->sc);
+       else
+               ath5k_led_on(led->sc);
+}
+
+static int
+ath5k_register_led(struct ath5k_softc *sc, struct ath5k_led *led,
+                  const char *name, char *trigger)
+{
+       int err;
+
+       led->sc = sc;
+       strncpy(led->name, name, sizeof(led->name));
+       led->led_dev.name = led->name;
+       led->led_dev.default_trigger = trigger;
+       led->led_dev.brightness_set = ath5k_led_brightness_set;
+
+       err = led_classdev_register(&sc->pdev->dev, &led->led_dev);
+       if (err)
+       {
+               ATH5K_WARN(sc, "could not register LED %s\n", name);
+               led->sc = NULL;
       }
+       return err;
 }
 
+static void
+ath5k_unregister_led(struct ath5k_led *led)
+{
+       if (!led->sc)
+               return;
+       led_classdev_unregister(&led->led_dev);
+       ath5k_led_off(led->sc);
+       led->sc = NULL;
+}
 
+static void
+ath5k_unregister_leds(struct ath5k_softc *sc)
+{
+       ath5k_unregister_led(&sc->rx_led);
+       ath5k_unregister_led(&sc->tx_led);
+}
+
+
+static int
+ath5k_init_leds(struct ath5k_softc *sc)
+{
+       int ret = 0;
+       struct ieee80211_hw *hw = sc->hw;
+       struct pci_dev *pdev = sc->pdev;
+       char name[ATH5K_LED_MAX_NAME_LEN + 1];
+
+       sc->led_on = 0;  /* active low */
+
+       /*
+        * Auto-enable soft led processing for IBM cards and for
+        * 5211 minipci cards.
+        */
+       if (pdev->device == PCI_DEVICE_ID_ATHEROS_AR5212_IBM ||
+           pdev->device == PCI_DEVICE_ID_ATHEROS_AR5211) {
+               __set_bit(ATH_STAT_LEDSOFT, sc->status);
+               sc->led_pin = 0;
+       }
+       /* Enable softled on PIN1 on HP Compaq nc6xx, nc4000 & nx5000 laptops */
+       if (pdev->subsystem_vendor == PCI_VENDOR_ID_COMPAQ) {
+               __set_bit(ATH_STAT_LEDSOFT, sc->status);
+               sc->led_pin = 1;
+       }
+       if (!test_bit(ATH_STAT_LEDSOFT, sc->status))
+               goto out;
+
+       ath5k_led_enable(sc);
+
+       snprintf(name, sizeof(name), "ath5k-%s::rx", wiphy_name(hw->wiphy));
+       ret = ath5k_register_led(sc, &sc->rx_led, name,
+               ieee80211_get_rx_led_name(hw));
+       if (ret)
+               goto out;
+
+       snprintf(name, sizeof(name), "ath5k-%s::tx", wiphy_name(hw->wiphy));
+       ret = ath5k_register_led(sc, &sc->tx_led, name,
+               ieee80211_get_tx_led_name(hw));
+out:
+       return ret;
+}
 
 
 /********************\
@@ -2651,8 +2624,6 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
               memmove(skb->data, skb->data+pad, hdrlen);
       }
 
-       sc->led_txrate = ieee80211_get_tx_rate(hw, info)->hw_value;
-
       spin_lock_irqsave(&sc->txbuflock, flags);
       if (list_empty(&sc->txbuf)) {
               ATH5K_ERR(sc, "no further txbuf available, dropping packet\n");
drivers/net/wireless/ath5k/base.h +18 -15
index bb4b26d..47f414b 100644
@@ -45,6 +45,7 @@
 #include <linux/list.h>
 #include <linux/wireless.h>
 #include <linux/if_ether.h>
+#include <linux/leds.h>
 
 #include "ath5k.h"
 #include "debug.h"
@@ -79,6 +80,19 @@ struct ath5k_txq {
       bool                    setup;
 };
 
+#define ATH5K_LED_MAX_NAME_LEN 31
+
+/*
+ * State for LED triggers
+ */
+struct ath5k_led
+{
+       char name[ATH5K_LED_MAX_NAME_LEN + 1];  /* name of the LED in sysfs */
+       struct ath5k_softc *sc;                 /* driver state */
+       struct led_classdev led_dev;            /* led classdev */
+};
+
+
 #if CHAN_DEBUG
 #define ATH_CHAN_MAX   (26+26+26+200+200)
 #else
@@ -118,13 +132,11 @@ struct ath5k_softc {
       size_t                  desc_len;       /* size of TX/RX descriptors */
       u16                     cachelsz;       /* cache line size */
 
-       DECLARE_BITMAP(status, 6);
+       DECLARE_BITMAP(status, 4);
 #define ATH_STAT_INVALID       0               /* disable hardware accesses */
 #define ATH_STAT_MRRETRY       1               /* multi-rate retry support */
 #define ATH_STAT_PROMISC       2
-#define ATH_STAT_LEDBLINKING   3               /* LED blink operation active */
-#define ATH_STAT_LEDENDBLINK   4               /* finish LED blink operation */
-#define ATH_STAT_LEDSOFT       5               /* enable LED gpio status */
+#define ATH_STAT_LEDSOFT       3               /* enable LED gpio status */
 
       unsigned int            filter_flags;   /* HW flags, AR5K_RX_FILTER_* */
       unsigned int            curmode;        /* current phy mode */
@@ -132,13 +144,6 @@ struct ath5k_softc {
 
       struct ieee80211_vif *vif;
 
-       struct {
-               u8      rxflags;        /* radiotap rx flags */
-               u8      txflags;        /* radiotap tx flags */
-               u16     ledon;          /* softled on time */
-               u16     ledoff;         /* softled off time */
-       } hwmap[32];                            /* h/w rate ix mappings */
-
       enum ath5k_int          imask;          /* interrupt mask copy */
 
       DECLARE_BITMAP(keymap, AR5K_KEYCACHE_SIZE); /* key use bit map */
@@ -148,9 +153,6 @@ struct ath5k_softc {
       unsigned int            led_pin,        /* GPIO pin for driving LED */
                               led_on,         /* pin setting for LED on */
                               led_off;        /* off time for current blink */
-       struct timer_list       led_tim;        /* led off timer */
-       u8                      led_rxrate;     /* current rx rate for LED */
-       u8                      led_txrate;     /* current tx rate for LED */
 
       struct tasklet_struct   restq;          /* reset tasklet */
 
@@ -159,6 +161,7 @@ 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;
@@ -167,6 +170,7 @@ struct ath5k_softc {
 
       struct ath5k_txq        *txq;           /* beacon and tx*/
       struct tasklet_struct   txtq;           /* tx intr tasklet */
+       struct ath5k_led        tx_led;         /* tx led */
 
       struct ath5k_buf        *bbuf;          /* beacon buffer */
       unsigned int            bhalq,          /* SW q for outgoing beacons */
--

1.5.4.2.182.gb3092

Quoted 4 lines

--
Bob Copeland %% www.bobcopeland.com

I don't have an IBM card either, anyway we need the cleanup more than blinking leds :-)

Signature · 3 lines
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
#2 lore raw

I do have a micro-update to this after reading over it one last time. I'll post a respin tonight.

Quoted 4 lines

drivers/net/wireless/ath5k/Kconfig line 5

Quoted 4 lines

tristate "Atheros 5xxx wireless cards support" depends on PCI && MAC80211 && WLAN_80211 && EXPERIMENTAL

drivers/net/wireless/ath5k/Kconfig +2 -0
+       select MAC80211_LEDS
+       select LEDS_CLASS

+ select NEW_LEDS

Randy Dunlap posted a patch series a few weeks ago that said NEW_LEDS was required where LEDS_CLASS was used.

Quoted 2 lines
Quoted 2 lines
drivers/net/wireless/ath5k/base.c +1 -1
-ath5k_led_off(unsigned long data)
+ath5k_led_enable(struct ath5k_softc *sc)

[..]

Quoted 3 lines

drivers/net/wireless/ath5k/base.c ath5k_calibrate(unsigned long data) · line 2477

Quoted 3 lines
drivers/net/wireless/ath5k/base.c +3 -0
+       if (test_bit(ATH_STAT_LEDSOFT, sc->status)) {
+               ath5k_hw_set_gpio_output(sc->ah, sc->led_pin);
+               ath5k_hw_set_gpio(sc->ah, sc->led_pin, 0);

That was carried over from the original code but I think it was supposed to be:

Quoted 1 line
Quoted 1 line

+ ath5k_hw_set_gpio(sc->ah, sc->led_pin, !sc->led_on);

(This changes it from 'on' to 'off' but either way I'm not sure it matters.)

Quoted 5 lines

I don't have an IBM card either, anyway we need the cleanup more than blinking leds :-)

Thanks!

Signature · 1 line
Bob Copeland %% www.bobcopeland.com
#3 lore raw

This change cleans up the ath5k LED code and converts it to use the standard LED device class along with the rx/tx LED triggers provided by mac80211.

---

Final version with small correction to led_enable and select NEW_LEDS

 drivers/net/wireless/ath5k/Kconfig |    3 +
 drivers/net/wireless/ath5k/base.c  |  255 ++++++++++++++++--------------------
 drivers/net/wireless/ath5k/base.h  |   32 +++--
 3 files changed, 134 insertions(+), 156 deletions(-)
drivers/net/wireless/ath5k/Kconfig +3 -0
index f1f2aea..75383a5 100644
@@ -1,6 +1,9 @@
 config ATH5K
 	tristate "Atheros 5xxx wireless cards support"
 	depends on PCI && MAC80211 && WLAN_80211 && EXPERIMENTAL
+	select MAC80211_LEDS
+	select LEDS_CLASS
+	select NEW_LEDS
 	---help---
 	  This module adds support for wireless adapters based on
 	  Atheros 5xxx chipset.
drivers/net/wireless/ath5k/base.c +113 -142

353 lines not on this page · raw message

drivers/net/wireless/ath5k/base.h +18 -14
index bb4b26d..47f414b 100644
@@ -45,6 +45,7 @@
 #include <linux/list.h>
 #include <linux/wireless.h>
 #include <linux/if_ether.h>
+#include <linux/leds.h>
 
 #include "ath5k.h"
 #include "debug.h"
@@ -79,6 +80,19 @@ struct ath5k_txq {
 	bool			setup;
 };
 
+#define ATH5K_LED_MAX_NAME_LEN 31
+
+/*
+ * State for LED triggers
+ */
+struct ath5k_led
+{
+	char name[ATH5K_LED_MAX_NAME_LEN + 1];	/* name of the LED in sysfs */
+	struct ath5k_softc *sc;			/* driver state */
+	struct led_classdev led_dev;		/* led classdev */
+};
+
+
 #if CHAN_DEBUG
 #define ATH_CHAN_MAX	(26+26+26+200+200)
 #else
@@ -118,13 +132,11 @@ struct ath5k_softc {
 	size_t			desc_len;	/* size of TX/RX descriptors */
 	u16			cachelsz;	/* cache line size */
 
-	DECLARE_BITMAP(status, 6);
+	DECLARE_BITMAP(status, 4);
 #define ATH_STAT_INVALID	0		/* disable hardware accesses */
 #define ATH_STAT_MRRETRY	1		/* multi-rate retry support */
 #define ATH_STAT_PROMISC	2
-#define ATH_STAT_LEDBLINKING	3		/* LED blink operation active */
-#define ATH_STAT_LEDENDBLINK	4		/* finish LED blink operation */
-#define ATH_STAT_LEDSOFT	5		/* enable LED gpio status */
+#define ATH_STAT_LEDSOFT	3		/* enable LED gpio status */
 
 	unsigned int		filter_flags;	/* HW flags, AR5K_RX_FILTER_* */
 	unsigned int		curmode;	/* current phy mode */
@@ -132,13 +144,6 @@ struct ath5k_softc {
 
 	struct ieee80211_vif *vif;
 
-	struct {
-		u8	rxflags;	/* radiotap rx flags */
-		u8	txflags;	/* radiotap tx flags */
-		u16	ledon;		/* softled on time */
-		u16	ledoff;		/* softled off time */
-	} hwmap[32];				/* h/w rate ix mappings */
-
 	enum ath5k_int		imask;		/* interrupt mask copy */
 
 	DECLARE_BITMAP(keymap, AR5K_KEYCACHE_SIZE); /* key use bit map */
@@ -148,9 +153,6 @@ struct ath5k_softc {
 	unsigned int		led_pin,	/* GPIO pin for driving LED */
 				led_on,		/* pin setting for LED on */
 				led_off;	/* off time for current blink */
-	struct timer_list	led_tim;	/* led off timer */
-	u8			led_rxrate;	/* current rx rate for LED */
-	u8			led_txrate;	/* current tx rate for LED */
 
 	struct tasklet_struct	restq;		/* reset tasklet */
 
@@ -159,6 +161,7 @@ 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;
@@ -167,6 +170,7 @@ struct ath5k_softc {
 
 	struct ath5k_txq	*txq;		/* beacon and tx*/
 	struct tasklet_struct	txtq;		/* tx intr tasklet */
+	struct ath5k_led	tx_led;		/* tx led */
 
 	struct ath5k_buf	*bbuf;		/* beacon buffer */
 	unsigned int		bhalq,		/* SW q for outgoing beacons */
Signature · 1 line
1.5.4.2.182.gb3092
#4 lore raw
Quoted 3 lines · Bob Copeland

This change cleans up the ath5k LED code and converts it to use the standard LED device class along with the rx/tx LED triggers provided by mac80211.

=2E..

Quoted 2 lines

I tested this with printk but I don't have the IBM 5212/5211/Compaq hardware to fully test. =C2=A0If anyone can give it a spin or has com=

ments

Quoted 1 line

please let me know.

I tested it against Linus' head version (it applied with minor modifications) on my HP NC6000 laptop with this built-in Atheros card:

02:04.0 Ethernet controller: Atheros Communications, Inc. AR5212 802.11= abg
NIC (rev 01)
Subsystem: Compaq Computer Corporation Unknown device 00e5

Basically it works, but I think the sc->led_on in:

drivers/net/wireless/ath5k/base.c ath5k_calibrate(unsigned long data)

Quoted 9 lines
drivers/net/wireless/ath5k/base.c +9 -0
+static int
+ath5k_init_leds(struct ath5k_softc *sc)
+{
+     int ret =3D 0;
+     struct ieee80211_hw *hw =3D sc->hw;
+     struct pci_dev *pdev =3D sc->pdev;
+     char name[ATH5K_LED_MAX_NAME_LEN + 1];
+
+     sc->led_on =3D 0;  /* active low */

needs to be:

Quoted 1 line

+ sc->led_on =3D 1; /* active high */

for the AR5212 in the HP laptop.

See also the changes to if_ath_pci.c in:
http://madwifi.org/attachment/ticket/1018/led.patch
and
http://madwifi.org/ticket/1018
for an old discussion about madwifi for LEDs in the HP box.

Without this change the LED stays on after a "rmmod ath5k" as well.

Helge

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

#5 lore raw
Quoted 2 lines · Helge Deller

I tested it against Linus' head version (it applied with minor modifications) on my HP NC6000 laptop with this built-in Atheros card:

[...]

Quoted 7 lines

Basically it works, but I think the sc->led_on in:

Quoted 1 line

+ sc->led_on = 0; /* active low */

needs to be:

Quoted 1 line

+ sc->led_on = 1; /* active high */

for the AR5212 in the HP laptop.

Thanks for the test!

Quoted 5 lines

See also the changes to if_ath_pci.c in:
http://madwifi.org/attachment/ticket/1018/led.patch
and
http://madwifi.org/ticket/1018
for an old discussion about madwifi for LEDs in the HP box.

So current madwifi doesn't have this change unless I missed something?
It appears to use active-low leds in the current version regardless
of the vendor.

Quoted 1 line

Without this change the LED stays on after a "rmmod ath5k" as well.

Yeah, that would definitely happen if the logic was backwards. I guess the conservative approach is to just use sc->led_on = 1 in the VENDOR_ID_COMPAQ branch.

Signature · 1 line
Bob Copeland %% www.bobcopeland.com
#6 lore raw
Quoted 2 lines · I wrote:

I guess the conservative approach is to just use sc->led_on = 1 in the VENDOR_ID_COMPAQ branch

...here's the obvious patch for this, let me know if it works for you.

Subject: [PATCH] ath5k: use positive logic for HP laptop LEDs

Helge Deller reports that HP laptops (NC4010 and NC6000) use active- high signals to turn on the LEDs. Previous code used active-low for all devices.

---
 drivers/net/wireless/ath5k/base.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/wireless/ath5k/base.c +2 -2
index a43e9b2..fb6fd14 100644
@@ -2551,8 +2551,6 @@ ath5k_init_leds(struct ath5k_softc *sc)
 	struct pci_dev *pdev = sc->pdev;
 	char name[ATH5K_LED_MAX_NAME_LEN + 1];
 
-	sc->led_on = 0;  /* active low */
-
 	/*
 	 * Auto-enable soft led processing for IBM cards and for
 	 * 5211 minipci cards.
@@ -2561,11 +2559,13 @@ ath5k_init_leds(struct ath5k_softc *sc)
 	    pdev->device == PCI_DEVICE_ID_ATHEROS_AR5211) {
 		__set_bit(ATH_STAT_LEDSOFT, sc->status);
 		sc->led_pin = 0;
+		sc->led_on = 0;  /* active low */
 	}
 	/* Enable softled on PIN1 on HP Compaq nc6xx, nc4000 & nx5000 laptops */
 	if (pdev->subsystem_vendor == PCI_VENDOR_ID_COMPAQ) {
 		__set_bit(ATH_STAT_LEDSOFT, sc->status);
 		sc->led_pin = 1;
+		sc->led_on = 1;  /* active high */
 	}
 	if (!test_bit(ATH_STAT_LEDSOFT, sc->status))
 		goto out;
Signature · 1 line
1.5.4.2.182.gb3092
#7 lore raw
Quoted 5 lines · Bob Copeland
Quoted 2 lines · I wrote:

I guess the conservative approach is to just use sc->led_on = 1 in the VENDOR_ID_COMPAQ branch

...here's the obvious patch for this, let me know if it works for you.

Thanks Bob!
Patch works OK.

Helge

Quoted 38 lines

Subject: [PATCH] ath5k: use positive logic for HP laptop LEDs

Helge Deller reports that HP laptops (NC4010 and NC6000) use active- high signals to turn on the LEDs. Previous code used active-low for all devices.

---
 drivers/net/wireless/ath5k/base.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/wireless/ath5k/base.c +2 -2
index a43e9b2..fb6fd14 100644
@@ -2551,8 +2551,6 @@ ath5k_init_leds(struct ath5k_softc *sc)
 	struct pci_dev *pdev = sc->pdev;
 	char name[ATH5K_LED_MAX_NAME_LEN + 1];
 
-	sc->led_on = 0;  /* active low */
-
 	/*
 	 * Auto-enable soft led processing for IBM cards and for
 	 * 5211 minipci cards.
@@ -2561,11 +2559,13 @@ ath5k_init_leds(struct ath5k_softc *sc)
 	    pdev->device == PCI_DEVICE_ID_ATHEROS_AR5211) {
 		__set_bit(ATH_STAT_LEDSOFT, sc->status);
 		sc->led_pin = 0;
+		sc->led_on = 0;  /* active low */
 	}
 	/* Enable softled on PIN1 on HP Compaq nc6xx, nc4000 & nx5000 laptops */
 	if (pdev->subsystem_vendor == PCI_VENDOR_ID_COMPAQ) {
 		__set_bit(ATH_STAT_LEDSOFT, sc->status);
 		sc->led_pin = 1;
+		sc->led_on = 1;  /* active high */
 	}
 	if (!test_bit(ATH_STAT_LEDSOFT, sc->status))
 		goto out;
#8 lore raw
In reply to Bob Copeland
Quoted 12 lines · Bob Copeland
Quoted 2 lines · Helge Deller

I tested it against Linus' head version (it applied with minor modifications) on my HP NC6000 laptop with this built-in Atheros card:

[...]

Quoted 5 lines

Basically it works, but I think the sc->led_on in:

Quoted 1 line

+ sc->led_on = 0; /* active low */

needs to be:

Quoted 1 line

+ sc->led_on = 1; /* active high */

for the AR5212 in the HP laptop.

Thanks for the test!

Quoted 7 lines
Quoted 5 lines

See also the changes to if_ath_pci.c in:
http://madwifi.org/attachment/ticket/1018/led.patch
and
http://madwifi.org/ticket/1018
for an old discussion about madwifi for LEDs in the HP box.

So current madwifi doesn't have this change unless I missed something?

Yes, the patch has not been applied yet.

Quoted 2 lines

It appears to use active-low leds in the current version regardless
of the vendor.

Yes.

Helge