The subject is a bit misleading IMHO - you don't skip all checks when
there's puncturing ...
maybe just say "wifi: cfg80211: skip regulatory for punctured
subchannels"
> However, these checks are also performed on frequencies that
> have been punctured, which should not be examined as they are
> not in use.
I'd argue subchannels are punctured (or really disabled, the whole
channel is punctured ... but we mix that up already), not frequencies
> static void cfg80211_set_chans_dfs_state(struct wiphy *wiphy, u32 center_freq,
> - u32 bandwidth,
> + u32 bandwidth, u16 punctured,
> enum nl80211_dfs_state dfs_state)
> {
> struct ieee80211_channel *c;
> u32 freq;
> + int subchan = 0;
>
> for (freq = center_freq - bandwidth/2 + 10;
> freq <= center_freq + bandwidth/2 - 10;
> freq += 20) {
> + if (punctured & BIT(subchan))
> + continue;
> + subchan++;
>
You never tested this code properly, it's clearly broken.
but anyway - for_each_subchan()?
> +#define for_each_subchan(wiphy, center_freq, bandwidth, punctured, \
> + subchan) \
I feel like we really should make this work on a chandef, not all these
arguments ... and cover both center_freq1 and center_freq2, because we
have all these duplicate calls like cfg80211_chandef_dfs_cac_time().
> + for (subchan = ieee80211_next_subchan(wiphy, center_freq, bandwidth, \
> + punctured, NULL); \
It should be especially easy if you're pulling it out into iterator
functions? Worst case, keep some extra state in the loop, like
for (u32 punctured = chandef->punctured,
freq = cfg80211_get_start_freq(chandef, 1),
_cf = 1;
freq = _cf == 1 ? 20, punctured >>= 1;
...)
if (!(punctured & 1))
I don't really mind a bit more complexity here, if it means we can get
rid of all the functions like cfg80211_get_chans_dfs_required() that get
called twice ...
johannes
