On 5 January 2017 at 11:02, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
> On 5 January 2017 at 10:31, Arend Van Spriel
> <arend.vanspriel@xxxxxxxxxxxx> wrote:
>> On 4-1-2017 22:19, Rafał Miłecki wrote:
>>> I'm not exactly happy with channels management in brcmfmac. Before
>>> calling wiphy_register it already disables channels unavailable for
>>> current country. This results in setting IEEE80211_CHAN_DISABLED in
>>
>> What do you mean by current country. There is none that we are aware off
>> in the driver. So we obtain the channels for the current
>> country/revision in the firmware and enable those before
>> wiphy_register(). This all is within the probe/init sequence so I do not
>> really see an issue. As the wiphy object is not yet registered there is
>> no user-space awareness
>
> It seems I'm terrible as describing my patches/problems/ideas :( Here
> I used 1 inaccurate word and you couldn't understand my point.
>
> By "current country" I meant current region (and so a set of
> regulatory rules) used by the firmware. I believe firmware describes
> it using "ccode" and "regrev".
>
> Now, about the issue I see:
>
> AFAIU if you set IEEE80211_CHAN_DISABLED in "orig_flags" it's meant to
> be there for good. Some reference code that makes me believe so
> (reg.c):
> pr_debug("Disabling freq %d MHz for good\n", chan->center_freq);
> chan->orig_flags |= IEEE80211_CHAN_DISABLED;
>
> This is what happens with brcmfmac right now. If firmware doesn't
> report some channels, you set "flags" to IEEE80211_CHAN_DISABLED for
> them. Then you call wiphy_register which copies that "flags" to the
> "orig_flags". I read it as: we are never going to use these channels.
>
> Now, consider you support regdom change (I do with my local patches).
> You translate alpha2 to a proper firmware request (board specific!),
> you execute it and then firmware may allow you to use channels that
> you marked as disabled for good. You would need to mess with
> orig_flags to recover from this issue.
I was wrong about this. Current notifier implementation in brcmfmac
doesn't care about "orig_flags" and it just sets "flags" as it wants.
This way it can enable even these channels that were believed to be
disabled for good (DISABLED in "orig_flags").
For my test I booted SR400ac with ccode & regrev set to values that
didn't allow me to use channels 149 - 165 (5735 - 5835). It matches my
current country:
country PL: DFS-ETSI
(2402 - 2482 @ 40), (20)
(5170 - 5250 @ 80), (20), AUTO-BW
(5250 - 5330 @ 80), (20), DFS, AUTO-BW
(5490 - 5710 @ 160), (27), DFS
# 60 GHz band channels 1-4, ref: Etsi En 302 567
(57000 - 66000 @ 2160), (40)
Then I used "iw reg set ??" to set country that allows these channels.
My locally modified brcmfmac driver sent proper "country" iovar to the
firmware and queried it for the updated "chanspecs". See my debugging
messages below:
brcmfmac: [brcmf_construct_chaninfo] hw_value:34 orig_flags:0x101 flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:36 orig_flags:0x1a0 flags:0x0a0
brcmfmac: [brcmf_construct_chaninfo] hw_value:38 orig_flags:0x101 flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:40 orig_flags:0x190 flags:0x090
brcmfmac: [brcmf_construct_chaninfo] hw_value:42 orig_flags:0x101 flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:44 orig_flags:0x1a0 flags:0x0a0
brcmfmac: [brcmf_construct_chaninfo] hw_value:46 orig_flags:0x101 flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:48 orig_flags:0x190 flags:0x090
brcmfmac: [brcmf_construct_chaninfo] hw_value:52 orig_flags:0x1aa flags:0x0aa
brcmfmac: [brcmf_construct_chaninfo] hw_value:56 orig_flags:0x19a flags:0x09a
brcmfmac: [brcmf_construct_chaninfo] hw_value:60 orig_flags:0x1aa flags:0x0aa
brcmfmac: [brcmf_construct_chaninfo] hw_value:64 orig_flags:0x19a flags:0x09a
brcmfmac: [brcmf_construct_chaninfo] hw_value:100 orig_flags:0x1aa flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:104 orig_flags:0x19a flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:108 orig_flags:0x1aa flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:112 orig_flags:0x19a flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:116 orig_flags:0x1aa flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:120 orig_flags:0x19a flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:124 orig_flags:0x1aa flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:128 orig_flags:0x19a flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:132 orig_flags:0x1aa flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:136 orig_flags:0x19a flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:140 orig_flags:0x1ba flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:144 orig_flags:0x101 flags:0x001
brcmfmac: [brcmf_construct_chaninfo] hw_value:149 orig_flags:0x101 flags:0x0a0
brcmfmac: [brcmf_construct_chaninfo] hw_value:153 orig_flags:0x101 flags:0x090
brcmfmac: [brcmf_construct_chaninfo] hw_value:157 orig_flags:0x101 flags:0x0a0
brcmfmac: [brcmf_construct_chaninfo] hw_value:161 orig_flags:0x101 flags:0x090
brcmfmac: [brcmf_construct_chaninfo] hw_value:165 orig_flags:0x101 flags:0x0b0
As you can see brcmfmac dropped IEEE80211_CHAN_DISABLED (hint: 0x1)
for channels 149 - 165 even though they were disabled according to the
"orig_flags".
So this patch with its
if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
continue;
could be considered some kind of regression. My change wouldn't allow
enabling such channels anymore.
Of course noone would notice this as noone uses country_codes table
yet I guess (except for me locally). Anyway, this patch should be
reworked to make sure it still allows implementing support for
country_codes tables in the future.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
