* Re: [PATCH] ath9k: Enable dynamic power save in ath9k.
2008-12-29 23:53 [PATCH] ath9k: Enable dynamic power save in ath9k Vivek Natarajan
@ 2008-12-29 12:11 ` Johannes Berg
2008-12-29 12:14 ` Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Johannes Berg @ 2008-12-29 12:11 UTC (permalink / raw)
To: Vivek Natarajan; +Cc: linville, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
On Mon, 2008-12-29 at 15:53 -0800, Vivek Natarajan wrote:
> +#define ATH9K_PS_WAKEUP(sc) \
> + do { \
> + if (!atomic_read(&sc->ps_usecount) && \
> + (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
> + ah->ah_restoreMode = ah->ah_powerMode; \
> + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
> + } \
> + atomic_inc(&sc->ps_usecount); \
> + } while (0);
> +
> +#define ATH9K_PS_RESTORE(sc) \
> + do { \
> + if (atomic_dec_and_test(&sc->ps_usecount) && \
> + (sc->hw->conf.flags & IEEE80211_CONF_PS)) \
> + ath9k_hw_setpower(sc->sc_ah, ah->ah_restoreMode); \
> + } while (0);
I think those would be better as static inlines rather than macros.
johannes
PS: Your clock seems to be off by 12.5 hours or so?
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Enable dynamic power save in ath9k.
2008-12-29 12:11 ` Johannes Berg
@ 2008-12-29 12:14 ` Johannes Berg
2008-12-29 12:23 ` Michael Buesch
2008-12-29 13:37 ` Vivek Natarajan
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2008-12-29 12:14 UTC (permalink / raw)
To: Vivek Natarajan; +Cc: linville, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 630 bytes --]
On Mon, 2008-12-29 at 13:11 +0100, Johannes Berg wrote:
> On Mon, 2008-12-29 at 15:53 -0800, Vivek Natarajan wrote:
>
> > +#define ATH9K_PS_WAKEUP(sc) \
> > + do { \
> > + if (!atomic_read(&sc->ps_usecount) && \
> > + (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
> > + ah->ah_restoreMode = ah->ah_powerMode; \
> > + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
> > + } \
> > + atomic_inc(&sc->ps_usecount); \
> > + } while (0);
Also, this seems racy, shouldn't it use something like if
(atomic_inc_return() == 1) ?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Enable dynamic power save in ath9k.
2008-12-29 12:11 ` Johannes Berg
2008-12-29 12:14 ` Johannes Berg
@ 2008-12-29 12:23 ` Michael Buesch
2008-12-29 13:37 ` Vivek Natarajan
2 siblings, 0 replies; 6+ messages in thread
From: Michael Buesch @ 2008-12-29 12:23 UTC (permalink / raw)
To: Johannes Berg; +Cc: Vivek Natarajan, linville, linux-wireless
On Monday 29 December 2008 13:11:00 Johannes Berg wrote:
> On Mon, 2008-12-29 at 15:53 -0800, Vivek Natarajan wrote:
>
> > +#define ATH9K_PS_WAKEUP(sc) \
> > + do { \
> > + if (!atomic_read(&sc->ps_usecount) && \
> > + (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
> > + ah->ah_restoreMode = ah->ah_powerMode; \
> > + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
> > + } \
> > + atomic_inc(&sc->ps_usecount); \
> > + } while (0);
> > +
> > +#define ATH9K_PS_RESTORE(sc) \
> > + do { \
> > + if (atomic_dec_and_test(&sc->ps_usecount) && \
> > + (sc->hw->conf.flags & IEEE80211_CONF_PS)) \
> > + ath9k_hw_setpower(sc->sc_ah, ah->ah_restoreMode); \
> > + } while (0);
>
> I think those would be better as static inlines rather than macros.
And the do-while c-block-protection of the macro is buggy. It has an extra
semicolon at the end, which defeats the whole purpose.
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Enable dynamic power save in ath9k.
2008-12-29 12:11 ` Johannes Berg
2008-12-29 12:14 ` Johannes Berg
2008-12-29 12:23 ` Michael Buesch
@ 2008-12-29 13:37 ` Vivek Natarajan
2009-01-09 19:51 ` John W. Linville
2 siblings, 1 reply; 6+ messages in thread
From: Vivek Natarajan @ 2008-12-29 13:37 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
On Mon, Dec 29, 2008 at 5:41 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2008-12-29 at 15:53 -0800, Vivek Natarajan wrote:
>
>> +#define ATH9K_PS_WAKEUP(sc) \
>> + do { \
>> + if (!atomic_read(&sc->ps_usecount) && \
>> + (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
>> + ah->ah_restoreMode = ah->ah_powerMode; \
>> + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
>> + } \
>> + atomic_inc(&sc->ps_usecount); \
>> + } while (0);
>> +
>> +#define ATH9K_PS_RESTORE(sc) \
>> + do { \
>> + if (atomic_dec_and_test(&sc->ps_usecount) && \
>> + (sc->hw->conf.flags & IEEE80211_CONF_PS)) \
>> + ath9k_hw_setpower(sc->sc_ah, ah->ah_restoreMode); \
>> + } while (0);
>
> I think those would be better as static inlines rather than macros.
>
> johannes
Thanks Johannes. I'll change them.
> Also, this seems racy, shouldn't it use something like if
> (atomic_inc_return() == 1) ?
Thanks.This seems much better.
Vivek.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ath9k: Enable dynamic power save in ath9k.
@ 2008-12-29 23:53 Vivek Natarajan
2008-12-29 12:11 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Vivek Natarajan @ 2008-12-29 23:53 UTC (permalink / raw)
To: linville; +Cc: linux-wireless
This patch implements dynamic power save feature for ath9k.
Signed-off-by: Vivek Natarajan <vnatarajan@atheros.com>
---
drivers/net/wireless/ath9k/ath9k.h | 2 +
drivers/net/wireless/ath9k/core.h | 19 ++++++++++++++++
drivers/net/wireless/ath9k/hw.c | 5 +--
drivers/net/wireless/ath9k/hw.h | 1 -
drivers/net/wireless/ath9k/main.c | 42 +++++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath9k/recv.c | 6 +++++
6 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
index d278135..c59e916 100644
--- a/drivers/net/wireless/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath9k/ath9k.h
@@ -790,6 +790,8 @@ struct ath_hal {
u16 ah_currentRD5G;
u16 ah_currentRD2G;
char ah_iso[4];
+ enum ath9k_power_mode ah_powerMode;
+ enum ath9k_power_mode ah_restoreMode;
struct ath9k_channel ah_channels[150];
struct ath9k_channel *ah_curchan;
diff --git a/drivers/net/wireless/ath9k/core.h b/drivers/net/wireless/ath9k/core.h
index 4ca2aed..d940ba8 100644
--- a/drivers/net/wireless/ath9k/core.h
+++ b/drivers/net/wireless/ath9k/core.h
@@ -692,6 +692,7 @@ enum PROT_MODE {
#define SC_OP_RFKILL_REGISTERED BIT(11)
#define SC_OP_RFKILL_SW_BLOCKED BIT(12)
#define SC_OP_RFKILL_HW_BLOCKED BIT(13)
+#define SC_OP_WAIT_FOR_BEACON BIT(14)
struct ath_softc {
struct ieee80211_hw *hw;
@@ -719,6 +720,8 @@ struct ath_softc {
DECLARE_BITMAP(sc_keymap, ATH_KEYMAX);
u8 sc_splitmic;
u8 sc_protrix;
+ atomic_t ps_usecount;
+
enum ath9k_int sc_imask;
enum PROT_MODE sc_protmode;
enum ath9k_ht_extprotspacing sc_ht_extprotspacing;
@@ -751,4 +754,20 @@ int ath_get_hal_qnum(u16 queue, struct ath_softc *sc);
int ath_get_mac80211_qnum(u32 queue, struct ath_softc *sc);
int ath_cabq_update(struct ath_softc *);
+#define ATH9K_PS_WAKEUP(sc) \
+ do { \
+ if (!atomic_read(&sc->ps_usecount) && \
+ (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
+ ah->ah_restoreMode = ah->ah_powerMode; \
+ ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
+ } \
+ atomic_inc(&sc->ps_usecount); \
+ } while (0);
+
+#define ATH9K_PS_RESTORE(sc) \
+ do { \
+ if (atomic_dec_and_test(&sc->ps_usecount) && \
+ (sc->hw->conf.flags & IEEE80211_CONF_PS)) \
+ ath9k_hw_setpower(sc->sc_ah, ah->ah_restoreMode); \
+ } while (0);
#endif /* CORE_H */
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index d2b0ecf..8b45a01 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -2735,7 +2735,7 @@ bool ath9k_hw_setpower(struct ath_hal *ah,
int status = true, setChip = true;
DPRINTF(ah->ah_sc, ATH_DBG_POWER_MGMT, "%s -> %s (%s)\n",
- modes[ahp->ah_powerMode], modes[mode],
+ modes[ah->ah_powerMode], modes[mode],
setChip ? "set chip " : "");
switch (mode) {
@@ -2754,8 +2754,7 @@ bool ath9k_hw_setpower(struct ath_hal *ah,
"Unknown power mode %u\n", mode);
return false;
}
- ahp->ah_powerMode = mode;
-
+ ah->ah_powerMode = mode;
return status;
}
diff --git a/drivers/net/wireless/ath9k/hw.h b/drivers/net/wireless/ath9k/hw.h
index 91d8f59..17608c5 100644
--- a/drivers/net/wireless/ath9k/hw.h
+++ b/drivers/net/wireless/ath9k/hw.h
@@ -830,7 +830,6 @@ struct ath_hal_5416 {
bool ah_chipFullSleep;
u32 ah_atimWindow;
u16 ah_antennaSwitchSwap;
- enum ath9k_power_mode ah_powerMode;
enum ath9k_ant_setting ah_diversityControl;
/* Calibration */
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 70affb7..948bc9e 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -262,6 +262,8 @@ static int ath_set_channel(struct ath_softc *sc, struct ath9k_channel *hchan)
if (sc->sc_flags & SC_OP_INVALID)
return -EIO;
+ ATH9K_PS_WAKEUP(sc);
+
if (hchan->channel != sc->sc_ah->ah_curchan->channel ||
hchan->channelFlags != sc->sc_ah->ah_curchan->channelFlags ||
(sc->sc_flags & SC_OP_CHAINMASK_UPDATE) ||
@@ -313,6 +315,7 @@ static int ath_set_channel(struct ath_softc *sc, struct ath9k_channel *hchan)
if (ath_startrecv(sc) != 0) {
DPRINTF(sc, ATH_DBG_FATAL,
"Unable to restart recv logic\n");
+ ATH9K_PS_RESTORE(sc);
return -EIO;
}
@@ -320,6 +323,7 @@ static int ath_set_channel(struct ath_softc *sc, struct ath9k_channel *hchan)
ath_update_txpow(sc);
ath9k_hw_set_interrupts(ah, sc->sc_imask);
}
+ ATH9K_PS_RESTORE(sc);
return 0;
}
@@ -591,8 +595,10 @@ static irqreturn_t ath_isr(int irq, void *dev)
ATH9K_HW_CAP_AUTOSLEEP)) {
/* Clear RxAbort bit so that we can
* receive frames */
+ ath9k_hw_setpower(ah, ATH9K_PM_AWAKE);
ath9k_hw_setrxabort(ah, 0);
sched = true;
+ sc->sc_flags |= SC_OP_WAIT_FOR_BEACON;
}
}
}
@@ -1071,6 +1077,7 @@ static void ath_radio_enable(struct ath_softc *sc)
struct ath_hal *ah = sc->sc_ah;
int status;
+ ATH9K_PS_WAKEUP(sc);
spin_lock_bh(&sc->sc_resetlock);
if (!ath9k_hw_reset(ah, ah->ah_curchan,
sc->tx_chan_width,
@@ -1108,6 +1115,7 @@ static void ath_radio_enable(struct ath_softc *sc)
ath9k_hw_set_gpio(ah, ATH_LED_PIN, 0);
ieee80211_wake_queues(sc->hw);
+ ATH9K_PS_RESTORE(sc);
}
static void ath_radio_disable(struct ath_softc *sc)
@@ -1115,7 +1123,7 @@ static void ath_radio_disable(struct ath_softc *sc)
struct ath_hal *ah = sc->sc_ah;
int status;
-
+ ATH9K_PS_WAKEUP(sc);
ieee80211_stop_queues(sc->hw);
/* Disable LED */
@@ -1149,6 +1157,7 @@ static void ath_radio_disable(struct ath_softc *sc)
ath9k_hw_phy_disable(ah);
ath9k_hw_setpower(ah, ATH9K_PM_FULL_SLEEP);
+ ATH9K_PS_RESTORE(sc);
}
static bool ath_is_rfkill_set(struct ath_softc *sc)
@@ -1296,8 +1305,11 @@ static int ath_start_rfkill_poll(struct ath_softc *sc)
static void ath_detach(struct ath_softc *sc)
{
struct ieee80211_hw *hw = sc->hw;
+ struct ath_hal *ah = sc->sc_ah;
int i = 0;
+ ATH9K_PS_WAKEUP(sc);
+
DPRINTF(sc, ATH_DBG_CONFIG, "Detach ATH hw\n");
#if defined(CONFIG_RFKILL) || defined(CONFIG_RFKILL_MODULE)
@@ -1322,6 +1334,7 @@ static void ath_detach(struct ath_softc *sc)
ath9k_hw_detach(sc->sc_ah);
ath9k_exit_debug(sc);
+ ATH9K_PS_RESTORE(sc);
}
static int ath_init(u16 devid, struct ath_softc *sc)
@@ -2135,6 +2148,27 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
struct ieee80211_conf *conf = &hw->conf;
mutex_lock(&sc->mutex);
+ if (changed & IEEE80211_CONF_CHANGE_PS) {
+ if (conf->flags & IEEE80211_CONF_PS) {
+ if ((sc->sc_imask & ATH9K_INT_TIM_TIMER) == 0) {
+ sc->sc_imask |= ATH9K_INT_TIM_TIMER;
+ ath9k_hw_set_interrupts(sc->sc_ah,
+ sc->sc_imask);
+ }
+ ath9k_hw_setrxabort(sc->sc_ah, 1);
+ ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_NETWORK_SLEEP);
+ } else {
+ ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE);
+ ath9k_hw_setrxabort(sc->sc_ah, 0);
+ sc->sc_flags &= ~SC_OP_WAIT_FOR_BEACON;
+ if (sc->sc_imask & ATH9K_INT_TIM_TIMER) {
+ sc->sc_imask &= ~ATH9K_INT_TIM_TIMER;
+ ath9k_hw_set_interrupts(sc->sc_ah,
+ sc->sc_imask);
+ }
+ }
+ }
+
if (changed & (IEEE80211_CONF_CHANGE_CHANNEL |
IEEE80211_CONF_CHANGE_HT)) {
struct ieee80211_channel *curchan = hw->conf.channel;
@@ -2357,8 +2391,10 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
struct ieee80211_key_conf *key)
{
struct ath_softc *sc = hw->priv;
+ struct ath_hal *ah = sc->sc_ah;
int ret = 0;
+ ATH9K_PS_WAKEUP(sc);
DPRINTF(sc, ATH_DBG_KEYCACHE, "Set HW Key\n");
switch (cmd) {
@@ -2380,6 +2416,7 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
ret = -EINVAL;
}
+ ATH9K_PS_RESTORE(sc);
return ret;
}
@@ -2441,8 +2478,10 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw,
u16 tid, u16 *ssn)
{
struct ath_softc *sc = hw->priv;
+ struct ath_hal *ah = sc->sc_ah;
int ret = 0;
+ ATH9K_PS_WAKEUP(sc);
switch (action) {
case IEEE80211_AMPDU_RX_START:
if (!(sc->sc_flags & SC_OP_RXAGGR))
@@ -2472,6 +2511,7 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw,
default:
DPRINTF(sc, ATH_DBG_FATAL, "Unknown AMPDU action\n");
}
+ ATH9K_PS_RESTORE(sc);
return ret;
}
diff --git a/drivers/net/wireless/ath9k/recv.c b/drivers/net/wireless/ath9k/recv.c
index 462e08c..35d9209 100644
--- a/drivers/net/wireless/ath9k/recv.c
+++ b/drivers/net/wireless/ath9k/recv.c
@@ -622,6 +622,12 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush)
} else {
sc->rx.rxotherant = 0;
}
+
+ if (ieee80211_is_beacon(hdr->frame_control) &&
+ (sc->sc_flags & SC_OP_WAIT_FOR_BEACON)) {
+ sc->sc_flags &= ~SC_OP_WAIT_FOR_BEACON;
+ ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_NETWORK_SLEEP);
+ }
requeue:
list_move_tail(&bf->list, &sc->rx.rxbuf);
ath_rx_buf_link(sc, bf);
--
1.6.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Enable dynamic power save in ath9k.
2008-12-29 13:37 ` Vivek Natarajan
@ 2009-01-09 19:51 ` John W. Linville
0 siblings, 0 replies; 6+ messages in thread
From: John W. Linville @ 2009-01-09 19:51 UTC (permalink / raw)
To: Vivek Natarajan; +Cc: Johannes Berg, linux-wireless
On Mon, Dec 29, 2008 at 07:07:41PM +0530, Vivek Natarajan wrote:
> On Mon, Dec 29, 2008 at 5:41 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Mon, 2008-12-29 at 15:53 -0800, Vivek Natarajan wrote:
> >
> >> +#define ATH9K_PS_WAKEUP(sc) \
> >> + do { \
> >> + if (!atomic_read(&sc->ps_usecount) && \
> >> + (ah->ah_powerMode != ATH9K_PM_AWAKE)) { \
> >> + ah->ah_restoreMode = ah->ah_powerMode; \
> >> + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); \
> >> + } \
> >> + atomic_inc(&sc->ps_usecount); \
> >> + } while (0);
> >> +
> >> +#define ATH9K_PS_RESTORE(sc) \
> >> + do { \
> >> + if (atomic_dec_and_test(&sc->ps_usecount) && \
> >> + (sc->hw->conf.flags & IEEE80211_CONF_PS)) \
> >> + ath9k_hw_setpower(sc->sc_ah, ah->ah_restoreMode); \
> >> + } while (0);
> >
> > I think those would be better as static inlines rather than macros.
> >
> > johannes
>
> Thanks Johannes. I'll change them.
>
> > Also, this seems racy, shouldn't it use something like if
> > (atomic_inc_return() == 1) ?
>
> Thanks.This seems much better.
Did you repost this? I don't see it. Please (re?)send it to the
list w/ the requested changes.
John
--
John W. Linville Linux should be at the core
linville@tuxdriver.com of your literate lifestyle.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-01-09 20:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-29 23:53 [PATCH] ath9k: Enable dynamic power save in ath9k Vivek Natarajan
2008-12-29 12:11 ` Johannes Berg
2008-12-29 12:14 ` Johannes Berg
2008-12-29 12:23 ` Michael Buesch
2008-12-29 13:37 ` Vivek Natarajan
2009-01-09 19:51 ` John W. Linville
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).