Re: [PATCH V5 3/3] cfg80211: support ieee80211-freq-limit DT property

["Rafał Miłecki" <zajec5@gmail.com>]

On 4 January 2017 at 14:26, Johannes Berg <johannes@sipsolutions.net> wrote=
:
>
>> V4: Move code to of.c
>>     Use one helper called at init time (no runtime hooks)
>>     Modify orig_flags
>
>
>> +/**
>> + * wiphy_read_of_freq_limits - read frequency limits from device
>> tree
>> + *
>> + * @wiphy: the wireless device to get extra limits for
>> + *
>> + * Some devices may have extra limitations specified in DT. This may
>> be useful
>> + * for chipsets that normally support more bands but are limited due
>> to board
>> + * design (e.g. by antennas or extermal power amplifier).
>> + *
>> + * This function reads info from DT and uses it to *modify* channels
>> (disable
>> + * unavailable ones). It's usually a *bad* idea to use it in drivers
>> with
>> + * shared channel data as DT limitations are device specific.
>> + *
>> + * As this function access device node it has to be called after
>> set_wiphy_dev.
>> + * It also modifies channels so they have to be set first.
>> + */
>
> It should also be called before wiphy_register(), I think? And I
> suppose you should add a comment about not being able to use shared
> channels.
>
>> +                             pr_debug("Disabling freq %d MHz as
>> it's out of OF limits\n",
>> +                                      chan->center_freq);
>> +                             chan->orig_flags |=3D
>> IEEE80211_CHAN_DISABLED;
>>
> But just setting orig_flags also won't work, since it'd be overwritten
> again by wiphy_register(), no?

I told you I successfully tested it, didn't I? Well, I quickly checked
wiphy_register and couldn't understand how it was possible it worked
for me...

OK, so after some debugging I understood why I got this working. It's
the way brcmfmac handles channels.

At the beginning all channels are disabled: see __wl_2ghz_channels &
__wl_5ghz_channels. They have IEEE80211_CHAN_DISABLED set in "flags"
for every channel.

In early phase brcmfmac calls wiphy_read_of_freq_limits which sets
IEEE80211_CHAN_DISABLED in "orig_flags" for unavailable channels.

Then brcmf_construct_chaninfo kicks in. Normally it removes
IEEE80211_CHAN_DISABLED from "flags" for most of channels, but it
doesn't happen anymore due to my change:
if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
        continue;

Then brcmfmac calls wiphy_apply_custom_regulatory which sets some bits
like IEEE80211_CHAN_NO_80MHZ and IEEE80211_CHAN_NO_160MHZ in "flags".

Finally wiphy_register is called which copies "flags" to
"original_flags". As brcmfmac /respected/ IEEE80211_CHAN_DISABLED set
in orig_flags, it also left IEEE80211_CHAN_DISABLED in flags. This way
I got IEEE80211_CHAN_DISABLED in orig_flags after overwriting that
field inside wiphy_register.

That's quite crazy, right?

I guess you're right after all, I should set IEEE80211_CHAN_DISABLED
in "flags" field, let wiphy_register copy that to "orig_flags" and
sanitize brcmfmac.

--=20
Rafa=C5=82