2010-06-15 05:20:23

by Vivek Natarajan

[permalink] [raw]
Subject: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

Some vendors require the LED to be ON always irrespective of any
radio activity. Introducing a module parameter to enable this,
so that one can choose between always on or led blink during
activity.

Signed-off-by: Vivek Natarajan <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/gpio.c | 9 ++++++---
drivers/net/wireless/ath/ath9k/init.c | 4 ++++
drivers/net/wireless/ath/ath9k/main.c | 4 +++-
4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 8d163ae..3a14630 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -628,6 +628,7 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)

extern struct ieee80211_ops ath9k_ops;
extern int modparam_nohwcrypt;
+extern int led_blink;

irqreturn_t ath_isr(int irq, void *dev);
int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index 0ee75e7..3a8ee99 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -76,7 +76,8 @@ static void ath_led_brightness(struct led_classdev *led_cdev,
case LED_FULL:
if (led->led_type == ATH_LED_ASSOC) {
sc->sc_flags |= SC_OP_LED_ASSOCIATED;
- ieee80211_queue_delayed_work(sc->hw,
+ if (led_blink)
+ ieee80211_queue_delayed_work(sc->hw,
&sc->ath_led_blink_work, 0);
} else if (led->led_type == ATH_LED_RADIO) {
ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 0);
@@ -143,7 +144,8 @@ void ath_init_leds(struct ath_softc *sc)
/* LED off, active low */
ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 1);

- INIT_DELAYED_WORK(&sc->ath_led_blink_work, ath_led_blink_work);
+ if (led_blink)
+ INIT_DELAYED_WORK(&sc->ath_led_blink_work, ath_led_blink_work);

trigger = ieee80211_get_radio_led_name(sc->hw);
snprintf(sc->radio_led.name, sizeof(sc->radio_led.name),
@@ -180,7 +182,8 @@ void ath_init_leds(struct ath_softc *sc)
return;

fail:
- cancel_delayed_work_sync(&sc->ath_led_blink_work);
+ if (led_blink)
+ cancel_delayed_work_sync(&sc->ath_led_blink_work);
ath_deinit_leds(sc);
}

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 514a401..b2bf0e8 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -33,6 +33,10 @@ int modparam_nohwcrypt;
module_param_named(nohwcrypt, modparam_nohwcrypt, int, 0444);
MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption");

+int led_blink;
+module_param_named(blink, led_blink, int, 0444);
+MODULE_PARM_DESC(blink, "Enable LED blink on activity");
+
/* We use the hw_value as an index into our private channel structure */

#define CHAN2G(_freq, _idx) { \
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index c8de50f..5af2596 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1241,7 +1241,9 @@ static void ath9k_stop(struct ieee80211_hw *hw)

aphy->state = ATH_WIPHY_INACTIVE;

- cancel_delayed_work_sync(&sc->ath_led_blink_work);
+ if (led_blink)
+ cancel_delayed_work_sync(&sc->ath_led_blink_work);
+
cancel_delayed_work_sync(&sc->tx_complete_work);
cancel_work_sync(&sc->paprd_work);

--
1.7.1



Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Tue, Jun 15, 2010 at 01:02:20PM +0530, Johannes Berg wrote:
> On Tue, 2010-06-15 at 10:50 +0530, Vivek Natarajan wrote:
>
> > case LED_FULL:
>
> I wonder if you can just use different triggers which may be easier?
> Look at what I suggested earlier for integrating the LEDs with iwlwifi:
>
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/51218/focus=51520

Sorry for a late reply. LED implementation with ath9k is completely
manual, fully controlled by driver (We have to set/clear a particular
gpio pin to turn on/off the LED every time). on/off period during blink
is based on the number of brightness_set() on tx/rx led trigger. Do
you think your suggested implementation would help in this case too?

thanks,

Vasanth

2010-06-16 05:51:29

by Vivek Natarajan

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Wed, Jun 16, 2010 at 1:21 AM, John W. Linville
<[email protected]> wrote:
> On Tue, Jun 15, 2010 at 10:50:17AM +0530, Vivek Natarajan wrote:
>> Some vendors require the LED to be ON always irrespective of any
>> radio activity. Introducing a module parameter to enable this,
>> so that one can choose between always on or led blink during
>> activity.
>>
>> Signed-off-by: Vivek Natarajan <[email protected]>
>
> Any particular reason the always-on behaviour is the default?

There is no specific reason for setting it as default. It is only that
some customers preferred the led to be always on instead of blinking.

Thanks
Vivek.

2010-06-22 05:46:02

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Wed, Jun 16, 2010 at 07:08:24PM +0530, John W. Linville wrote:
> On Wed, Jun 16, 2010 at 11:21:27AM +0530, Vivek Natarajan wrote:
> > On Wed, Jun 16, 2010 at 1:21 AM, John W. Linville
> > <[email protected]> wrote:
> > > On Tue, Jun 15, 2010 at 10:50:17AM +0530, Vivek Natarajan wrote:
> > >> Some vendors require the LED to be ON always irrespective of any
> > >> radio activity. Introducing a module parameter to enable this,
> > >> so that one can choose between always on or led blink during
> > >> activity.
> > >>
> > >> Signed-off-by: Vivek Natarajan <[email protected]>
> > >
> > > Any particular reason the always-on behaviour is the default?
> >
> > There is no specific reason for setting it as default. It is only that
> > some customers preferred the led to be always on instead of blinking.
>
> Well it is just that as I read the patch, you are changing the
> behaviour for everyone rather than simply giving a new option for
> those that don't like it that way it already is.
>
> Look, I don't particular care about the behaviour. Blinking w/
> activity makes sense to me. But I also get to read the seemingly
> endless, whining complaints about how the blinking is distracting for
> laptop users. I just would rather not add another endless stream of
> complaints that say "my wifi LED used to blink w/ activity, now it
> doesn't -- fix it!"
I agree with John. Let's not change the existing default pattern
(blinking during activity) and use this option to disable blinking
if someone wants it.
>
> Perhaps we could standardize this somehow? Anyone care to make
> a proposal?
>
> John
> --
> John W. Linville Someday the world will need a hero, and you
> [email protected] might be all we have. Be ready.
> --
> 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

2010-06-16 13:45:10

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Wed, Jun 16, 2010 at 11:21:27AM +0530, Vivek Natarajan wrote:
> On Wed, Jun 16, 2010 at 1:21 AM, John W. Linville
> <[email protected]> wrote:
> > On Tue, Jun 15, 2010 at 10:50:17AM +0530, Vivek Natarajan wrote:
> >> Some vendors require the LED to be ON always irrespective of any
> >> radio activity. Introducing a module parameter to enable this,
> >> so that one can choose between always on or led blink during
> >> activity.
> >>
> >> Signed-off-by: Vivek Natarajan <[email protected]>
> >
> > Any particular reason the always-on behaviour is the default?
>
> There is no specific reason for setting it as default. It is only that
> some customers preferred the led to be always on instead of blinking.

Well it is just that as I read the patch, you are changing the
behaviour for everyone rather than simply giving a new option for
those that don't like it that way it already is.

Look, I don't particular care about the behaviour. Blinking w/
activity makes sense to me. But I also get to read the seemingly
endless, whining complaints about how the blinking is distracting for
laptop users. I just would rather not add another endless stream of
complaints that say "my wifi LED used to blink w/ activity, now it
doesn't -- fix it!"

Perhaps we could standardize this somehow? Anyone care to make
a proposal?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-06-15 07:32:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Tue, 2010-06-15 at 10:50 +0530, Vivek Natarajan wrote:

> case LED_FULL:

I wonder if you can just use different triggers which may be easier?
Look at what I suggested earlier for integrating the LEDs with iwlwifi:

http://thread.gmane.org/gmane.linux.kernel.wireless.general/51218/focus=51520

Up to you, of course. I would love to do the iwlwifi stuff I suggested
but simply don't have time for it right now.

johannes


2010-06-15 20:00:12

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Tue, Jun 15, 2010 at 10:50:17AM +0530, Vivek Natarajan wrote:
> Some vendors require the LED to be ON always irrespective of any
> radio activity. Introducing a module parameter to enable this,
> so that one can choose between always on or led blink during
> activity.
>
> Signed-off-by: Vivek Natarajan <[email protected]>

Any particular reason the always-on behaviour is the default?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-06-22 07:01:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Tue, 2010-06-22 at 12:08 +0530, Vasanthakumar Thiagarajan wrote:

> > http://thread.gmane.org/gmane.linux.kernel.wireless.general/51218/focus=51520
>
> Sorry for a late reply. LED implementation with ath9k is completely
> manual, fully controlled by driver (We have to set/clear a particular
> gpio pin to turn on/off the LED every time). on/off period during blink
> is based on the number of brightness_set() on tx/rx led trigger. Do
> you think your suggested implementation would help in this case too?

I'm not sure. I was mostly describing the use in iwlwifi, and since I
don't know how ath9k works I can't really comment much. It just seemed
to me that the driver was also hard-coding the behaviour of the LEDs,
which the user in that thread didn't like for iwlwifi, so integrating
with the LED trigger stuff could be worthwhile for ath9k? Or maybe it
does and I'm reading the patches incorrectly.

johannes


Subject: Re: [PATCH] ath9k: Modify LED blinking pattern during wifi activity.

On Tue, Jun 22, 2010 at 12:31:27PM +0530, Johannes Berg wrote:
> On Tue, 2010-06-22 at 12:08 +0530, Vasanthakumar Thiagarajan wrote:
>
> > > http://thread.gmane.org/gmane.linux.kernel.wireless.general/51218/focus=51520
> >
> > Sorry for a late reply. LED implementation with ath9k is completely
> > manual, fully controlled by driver (We have to set/clear a particular
> > gpio pin to turn on/off the LED every time). on/off period during blink
> > is based on the number of brightness_set() on tx/rx led trigger. Do
> > you think your suggested implementation would help in this case too?
>
> I'm not sure. I was mostly describing the use in iwlwifi, and since I
> don't know how ath9k works I can't really comment much.

With ath9k hw itself does not blink LED based on the traffic, it is
the driver which turns on/off LED based on tx/rx.

> It just seemed
> to me that the driver was also hard-coding the behaviour of the LEDs,
> which the user in that thread didn't like for iwlwifi.

Yeah, before Vivek's patch the driver was hard-coding the LED
behaviour. After this patch, the behaviour would be based on
a modparam.

> so integrating
> with the LED trigger stuff could be worthwhile for ath9k? Or maybe it
> does and I'm reading the patches incorrectly.

The existing LED implementation looks simple. As the blink rate is
determined in driver dynamically, I'm not quite sure integrating LED
trigger, would review this in detail later.

Vasanth