Re: [RFC PATCH 4/8] brcm80211: smac: inform mac80211 of the X2 regulatory domain

[Seth Forshee <seth.forshee@canonical.com>]

On Sat, May 26, 2012 at 12:33:15AM +0200, Arend van Spriel wrote:
> > +#define BRCM_2GHZ_2412_2462	REG_RULE(2412-10, 2462+10, 40, 0, 19, 0)
> > +#define BRCM_2GHZ_2467_2472	REG_RULE(2467-10, 2472+10, 20, 0, 19, \
> > +					 NL80211_RRF_PASSIVE_SCAN | \
> > +					 NL80211_RRF_NO_IBSS)
> > +#define BRCM_2GHZ_2484		REG_RULE(2484-10, 2484+10, 40, 0, 19, \
> > +					 NL80211_RRF_PASSIVE_SCAN | \
> > +					 NL80211_RRF_NO_OFDM)
> > +
> 
> If channels 12 and 13 are 20MHz only, shouldn't channel 14 above
> (2484MHz) be 20MHz as well?

Heh, good point. I'm trying to remember why I made 12 and 13 20MHz only
though; maybe that's a mistake. Is there any reason why they couldn't
all be 40MHz?

> > +static const struct ieee80211_regdomain brcms_regdom_x2 = {
> > +	.n_reg_rules = 7,
> > +	.alpha2 = "X2",
> > +	.reg_rules = {
> > +		BRCM_2GHZ_2412_2462,
> > +		BRCM_2GHZ_2467_2472,
> > +		BRCM_2GHZ_2484,
> 
> The X2 domain did not support channel 14 so this is cheating ;-)

Yep, my mistake. I'll remove it.

> > +	/* Worldwide Row 2, must always be at index 0 */
> 
> Row is abbreviation for Rest of World, hence RoW. Come to think of it,
> this actually makes the term Worldwide a bit redundant.

I'll fix it.

> > +bool brcms_is_radar_freq(u16 center_freq)
> > +{
> > +	return center_freq >= 5260 && center_freq <= 5700;
> 
> You can avoid this code if you/we do not add the channels to
> wiphy->bands[IEEE80211_BAND_5GHZ]. The driver does not support DFS so we
> better stay away from these channels, which was your original issue to
> start this work if I recall correct :-( Sorry.

This is what originally got me looking at the brcmsmac regulatory
support, so it would be disappointing if at the end these channels still
aren't usable.

My understanding of DFS is pretty rudamentary. Could you explain what
support is needed? I do see that mac80211 doesn't really support power
measurement requests, but if we can at least stop transmission when
radar is present couldn't we still operate on the channels?

Luis, any insight you can offer on the subject would be appreciated.

> > +			} else {
> > +				if (ch->beacon_found)
> 
> can save one indent when using else if () immediately here.

I'll change this.

> > diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
> > index aa15558..f26e93c 100644
> > --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
> > +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
> > @@ -1059,6 +1059,8 @@ static struct brcms_info *brcms_attach(struct bcma_device *pdev)
> >  		goto fail;
> >  	}
> >  
> > +	brcms_c_regd_init(wl->wlc);
> > +
> 
> can this be moved in brcms_c_channel_mgr_attach()?

Not without rearranging the initialization, as brcms_c_regd_init() must
be called after wiphy->bands has been set up. I don't see any reason why
setting up the bands couldn't be moved to before channel_mgr_attach()
though, and in that case regd_init() could be moved into
channel_mgr_attach().