On 10/08/2010 11:13 AM, Bob Copeland wrote:
On Fri, Oct 8, 2010 at 2:06 PM, Ben Greear<greearb@xxxxxxxxxxxxxxx> wrote:
On 10/08/2010 11:03 AM, Bob Copeland wrote:
On Fri, Oct 8, 2010 at 12:43 PM,<greearb@xxxxxxxxxxxxxxx> wrote:
@@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc,
struct ieee80211_vif *vif)
memset(&iter_data.mask, 0xff, ETH_ALEN);
iter_data.found_active = false;
iter_data.need_set_hw_addr = true;
+ iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED;
if (vif)
ath_vif_iter(&iter_data, vif->addr, vif);
@@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc,
struct ieee80211_vif *vif)
&iter_data);
memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN);
+ if (update_opmode&& sc->opmode != iter_data.opmode) {
+ sc->opmode = iter_data.opmode;
+ ath_do_set_opmode(sc);
+ }
+
Should we really couple updating bssid mask and configuring
the opmode? Generally, I dislike adding boolean flags to
functions because it's hard to figure out from the callsite
what is happening (you have to go back to the prototype), and
it usually indicates that the abstraction is a little broken.
Well, we need to do the iteration over all VIFS to figure out what to set
this
too. Seems doing one iteration v/s doing two is worth the
extra flag?
I admit I haven't looked at the context, so I'm not sure how ugly it
is, but you can often do this kind of thing by inverting the loop:
update_vif_data()
{
for all vifs:
compute new bssid& opmode
update_bssid(new_bssid)
update_opmode(neW_opmode)
}
and call that instead (even in the single vif case).
I can just always set the opmode if you prefer that. Maybe
change the name of that method.
I'll work on a patch.
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.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
