[PATCH] staging: wilc1000: Connect to highest RSSI value for required SSID

[Aditya Shankar]

Connect to the highest rssi with the required SSID in the shadow
table if the connection criteria is based only on the SSID.
For the first matching SSID, an index to the table is saved.
Later the index is updated if matching SSID has a higher
RSSI value than the last saved index.

However if decision is made based on BSSID, there is only one match
in the table and corresponding index is used.

Signed-off-by: Aditya Shankar 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index c1a24f7..32206b8 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -665,6 +665,7 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
 {
s32 s32Error = 0;
u32 i;
+   u32 sel_bssi_idx = last_scanned_cnt + 1;
u8 u8security = NO_ENCRYPT;
enum AUTHTYPE tenuAuth_type = ANY;
 
@@ -688,18 +689,25 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
memcmp(last_scanned_shadow[i].ssid,
   sme->ssid,
   sme->ssid_len) == 0) {
-   if (!sme->bssid)
-   break;
-   else
+   if (!sme->bssid) {
+   if (sel_bssi_idx == (last_scanned_cnt + 1))
+   sel_bssi_idx = i;
+   else if (last_scanned_shadow[i].rssi >
+last_scanned_shadow[sel_bssi_idx].rssi)
+   sel_bssi_idx = i;
+   } else {
if (memcmp(last_scanned_shadow[i].bssid,
   sme->bssid,
-  ETH_ALEN) == 0)
+  ETH_ALEN) == 0){
+   sel_bssi_idx = i;
break;
+   }
+   }
}
}
 
-   if (i < last_scanned_cnt) {
-   pstrNetworkInfo = _scanned_shadow[i];
+   if (sel_bssi_idx < last_scanned_cnt) {
+   pstrNetworkInfo = _scanned_shadow[sel_bssi_idx];
} else {
s32Error = -ENOENT;
wilc_connecting = 0;
-- 
2.7.4

Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits

[Rafał Miłecki]

On 4 January 2017 at 21:07, Arend Van Spriel
 wrote:
> On 4-1-2017 18:58, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>>
>> 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 in DT to support such devices properly.
>>
>> Now there is a cfg80211 helper for reading such info use it in brcmfmac.
>> This patch adds check for channel being disabled with orig_flags which
>> is how this wiphy helper and wiphy_register work.
>>
>> Signed-off-by: Rafał Miłecki 
>> ---
>> This patch should probably go through wireless-driver-next which is why
>> it got weird number 4/3. I'm sending it just as a proof of concept.
>> It was succesfully tested on SmartRG SR400ac with BCM43602.
>>
>> V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
>> V5: Update commit message
>> V6: Call wiphy_read_of_freq_limits after brcmf_setup_wiphybands to make it 
>> work
>> with helper setting "flags" instead of "orig_flags".
>> ---
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index ccae3bb..a008ba5 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct 
>> brcmf_cfg80211_info *cfg,
>>  band->band);
>>   channel[index].hw_value = ch.control_ch_num;
>>
>> + if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
>> + continue;
>> +
>
> So to be clear this is still needed for subsequent calls to
> brcmf_setup_wiphybands(). The subsequent calls are done from the
> regulatory notifier. So I think we have an issue with this approach. Say
> the device comes up with US. That would set DISABLED flags for channels
> 12 to 14. With a country update to PL we would want to enable channels
> 12 and 13, right? The orig_flags are copied from the initial flags
> during wiphy_register() so it seems we will skip enabling 12 and 13. I
> think we should remove the check above and call
> wiphy_read_of_freq_limits() as a last step within
> brcmf_setup_wiphybands(). It means it is called every time, but it
> safeguards that the limits in DT are always applied.

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
orig_flags of channels that may become available later, after country
change. Please note it happens even right now, without this patch.
Maybe you can workaround this by ignoring orig_flags in custom
regulatory code, but I'd just prefer to have it nicely handled in the
first place.

This is the next feature I'm going to work on. AFAIU this patch won't
be applied for now (it's for wireless-drivers-next and we first need
to get wiphy_read_of_freq_limits in that tree). By the time that
happens I may have another patchset cleaning brcmfmac ready. And FWIW
this patch wouldn't make things worse *at this point* as we don't
really support country switching for any device yet.

So I hope problem with channels in brcmfmac doesn't mean we need to
postpone patches 1-3.

-- 
Rafał

Re: [PATCH] RFC: Universal scan proposal

[Dmitry Shmidt]

On Wed, Jan 4, 2017 at 5:28 AM, Johannes Berg  wrote:
> 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?

If we go with approach to use parameters and let FW or MAC80211
layer to decide what type of scan to do, then in general the only
difference between different types of scan is what to do with result:
- Normal scan: ssid list, channel list, dwell params, etc...
- Sched scan: ssid list, channel list, interval
- BSSID scan: bssid list, channel list, interval
Action: Report when suitable results are found (in case of Normal
scan it will be at the end of scan)

- Roaming / Autojoin: ssid list, channel list, interval
Action: Connect when suitable results are found

- History scan: bssid list, channel list, interval
Action: Report when buffer is full / almost full

So we can literally distinguish scan types by final
action. And for History scan we need new get_scan_results()
command.

Does it sound reasonable?

> johannes

Re: [PATCH v4] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT

[Andrew Zaborowski]

On 4 January 2017 at 10:40, Johannes Berg  wrote:
>> +++ 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?

In this case yes, I think I should perform the same bssid comparison.
But elsewhere we want conn_owner_nlportid to be set earlier than
current_bss, and reset earlier than current_bss because (1) we want to
be able to interrupt an ongoing attempt, and (2) we also don't want to
trigger another disconnect / deauth if one is already in progress.

>
>> @@ -14539,13 +14554,21 @@ static int nl80211_netlink_notify(struct
>> notifier_block * nb,
>>   spin_unlock(
>> >destroy_list_lock);
>>   schedule_work(>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?

This is only a syntactic change though.  The "continue" is now in the
"if (schedule_destroy_work)" block so the other actions will not be
scheduled is the interface is being destroyed.

Best regards

Re: [PATCH v2 3/4] cfg80211: Accept multiple RSSI thresholds for CQM

[Andrew Zaborowski]

Hi,

On 4 January 2017 at 10:53, Johannes Berg  wrote:
> Should userspace really just get -EOPNOTSUPP back?

In what circumstance?

>
> 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.

True, I'd assumed that netlink would check for exact attribute length
with NLA_S32.

I'll add the feature bit but I wonder if it's better as a
driver/device feature (enum nl80211_ext_feature_index) so that it can
take into account whether rdev->set_cqm_rssi_range_config is set.

>
>
>> + 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 :)

Ok.

Best regards

Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits

[Arend Van Spriel]

On 4-1-2017 18:58, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> 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 in DT to support such devices properly.
> 
> Now there is a cfg80211 helper for reading such info use it in brcmfmac.
> This patch adds check for channel being disabled with orig_flags which
> is how this wiphy helper and wiphy_register work.
> 
> Signed-off-by: Rafał Miłecki 
> ---
> This patch should probably go through wireless-driver-next which is why
> it got weird number 4/3. I'm sending it just as a proof of concept.
> It was succesfully tested on SmartRG SR400ac with BCM43602.
> 
> V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
> V5: Update commit message
> V6: Call wiphy_read_of_freq_limits after brcmf_setup_wiphybands to make it 
> work
> with helper setting "flags" instead of "orig_flags".
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index ccae3bb..a008ba5 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct 
> brcmf_cfg80211_info *cfg,
>  band->band);
>   channel[index].hw_value = ch.control_ch_num;
>  
> + if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
> + continue;
> +

So to be clear this is still needed for subsequent calls to
brcmf_setup_wiphybands(). The subsequent calls are done from the
regulatory notifier. So I think we have an issue with this approach. Say
the device comes up with US. That would set DISABLED flags for channels
12 to 14. With a country update to PL we would want to enable channels
12 and 13, right? The orig_flags are copied from the initial flags
during wiphy_register() so it seems we will skip enabling 12 and 13. I
think we should remove the check above and call
wiphy_read_of_freq_limits() as a last step within
brcmf_setup_wiphybands(). It means it is called every time, but it
safeguards that the limits in DT are always applied.

Regards,
Arend

>   /* assuming the chanspecs order is HT20,
>* HT40 upper, HT40 lower, and VHT80.
>*/
> @@ -6478,7 +6481,11 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, 
> struct brcmf_if *ifp)
>   }
>   }
>   err = brcmf_setup_wiphybands(wiphy);
> - return err;
> + if (err)
> + return err;
> + wiphy_read_of_freq_limits(wiphy);
> +
> + return 0;
>  }
>  
>  static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
> 

Re: [1/2] ath10k: add accounting for the extended peer statistics

[Christian Lamparter]

Hello Shafi and Kalle,

On Tuesday, January 3, 2017 10:58:27 AM CET Mohammed Shafi Shajakhan wrote:
> On Fri, Dec 30, 2016 at 03:35:10PM +0100, Christian Lamparter wrote:
> > On Thu, Dec 29, 2016 at 3:11 PM, Kalle Valo  wrote:
> > > Christian Lamparter  wrote:
> > >> The 10.4 firmware adds extended peer information to the
> > >> firmware's statistics payload. This additional info is
> > >> stored as a separate data field and the elements are
> > >> stored in their own "peers_extd" list.
> > >>
> > >> These elements can pile up in the same way as the peer
> > >> information elements. This is because the
> > >> ath10k_wmi_10_4_op_pull_fw_stats() function tries to
> > >> pull the same amount (num_peer_stats) for every statistic
> > >> data unit.
> > >>
> > >> Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
> > >> Signed-off-by: Christian Lamparter 
> > >
> > > My understanding is that I should skip this patch 1. Please let me know if
> > > I misunderstood. But I'm still plannning to apply patch 2.
> > 
> > I added Mohammed (I hope, he can reply to the open question when he
> > returns), Since I'm not sure what he wants or not.
> > 
> > As far as I'm aware, the "extended" boolean serves no purpose
> > because it was only used in once place in debugfs_sta which was
> > removed in the patch. ( "ath10k_sta_update_stats_rx_duration"
> > and "ath10k_sta_update_extd_stats_rx_duration" have been unified).
> 
> [shafi] We will wait for Kalle to review from the de-ferred stage
> and get his opinion as well(regarding the design change).
> I have no concerns as long this does not changes the existing behaviour.
> thank you !

Thank you Shafi for sticking around. I just fished your response to 
"Re: [PATCH] ath10k: merge extended peer info data with existing peers info" 
[0].
out of my spam-bucket. Kalle, please look if your copy of the mail got 
flagged/deleted as well. Judging from the reply in this thread, I think you
overlooked it as well? 

After reading it, I think the previous post and the request to put the patch
on wait was unnecessary. As of now, it seems to me that the open questions
between us have been settled amically (so to speak). Kalle, do you have any
concerns or can you put this in the next round then?

Regards,
Christian

[0] 

Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties

[Arend Van Spriel]

On 4-1-2017 15:53, Rafał Miłecki wrote:
> On 4 January 2017 at 15:32, Kalle Valo  wrote:
>> Rafał Miłecki  writes:
>>
>>> On 3 January 2017 at 20:55, Rob Herring  wrote:
 On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki 
>
> 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?
> 
> 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."

Just to be clear, it is does not need to be a hardware limitation, but
simply a way to assure that multiple wlan radios are not interfering
with each other and crank up the total bandwidth that vendors can put on
their box.

Regards,
Arend

Re: [PATCH V6 1/3] dt-bindings: document common IEEE 802.11 frequency limit property

[Rob Herring]

On Wed, Jan 4, 2017 at 11:58 AM, Rafał Miłecki  wrote:
> From: Rafał Miłecki 
>
> This new file should be used for properties that apply to all wireless
> devices.
>
> Signed-off-by: Rafał Miłecki 
> ---
> V2: Switch to a single ieee80211-freq-limit property that allows specifying
> *multiple* ranges. This resolves problem with more complex rules as 
> pointed
> by Felx.
> Make description implementation agnostic as pointed by Arend.
> Rename node to wifi as suggested by Martin.
> V3: Use more real-life frequencies in the example.
> V5: Describe hardware design as possible use for this property
> V6: Even better property description, thanks Rob for your help!
> ---
>  .../devicetree/bindings/net/wireless/ieee80211.txt | 24 
> ++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ieee80211.txt

Acked-by: Rob Herring 

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

[Rafał Miłecki]

From: Rafał Miłecki 

This patch adds a helper for reading that new property and applying
limitations of supported channels specified this way.
It is used with devices that normally support a wide wireless band but
in a given config are limited to some part of it (usually due to board
design). For example a dual-band chipset may be able to support one band
only because of used antennas.
It's also common that tri-band routers have separated radios for lower
and higher part of 5 GHz band and it may be impossible to say which is
which without a DT info.

Signed-off-by: Rafał Miłecki 
---
V2: Put main code in core.c as it isn't strictly part of regulatory - pointed
by Arend.
Update to support ieee80211-freq-limit (new property).
V3: Introduce separated wiphy_read_of_freq_limits function.
Add extra sanity checks for DT data.
Move code back to reg.c as suggested by Johannes.
V4: Move code to of.c
Use one helper called at init time (no runtime hooks)
Modify orig_flags
V5: Make wiphy_read_of_freq_limits return void as there isn't much point of
handling errors.
V6: Set IEEE80211_CHAN_DISABLED in "flags" instead of "orig_flags" as they will
be copied during wiphy_register. Also improve description & note that helper
has to be called before wiphy_register.
---
 include/net/cfg80211.h |  28 ++
 net/wireless/Makefile  |   1 +
 net/wireless/of.c  | 138 +
 3 files changed, 167 insertions(+)
 create mode 100644 net/wireless/of.c

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ca2ac1c..30841a9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -311,6 +311,34 @@ struct ieee80211_supported_band {
struct ieee80211_sta_vht_cap vht_cap;
 };
 
+/**
+ * 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 external 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. You should make
+ * sure to call it only if channels in wiphy are copied and can be modified
+ * without affecting other devices.
+ *
+ * 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.
+ * If using this helper, call it before wiphy_register.
+ */
+#ifdef CONFIG_OF
+void wiphy_read_of_freq_limits(struct wiphy *wiphy);
+#else /* CONFIG_OF */
+static inline void wiphy_read_of_freq_limits(struct wiphy *wiphy)
+{
+}
+#endif /* !CONFIG_OF */
+
+
 /*
  * Wireless hardware/device configuration structures and methods
  */
diff --git a/net/wireless/Makefile b/net/wireless/Makefile
index 4c9e39f..95b4c09 100644
--- a/net/wireless/Makefile
+++ b/net/wireless/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_WEXT_PRIV) += wext-priv.o
 
 cfg80211-y += core.o sysfs.o radiotap.o util.o reg.o scan.o nl80211.o
 cfg80211-y += mlme.o ibss.o sme.o chan.o ethtool.o mesh.o ap.o trace.o ocb.o
+cfg80211-$(CONFIG_OF) += of.o
 cfg80211-$(CONFIG_CFG80211_DEBUGFS) += debugfs.o
 cfg80211-$(CONFIG_CFG80211_WEXT) += wext-compat.o wext-sme.o
 cfg80211-$(CONFIG_CFG80211_INTERNAL_REGDB) += regdb.o
diff --git a/net/wireless/of.c b/net/wireless/of.c
new file mode 100644
index 000..de221f0
--- /dev/null
+++ b/net/wireless/of.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 2017 Rafał Miłecki 
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include "core.h"
+
+static bool wiphy_freq_limits_valid_chan(struct wiphy *wiphy,
+struct ieee80211_freq_range 
*freq_limits,
+unsigned int n_freq_limits,
+struct ieee80211_channel *chan)
+{
+   u32 bw = MHZ_TO_KHZ(20);
+   int i;
+
+   for (i = 0; i < n_freq_limits; i++) {
+   struct 

[PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits

[Rafał Miłecki]

From: Rafał Miłecki 

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 in DT to support such devices properly.

Now there is a cfg80211 helper for reading such info use it in brcmfmac.
This patch adds check for channel being disabled with orig_flags which
is how this wiphy helper and wiphy_register work.

Signed-off-by: Rafał Miłecki 
---
This patch should probably go through wireless-driver-next which is why
it got weird number 4/3. I'm sending it just as a proof of concept.
It was succesfully tested on SmartRG SR400ac with BCM43602.

V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
V5: Update commit message
V6: Call wiphy_read_of_freq_limits after brcmf_setup_wiphybands to make it work
with helper setting "flags" instead of "orig_flags".
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ccae3bb..a008ba5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct 
brcmf_cfg80211_info *cfg,
   band->band);
channel[index].hw_value = ch.control_ch_num;
 
+   if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
+   continue;
+
/* assuming the chanspecs order is HT20,
 * HT40 upper, HT40 lower, and VHT80.
 */
@@ -6478,7 +6481,11 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct 
brcmf_if *ifp)
}
}
err = brcmf_setup_wiphybands(wiphy);
-   return err;
+   if (err)
+   return err;
+   wiphy_read_of_freq_limits(wiphy);
+
+   return 0;
 }
 
 static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
-- 
2.10.1

[PATCH V6 1/3] dt-bindings: document common IEEE 802.11 frequency limit property

[Rafał Miłecki]

From: Rafał Miłecki 

This new file should be used for properties that apply to all wireless
devices.

Signed-off-by: Rafał Miłecki 
---
V2: Switch to a single ieee80211-freq-limit property that allows specifying
*multiple* ranges. This resolves problem with more complex rules as pointed
by Felx.
Make description implementation agnostic as pointed by Arend.
Rename node to wifi as suggested by Martin.
V3: Use more real-life frequencies in the example.
V5: Describe hardware design as possible use for this property
V6: Even better property description, thanks Rob for your help!
---
 .../devicetree/bindings/net/wireless/ieee80211.txt | 24 ++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt 
b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
new file mode 100644
index 000..f6442b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
@@ -0,0 +1,24 @@
+Common IEEE 802.11 properties
+
+This provides documentation of common properties that are valid for all 
wireless
+devices.
+
+Optional properties:
+ - ieee80211-freq-limit : list of supported frequency ranges in KHz. This can 
be
+   used for devices that in a given config support less channels than
+   normally. It may happen chipset supports a wide wireless band but it is
+   limited to some part of it due to used antennas or power amplifier.
+   An example case for this can be tri-band wireless router with two
+   identical chipsets used for two different 5 GHz subbands. Using them
+   incorrectly could not work or decrease performance noticeably.
+
+Example:
+
+pcie@0,0 {
+   reg = <0x 0 0 0 0>;
+   wifi@0,0 {
+   reg = <0x 0 0 0 0>;
+   ieee80211-freq-limit = <2402000 2482000>,
+  <517 525>;
+   };
+};
-- 
2.10.1

[PATCH V6 2/3] cfg80211: move function checking range fit to util.c

[Rafał Miłecki]

From: Rafał Miłecki 

It is needed for another cfg80211 helper that will be out of reg.c so
move it to common util.c file and make it non-static.

Signed-off-by: Rafał Miłecki 
---
V5: Add this patch as suggested by Arend
---
 net/wireless/core.h |  3 +++
 net/wireless/reg.c  | 27 +++
 net/wireless/util.c | 15 +++
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/net/wireless/core.h b/net/wireless/core.h
index af6e023..bc8ba6e 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -430,6 +430,9 @@ int cfg80211_change_iface(struct cfg80211_registered_device 
*rdev,
 void cfg80211_process_rdev_events(struct cfg80211_registered_device *rdev);
 void cfg80211_process_wdev_events(struct wireless_dev *wdev);
 
+bool cfg80211_does_bw_fit_range(const struct ieee80211_freq_range *freq_range,
+   u32 center_freq_khz, u32 bw_khz);
+
 /**
  * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable
  * @wiphy: the wiphy to validate against
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..753efcd 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -748,21 +748,6 @@ static bool is_valid_rd(const struct ieee80211_regdomain 
*rd)
return true;
 }
 
-static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
-   u32 center_freq_khz, u32 bw_khz)
-{
-   u32 start_freq_khz, end_freq_khz;
-
-   start_freq_khz = center_freq_khz - (bw_khz/2);
-   end_freq_khz = center_freq_khz + (bw_khz/2);
-
-   if (start_freq_khz >= freq_range->start_freq_khz &&
-   end_freq_khz <= freq_range->end_freq_khz)
-   return true;
-
-   return false;
-}
-
 /**
  * freq_in_rule_band - tells us if a frequency is in a frequency band
  * @freq_range: frequency rule we want to query
@@ -1070,7 +1055,7 @@ freq_reg_info_regd(u32 center_freq,
if (!band_rule_found)
band_rule_found = freq_in_rule_band(fr, center_freq);
 
-   bw_fits = reg_does_bw_fit(fr, center_freq, bw);
+   bw_fits = cfg80211_does_bw_fit_range(fr, center_freq, bw);
 
if (band_rule_found && bw_fits)
return rr;
@@ -1138,11 +1123,13 @@ static uint32_t reg_rule_to_chan_bw_flags(const struct 
ieee80211_regdomain *regd
max_bandwidth_khz = reg_get_max_bandwidth(regd, reg_rule);
 
/* If we get a reg_rule we can assume that at least 5Mhz fit */
-   if (!reg_does_bw_fit(freq_range, MHZ_TO_KHZ(chan->center_freq),
-MHZ_TO_KHZ(10)))
+   if (!cfg80211_does_bw_fit_range(freq_range,
+   MHZ_TO_KHZ(chan->center_freq),
+   MHZ_TO_KHZ(10)))
bw_flags |= IEEE80211_CHAN_NO_10MHZ;
-   if (!reg_does_bw_fit(freq_range, MHZ_TO_KHZ(chan->center_freq),
-MHZ_TO_KHZ(20)))
+   if (!cfg80211_does_bw_fit_range(freq_range,
+   MHZ_TO_KHZ(chan->center_freq),
+   MHZ_TO_KHZ(20)))
bw_flags |= IEEE80211_CHAN_NO_20MHZ;
 
if (max_bandwidth_khz < MHZ_TO_KHZ(10))
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 2cf7df8..62dc214 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1847,6 +1847,21 @@ void cfg80211_free_nan_func(struct cfg80211_nan_func *f)
 }
 EXPORT_SYMBOL(cfg80211_free_nan_func);
 
+bool cfg80211_does_bw_fit_range(const struct ieee80211_freq_range *freq_range,
+   u32 center_freq_khz, u32 bw_khz)
+{
+   u32 start_freq_khz, end_freq_khz;
+
+   start_freq_khz = center_freq_khz - (bw_khz / 2);
+   end_freq_khz = center_freq_khz + (bw_khz / 2);
+
+   if (start_freq_khz >= freq_range->start_freq_khz &&
+   end_freq_khz <= freq_range->end_freq_khz)
+   return true;
+
+   return false;
+}
+
 /* See IEEE 802.1H for LLC/SNAP encapsulation/decapsulation */
 /* Ethernet-II snap header (RFC1042 for most EtherTypes) */
 const unsigned char rfc1042_header[] __aligned(2) =
-- 
2.10.1

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

[Rafał Miłecki]

On 4 January 2017 at 14:26, Johannes Berg  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 |=
>> 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.

-- 
Rafał

Re: Can't authenticate to WPA network with rt2800usb driver

[Xavier Bestel]

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]:   [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.83] 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

Re: [PATCH v2 3/4] cfg80211: Accept multiple RSSI thresholds for CQM

[Johannes Berg]

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

Re: [PATCH v2 1/4] mac80211: Pass new RSSI level in CQM RSSI notification

[Johannes Berg]

I think this ought to have some commit log explaining a bit what this
is about :-)

johannes

Re: [PATCH v4] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT

[Johannes Berg]

> +++ 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(
> >destroy_list_lock);
>   schedule_work(>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

Re: ath10k mesh error 80MHz channel

[Matteo Grandi]

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
:
> 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 :
>>> 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:
>>>
>>> 

Re: Can't authenticate to WPA network with rt2800usb driver

[Stanislaw Gruszka]

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]:   [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.83] 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

Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties

[Rafał Miłecki]

On 4 January 2017 at 15:32, Kalle Valo  wrote:
> Rafał Miłecki  writes:
>
>> On 3 January 2017 at 20:55, Rob Herring  wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
 From: Rafał Miłecki 

 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?

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."

-- 
Rafał

Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties

[Kalle Valo]

Rafał Miłecki  writes:

> On 3 January 2017 at 20:55, Rob Herring  wrote:
>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki 
>>>
>>> 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?

-- 
Kalle Valo

Re: [PATCH V5 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits

[Rob Herring]

On Tue, Jan 03, 2017 at 11:57:15PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> 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

Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs

[Johannes Berg]

> 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

Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs

[Johannes Berg]

On Tue, 2016-12-20 at 20:52 +, 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

Re: [PATCH] RFC: Universal scan proposal

[Johannes Berg]

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

Re: [PATCH V3] brcmfmac: avoid writing channel out of allocated array

[Arend Van Spriel]

On 4-1-2017 12:09, Rafał Miłecki wrote:
> From: Rafał Miłecki 

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 
> Signed-off-by: Rafał Miłecki 
> ---
> 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 = >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([index], );
> + brcmf_update_bw40_channel_flag(channel, );
>   } 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();
>   chaninfo = ch.chspec;
> @@ -5907,11 +5909,11 @@ static int brcmf_construct_chaninfo(struct 
> brcmf_cfg80211_info *cfg,
>  );
>   if (!err) {
>   if (chaninfo & WL_CHAN_RADAR)
> - channel[index].flags |=
> + channel->flags |=
> 

[PATCH V3] brcmfmac: avoid writing channel out of allocated array

[Rafał Miłecki]

From: Rafał Miłecki 

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 
---
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 = >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([index], );
+   brcmf_update_bw40_channel_flag(channel, );
} 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();
chaninfo = ch.chspec;
@@ -5907,11 +5909,11 @@ static int brcmf_construct_chaninfo(struct 
brcmf_cfg80211_info *cfg,
   );
if (!err) {
if (chaninfo & WL_CHAN_RADAR)
-   channel[index].flags |=
+   channel->flags |=
(IEEE80211_CHAN_RADAR |
 IEEE80211_CHAN_NO_IR);
if (chaninfo & WL_CHAN_PASSIVE)
-   

Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array

[Rafał Miłecki]

On 4 January 2017 at 11:48, Arend Van Spriel
 wrote:
> On 4-1-2017 11:40, Rafał Miłecki wrote:
>> On 4 January 2017 at 10:39, Arend Van Spriel
>>  wrote:
>>> On 3-1-2017 17:49, Rafał Miłecki wrote:
 From: Rafał Miłecki 

 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 
 ---
 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 = >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

... sorry, it seems I should take a break ;)

-- 
Rafał

[PATCH] cfg80211: only pass sband to set_mandatory_flags_band()

[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 
---
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

Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array

[Arend Van Spriel]

On 4-1-2017 11:40, Rafał Miłecki wrote:
> On 4 January 2017 at 10:39, Arend Van Spriel
>  wrote:
>> On 3-1-2017 17:49, Rafał Miłecki wrote:
>>> From: Rafał Miłecki 
>>>
>>> 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 
>>> ---
>>> 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 = >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

Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array

[Rafał Miłecki]

On 4 January 2017 at 10:39, Arend Van Spriel
 wrote:
> On 3-1-2017 17:49, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>>
>> 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 
>> ---
>> 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 = >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.

-- 
Rafał

Can't authenticate to WPA network with rt2800usb driver

[Xavier Bestel]

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]:   [1483525349.1350] device 
(wlx7cdd90463ef8): Activation: starting connection 'MySecretSSID 2' 
(e6482e2d-adbc-4529-bafc-24c3e54f07e6) 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [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]:   [1483525349.1352] device 
(wlx7cdd90463ef8): state change: disconnected -> prepare (reason 'none') [30 40 
0] 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [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]:   [1483525349.3266] device 
(wlx7cdd90463ef8): state change: prepare -> config (reason 'none') [40 50 0] 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3267] device 
(wlx7cdd90463ef8): Activation: (wifi) access point 'MySecretSSID 2' has 
security, but secrets are required. 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3267] device 
(wlx7cdd90463ef8): state change: config -> need-auth (reason 'none') [50 60 0]
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3287] device 
(wlx7cdd90463ef8): state change: need-auth -> prepare (reason 'none') [60 40 0] 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3289] device 
(wlx7cdd90463ef8): state change: prepare -> config (reason 'none') [40 50 0] 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [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]:   [1483525349.3291] Config: 
added 'ssid' value 'MySecretSSID' 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3291] Config: 
added 'scan_ssid' value '1' 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3291] Config: 
added 'key_mgmt' value 'WPA-PSK' 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3291] Config: 
added 'auth_alg' value 'OPEN' 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [1483525349.3291] Config: 
added 'psk' value '' 
Jan  4 11:22:29 pcxav NetworkManager[15150]:   [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]:   [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.83] 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]:   [1483525354.4250] device 
(wlx7cdd90463ef8): supplicant interface state: authenticating -> disconnected 
Jan  4 11:22:44 pcxav NetworkManager[15150]:   [1483525364.4264] device 
(wlx7cdd90463ef8): supplicant 

Re: [RFC] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

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

Re: [RFC] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

> 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

Re: [RFC] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

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

Re: [PATCH] brcmfmac: avoid writing channel out of allocated array

[Kalle Valo]

Rafał Miłecki  writes:

>>> @@ -5873,33 +5872,33 @@ 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 = >channels[j];
>>>   break;
>>>   }
>>>   }
>>
>> You could have kept the index construct and simply check if j ==
>> 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.

-- 
Kalle Valo

Re: [PATCH V5 1/3] dt-bindings: document common IEEE 802.11 frequency limit property

[Arend Van Spriel]

On 4-1-2017 7:20, Rafał Miłecki wrote:
> Hi Rob,
> 
> On 01/03/2017 11:57 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>>
>> This new file should be used for properties that apply to all wireless
>> devices.
>>
>> Signed-off-by: Rafał Miłecki 
>> ---
>> V2: Switch to a single ieee80211-freq-limit property that allows
>> specifying
>> *multiple* ranges. This resolves problem with more complex rules
>> as pointed
>> by Felx.
>> Make description implementation agnostic as pointed by Arend.
>> Rename node to wifi as suggested by Martin.
>> V3: Use more real-life frequencies in the example.
>> V5: Describe hardware design as possible use for this property
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt   | 20
>> 
>>  1 file changed, 20 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 000..1c82c16
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,20 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are valid for
>> all wireless
>> +devices.
>> +
>> +Optional properties:
>> + - ieee80211-freq-limit : list of supported frequency ranges in KHz.
>> This can be
>> +used to specify extra hardware limitations caused by e.g. used
>> antennas
>> +or power amplifiers.
> 
> Do you find this description sufficient now? I'm not sure how/if I could
> answer
> "Where would this data normally come from?" question.
>
> One vendor may hardcode choice of channels in their PHP web UI.
> Another one may do it in Andoid app.
> OpenWrt so far was describing this limitation on their wiki page.
> 
> It doesn't sound like any valuable info if I understand this correctly.
> We also
> don't describe where to get information about amount o RAM. One may just
> check
> the hardware, one may use vendor firmware, one could check product data
> sheet.
> 
> If I missed the point, could you help me get this?


There is probably no easy answer. DT is used to describe device
properties that are not otherwise discoverable (at least that is my rule
of thumb). So what is the "device" in this context? You may consider
just the chip, but in this case it is combination of the chip and its RF
path that determine the frequency range that it can operate in.
Apparently this was assured in user-space due to lack of a better
option. Having this specified in DT seems a viable option getting rid of
having a particular platform impose a requirement upon user-space.

You could consider these properties global as they are describing the
platform, but again this depends on what you consider the "device". If
you want to do this global you may add a global node for wifi properties
and use references to the device.

Regards,
Arend

Re: [RFC] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

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.

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?

> I guess your remark means this clarifies your earlier question about
> the request id, right?

yeah.

johannes

Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array

[Arend Van Spriel]

On 3-1-2017 17:49, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> 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 
> ---
> 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 = >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, ...).

Regards,
Arend

Re: [PATCH] brcmfmac: avoid writing channel out of allocated array

[Kalle Valo]

Rafał Miłecki  writes:

> On 3 January 2017 at 15:14, Rafał Miłecki  wrote:
>> On 3 January 2017 at 14:19, Arend Van Spriel
>>  wrote:
>>> On 3-1-2017 12:31, Rafał Miłecki wrote:
>> + if (!channel) {
>> + brcmf_err("Firmware reported unexpected channel 
>> %d\n",
>> +   ch.control_ch_num);
>> + continue;
>> + }
> As stated above something is really off when this happens so should we
> continue and try to make sense of what firmware provides or simply fail.
 Well, I could image something like this happening and not being critical.
 The simplest case: Broadcom team releases a new firmware which
 supports extra 5 GHz channels (e.g. due to the IEEE standard change).
 Why should we refuse to run & support all "old" channel just because of 
 that?
>>>
>>> Fair enough. I was assuming we keep __wl_{2,5}ghz_channels up to date
>>> with IEEE standard.
>>>
 What do you mean by "make sense of what firmware provides"? Would kind
 of solution would you suggest?
>>>
>>> When the above assumption can be assured (by us) the only other scenario
>>> would be a change in the firmware API where we wrongly interpret the
>>> information retrieved. In this case all subsequent channels will likely
>>> result in bogus or accidental matches hence it seems better to bail out
>>> early.
>>
>> Good point, this actually gave me an idea for a small nice
>> improvement. I remember I saw something like WL_VER in wl ioctl
>> protocol, it was already bumped at some point by Broadcom, when
>> chanspec format has changed. We should probably read this number from
>> firmware and maybe refuse to run if version is newer than the one we
>> know.
>
> I was thinking about WLC_GET_VERSION and you seem to already have it
> in brcmfmac under nam BRCMF_C_GET_VERSION. If you wish to be prepared
> for firmware API change, I guess you should check version. It seems
> brcmfmac supports 1 and 2.
>
> On the other hand if adding firmware with incompatible API you may
> want to have different directory or file names. I think this is what
> Intel does. This allows one to have multiple firmwares in
> /lib/firmware/ and switching between kernels freely.

ath10k does something similar. IIRC we currently support four different,
and incompatible, firmware releases now.

-- 
Kalle Valo

Re: [PATCH] brcmfmac: avoid writing channel out of allocated array

[Kalle Valo]

Arend Van Spriel  writes:

> On 3-1-2017 9:38, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>> 
>> 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.
>> 
>> Fix this by detecting unexpected channel and ignoring it.
>> 
>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy 
>> bands")
>> Signed-off-by: Rafał Miłecki 
>> ---
>> I'm not sure what kind of material it is. It fixes possible memory corruption
>> (serious thing?) but this bug was there since Apr 2015, so is it worth fixing
>> in 4.10? Or maybe I should even cc stable?
>> I don't think any released firmware reports any unexpected channel, so I 
>> guess
>> noone ever hit this problem. I just noticed this possible problem when 
>> working
>> on another feature.
>
> Looking at the change I was going to ask if you actually hit the issue
> you are addressing here. The channels in __wl_2ghz_channels and
> __wl_5ghz_channels are complete list of channels for the particular band
> so it would mean firmware behaves out-of-spec or firmware api was
> changed. For robustness a change is acceptable I guess.
>
> My general policy is to submit fixes to wireless-drivers (and stable)
> only if it resolves a critical issue found during testing or a reported
> issue.

That's also my preference. And I read somewhere (forgot where) that in
kernel summit there was a discussion about having only regression fixes
in -rc kernels. So the rules are getting stricter, which is a good thing
as then we can make releases in a shorter cycle.

-- 
Kalle Valo