* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Kalle Valo @ 2017-01-04 8:14 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <CACna6rzGRHLCdzxS_+MHXeZwF=BW4S52pDb9g9MuanNGyToEOw@mail.gmail.com>
Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:
>>> @@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcm=
f_cfg80211_info *cfg,
>>> ch.bw =3D=3D BRCMU_CHAN_BW_80)
>>> continue;
>>>
>>> - channel =3D band->channels;
>>> - index =3D band->n_channels;
>>> + channel =3D NULL;
>>> for (j =3D 0; j < band->n_channels; j++) {
>>> - if (channel[j].hw_value =3D=3D ch.control_ch_num)=
{
>>> - index =3D j;
>>> + if (band->channels[j].hw_value =3D=3D ch.control_=
ch_num) {
>>> + channel =3D &band->channels[j];
>>> break;
>>> }
>>> }
>>
>> You could have kept the index construct and simply check if j =3D=3D
>> band->n_channels here to determine something is wrong.
>
> I wanted to simplify code at the same time. Having channel[index]
> repeated 7 times was a hint for me it could be handled better. I
> should have made that clear, I'll fix improve this in V2.
If you are making a patch to stable or -rc releases you should keep the
patch as simple as possible and do all the cleanup later. But I see that
you dropped "cc stable" in this patch so all is good, just a general
remark.
--=20
Kalle Valo
^ permalink raw reply
* Re: [RFC] nl80211: allow multiple active scheduled scan requests
From: Arend Van Spriel @ 2017-01-04 10:20 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Dmitry Shmidt
In-Reply-To: <1483523988.15591.9.camel@sipsolutions.net>
On 4-1-2017 10:59, Johannes Berg wrote:
> On Tue, 2017-01-03 at 13:25 +0100, Arend Van Spriel wrote:
>> On 2-1-2017 11:44, Johannes Berg wrote:
>>>
>>>> + /*
>>>> + * allow only one legacy scheduled scan if user-space
>>>> + * does not indicate multiple scheduled scan support.
>>>> + */
>>>> + if (!info->attrs[NL80211_ATTR_SCHED_SCAN_MULTI] &&
>>>> + cfg80211_legacy_sched_scan_active(rdev))
>>>> return -EINPROGRESS;
>>>
>>> That probably doesn't go far enough - if legacy one is active then
>>> we
>>> probably shouldn't allow a new MULTI one either (or abandon the
>>> legacy
>>> one) so that older userspace doesn't get confused with multiple
>>> notifications from sched scans it didn't start.
>>
>> I considered that although not taking the notifications into account.
>> Will change it. Abandoning the legacy one would be a behavioral
>> change so probably not acceptable, right?
>
> Well, it would be acceptable since it's documented that it's possible
> it might stop for any reason. However, we need to prefer something -
> always preferring the new sched scan could lead to bounces, so we can
> prefer (1) existing, (2) legacy-single type or (3) new-multi type, but
> not (4) new sched scan.
Not sure I can follow. What is the difference between (1) and (2). Also
what do you consider (4) new sched scan. You mean the additional
parameterization of the scheduled scan?
> I think preferring the existing would probably be best, i.e. refuse
> legacy if any sched scan is running, and refuse multi if legacy is
> running?
Whatever the response above, I can understand this and it seems most
straightforward. So I tend agree this is our best option although maybe
for the wrong reason.
>> I guess your remark means this clarifies your earlier question about
>> the request id, right?
>
> yeah.
Thanks,
Arend
^ permalink raw reply
* Re: [RFC] nl80211: allow multiple active scheduled scan requests
From: Johannes Berg @ 2017-01-04 10:30 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless, Dmitry Shmidt
In-Reply-To: <5ad9a594-29b3-8c52-a88f-c4186511fe4f@broadcom.com>
> However, we need to prefer something
> > -
> > always preferring the new sched scan could lead to bounces, so we
> > can
> > prefer (1) existing, (2) legacy-single type or (3) new-multi type,
> > but
> > not (4) new sched scan.
>
> Not sure I can follow. What is the difference between (1) and (2).
(1) would never cancel any existing sched scan, regardless of type
(legacy vs. multi-capable)
(2) would cancel an existing sched scan (in favour of a new one) if the
existing one is multi-capable
(3) would cancel an existing sched scan (in favour of a new one) if the
existing one is legacy type
> Also
> what do you consider (4) new sched scan. You mean the additional
> parameterization of the scheduled scan?
No, I just meant any new request.
> > I think preferring the existing would probably be best, i.e. refuse
> > legacy if any sched scan is running, and refuse multi if legacy is
> > running?
>
> Whatever the response above, I can understand this and it seems most
> straightforward. So I tend agree this is our best option although
> maybe for the wrong reason.
:)
johannes
^ permalink raw reply
* Re: [RFC] nl80211: allow multiple active scheduled scan requests
From: Arend Van Spriel @ 2017-01-04 10:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Dmitry Shmidt
In-Reply-To: <1483525836.7312.1.camel@sipsolutions.net>
On 4-1-2017 11:30, Johannes Berg wrote:
>> However, we need to prefer something
>>> -
>>> always preferring the new sched scan could lead to bounces, so we
>>> can
>>> prefer (1) existing, (2) legacy-single type or (3) new-multi type,
>>> but
>>> not (4) new sched scan.
>>
>> Not sure I can follow. What is the difference between (1) and (2).
>
> (1) would never cancel any existing sched scan, regardless of type
> (legacy vs. multi-capable)
>
> (2) would cancel an existing sched scan (in favour of a new one) if the
> existing one is multi-capable
>
> (3) would cancel an existing sched scan (in favour of a new one) if the
> existing one is legacy type
yes, yes. sinking in :-p
>> Also
>> what do you consider (4) new sched scan. You mean the additional
>> parameterization of the scheduled scan?
>
> No, I just meant any new request.
Yeah, so there is already an existing/active sched scan. Clear.
>>> I think preferring the existing would probably be best, i.e. refuse
>>> legacy if any sched scan is running, and refuse multi if legacy is
>>> running?
>>
>> Whatever the response above, I can understand this and it seems most
>> straightforward. So I tend agree this is our best option although
>> maybe for the wrong reason.
>
> :)
Thanks for clarifying it.
Gr. AvS
^ permalink raw reply
* Can't authenticate to WPA network with rt2800usb driver
From: Xavier Bestel @ 2017-01-04 10:37 UTC (permalink / raw)
To: linux-wireless
Hi,
I have this USB WiFi dongle:
[2921541.271677] usb 3-12.6: new high-speed USB device number 65 using xhci_hcd
[2921541.393139] usb 3-12.6: New USB device found, idVendor=148f, idProduct=5370
[2921541.393141] usb 3-12.6: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[2921541.393142] usb 3-12.6: Product: 802.11 n WLAN
[2921541.393142] usb 3-12.6: Manufacturer: Ralink
[2921541.393143] usb 3-12.6: SerialNumber: 1.0
[2921541.471914] usb 3-12.6: reset high-speed USB device number 65 using xhci_hcd
[2921541.586974] ieee80211 phy4: rt2x00_set_rt: Info - RT chipset 5390, rev 0502 detected
[2921541.596791] ieee80211 phy4: rt2x00_set_rf: Info - RF chipset 5370 detected
[2921541.597005] ieee80211 phy4: Selected rate control algorithm 'minstrel_ht'
[2921541.806292] rt2800usb 3-12.6:1.0 wlx7cdd90463ef8: renamed from wlan0
[2921541.979061] IPv6: ADDRCONF(NETDEV_UP): wlx7cdd90463ef8: link is not ready
[2921541.979088] ieee80211 phy4: rt2x00lib_request_firmware: Info - Loading firmware file 'rt2870.bin'
[2921542.027030] rt2800usb 3-12.6:1.0: firmware: direct-loading firmware rt2870.bin
[2921542.027035] ieee80211 phy4: rt2x00lib_request_firmware: Info - Firmware detected - version: 0.36
It's inserted into a debian jessie box, kernel 4.8.0-1-amd64.
The box refuses to connect to a WPA AP (although it connects fine with
a PCI WiFi card):
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.1350] device (wlx7cdd90463ef8): Activation: starting connection 'MySecretSSID 2' (e6482e2d-adbc-4529-bafc-24c3e54f07e6)
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.1351] audit: op="connection-activate" uuid="e6482e2d-adbc-4529-bafc-24c3e54f07e6" name="MySecretSSID 2" pid=1599 uid=1000 result="success"
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.1352] device (wlx7cdd90463ef8): state change: disconnected -> prepare (reason 'none') [30 40 0]
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.1445] device (wlx7cdd90463ef8): set-hw-addr: set-cloned MAC address to 7C:DD:90:46:3E:F8 (permanent)
Jan 4 11:22:29 pcxav kernel: [2921135.939166] IPv6: ADDRCONF(NETDEV_UP): wlx7cdd90463ef8: link is not ready
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3266] device (wlx7cdd90463ef8): state change: prepare -> config (reason 'none') [40 50 0]
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3267] device (wlx7cdd90463ef8): Activation: (wifi) access point 'MySecretSSID 2' has security, but secrets are required.
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3267] device (wlx7cdd90463ef8): state change: config -> need-auth (reason 'none') [50 60 0]
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3287] device (wlx7cdd90463ef8): state change: need-auth -> prepare (reason 'none') [60 40 0]
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3289] device (wlx7cdd90463ef8): state change: prepare -> config (reason 'none') [40 50 0]
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3290] device (wlx7cdd90463ef8): Activation: (wifi) connection 'MySecretSSID 2' has security, and secrets exist. No new secrets needed.
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3291] Config: added 'ssid' value 'MySecretSSID'
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3291] Config: added 'scan_ssid' value '1'
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3291] Config: added 'key_mgmt' value 'WPA-PSK'
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3291] Config: added 'auth_alg' value 'OPEN'
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3291] Config: added 'psk' value '<omitted>'
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3529] sup-iface[0x55f2087344e0,wlx7cdd90463ef8] config: set interface ap_scan to 1
Jan 4 11:22:29 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: SME: Trying to authenticate with 5c:33:8e:56:78:d3 (SSID='MySecretSSID' freq=2437 MHz)
Jan 4 11:22:29 pcxav kernel: [2921135.972360] wlx7cdd90463ef8: authenticate with 5c:33:8e:56:78:d3
Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3774] device (wlx7cdd90463ef8): supplicant interface state: inactive -> authenticating
Jan 4 11:22:29 pcxav kernel: [2921135.990588] wlx7cdd90463ef8: send auth to 5c:33:8e:56:78:d3 (try 1/3)
Jan 4 11:22:29 pcxav kernel: [2921135.999983] wlx7cdd90463ef8: authenticated
Jan 4 11:22:34 pcxav kernel: [2921140.991625] wlx7cdd90463ef8: aborting authentication with 5c:33:8e:56:78:d3 by local choice (Reason: 3=DEAUTH_LEAVING)
Jan 4 11:22:34 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: CTRL-EVENT-SSID-TEMP-DISABLED id=1 ssid="MySecretSSID" auth_failures=1 duration=10 reason=CONN_FAILED
Jan 4 11:22:34 pcxav NetworkManager[15150]: <info> [1483525354.4250] device (wlx7cdd90463ef8): supplicant interface state: authenticating -> disconnected
Jan 4 11:22:44 pcxav NetworkManager[15150]: <info> [1483525364.4264] device (wlx7cdd90463ef8): supplicant interface state: disconnected -> scanning
Jan 4 11:22:45 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: CTRL-EVENT-SSID-REENABLED id=1 ssid="MySecretSSID"
Jan 4 11:22:45 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: SME: Trying to authenticate with 5c:33:8e:56:78:d3 (SSID='MySecretSSID' freq=2437 MHz)
Jan 4 11:22:45 pcxav kernel: [2921152.045396] wlx7cdd90463ef8: authenticate with 5c:33:8e:56:78:d3
Jan 4 11:22:45 pcxav NetworkManager[15150]: <info> [1483525365.4655] device (wlx7cdd90463ef8): supplicant interface state: scanning -> authenticating
Jan 4 11:22:45 pcxav kernel: [2921152.079166] wlx7cdd90463ef8: send auth to 5c:33:8e:56:78:d3 (try 1/3)
Jan 4 11:22:45 pcxav kernel: [2921152.081749] wlx7cdd90463ef8: authenticated
Jan 4 11:22:50 pcxav kernel: [2921157.080963] wlx7cdd90463ef8: aborting authentication with 5c:33:8e:56:78:d3 by local choice (Reason: 3=DEAUTH_LEAVING)
Jan 4 11:22:50 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: CTRL-EVENT-SSID-TEMP-DISABLED id=1 ssid="MySecretSSID" auth_failures=2 duration=20 reason=CONN_FAILED
Jan 4 11:22:50 pcxav NetworkManager[15150]: <info> [1483525370.5091] device (wlx7cdd90463ef8): supplicant interface state: authenticating -> disconnected
Jan 4 11:22:54 pcxav NetworkManager[15150]: <warn> [1483525374.7275] device (wlx7cdd90463ef8): Activation: (wifi) association took too long, failing activation
Jan 4 11:22:54 pcxav NetworkManager[15150]: <info> [1483525374.7275] device (wlx7cdd90463ef8): state change: config -> failed (reason 'ssid-not-found') [50 120 53]
Jan 4 11:22:54 pcxav NetworkManager[15150]: <warn> [1483525374.7279] device (wlx7cdd90463ef8): Activation: failed for connection 'MySecretSSID 2'
Jan 4 11:22:54 pcxav NetworkManager[15150]: <info> [1483525374.7282] device (wlx7cdd90463ef8): state change: failed -> disconnected (reason 'none') [120 30 0]
Jan 4 11:22:54 pcxav kernel: [2921161.343145] IPv6: ADDRCONF(NETDEV_UP): wlx7cdd90463ef8: link is not ready
Jan 4 11:22:54 pcxav NetworkManager[15150]: <info> [1483525374.7454] device (wlx7cdd90463ef8): set-hw-addr: set MAC address to EA:EC:7C:7D:D6:86 (scanning)
Jan 4 11:22:54 pcxav kernel: [2921161.536075] IPv6: ADDRCONF(NETDEV_UP): wlx7cdd90463ef8: link is not ready
Jan 4 11:22:55 pcxav NetworkManager[15150]: <info> [1483525375.9561] device (wlx7cdd90463ef8): supplicant interface state: disconnected -> inactive
Does anyone have a hint to make it work ?
Regards,
Xav
^ permalink raw reply
* Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-04 10:40 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <17d4eeb8-0cc2-4e0a-ad16-442dfec32a08@broadcom.com>
On 4 January 2017 at 10:39, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 17:49, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> Our code was assigning number of channels to the index variable by
>> default. If firmware reported channel we didn't predict this would
>> result in using that initial index value and writing out of array. This
>> never happened so far (we got a complete list of supported channels) but
>> it means possible memory corruption so we should handle it anyway.
>>
>> This patch simply detects unexpected channel and ignores it.
>>
>> As we don't try to create new entry now, it's also safe to drop hw_value
>> and center_freq assignment. For known channels we have these set anyway.
>>
>> I decided to fix this issue by assigning NULL or a target channel to the
>> channel variable. This was one of possible ways, I prefefred this one as
>> it also avoids using channel[index] over and over.
>>
>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiph=
y bands")
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> V2: Add extra comment in code for not-found channel.
>> Make it clear this problem have never been seen so far
>> Explain why it's safe to drop extra assignments
>> Note & reason changing channel variable usage
>> ---
>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++---=
-------
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c=
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 9c2c128..a16dd7b 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_c=
fg80211_info *cfg,
>> u32 i, j;
>> u32 total;
>> u32 chaninfo;
>> - u32 index;
>>
>> pbuf =3D kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>>
>> @@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf=
_cfg80211_info *cfg,
>> ch.bw =3D=3D BRCMU_CHAN_BW_80)
>> continue;
>>
>> - channel =3D band->channels;
>> - index =3D band->n_channels;
>> + channel =3D NULL;
>> for (j =3D 0; j < band->n_channels; j++) {
>> - if (channel[j].hw_value =3D=3D ch.control_ch_num) =
{
>> - index =3D j;
>> + if (band->channels[j].hw_value =3D=3D ch.control_c=
h_num) {
>> + channel =3D &band->channels[j];
>> break;
>> }
>> }
>> - channel[index].center_freq =3D
>> - ieee80211_channel_to_frequency(ch.control_ch_num,
>> - band->band);
>> - channel[index].hw_value =3D ch.control_ch_num;
>> + if (!channel) {
>> + /* It seems firmware supports some channel we neve=
r
>> + * considered. Something new in IEEE standard?
>> + */
>> + brcmf_err("Firmware reported unexpected channel %d=
\n",
>> + ch.control_ch_num);
>
> Maybe rephrase to "Ignoring unexpected firmware channel %d\n" so
> end-users are not alarmed by this error message. I think using
> brcmf_err() is justified, but you may even consider chiming down to
> brcmf_dbg(INFO, ...).
Can you suggest a better error message? It seems I'm too brave and I
don't find this one alarming, so I need suggestion.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array
From: Arend Van Spriel @ 2017-01-04 10:48 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <CACna6rzFvpjfV_aTmYOac6_jE4XPmq6k6kNGwFM9_xFKgJ7MuA@mail.gmail.com>
On 4-1-2017 11:40, Rafał Miłecki wrote:
> On 4 January 2017 at 10:39, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 3-1-2017 17:49, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Our code was assigning number of channels to the index variable by
>>> default. If firmware reported channel we didn't predict this would
>>> result in using that initial index value and writing out of array. This
>>> never happened so far (we got a complete list of supported channels) but
>>> it means possible memory corruption so we should handle it anyway.
>>>
>>> This patch simply detects unexpected channel and ignores it.
>>>
>>> As we don't try to create new entry now, it's also safe to drop hw_value
>>> and center_freq assignment. For known channels we have these set anyway.
>>>
>>> I decided to fix this issue by assigning NULL or a target channel to the
>>> channel variable. This was one of possible ways, I prefefred this one as
>>> it also avoids using channel[index] over and over.
>>>
>>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> V2: Add extra comment in code for not-found channel.
>>> Make it clear this problem have never been seen so far
>>> Explain why it's safe to drop extra assignments
>>> Note & reason changing channel variable usage
>>> ---
>>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++----------
>>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 9c2c128..a16dd7b 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
>>> u32 i, j;
>>> u32 total;
>>> u32 chaninfo;
>>> - u32 index;
>>>
>>> pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>>>
>>> @@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
>>> ch.bw == BRCMU_CHAN_BW_80)
>>> continue;
>>>
>>> - channel = band->channels;
>>> - index = band->n_channels;
>>> + channel = NULL;
>>> for (j = 0; j < band->n_channels; j++) {
>>> - if (channel[j].hw_value == ch.control_ch_num) {
>>> - index = j;
>>> + if (band->channels[j].hw_value == ch.control_ch_num) {
>>> + channel = &band->channels[j];
>>> break;
>>> }
>>> }
>>> - channel[index].center_freq =
>>> - ieee80211_channel_to_frequency(ch.control_ch_num,
>>> - band->band);
>>> - channel[index].hw_value = ch.control_ch_num;
>>> + if (!channel) {
>>> + /* It seems firmware supports some channel we never
>>> + * considered. Something new in IEEE standard?
>>> + */
>>> + brcmf_err("Firmware reported unexpected channel %d\n",
>>> + ch.control_ch_num);
>>
>> Maybe rephrase to "Ignoring unexpected firmware channel %d\n" so
>> end-users are not alarmed by this error message. I think using
>> brcmf_err() is justified, but you may even consider chiming down to
>> brcmf_dbg(INFO, ...).
>
> Can you suggest a better error message? It seems I'm too brave and I
> don't find this one alarming, so I need suggestion.
Uhm. There is a suggestion above. :-p
Regards,
Arend
^ permalink raw reply
* [PATCH] cfg80211: only pass sband to set_mandatory_flags_band()
From: Arend van Spriel @ 2017-01-04 10:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Arend van Spriel
The supported band structure contains the band is applies to
so no need to pass it separately. Also added a default case
to the switch for completeness. The current code base does not
call this function with NUM_NL80211_BANDS but kept that case
statement although default case would cover that.
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Stumbled upon this function and wanted to start the new year lightly.
It applies to master branch of mac80211-next repo.
Best wishes,
Arend
---
net/wireless/util.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 2cf7df8..c91bc25 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -136,12 +136,11 @@ struct ieee80211_channel *ieee80211_get_channel(struct wiphy *wiphy, int freq)
}
EXPORT_SYMBOL(ieee80211_get_channel);
-static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
- enum nl80211_band band)
+static void set_mandatory_flags_band(struct ieee80211_supported_band *sband)
{
int i, want;
- switch (band) {
+ switch (sband->band) {
case NL80211_BAND_5GHZ:
want = 3;
for (i = 0; i < sband->n_bitrates; i++) {
@@ -191,6 +190,7 @@ static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
WARN_ON((sband->ht_cap.mcs.rx_mask[0] & 0x1e) != 0x1e);
break;
case NUM_NL80211_BANDS:
+ default:
WARN_ON(1);
break;
}
@@ -202,7 +202,7 @@ void ieee80211_set_bitrate_flags(struct wiphy *wiphy)
for (band = 0; band < NUM_NL80211_BANDS; band++)
if (wiphy->bands[band])
- set_mandatory_flags_band(wiphy->bands[band], band);
+ set_mandatory_flags_band(wiphy->bands[band]);
}
bool cfg80211_supported_cipher_suite(struct wiphy *wiphy, u32 cipher)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-04 11:04 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <f2f23dca-d5e6-b127-7fa1-ebebdcbf19be@broadcom.com>
On 4 January 2017 at 11:48, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 4-1-2017 11:40, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 4 January 2017 at 10:39, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 3-1-2017 17:49, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> Our code was assigning number of channels to the index variable by
>>>> default. If firmware reported channel we didn't predict this would
>>>> result in using that initial index value and writing out of array. Thi=
s
>>>> never happened so far (we got a complete list of supported channels) b=
ut
>>>> it means possible memory corruption so we should handle it anyway.
>>>>
>>>> This patch simply detects unexpected channel and ignores it.
>>>>
>>>> As we don't try to create new entry now, it's also safe to drop hw_val=
ue
>>>> and center_freq assignment. For known channels we have these set anywa=
y.
>>>>
>>>> I decided to fix this issue by assigning NULL or a target channel to t=
he
>>>> channel variable. This was one of possible ways, I prefefred this one =
as
>>>> it also avoids using channel[index] over and over.
>>>>
>>>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wi=
phy bands")
>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>> ---
>>>> V2: Add extra comment in code for not-found channel.
>>>> Make it clear this problem have never been seen so far
>>>> Explain why it's safe to drop extra assignments
>>>> Note & reason changing channel variable usage
>>>> ---
>>>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++-=
---------
>>>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=
.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> index 9c2c128..a16dd7b 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf=
_cfg80211_info *cfg,
>>>> u32 i, j;
>>>> u32 total;
>>>> u32 chaninfo;
>>>> - u32 index;
>>>>
>>>> pbuf =3D kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>>>>
>>>> @@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brc=
mf_cfg80211_info *cfg,
>>>> ch.bw =3D=3D BRCMU_CHAN_BW_80)
>>>> continue;
>>>>
>>>> - channel =3D band->channels;
>>>> - index =3D band->n_channels;
>>>> + channel =3D NULL;
>>>> for (j =3D 0; j < band->n_channels; j++) {
>>>> - if (channel[j].hw_value =3D=3D ch.control_ch_num=
) {
>>>> - index =3D j;
>>>> + if (band->channels[j].hw_value =3D=3D ch.control=
_ch_num) {
>>>> + channel =3D &band->channels[j];
>>>> break;
>>>> }
>>>> }
>>>> - channel[index].center_freq =3D
>>>> - ieee80211_channel_to_frequency(ch.control_ch_num=
,
>>>> - band->band);
>>>> - channel[index].hw_value =3D ch.control_ch_num;
>>>> + if (!channel) {
>>>> + /* It seems firmware supports some channel we ne=
ver
>>>> + * considered. Something new in IEEE standard?
>>>> + */
>>>> + brcmf_err("Firmware reported unexpected channel =
%d\n",
>>>> + ch.control_ch_num);
>>>
>>> Maybe rephrase to "Ignoring unexpected firmware channel %d\n" so
>>> end-users are not alarmed by this error message. I think using
>>> brcmf_err() is justified, but you may even consider chiming down to
>>> brcmf_dbg(INFO, ...).
>>
>> Can you suggest a better error message? It seems I'm too brave and I
>> don't find this one alarming, so I need suggestion.
>
> Uhm. There is a suggestion above. :-p
... sorry, it seems I should take a break ;)
--=20
Rafa=C5=82
^ permalink raw reply
* [PATCH V3] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-04 11:09 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170103164930.29989-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Our code was assigning number of channels to the index variable by
default. If firmware reported channel we didn't predict this would
result in using that initial index value and writing out of array. This
never happened so far (we got a complete list of supported channels) but
it means possible memory corruption so we should handle it anyway.
This patch simply detects unexpected channel and ignores it.
As we don't try to create new entry now, it's also safe to drop hw_value
and center_freq assignment. For known channels we have these set anyway.
I decided to fix this issue by assigning NULL or a target channel to the
channel variable. This was one of possible ways, I prefefred this one as
it also avoids using channel[index] over and over.
Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Add extra comment in code for not-found channel.
Make it clear this problem have never been seen so far
Explain why it's safe to drop extra assignments
Note & reason changing channel variable usage
V3: Update error message as suggested by Arend.
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++----------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 9c2c128..75ef23b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
u32 i, j;
u32 total;
u32 chaninfo;
- u32 index;
pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
@@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
ch.bw == BRCMU_CHAN_BW_80)
continue;
- channel = band->channels;
- index = band->n_channels;
+ channel = NULL;
for (j = 0; j < band->n_channels; j++) {
- if (channel[j].hw_value == ch.control_ch_num) {
- index = j;
+ if (band->channels[j].hw_value == ch.control_ch_num) {
+ channel = &band->channels[j];
break;
}
}
- channel[index].center_freq =
- ieee80211_channel_to_frequency(ch.control_ch_num,
- band->band);
- channel[index].hw_value = ch.control_ch_num;
+ if (!channel) {
+ /* It seems firmware supports some channel we never
+ * considered. Something new in IEEE standard?
+ */
+ brcmf_err("Ignoring unexpected firmware channel %d\n",
+ ch.control_ch_num);
+ continue;
+ }
/* assuming the chanspecs order is HT20,
* HT40 upper, HT40 lower, and VHT80.
*/
if (ch.bw == BRCMU_CHAN_BW_80) {
- channel[index].flags &= ~IEEE80211_CHAN_NO_80MHZ;
+ channel->flags &= ~IEEE80211_CHAN_NO_80MHZ;
} else if (ch.bw == BRCMU_CHAN_BW_40) {
- brcmf_update_bw40_channel_flag(&channel[index], &ch);
+ brcmf_update_bw40_channel_flag(channel, &ch);
} else {
/* enable the channel and disable other bandwidths
* for now as mentioned order assure they are enabled
* for subsequent chanspecs.
*/
- channel[index].flags = IEEE80211_CHAN_NO_HT40 |
- IEEE80211_CHAN_NO_80MHZ;
+ channel->flags = IEEE80211_CHAN_NO_HT40 |
+ IEEE80211_CHAN_NO_80MHZ;
ch.bw = BRCMU_CHAN_BW_20;
cfg->d11inf.encchspec(&ch);
chaninfo = ch.chspec;
@@ -5907,11 +5909,11 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
&chaninfo);
if (!err) {
if (chaninfo & WL_CHAN_RADAR)
- channel[index].flags |=
+ channel->flags |=
(IEEE80211_CHAN_RADAR |
IEEE80211_CHAN_NO_IR);
if (chaninfo & WL_CHAN_PASSIVE)
- channel[index].flags |=
+ channel->flags |=
IEEE80211_CHAN_NO_IR;
}
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH V3] brcmfmac: avoid writing channel out of allocated array
From: Arend Van Spriel @ 2017-01-04 11:11 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170104110941.21261-1-zajec5@gmail.com>
On 4-1-2017 12:09, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
Not taking a break?
> Our code was assigning number of channels to the index variable by
> default. If firmware reported channel we didn't predict this would
> result in using that initial index value and writing out of array. This
> never happened so far (we got a complete list of supported channels) but
> it means possible memory corruption so we should handle it anyway.
>
> This patch simply detects unexpected channel and ignores it.
>
> As we don't try to create new entry now, it's also safe to drop hw_value
> and center_freq assignment. For known channels we have these set anyway.
>
> I decided to fix this issue by assigning NULL or a target channel to the
> channel variable. This was one of possible ways, I prefefred this one as
> it also avoids using channel[index] over and over.
>
> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Add extra comment in code for not-found channel.
> Make it clear this problem have never been seen so far
> Explain why it's safe to drop extra assignments
> Note & reason changing channel variable usage
> V3: Update error message as suggested by Arend.
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++----------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 9c2c128..75ef23b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> u32 i, j;
> u32 total;
> u32 chaninfo;
> - u32 index;
>
> pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>
> @@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> ch.bw == BRCMU_CHAN_BW_80)
> continue;
>
> - channel = band->channels;
> - index = band->n_channels;
> + channel = NULL;
> for (j = 0; j < band->n_channels; j++) {
> - if (channel[j].hw_value == ch.control_ch_num) {
> - index = j;
> + if (band->channels[j].hw_value == ch.control_ch_num) {
> + channel = &band->channels[j];
> break;
> }
> }
> - channel[index].center_freq =
> - ieee80211_channel_to_frequency(ch.control_ch_num,
> - band->band);
> - channel[index].hw_value = ch.control_ch_num;
> + if (!channel) {
> + /* It seems firmware supports some channel we never
> + * considered. Something new in IEEE standard?
> + */
> + brcmf_err("Ignoring unexpected firmware channel %d\n",
> + ch.control_ch_num);
> + continue;
> + }
>
> /* assuming the chanspecs order is HT20,
> * HT40 upper, HT40 lower, and VHT80.
> */
> if (ch.bw == BRCMU_CHAN_BW_80) {
> - channel[index].flags &= ~IEEE80211_CHAN_NO_80MHZ;
> + channel->flags &= ~IEEE80211_CHAN_NO_80MHZ;
> } else if (ch.bw == BRCMU_CHAN_BW_40) {
> - brcmf_update_bw40_channel_flag(&channel[index], &ch);
> + brcmf_update_bw40_channel_flag(channel, &ch);
> } else {
> /* enable the channel and disable other bandwidths
> * for now as mentioned order assure they are enabled
> * for subsequent chanspecs.
> */
> - channel[index].flags = IEEE80211_CHAN_NO_HT40 |
> - IEEE80211_CHAN_NO_80MHZ;
> + channel->flags = IEEE80211_CHAN_NO_HT40 |
> + IEEE80211_CHAN_NO_80MHZ;
> ch.bw = BRCMU_CHAN_BW_20;
> cfg->d11inf.encchspec(&ch);
> chaninfo = ch.chspec;
> @@ -5907,11 +5909,11 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> &chaninfo);
> if (!err) {
> if (chaninfo & WL_CHAN_RADAR)
> - channel[index].flags |=
> + channel->flags |=
> (IEEE80211_CHAN_RADAR |
> IEEE80211_CHAN_NO_IR);
> if (chaninfo & WL_CHAN_PASSIVE)
> - channel[index].flags |=
> + channel->flags |=
> IEEE80211_CHAN_NO_IR;
> }
> }
>
^ permalink raw reply
* Re: [PATCH V5 3/3] cfg80211: support ieee80211-freq-limit DT property
From: Johannes Berg @ 2017-01-04 13:26 UTC (permalink / raw)
To: Rafał Miłecki, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103225715.14072-3-zajec5@gmail.com>
> 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 |=
> IEEE80211_CHAN_DISABLED;
>
But just setting orig_flags also won't work, since it'd be overwritten
again by wiphy_register(), no?
johannes
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2017-01-04 13:28 UTC (permalink / raw)
To: Dmitry Shmidt; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <CAH7ZN-zm-CUv=QzoUojdyrcONRAHtTG18PiwGZ4sYY0qqft1BA@mail.gmail.com>
On Tue, 2017-01-03 at 12:45 -0800, Dmitry Shmidt wrote:
>
> We can either use alternative structure in kernel wireless stack,
> or alternative structure in userspace (in wpa_supplicant), and we
> will most likely need special command for this case at least to
> retrieve results.
Yes, I tend to agree - we need to have some new structure (and netlink
attributes etc.) to report the aggregated/partial results. But it seems
to me that it'll have to be very obvious which way of obtaining results
must be used for a given scan?
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Johannes Berg @ 2017-01-04 13:32 UTC (permalink / raw)
To: Arend Van Spriel, Malinen, Jouni
Cc: Vamsi, Krishna, linux-wireless@vger.kernel.org
In-Reply-To: <c807ce00-46bb-e214-8bef-3544e301aad6@broadcom.com>
> Also I don't see the array issue. @relative_rssi_5g_pref with s8
> value seems same as @rssi_adjust with (band=5g, s8 value) packed
> together. Or am I missing something here.
Jouni is arguing that if you can specify which band to adjust, you
might want to adjust more than one band - and need an array of
(band,adjustment) rather than just a single such tuple.
But if we introduce this attribute with the tuple now, we can extend it
to an array of tuples later if we really need that.
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Johannes Berg @ 2017-01-04 13:34 UTC (permalink / raw)
To: Malinen, Jouni
Cc: Arend Van Spriel, Vamsi, Krishna, linux-wireless@vger.kernel.org
In-Reply-To: <20161220205221.GB9756@jouni.qca.qualcomm.com>
On Tue, 2016-12-20 at 20:52 +0000, Malinen, Jouni wrote:
> > I think between that (unless we add that, we could technically
> > extend flag attributes to allow them being an int as well, or add a
> > new one) and the fact that the device may not support different
> > relative RSSI matches in different matchsets, I'm almost convinced
> > that adding new attributes is better.
>
> I'm not completely sure how to interpret all this and also the last
> email from Arend in this thread. Could either (or both :) of you
> providemore detailed suggestion on what exactly you would like us to
> change, if anything, in the attribute design now so that we can try
> to close on this?
I guess I'm thinking out loud too much :-)
Moving to the (band,adjustment) struct thing, like we have it in the
BSS select now, would be good I think - instead of the single value
that has the band hardcoded in the name. It's fairly useless with just
two bands since you can adjust one or the other to achieve the same
result, but for consistency it'd be good.
And then doing a match attribute we more or less discarded here, so
conceptually nothing else really changes.
johannes
^ permalink raw reply
* Re: [PATCH V5 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rob Herring @ 2017-01-04 14:20 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Johannes Berg, linux-wireless, Martin Blumenstingl, Felix Fietkau,
Arend van Spriel, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20170103225715.14072-4-zajec5@gmail.com>
On Tue, Jan 03, 2017 at 11:57:15PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> There are some devices (e.g. Netgear R8000 home router) with one chipset
> model used for different radios, some of them limited to subbands. NVRAM
> entries don't contain any extra info on such limitations and firmware
> reports full list of channels to us. We need to store extra limitation
> info on DT to support such devices properly.
This is the type of explanation that I'm looking for in the DT patch as
well as the examples of where this data comes from now. Certainly, I
think having per platform settings in userspace is not what you want.
Rob
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Kalle Valo @ 2017-01-04 14:32 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rob Herring, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CACna6rxmgN2zz-Fc1-Uu02oqMQreNkMJt0jGjLOMrXM=6iujFw@mail.gmail.com>
Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:
> On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafa=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>
>>> This new file should be used for properties handled at higher level and
>>> so usable with all drivers.
>>
>> Why is this needed? Where would this data normally come from?
>
> Vendors limit choice of channels at their web user interface level. I
> want to do better and report proper channels directly at kernel level
> instead of masking them in every possible configuration tool.
Why do vendors limit the channels? Is it because of a hardware
limitation (antenna does not support, or not calibrated, for a certain
band etc) or something else?
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2017-01-04 14:53 UTC (permalink / raw)
To: Kalle Valo
Cc: Rob Herring, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <874m1ej3oz.fsf@kamboji.qca.qualcomm.com>
On 4 January 2017 at 15:32, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:
>
>> On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafa=C5=82 Mi=C5=82ecki wrote=
:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level an=
d
>>>> so usable with all drivers.
>>>
>>> Why is this needed? Where would this data normally come from?
>>
>> Vendors limit choice of channels at their web user interface level. I
>> want to do better and report proper channels directly at kernel level
>> instead of masking them in every possible configuration tool.
>
> Why do vendors limit the channels? Is it because of a hardware
> limitation (antenna does not support, or not calibrated, for a certain
> band etc) or something else?
Please review & comment on the latest version, currently V5:
https://patchwork.kernel.org/patch/9495795/
"This can be used to specify extra hardware limitations caused by e.g.
used antennas or power amplifiers."
--=20
Rafa=C5=82
^ permalink raw reply
* Re: Can't authenticate to WPA network with rt2800usb driver
From: Stanislaw Gruszka @ 2017-01-04 15:02 UTC (permalink / raw)
To: Xavier Bestel; +Cc: linux-wireless
In-Reply-To: <1483526270.8560.70.camel@free.fr>
On Wed, Jan 04, 2017 at 11:37:50AM +0100, Xavier Bestel wrote:
> Jan 4 11:22:29 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: SME: Trying to authenticate with 5c:33:8e:56:78:d3 (SSID='MySecretSSID' freq=2437 MHz)
> Jan 4 11:22:29 pcxav kernel: [2921135.972360] wlx7cdd90463ef8: authenticate with 5c:33:8e:56:78:d3
> Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3774] device (wlx7cdd90463ef8): supplicant interface state: inactive -> authenticating
> Jan 4 11:22:29 pcxav kernel: [2921135.990588] wlx7cdd90463ef8: send auth to 5c:33:8e:56:78:d3 (try 1/3)
> Jan 4 11:22:29 pcxav kernel: [2921135.999983] wlx7cdd90463ef8: authenticated
> Jan 4 11:22:34 pcxav kernel: [2921140.991625] wlx7cdd90463ef8: aborting authentication with 5c:33:8e:56:78:d3 by local choice (Reason: 3=DEAUTH_LEAVING)
This looks like problem when wpa_supplicant do not talk to correct
kernel socket when cfg80211 module was removed and then loaded again.
This can happen when you remove wireless device driver using
modprobe -r i.e:
modprobe -r rt2800usb
modprobe rt2800pci
This problem is fixed in current version of wpa_supplicant or maybe
in kernel.
> Does anyone have a hint to make it work ?
"systemctl restart wpa_supplicant.service" recovers from bad state.
Stanislaw
^ permalink raw reply
* Re: ath10k mesh error 80MHz channel
From: Matteo Grandi @ 2017-01-04 15:30 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: LinuxWireless Mailing List
In-Reply-To: <585CB390.2010207@qti.qualcomm.com>
Hi Vasanth,
Thanks for the hint!
My version of iw (4.9 October2016) didn't provide any support for the
cac parameter, so I needed to checkout the latest git version as you
suggested.
Sometimes I receive the error "invalid argument (-22)" while
performing sudo iw wlanX cac trigger channel 52 80MHZ, but it seems to
happen randomly.
Thanks a lot!
Matteo
2016-12-23 6:18 GMT+01:00 Thiagarajan, Vasanthakumar
<vthiagar@qti.qualcomm.com>:
> On Wednesday 21 December 2016 08:51 PM, Matteo Grandi wrote:
>> Hi all again, just an update to the previous message:
>>
>> looking at iw list I found this situation:
>>
>> Frequencies:
>> * 5180 MHz [36] (20.0 dBm)
>> * 5200 MHz [40] (20.0 dBm)
>> * 5220 MHz [44] (20.0 dBm)
>> * 5240 MHz [48] (20.0 dBm)
>> * 5260 MHz [52] (20.0 dBm) (no IR, radar detection)
>> * 5280 MHz [56] (20.0 dBm) (no IR, radar detection)
>> * 5300 MHz [60] (20.0 dBm) (no IR, radar detection)
>> * 5320 MHz [64] (20.0 dBm) (no IR, radar detection)
>> * 5500 MHz [100] (23.0 dBm) (no IR, radar detection)
>> * 5520 MHz [104] (23.0 dBm) (no IR, radar detection)
>> * 5540 MHz [108] (23.0 dBm) (no IR, radar detection)
>> * 5560 MHz [112] (23.0 dBm) (no IR, radar detection)
>> * 5580 MHz [116] (23.0 dBm) (no IR, radar detection)
>> * 5600 MHz [120] (23.0 dBm) (no IR, radar detection)
>> * 5620 MHz [124] (23.0 dBm) (no IR, radar detection)
>> * 5640 MHz [128] (23.0 dBm) (no IR, radar detection)
>> * 5660 MHz [132] (23.0 dBm) (no IR, radar detection)
>> * 5680 MHz [136] (23.0 dBm) (no IR, radar detection)
>> * 5700 MHz [140] (23.0 dBm) (no IR, radar detection)
>> * 5745 MHz [149] (disabled)
>> * 5765 MHz [153] (disabled)
>> * 5785 MHz [157] (disabled)
>> * 5805 MHz [161] (disabled)
>> * 5825 MHz [165] (disabled)
>> Supported commands:
>> * new_interface
>> * set_interface
>> * new_key
>>
>> The no Initial Radiation prevent the interface to start transmitting.
>> I'm almost sure that it's a regulatory domain issue, but i already
>> tried to reinstall the CRD Agent, and even changing the regulatory
>> domain it change only the global (i.e. CH) while the iw list provide
>> the same results.
>> root@MrProper:~# iw reg get
>> global
>> country CH: DFS-ETSI
>> (2402 - 2482 @ 40), (N/A, 20), (N/A)
>> (5170 - 5250 @ 80), (N/A, 20), (N/A), AUTO-BW
>> (5250 - 5330 @ 80), (N/A, 20), (0 ms), DFS, AUTO-BW
>> (5490 - 5710 @ 160), (N/A, 27), (0 ms), DFS
>> (57000 - 66000 @ 2160), (N/A, 40), (N/A)
>>
>> phy#1
>> country US: DFS-FCC
>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> phy#0
>> country US: DFS-FCC
>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> phy#2
>> country US: DFS-FCC
>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> Is there a way to remove the no IR flags in the allowed channels?
>
>
> I assume you are trying to bring up mesh on one of DFS channels where
> CAC is mandatory before starting the operation. Perhaps try completing
> CAC on the channel and bring up mesh.
>
> If you want to operate mesh network on channel 52 with 80 Mhz bandwidth,
> you may need to run cac something like below from iw (a recent one which has
> support to trigger CAC).
>
> sudo iw wlanX cac trigger channel 52 80MHZ
>
>
> Vasanth
>
>>
>> 2016-12-21 15:19 GMT+01:00 Matteo Grandi <iu5bdp@gmail.com>:
>>> Dear all,
>>>
>>> I'm configuring a simple mesh network using four miniPCIe adapters
>>> (Compex WLE900VX) on two Gateworks Ventana 5410 boards.
>>> Actually, based on the guide
>>> https://wireless.wiki.kernel.org/en/users/drivers/ath10k/mesh
>>> I don't have any problem setting up the mesh network on channels 36,
>>> 149, 153, respectively 5180 80 5210, 5745 80 5775, and 5765 80 5795
>>> (even if MIMO is not working).
>>>
>>> But while configuring the interfaces to work in 80MHz channel
>>> bandwidth (802.11ac using ath10k driver) on a different channel I
>>> bump into the error:
>>>
>>> command failed: Invalid argument (-22)
>>>
>>> immediately after launching the command for joining the mesh: iw dev
>>> <if_name> mesh join <mesh_name>.
>>>
>>> The syslog provide only this:
>>>
>>> Dec 21 13:52:03 MrProper kernel: [ 7257.058617] util.c |
>>> ieee80211_set_wmm_default: ac=0, enable_qos=true, vif.type=7,
>>> NL80211_IFTYPE_STATION=2
>>> Dec 21 13:52:03 MrProper kernel: [ 7257.059654] IPv6:
>>> ADDRCONF(NETDEV_UP): mpp1: link is not ready
>>> Dec 21 13:52:03 MrProper kernel: [ 7257.205911] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:03 MrProper kernel: [ 7257.581898] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:03 MrProper kernel: [ 7257.613627] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:04 MrProper kernel: [ 7258.084733] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:04 MrProper kernel: [ 7258.180963] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:04 MrProper kernel: [ 7258.325382] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:04 MrProper kernel: [ 7258.661534] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:05 MrProper kernel: [ 7259.029525] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:05 MrProper kernel: [ 7259.392563] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:08 MrProper kernel: [ 7262.386101] ath10k_warn: 18
>>> callbacks suppressed
>>> Dec 21 13:52:08 MrProper kernel: [ 7262.386154] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7262.737520] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7262.785597] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7262.971144] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7262.981414] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7263.091578] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7263.445603] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7263.458727] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7263.582180] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:09 MrProper kernel: [ 7263.589649] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:14 MrProper kernel: [ 7267.742838] ath10k_warn: 44
>>> callbacks suppressed
>>> Dec 21 13:52:14 MrProper kernel: [ 7267.742887] ath10k_pci
>>> 0000:07:00.0: no channel configured; ignoring frame(s)!
>>> Dec 21 13:52:14 MrProper kernel: [ 7267.744466] ath10k_pci
>>> 0000:07:00.0: no channel
>>>
>>> Did someone have a similar issue?
>>> Are there, maybe, some channels that even present in the regulatory
>>> domain with the @80 flags are not configurable in this way? (36, 52,
>>> 100, 116, 132, 149 should be ok, but only 36 and 149 work)
>>> Or maybe it's my reg-domain issue (see below)?
>>>
>>> root@MrProper:~# iw reg get
>>> global
>>> country US: DFS-FCC
>>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>>
>>> phy#1
>>> country US: DFS-FCC
>>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>>
>>> phy#0
>>> country US: DFS-FCC
>>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>>
>>> phy#2
>>> country US: DFS-FCC
>>> (2402 - 2472 @ 40), (N/A, 30), (N/A)
>>> (5170 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
>>> (5250 - 5330 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
>>> (5490 - 5730 @ 160), (N/A, 23), (0 ms), DFS
>>> (5735 - 5835 @ 80), (N/A, 30), (N/A)
>>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>>
>>>
>>> Thanks a lot for every hint!
>>>
>>> Matteo
^ permalink raw reply
* Re: [PATCH v4] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2017-01-04 15:40 UTC (permalink / raw)
To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20161218002554.6362-1-andrew.zaborowski@intel.com>
> +++ b/net/wireless/mlme.c
> @@ -340,6 +340,8 @@ int cfg80211_mlme_deauth(struct
> cfg80211_registered_device *rdev,
>
> ASSERT_WDEV_LOCK(wdev);
>
> + wdev->conn_owner_nlportid = 0;
Is this really correct? The deauth might not be to the current_bss, as
you can see in the following if statement:
> if (local_state_change &&
> (!wdev->current_bss ||
> !ether_addr_equal(wdev->current_bss->pub.bssid, bssid)))
It seems that perhaps this should go into some other place, perhaps
only be reset when current_bss is also reset to NULL?
> @@ -14539,13 +14554,21 @@ static int nl80211_netlink_notify(struct
> notifier_block * nb,
> spin_unlock(&rdev-
> >destroy_list_lock);
> schedule_work(&rdev->destroy_work);
> }
> - } else if (schedule_scan_stop) {
> +
> + continue;
> + }
This also doesn't seem right - the same socket could possibly own both
an interface and a connection? If the connection is on the same
interface you might not really want to do both - though it shouldn't
hurt if all the cancel_work is in the right place - but it could be a
different interface?
johannes
^ permalink raw reply
* Re: [PATCH v2 1/4] mac80211: Pass new RSSI level in CQM RSSI notification
From: Johannes Berg @ 2017-01-04 15:44 UTC (permalink / raw)
To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20161212015155.21272-1-andrew.zaborowski@intel.com>
I think this ought to have some commit log explaining a bit what this
is about :-)
johannes
^ permalink raw reply
* Re: [PATCH v2 3/4] cfg80211: Accept multiple RSSI thresholds for CQM
From: Johannes Berg @ 2017-01-04 15:53 UTC (permalink / raw)
To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20161212015213.21323-3-andrew.zaborowski@intel.com>
Should userspace really just get -EOPNOTSUPP back?
Also, this whole business with using an array in the existing
NL80211_ATTR_CQM_RSSI_THOLD is not very backward compatible, because an
old kernel would interpret this as just a single value (the first one
in your array) - ignoring entirely the fact that you requested
multiple.
Thus, you either need an nl80211 protocol feature bit (enum
nl80211_protocol_features) or a new attribute, or so, I think.
> + cqm_config = kzalloc(sizeof(struct
> cfg80211_cqm_config) +
> + n_thresholds * sizeof(s32),
> GFP_KERNEL);
> + cqm_config->rssi_hyst = hysteresis;
You definitely need error checking here :)
johannes
^ permalink raw reply
* Re: Can't authenticate to WPA network with rt2800usb driver
From: Xavier Bestel @ 2017-01-04 16:02 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: linux-wireless
In-Reply-To: <20170104150243.GA11582@redhat.com>
Le mercredi 04 janvier 2017 à 16:02 +0100, Stanislaw Gruszka a écrit :
> On Wed, Jan 04, 2017 at 11:37:50AM +0100, Xavier Bestel wrote:
> > Jan 4 11:22:29 pcxav wpa_supplicant[13666]: wlx7cdd90463ef8: SME: Trying to authenticate with 5c:33:8e:56:78:d3 (SSID='MySecretSSID' freq=2437 MHz)
> > Jan 4 11:22:29 pcxav kernel: [2921135.972360] wlx7cdd90463ef8: authenticate with 5c:33:8e:56:78:d3
> > Jan 4 11:22:29 pcxav NetworkManager[15150]: <info> [1483525349.3774] device (wlx7cdd90463ef8): supplicant interface state: inactive -> authenticating
> > Jan 4 11:22:29 pcxav kernel: [2921135.990588] wlx7cdd90463ef8: send auth to 5c:33:8e:56:78:d3 (try 1/3)
> > Jan 4 11:22:29 pcxav kernel: [2921135.999983] wlx7cdd90463ef8: authenticated
> > Jan 4 11:22:34 pcxav kernel: [2921140.991625] wlx7cdd90463ef8: aborting authentication with 5c:33:8e:56:78:d3 by local choice (Reason: 3=DEAUTH_LEAVING)
>
> This looks like problem when wpa_supplicant do not talk to correct
> kernel socket when cfg80211 module was removed and then loaded again.
> This can happen when you remove wireless device driver using
> modprobe -r i.e:
>
> modprobe -r rt2800usb
> modprobe rt2800pci
>
> This problem is fixed in current version of wpa_supplicant or maybe
> in kernel.
>
> > Does anyone have a hint to make it work ?
>
> "systemctl restart wpa_supplicant.service" recovers from bad state.
I just upgraded wpa_supplicant from 2.5 to 2.6 (latest version AFAIK)
and restarted it but still no luck:
Jan 4 17:01:19 pcxav kernel: [2941466.617276] wlx7cdd90463ef8: aborting authentication with 5c:33:8e:56:78:d3 by local choice (Reason: 3=DEAUTH_LEAVING)
Do you know which kernel should I run to have the fix ?
Regards,
Xav
^ permalink raw reply
* Re: [PATCH V5 3/3] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-04 16:13 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arend van Spriel, Arnd Bergmann,
devicetree@vger.kernel.org, Rafał Miłecki
In-Reply-To: <1483536406.7312.3.camel@sipsolutions.net>
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox