Hi bob： Please review below patch which has been modified according to your comments.
From: Martin Xu <[EMAIL PROTECTED]> Subject: disable beacon filter when station is not associated with any AP. Ath5k driver has too many interrupts per second at idle http://bugzilla.kernel.org/show_bug.cgi?id=11749 Signed-off-by: Martin Xu <[EMAIL PROTECTED]> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c index 34cd1a4..bed1376 100644 --- a/drivers/net/wireless/ath5k/base.c +++ b/drivers/net/wireless/ath5k/base.c @@ -241,6 +241,10 @@ static int ath5k_get_tx_stats(struct ieee80211_hw *hw, static u64 ath5k_get_tsf(struct ieee80211_hw *hw); static void ath5k_reset_tsf(struct ieee80211_hw *hw); static int ath5k_beacon_update(struct ath5k_softc *sc, struct sk_buff *skb); +static void ath5k_bss_info_changed(struct ieee80211_hw *hw, + struct ieee80211_vif *vif, + struct ieee80211_bss_conf *bss_conf, + u32 changes); static struct ieee80211_ops ath5k_hw_ops = { .tx = ath5k_tx, @@ -2952,7 +2956,7 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw, sc->opmode != NL80211_IFTYPE_MESH_POINT && test_bit(ATH_STAT_PROMISC, sc->status)) rfilt |= AR5K_RX_FILTER_PROM; - if (sc->opmode == NL80211_IFTYPE_STATION || + if ((sc->opmode == NL80211_IFTYPE_STATION && sc->assoc) || sc->opmode == NL80211_IFTYPE_ADHOC || sc->opmode == NL80211_IFTYPE_AP) rfilt |= AR5K_RX_FILTER_BEACON; @@ -3092,4 +3096,39 @@ ath5k_beacon_update(struct ath5k_softc *sc, struct sk_buff *skb) return ret; } +static void +set_beacon_filter(struct ieee80211_hw *hw, bool enable) +{ + struct ath5k_softc *sc = hw->priv; + struct ath5k_hw *ah = sc->ah; + u32 rfilt; + rfilt = ath5k_hw_get_rx_filter(ah); + if (enable) + rfilt |= AR5K_RX_FILTER_BEACON; + else + rfilt &= ~AR5K_RX_FILTER_BEACON; + ath5k_hw_set_rx_filter(ah, rfilt); + sc->filter_flags = rfilt; + return; +} + +static void ath5k_bss_info_changed(struct ieee80211_hw *hw, + struct ieee80211_vif *vif, + struct ieee80211_bss_conf *bss_conf, + u32 changes) +{ + struct ath5k_softc *sc = hw->priv; + if (changes & BSS_CHANGED_ASSOC) { + mutex_lock(&sc->lock); + sc->assoc = bss_conf->assoc; + if (sc->opmode == NL80211_IFTYPE_STATION) { + if (sc->assoc) + set_beacon_filter(hw, 1); + else + set_beacon_filter(hw, 0); + } + mutex_unlock(&sc->lock); + } + return; +} diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h index 06d1054..facc60d 100644 --- a/drivers/net/wireless/ath5k/base.h +++ b/drivers/net/wireless/ath5k/base.h @@ -179,6 +179,7 @@ struct ath5k_softc { struct timer_list calib_tim; /* calibration timer */ int power_level; /* Requested tx power in dbm */ + bool assoc; /* assocate state */ }; #define ath5k_hw_hasbssidmask(_ah) \ -----Original Message----- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Bob Copeland Sent: 2008年11月14日 0:26 To: Xu, Martin Cc: Nick Kossifidis; Luis R. Rodriguez; linux-wireless; [EMAIL PROTECTED]; Vikram Kandukuri; Jothikumar Mothilal; [email protected]; Liu, Bing Wei; Selbak, Rolla N; Wang, Yong Y Subject: Re: [ath5k-devel] [Bug 11749] Ath5k driver has too many interrupts per second at idle On Thu, Nov 13, 2008 at 1:12 AM, Xu, Martin <[EMAIL PROTECTED]> wrote: > Hi all: > I have a patch that can be used to fix the bug. > The patch resolved the issue by disabling the beacon filter when > disassociated with AP and enabling beacon when associate with AP. > See http://bugzilla.kernel.org/show_bug.cgi?id=11749 > Please review it. Thanks! Thanks for the patch! I think the basic idea is ok (we only have beacons if PRBRESP_PROMISC is set or if we are in STA mode _and_ associated, or in IBSS mode). However, there are lots of CodingStyle issues with the patch. Please run scripts/checkpatch.pl on it and fix the corresponding whitespace issues. +static void +enable_beacon_filter(struct ieee80211_hw *hw) +{ + struct ath5k_softc *sc = hw->priv; + struct ath5k_hw *ah = sc->ah; + u32 rfilt; + rfilt = ath5k_hw_get_rx_filter(ah); + if ( !(rfilt & AR5K_RX_FILTER_BEACON) ){ Probably not worth it to do the test, since this isn't going to get called that often. + rfilt |= AR5K_RX_FILTER_BEACON; + ath5k_hw_set_rx_filter(ah,rfilt); + sc->filter_flags = rfilt; + } + rfilt = ath5k_hw_get_rx_filter(ah); + return; Above two lines are unnecessary. +} You remove a heap of code by making this set_beacon_filter(hw, bool enable) and cleaning up the branches in bss_info_changed. I believe this can race with configure_filter as well (but configure_filter already races with anything that touches sc->status...). > @@ -179,6 +179,7 @@ struct ath5k_softc { > > struct timer_list calib_tim; /* calibration timer */ > int power_level; /* Requested tx power in dbm */ > + bool assoc; /* assocate state */ > }; s/assocate/associate. Also in ath5k we sometimes use sc->status for such flags, though this could go either way. -- Bob Copeland %% www.bobcopeland.com _______________________________________________ ath5k-devel mailing list [email protected] https://lists.ath5k.org/mailman/listinfo/ath5k-devel
