Search Linux Wireless

Re: [PATCH 1/2 v4] cfg80211: Add new wireless regulatory infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> +enum nl80211_reg_rule_flags {
> +	NL80211_RRF_NO_OFDM		= 1<<0,
> +	NL80211_RRF_NO_CCK		= 1<<1,
> +	NL80211_RRF_NO_INDOOR		= 1<<2,
> +	NL80211_RRF_NO_OUTDOOR		= 1<<3,

> +	NL80211_RRF_DFS			= 1<<4,
> +	NL80211_RRF_PTP_ONLY		= 1<<4,
> +	NL80211_RRF_PTMP_ONLY		= 1<<4,
> +	NL80211_RRF_PASSIVE_SCAN	= 1<<4,

That seems a bit wrong?

> +	NL80211_RRF_NO_IBSS		= 1<<8,
> +	/* hole at 9, used to be NO_HT20  */
> +	NL80211_RRF_NO_HT40		= 1<<10,

and I'm still not sure we should be using the same bit assignments as in
the userspace part, that'll just make people think we have to do that,
while we don't really, I'd rather see a clean set here and have the
userspace code contain an internal mapping from its values to the kernel
values.


> +struct ieee80211_freq_range {
> +	u32 start_freq;
> +	u32 end_freq;
> +	u32 max_bandwidth;

u32 seems excessive for the bandwidth, no?

> +struct ieee80211_reg_rule {
> +	struct ieee80211_freq_range freq_range;
> +	struct ieee80211_power_rule power_rule;
> +	u32 flags;

a note what those flags are would probably be good.

>  	u16 center_freq;
> +	u8 max_bandwidth;

and here you're using u8 anyway :)

> +extern int __regulatory_hint(struct wiphy *wiphy, enum reg_set_by set_by,
> +		const char *alpha2, struct ieee80211_regdomain *rd);

generally extern is left out at least in code I did, but I don't care
much; however, why is this one exported in the global header file?

> +		/* IEEE 802.11b/g, channels 1..11 */
> +		REG_RULE(2412-40, 2462+40, 40, 6, 27, 0),

that's wrong, should be -20/+20, same for all the others. Also, doesn't
it have to be in KHz? I can't really tell right now.

> + * In addition to all this we provide an extra layer of regulatory
> + * comformance. For drivers which do not have any regulatory

typo, conformance

> + * Note: When number of rules --> infinity we will not be able to
> + * index on alpha2 any more, instead we'll probably have to
> + * rely on some SHA1 checksum of the regdomain for example.

don't understand?

> +	/* ASCII 0 */
> +	if (alpha2[0] == 48 && alpha2[1] == 48)

You could use '0' instead of 48 and save the comment :)


> +				if (alpha2_equal(alpha2,
> +						cfg80211_regdomain->alpha2))
> +					return -EALREADY;
> +				/* Driver should not be trying to hint
> +				 * different regulatory domains! */
> +				BUG_ON(!alpha2_equal(alpha2,
> +						cfg80211_regdomain->alpha2));

Huh? Doesn't that like always trigger due to the if above?

> +EXPORT_SYMBOL(__regulatory_hint);

For whom does that need to be exported?

johannes

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux