2010-06-23 06:38:35

by Vivek Natarajan

[permalink] [raw]
Subject: [PATCH] ath9k: Fix the LED behaviour in idle unassociated state.

LED should be ON when the radio is put into FULL SLEEP mode during the idle
unassociated state.

Signed-off-by: Vivek Natarajan <[email protected]>
---
drivers/net/wireless/ath/ath9k/main.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 5af2596..c908657 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -829,9 +829,14 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
ath9k_ps_wakeup(sc);
ieee80211_stop_queues(hw);

- /* Disable LED */
- ath9k_hw_set_gpio(ah, ah->led_pin, 1);
- ath9k_hw_cfg_gpio_input(ah, ah->led_pin);
+ /*
+ * Keep the LED on when the radio is disabled
+ * during idle unassociated state.
+ */
+ if (!sc->ps_idle) {
+ ath9k_hw_set_gpio(ah, ah->led_pin, 1);
+ ath9k_hw_cfg_gpio_input(ah, ah->led_pin);
+ }

/* Disable interrupts */
ath9k_hw_set_interrupts(ah, 0);
--
1.7.1



2010-06-23 17:55:47

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Fix the LED behaviour in idle unassociated state.

On Wed, 2010-06-23 at 12:08 +0530, Vivek Natarajan wrote:
> LED should be ON when the radio is put into FULL SLEEP mode during the idle
> unassociated state.

Why? Is there any written policy for the LED behavior? I think we want
to conserve power if sleeping, which means turning LEDs off.

Or is it a workaround for some quirk in Atheros hardware?

In any case, I don't like the new comment. The comment says "Keep the
LED on...", but the code is still turning the LED off.

--
Regards,
Pavel Roskin

Subject: Re: [PATCH] ath9k: Fix the LED behaviour in idle unassociated state.

On Wed, Jun 23, 2010 at 11:25:40PM +0530, Pavel Roskin wrote:
> On Wed, 2010-06-23 at 12:08 +0530, Vivek Natarajan wrote:
> > LED should be ON when the radio is put into FULL SLEEP mode during the idle
> > unassociated state.
>
> Why? Is there any written policy for the LED behavior? I think we want
> to conserve power if sleeping, which means turning LEDs off.

There is no written policy for this behaviour, may be the comment
needs to be changed. It makes sense to turn off the LED to save
power but turning off the LED while wireless interface is still up
is kind of misleading, also this one we got as a requirement
internally and did not want to maintain any private patch.

>
> Or is it a workaround for some quirk in Atheros hardware?

nope.

>
> In any case, I don't like the new comment. The comment says "Keep the
> LED on...", but the code is still turning the LED off.

yeah, this comment seems to be implicit. thanks for the review.

Vasanth