On Fri, Jul 31, 2009 at 10:28 AM, Luis R. Rodriguez<lrodriguez@xxxxxxxxxxx> wrote: > On Fri, Jul 31, 2009 at 9:43 AM, Johannes Berg<johannes@xxxxxxxxxxxxxxxx> wrote: >> On Fri, 2009-07-31 at 09:32 -0700, Luis R. Rodriguez wrote: >> >>> Its not that simple. Consider the fact we build our own custom >>> regulatory domain and stuff what is intended for each one of them. The >>> _orig stuff is set upon channel registration so it is correct that >>> we'd have to avoid setting the channel flags prior to wiphy >>> registration. How you do that is left up to implementation. Since >>> cfg80211 will set the channel *_orig params based on >>> wiphy_registration() you're only option is to either prevent your >>> wiphy being registered with the flags set or to reset the channel >>> flags on the reg_notifier(). The later seems like the way to go but we >>> currently do not call the reg_notifier() upon beacon hints -- we >>> currently only call the reg_notifier() upon regulatory domain changes >>> and upon wiphy registration. My point is both of these options require >>> considerable changes for 2.6.31. >> >> Aha. So the problem really is that we don't have a reg notifier on >> updates due to beacons. > > Yeah, seems that's the core issue from the original implementation. We > should have added that. > >>> > and set them in ath_reg_apply_beaconing_flags and >>> > ath_reg_apply_active_scan_flags, changing the polarity? >>> > >>> > I mean, right now you tell cfg80211 you don't support it, >>> > and then try >>> > to support it anyhow. >>> >>> Not sure I follow, support what? The reg_notifier() is there for >>> regulatory domain changes, we didn't call it upon beacon hints. We >>> can, and I agree its the right approach, but not for 2.6.31. >> >> I'm thinking more in terms of iwlwifi here, for all intents and purposes >> it doesn't support beaconing on channel N, so it only supports NO_IBSS >> on that channel. Which is what it puts into flags before registration. > > Ah I see, well but remember also that devices may have custom world > regulatory domains for which they can set the no-ibss/passive scan > flags and then allow for it due to a beacon hint. > >>> > Instead, you could tell cfg80211 you _do_ support >>> > it, and then not support them depending on the notifier? It seems like >>> > that should work and not break cfg80211's assumption that you can never >>> > ever support _more_ than registration flags (orig_flags). >>> >>> I'm not following at all, orig_flags do not tell cfg80211 the channel >>> flags the device supports. Most devices set flag to 0 upon wiphy >>> registration and therefore cfg80211 sets orig_flags to 0 as well >>> during wiphy registration. >> >> Well ok, "supports" is a bad word since we're talking about >> restrictions. But for iwlwifi the orig_flags _do_ tell cfg80211 the >> channel restrictions that the device _requires_. > > Oh ok, got it, yeah agreed, but its important to note we never > documented nor was I aware we wanted to treat flags set prior to wiphy > registration as capabilities. Truth is the device is actually capable > of active scanning and beaconing on those channels but the design of > the firmware doesn't allow for it. > >> Your devices, however, do not _strictly_ _require_ those channel flags, >> they just require them for compliance. Unlike iwlwifi which crashes when >> you don't do it right :) >> >> So I think we're in agreement -- the proper solution would be to call >> the reg notifier on beacon hints too. > > Yeah, unless we want to just document an exception to the rules, but I > do agree that seems like a slippery slope. > >> What we do for .31 I can't say I >> really care, > > OK, I do, please consider this patch for 2.6.31 and also on > wireless-testing for now. > >> how much work do you think this would be? It doesn't seem >> all _that_ bad, at least for core changes? > > After some thinking the reg_notifier() does not seem to be the > appropriate place for this. The reg_notifier() was designed to account > for regulatory domain changes, not enhancements such as beacon hints. > Consider the regulatory_request structure passed as the second > argument to reg_notifier() and the currently supported types of > regulatory domain changes, %REGDOM_SET_BY_*. > > What seems cleaner is to add a beacon_hint() notifier and use that at > the end of handle_reg_beacon() so that we already give the driver the > channel on which a beacon was received. And this in itself would be a no-op if we use the patch I posted, and we treat iwlwifi on this matter as an exception. So I'm now more happy to support this patch as-is. Luis -- 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
