On Thu, Nov 13, 2008 at 1:12 AM, Xu, Martin <martin.xu@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
