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
