From 0e07433f7013b2ec4a960511cf489f62625b470f Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 14 Sep 2010 18:41:46 -0700 Subject: [PATCH 1/6] ath9k: fix power save race conditions ath9k has a race on putting the chip into network sleep and having registers read from hardware. The race occurs because although ath9k_ps_restore() locks its own callers it makes use of some variables which get altered in the driver at different code paths. The variables are the ps_enabled and ps_flags. This is easily reprodicible in large network environments when roaming with the wpa_supplicant simple bgscan. You'd get some 0xdeadbeef read out on certain registers such as: ath: timeout (100000 us) on reg 0x806c: 0xdeadbeef & 0x01f00000 != 0x00000000 ath: RX failed to go idle in 10 ms RXSM=0xdeadbeef ath: timeout (100000 us) on reg 0x7000: 0xdeadbeef & 0x00000003 != 0x00000000 ath: Chip reset failed The fix is to protect the ath9k_config(hw, IEEE80211_CONF_CHANGE_PS) calls with a spin_lock_irqsave() which will disable contendors for these variables from interrupt context, timers, re-entry from mac80211 on the same callback, and most importantly from ath9k_ps_restore() which is the only call which will put the device into network sleep. There are quite a few threads and bug reports on these a few of them are: https://bugs.launchpad.net/ubuntu/karmic/+source/linux/+bug/407040 http://code.google.com/p/chromium-os/issues/detail?id=5709 http://code.google.com/p/chromium-os/issues/detail?id=5943 Stable fixes apply to [2.6.32+] Cc: stable@kernel.org Cc: Paul Stewart Cc: Amod Bodas Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/main.c | 5 ++++- drivers/net/wireless/ath/ath9k/recv.c | 3 +++ 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 115e1ae..c82e4dd 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -1557,6 +1557,8 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed) * IEEE80211_CONF_CHANGE_PS is only passed by mac80211 for STA mode. */ if (changed & IEEE80211_CONF_CHANGE_PS) { + unsigned long flags; + spin_lock_irqsave(&sc->sc_pm_lock, flags); if (conf->flags & IEEE80211_CONF_PS) { sc->ps_flags |= PS_ENABLED; /* @@ -1571,7 +1573,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed) sc->ps_enabled = false; sc->ps_flags &= ~(PS_ENABLED | PS_NULLFUNC_COMPLETED); - ath9k_setpower(sc, ATH9K_PM_AWAKE); + ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE); if (!(ah->caps.hw_caps & ATH9K_HW_CAP_AUTOSLEEP)) { ath9k_hw_setrxabort(sc->sc_ah, 0); @@ -1586,6 +1588,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed) } } } + spin_unlock_irqrestore(&sc->sc_pm_lock, flags); } if (changed & IEEE80211_CONF_CHANGE_MONITOR) { diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index 1ca42e5..d092f77 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -491,6 +491,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush) struct ieee80211_hdr *hdr; int retval; bool decrypt_error = false; + unsigned long flags; spin_lock_bh(&sc->rx.rxbuflock); @@ -633,10 +634,12 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush) sc->rx.rxotherant = 0; } + spin_lock_irqsave(&sc->sc_pm_lock, flags); if (unlikely(sc->ps_flags & (PS_WAIT_FOR_BEACON | PS_WAIT_FOR_CAB | PS_WAIT_FOR_PSPOLL_DATA))) ath_rx_ps(sc, skb); + spin_unlock_irqrestore(&sc->sc_pm_lock, flags); ath_rx_send_to_mac80211(hw, sc, skb, rxs); -- 1.7.0.4 From 6b90529b477862628c4e95c857bedb82fb440777 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Mon, 13 Sep 2010 16:33:17 -0700 Subject: [PATCH 2/6] mac80211: make the beacon monitor available externally This will be used by other components next. Cc: stable@kernel.org [2.6.34+] Cc: Amod Bodas Signed-off-by: Luis R. Rodriguez --- net/mac80211/ieee80211_i.h | 1 + net/mac80211/mlme.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 241533e..9c2a657 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -982,6 +982,7 @@ void ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata, struct ieee80211_bss *bss); void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata); void ieee80211_sta_restart(struct ieee80211_sub_if_data *sdata); +void ieee80211_sta_reset_beacon_monitor(struct ieee80211_sub_if_data *sdata); /* IBSS code */ void ieee80211_ibss_notify_scan_completed(struct ieee80211_local *local); diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 1349a09..118888b 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -102,7 +102,7 @@ static void run_again(struct ieee80211_if_managed *ifmgd, mod_timer(&ifmgd->timer, timeout); } -static void mod_beacon_timer(struct ieee80211_sub_if_data *sdata) +void ieee80211_sta_reset_beacon_monitor(struct ieee80211_sub_if_data *sdata) { if (sdata->local->hw.flags & IEEE80211_HW_BEACON_FILTER) return; @@ -1161,7 +1161,7 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk, * Also start the timer that will detect beacon loss. */ ieee80211_sta_rx_notify(sdata, (struct ieee80211_hdr *)mgmt); - mod_beacon_timer(sdata); + ieee80211_sta_reset_beacon_monitor(sdata); return true; } @@ -1259,7 +1259,7 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata, * we have or will be receiving any beacons or data, so let's * schedule the timers again, just in case. */ - mod_beacon_timer(sdata); + ieee80211_sta_reset_beacon_monitor(sdata); mod_timer(&ifmgd->conn_mon_timer, round_jiffies_up(jiffies + IEEE80211_CONNECTION_IDLE_TIME)); @@ -1345,7 +1345,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, * Push the beacon loss detection into the future since * we are processing a beacon from the AP just now. */ - mod_beacon_timer(sdata); + ieee80211_sta_reset_beacon_monitor(sdata); ncrc = crc32_be(0, (void *)&mgmt->u.beacon.beacon_int, 4); ncrc = ieee802_11_parse_elems_crc(mgmt->u.beacon.variable, -- 1.7.0.4 From c433d2d8c1d04605836bef4df4d27011f2985aad Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Mon, 13 Sep 2010 16:36:45 -0700 Subject: [PATCH 3/6] mac80211: disable beacon monitor while going offchannel The beacon monitor should be disabled when going off channel to prevent spurious warnings and triggering connection deterioration work such as sending probe requests. Re-enable the beacon monitor once we come back to the home channel. Cc: stable@kernel.org [2.6.34+] Cc: Amod Bodas Signed-off-by: Luis R. Rodriguez --- net/mac80211/offchannel.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c index c36b191..5839045 100644 --- a/net/mac80211/offchannel.c +++ b/net/mac80211/offchannel.c @@ -22,12 +22,14 @@ static void ieee80211_offchannel_ps_enable(struct ieee80211_sub_if_data *sdata) { struct ieee80211_local *local = sdata->local; + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; local->offchannel_ps_enabled = false; /* FIXME: what to do when local->pspolling is true? */ del_timer_sync(&local->dynamic_ps_timer); + del_timer_sync(&ifmgd->bcn_mon_timer); cancel_work_sync(&local->dynamic_ps_enable_work); if (local->hw.conf.flags & IEEE80211_CONF_PS) { @@ -85,6 +87,8 @@ static void ieee80211_offchannel_ps_disable(struct ieee80211_sub_if_data *sdata) mod_timer(&local->dynamic_ps_timer, jiffies + msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout)); } + + ieee80211_sta_reset_beacon_monitor(sdata); } void ieee80211_offchannel_stop_beaconing(struct ieee80211_local *local) -- 1.7.0.4 From 6b0d4f86b2ce3678117c04d33d3e8c9fd9e9b3e0 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 8 Sep 2010 15:12:16 -0700 Subject: [PATCH 4/6] mac80211: send last 3/5 probe requests as unicast Some buggy APs do not respond to unicast probe requests or send unicast probe requests very delayed so in the worst case we should try to send broadcast probe requests, otherwise we can get disconnected from these APs. Even if drivers do not have filters to disregard probe responses from foreign APs mac80211 will only process probe responses from our associated AP for re-arming connection monitoring. We need to do this since the beacon monitor does not push back the connection monitor by design so even if we are getting beacons from these type of APs our connection monitor currently relies heavily on the way the probe requests are received on the AP. An example of an AP affected by this is the Nexus One, but this has also been observed with random APs. We can probably optimize this later by using null funcs instead of probe requests. For more details refer to: http://code.google.com/p/chromium-os/issues/detail?id=5715 This patch has fixes for stable kernels [2.6.35+]. Cc: stable@kernel.org Cc: Paul Stewart Cc: Amod Bodas Signed-off-by: Luis R. Rodriguez --- net/mac80211/mlme.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 118888b..aa9184b 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -865,10 +865,19 @@ static void ieee80211_mgd_probe_ap_send(struct ieee80211_sub_if_data *sdata) { struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; const u8 *ssid; + u8 *dst = ifmgd->associated->bssid; + u8 unicast_limit = max(1, IEEE80211_MAX_PROBE_TRIES - 3); + + /* + * Try sending broadcast probe requests for the last three + * probe requests after the first ones failed since some + * buggy APs only support broadcast probe requests. + */ + if (ifmgd->probe_send_count >= unicast_limit) + dst = NULL; ssid = ieee80211_bss_get_ie(ifmgd->associated, WLAN_EID_SSID); - ieee80211_send_probe_req(sdata, ifmgd->associated->bssid, - ssid + 2, ssid[1], NULL, 0); + ieee80211_send_probe_req(sdata, dst, ssid + 2, ssid[1], NULL, 0); ifmgd->probe_send_count++; ifmgd->probe_timeout = jiffies + IEEE80211_PROBE_WAIT; -- 1.7.0.4 From 52a874146447f722da45c3d59f6af49b5ccb78c8 Mon Sep 17 00:00:00 2001 From: Senthil Balasubramanian Date: Mon, 13 Sep 2010 17:00:01 -0700 Subject: [PATCH 5/6] ath9k: fix regression which prevents chip sleep after CAB data The patch: commit 293dc5dfdbcc16cde06e40a688394cc8ab083e48 Author: Gabor Juhos Date: Fri Jun 19 12:17:48 2009 +0200 ath9k: remove ath_rx_ps_back_to_sleep helper This helper only clears the SC_OP_WAIT_FOR_{BEACON,CAB} flags. Remove it and clear these flags directly in the approptiate places instead. Changes-licensed-under: ISC Signed-off-by: Gabor Juhos Signed-off-by: John W. Linville introduced a regression which forgot to lift the beacon flag after we received all broadcast and multicast data. This meant we never went to sleep consuming about ~650mW on idle. This pretty much broke power save completely. This patch has fixes for stable kernels [2.6.32+]. Cc: stable@kernel.org Cc: Paul Stewart Cc: Sameer Nanda Cc: Gabor Juhos Cc: Amod Bodas Signed-off-by: Senthil Balasubramanian Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/recv.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index d092f77..2ad8074 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -420,7 +420,7 @@ static void ath_rx_ps(struct ath_softc *sc, struct sk_buff *skb) * No more broadcast/multicast frames to be received at this * point. */ - sc->ps_flags &= ~PS_WAIT_FOR_CAB; + sc->ps_flags &= ~(PS_WAIT_FOR_CAB | PS_WAIT_FOR_BEACON); ath_print(common, ATH_DBG_PS, "All PS CAB frames received, back to sleep\n"); } else if ((sc->ps_flags & PS_WAIT_FOR_PSPOLL_DATA) && -- 1.7.0.4 From 838ac31c0b26139fdbded6fc08ab873290fc0c03 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Fri, 27 Aug 2010 18:54:36 -0700 Subject: [PATCH 6/6] ath9k: fix regression which disabled ps on ath9k The patch titled "ath9k: Add new file init.c" shuffled some code around but in dong so for some reason also removed the revision check for disablign power save. Add this revision check again so we can get power save re-enabled again by default on cards newer than AR5416 and AR5418. $ git describe --contains 556242049cc3992d0ee625e9f15c4b00ea4baac8 v2.6.34-rc1~233^2~49^2~343 This patch has fixes for stable kernels [2.6.34+]. Cc: stable@kernel.org Cc: Paul Stewart Cc: Amod Bodas Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/init.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c index b78308c..7e5fc3f 100644 --- a/drivers/net/wireless/ath/ath9k/init.c +++ b/drivers/net/wireless/ath/ath9k/init.c @@ -639,7 +639,8 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw) BIT(NL80211_IFTYPE_ADHOC) | BIT(NL80211_IFTYPE_MESH_POINT); - hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT; + if (AR_SREV_5416(sc->sc_ah)) + hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT; hw->queues = 4; hw->max_rates = 4; -- 1.7.0.4