[PATCH] ath9k: Handle power modes in isr for power save.

[PATCH] ath9k: Handle power modes in isr for power save.

[Vivek Natarajan]

Restore network sleep mode in isr if power save is enabled.

Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
 drivers/net/wireless/ath9k/ath9k.h |    3 ++-
 drivers/net/wireless/ath9k/main.c  |    2 ++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
index 6481ea4..69292f3 100644
--- a/drivers/net/wireless/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath9k/ath9k.h
@@ -677,7 +677,8 @@ static inline void ath9k_ps_wakeup(struct ath_softc *sc)
 static inline void ath9k_ps_restore(struct ath_softc *sc)
 {
 	if (atomic_dec_and_test(&sc->ps_usecount))
-		if (sc->hw->conf.flags & IEEE80211_CONF_PS)
+		if ((sc->hw->conf.flags & IEEE80211_CONF_PS) &&
+		    !(sc->sc_flags & SC_OP_WAIT_FOR_BEACON))
 			ath9k_hw_setpower(sc->sc_ah,
 					  sc->sc_ah->restore_mode);
 }
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index fd6cc73..96194ef 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -516,6 +516,7 @@ irqreturn_t ath_isr(int irq, void *dev)
 			return IRQ_NONE;
 
 		sc->intrstatus = status;
+		ath9k_ps_wakeup(sc);
 
 		if (status & ATH9K_INT_FATAL) {
 			/* need a chip reset */
@@ -581,6 +582,7 @@ irqreturn_t ath_isr(int irq, void *dev)
 				sched = true;
 			}
 		}
+		ath9k_ps_restore(sc);
 	} while (0);
 
 	ath_debug_stat_interrupt(sc, status);
-- 
1.6.0.1

[PATCH] ath9k: Handle power modes in isr for power save.

[Sujith]

Vivek Natarajan wrote:
> Restore network sleep mode in isr if power save is enabled.
> 
> Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
> ---
>  drivers/net/wireless/ath9k/ath9k.h |    3 ++-
>  drivers/net/wireless/ath9k/main.c  |    2 ++
>  2 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
> index 6481ea4..69292f3 100644
> --- a/drivers/net/wireless/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath9k/ath9k.h
> @@ -677,7 +677,8 @@ static inline void ath9k_ps_wakeup(struct ath_softc *sc)
>  static inline void ath9k_ps_restore(struct ath_softc *sc)
>  {
>  	if (atomic_dec_and_test(&sc->ps_usecount))
> -		if (sc->hw->conf.flags & IEEE80211_CONF_PS)
> +		if ((sc->hw->conf.flags & IEEE80211_CONF_PS) &&
> +		    !(sc->sc_flags & SC_OP_WAIT_FOR_BEACON))
>  			ath9k_hw_setpower(sc->sc_ah,
>  					  sc->sc_ah->restore_mode);
>  }
> diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
> index fd6cc73..96194ef 100644
> --- a/drivers/net/wireless/ath9k/main.c
> +++ b/drivers/net/wireless/ath9k/main.c
> @@ -516,6 +516,7 @@ irqreturn_t ath_isr(int irq, void *dev)
>  			return IRQ_NONE;
>  
>  		sc->intrstatus = status;
> +		ath9k_ps_wakeup(sc);
>  
>  		if (status & ATH9K_INT_FATAL) {
>  			/* need a chip reset */
> @@ -581,6 +582,7 @@ irqreturn_t ath_isr(int irq, void *dev)
>  				sched = true;
>  			}
>  		}
> +		ath9k_ps_restore(sc);
>  	} while (0);
>  

This needs proper locking.

Sujith

Re: [PATCH] ath9k: Handle power modes in isr for power save.

[Vivek Natarajan]

On Mon, Mar 2, 2009 at 11:12 PM, Sujith <m.sujith@gmail.com> wrote:
> Vivek Natarajan wrote:
>> Restore network sleep mode in isr if power save is enabled.
>>
>> Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
>> ---
>> =A0drivers/net/wireless/ath9k/ath9k.h | =A0 =A03 ++-
>> =A0drivers/net/wireless/ath9k/main.c =A0| =A0 =A02 ++
>> =A02 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wirele=
ss/ath9k/ath9k.h
>> index 6481ea4..69292f3 100644
>> --- a/drivers/net/wireless/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath9k/ath9k.h
>> @@ -677,7 +677,8 @@ static inline void ath9k_ps_wakeup(struct ath_so=
ftc *sc)
>> =A0static inline void ath9k_ps_restore(struct ath_softc *sc)
>> =A0{
>> =A0 =A0 =A0 if (atomic_dec_and_test(&sc->ps_usecount))
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (sc->hw->conf.flags & IEEE80211_CONF_PS=
)
>> + =A0 =A0 =A0 =A0 =A0 =A0 if ((sc->hw->conf.flags & IEEE80211_CONF_P=
S) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 !(sc->sc_flags & SC_OP_WAIT_FOR_BE=
ACON))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ath9k_hw_setpower(sc->sc=
_ah,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 sc->sc_ah->restore_mode);
>> =A0}
>
> This needs proper locking.

SC_OP_WAIT_FOR_BEACON is being changed in rx_tasklet and in isr which
are mutually exclusive. Even though it might be changed  in these two
regions, it won't affect the ps_wake/ps_restore cycle.

IEEE80211_CONF_PS is changed in siwpower() and also in
dynamic_ps_work. Even if it is changed while we are executing this
ps_restore, there won't be any race condition on ps_wake/ps_restore
cycle and it will work fine.

hw_setpower is just a register write function which may not need a prot=
ection(?)

And for the protection of ps_wake/ps_restore itself, there is
ps_count(atomic) which takes care of properly restoring power save
state.

Overall, from my perspective there is no need for any more lock.

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

Re: [PATCH] ath9k: Handle power modes in isr for power save.

[Sujith]

Vivek Natarajan wrote:
> SC_OP_WAIT_FOR_BEACON is being changed in rx_tasklet and in isr which
> are mutually exclusive. Even though it might be changed  in these two
> regions, it won't affect the ps_wake/ps_restore cycle.
> 
> IEEE80211_CONF_PS is changed in siwpower() and also in
> dynamic_ps_work. Even if it is changed while we are executing this
> ps_restore, there won't be any race condition on ps_wake/ps_restore
> cycle and it will work fine.
> 
> hw_setpower is just a register write function which may not need a protection(?)
> 
> And for the protection of ps_wake/ps_restore itself, there is
> ps_count(atomic) which takes care of properly restoring power save
> state.
> 
> Overall, from my perspective there is no need for any more lock.

Sorry, I saw sc_flags and just assumed it would be set from process context.
Thanks for the explanation.

Sujith