* Re: [PATCH v2 3/4] cfg80211: Accept multiple RSSI thresholds for CQM
From: Johannes Berg @ 2017-01-05 11:49 UTC (permalink / raw)
To: Andrew Zaborowski; +Cc: linux-wireless
In-Reply-To: <CAOq732JfxTx7KrPhic+2aej0ddj6igaqjcGzg+9MJ6=zDYfWAw@mail.gmail.com>
On Wed, 2017-01-04 at 15:19 -0500, Andrew Zaborowski wrote:
> Hi,
>
> On 4 January 2017 at 10:53, Johannes Berg <johannes@sipsolutions.net>
> wrote:
> > Should userspace really just get -EOPNOTSUPP back?
>
> In what circumstance?
When multiple levels aren't supported, and it's requesting them. Rather
than, for example, being able to determine up-front (through a feature
flag) whether it's supported or not.
> > 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.
Well, it's even more complicated than that because mac80211 may have
the callback but still not be able to support it, with filtering,
perhaps? Not quite sure now.
Doing an (extended) feature flag this would address both points (also
the first, see above) in a single feature flag. Adding the protocol
feature flag would've addressed only the basic netlink attribute length
issue - so if we agree on the first point that a feature flag would be
useful for userspace to predict usability of the feature then we should
have it per device.
johannes
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2017-01-05 11:46 UTC (permalink / raw)
To: Dmitry Shmidt; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <CAH7ZN-zenL8SsqUAzMH5-Hb1RmUDDwgqqUKWEbPX=qaPV7e3TA@mail.gmail.com>
> If we go with approach to use parameters and let FW or MAC80211
> layer to decide what type of scan to do,
At that point though, is it even meaningful to ask "what type of scan
is this"? Or put another way - what does "scan type" even mean?
> 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
Exactly. But the type of action is something set by the entity that
triggered the scan, right? normal and roam would be equivalent anyway,
no? wpa_supplicant would make a decision to connect - after the results
are coming in.
Oh, then again, maybe you're thinking of full-MAC devices - does a
roam/autojoin scan really already *imply* a new connection? And if so,
do we have to do it that way, or can we remove that type of action and
make a connection decision in higher layers, so it's really the same as
"report when suitable results are found"?
> So we can literally distinguish scan types by final action.
Actually I think I'm just misinterpreting your wording - you mean that
we can use the different final actions for the different scan types,
not that we should actually say - in driver/firmware/... - "this is a
history scan because the action is to report buffer full", right?
> And for History scan we need new get_scan_results() command.
>
> Does it sound reasonable?
I think it does.
There's a bit more complication wrt. the level of detail in results
though - sometimes the result may include all IEs (normal/sched scan),
sometimes it may not ("history scan") - are we sure we really only need
one new get_scan_results()?
johannes
^ permalink raw reply
* Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION
From: Johannes Berg @ 2017-01-05 11:38 UTC (permalink / raw)
To: Jorge Ramirez, netdev, Daniel Lezcano; +Cc: linux-wireless
In-Reply-To: <685811c3-6247-77fd-8c70-617951886451@linaro.org>
+linux-wireless, where this should've gone
> I am running a single wlan0 interface in managed mode (no aliases,
> no other wireless interfaces).
> The association with the AP still hasn't happened.
>
> I noticed that if trying to change the frequency to one of the valid
> values, the driver returns EBUSY.
>
> The call stack is
> cfg80211_wext_siwfreq
> -->cfg80211_mgd_wext_siwfreq
> --->cfg80211_set_monitor_channel (notice call to set 'monitor'
> channel
> in managed mode)
> ----> fails with EBUSY
>
> Is therefore the expected behavior to fail under the above
> circumstances
> (managed mode && single wlan0 interface && no association)?
> And if it is, please could you clarify when would it be valid to
> change the frequency in managed mode?
Frankly, I don't remember - all of this is plastered all over with
backward compatibility hooks etc.
How are you running into this? Why are you even trying to do this? You
really shouldn't use wireless extensions any more.
Also, there shouldn't be much reason to be setting the channel anyway,
unless you want to trigger a connection specifically on that channel,
but then when you use nl80211 you get that included in the CONNECT
command there.
Finally, I suspect that this particular backward compatibility hook
can't really work anyway and could be removed, but I'm not sure that
would have the effect you want either.
johannes
^ permalink raw reply
* [bug report] rtlwifi: Remove some redundant code
From: Dan Carpenter @ 2017-01-05 11:11 UTC (permalink / raw)
To: Larry.Finger; +Cc: linux-wireless
Hello Larry Finger,
The patch c93ac39da006: "rtlwifi: Remove some redundant code" from
Dec 15, 2016, leads to the following static checker warning:
drivers/net/wireless/realtek/rtlwifi/rtl8192de/fw.c:326 rtl92d_download_fw()
warn: curly braces intended?
drivers/net/wireless/realtek/rtlwifi/rtl8192de/fw.c
306 /* If 8051 is running in RAM code, driver should
307 * inform Fw to reset by itself, or it will cause
308 * download Fw fail.*/
309 /* 8051 RAM code */
310 if (rtl_read_byte(rtlpriv, REG_MCUFWDL) & BIT(7)) {
311 rtl92d_firmware_selfreset(hw);
312 rtl_write_byte(rtlpriv, REG_MCUFWDL, 0x00);
313 }
314 _rtl92d_enable_fw_download(hw, true);
315 _rtl92d_write_fw(hw, version, pfwdata, fwsize);
316 _rtl92d_enable_fw_download(hw, false);
317 spin_lock_irqsave(&globalmutex_for_fwdownload, flags);
318 err = _rtl92d_fw_free_to_go(hw);
319 /* download fw over,clear 0x1f[5] */
320 value = rtl_read_byte(rtlpriv, 0x1f);
321 value &= (~BIT(5));
322 rtl_write_byte(rtlpriv, 0x1f, value);
323 spin_unlock_irqrestore(&globalmutex_for_fwdownload, flags);
324 if (err)
325 pr_err("fw is not ready to run!\n");
326 goto exit;
I guess we could add the braces back.
327 exit:
328 err = _rtl92d_fw_init(hw);
Should we even be running _rtl92d_fw_init() if _rtl92d_fw_free_to_go()
fails? What about preserving the error code?
329 return err;
330 }
regards,
dan carpenter
^ permalink raw reply
* Re: Can't authenticate to WPA network with rt2800usb driver
From: Stanislaw Gruszka @ 2017-01-05 10:56 UTC (permalink / raw)
To: Xavier Bestel; +Cc: linux-wireless
In-Reply-To: <1483545757.8560.79.camel@free.fr>
On Wed, Jan 04, 2017 at 05:02:37PM +0100, Xavier Bestel wrote:
> 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 ?
4.7 kernel and 2.5 supplicant works here. But if wpa_supplicant
restart do not fix the problem this is likely a different issue
than I describe.
Stanislaw
^ permalink raw reply
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-05 10:02 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, linux-wireless@vger.kernel.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree@vger.kernel.org, Rob Herring,
Rafał Miłecki
In-Reply-To: <e2bbce35-af09-adfa-09ea-e9fcf57b4d09@broadcom.com>
On 5 January 2017 at 10:31, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 4-1-2017 22:19, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 4 January 2017 at 21:07, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 4-1-2017 18:58, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> There are some devices (e.g. Netgear R8000 home router) with one chips=
et
>>>> model used for different radios, some of them limited to subbands. NVR=
AM
>>>> 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 brcmfma=
c.
>>>> 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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>> ---
>>>> This patch should probably go through wireless-driver-next which is wh=
y
>>>> 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 mak=
e 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 =3D 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. Sa=
y
>>> 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
>
> What do you mean by current country. There is none that we are aware off
> in the driver. So we obtain the channels for the current
> country/revision in the firmware and enable those before
> wiphy_register(). This all is within the probe/init sequence so I do not
> really see an issue. As the wiphy object is not yet registered there is
> no user-space awareness
It seems I'm terrible as describing my patches/problems/ideas :( Here
I used 1 inaccurate word and you couldn't understand my point.
By "current country" I meant current region (and so a set of
regulatory rules) used by the firmware. I believe firmware describes
it using "ccode" and "regrev".
Now, about the issue I see:
AFAIU if you set IEEE80211_CHAN_DISABLED in "orig_flags" it's meant to
be there for good. Some reference code that makes me believe so
(reg.c):
pr_debug("Disabling freq %d MHz for good\n", chan->center_freq);
chan->orig_flags |=3D IEEE80211_CHAN_DISABLED;
This is what happens with brcmfmac right now. If firmware doesn't
report some channels, you set "flags" to IEEE80211_CHAN_DISABLED for
them. Then you call wiphy_register which copies that "flags" to the
"orig_flags". I read it as: we are never going to use these channels.
Now, consider you support regdom change (I do with my local patches).
You translate alpha2 to a proper firmware request (board specific!),
you execute it and then firmware may allow you to use channels that
you marked as disabled for good. You would need to mess with
orig_flags to recover from this issue.
Does my explanation make more sense of this issue now?
>> orig_flags of channels that may become available later, after country
>> change. Please note it happens even right now, without this patch.
>
> Nope. As stated earlier the country setting in firmware is not updated
> unless you provide a *proper* mapping of user-space country code to
> firmware country code/revision. That is the reason, ie. firmware simply
> returns the same list of channels as nothing changed from its
> perspective. We may actually drop 11d support.
I implemented mapping support locally, this is the feature I'm talking abou=
t.
>> 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.
>
> Please care to explain your ideas before putting any effort in this
> "feature". As the author of the code that makes you unhappy and as
> driver maintainer I would like to get a clearer picture of your point of
> view. What exactly is the issue that makes you unhappy.
I meant that problem with "orig_flags" I described in the first
paragraph. I wasn't trying to hide whatever issue I'm seeing, I swear
;)
>> 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.
>
> Now who is *we*? We as Broadcom can, because we know how to map the ISO
> 3166-1 country code to firmware country code/revision for a specific
> firmware release. Firmware uses its own regulatory rules which may
> differ from what regdb has. Now I know Intel submitted a mechanism to
> export firmware rules to regdb so maybe we should consider switching to
> that api if that has been upstreamed. Need to check.
We as a driver developers. Please read
"we don't really support country switching for any device yet"
as
"brcmfmac doesn't really support country switching for any device yet"
Does it help to get the context?
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH] brcmfmac: check if we can support used firmware API version
From: Rafał Miłecki @ 2017-01-05 10:06 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: <1acc759f-0724-0ad4-fc2b-75bd41715cbb@broadcom.com>
On 5 January 2017 at 10:50, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 17:11, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> Every new firmware API will most likely require changes in our code to
>> support it. Right now we support 2 versions only. Refuse to init if we
>> detect newer version.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> Hi Arend,
>>
>> I think you were concerned about possible firmware API changes. Please
>> review this patch, I hope it's a proper check for running unsupported
>> firmware version which could result in broken communication between host
>> driver and a device.
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c=
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 0babfc7..c69ae84 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -6816,6 +6816,11 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach=
(struct brcmf_pub *drvr,
>> brcmf_err("Failed to get D11 version (%d)\n", err);
>> goto priv_out;
>> }
>> + if (io_type > BRCMU_D11AC_IOTYPE) {
>> + brcmf_err("Unsupported IO version %d\n", io_type);
>> + goto priv_out;
>> + }
>
> I prefer to have this in brcmu_d11_attach() and have it return an error.
My too, but since you made brcmu_d11_attach part of *utils* and new IO
version support may require driver changes, I didn't want to mess
these two.
If it was up to me, I would keep brcmu_d11_attach in brcmfmac code and
then add a proper check in this function indeed.
Is there any reason you made brcmu_d11_attach part of utils?
^ permalink raw reply
* [PATCH] nl80211: fix sched scan netlink socket owner destruction
From: Johannes Berg @ 2017-01-05 10:00 UTC (permalink / raw)
To: linux-wireless; +Cc: Jukka Rissanen, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
A single netlink socket might own multiple interfaces *and* a
scheduled scan request (which might belong to another interface),
so when it goes away both may need to be destroyed.
Remove the schedule_scan_stop indirection to fix this - it's only
needed for interface destruction because of the way this works
right now, with a single work taking care of all interfaces.
Cc: stable@vger.kernel.org
Fixes: 93a1e86ce10e4 ("nl80211: Stop scheduled scan if netlink client disappears")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3df85a751a85..ef5eff93a8b8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -14502,13 +14502,17 @@ static int nl80211_netlink_notify(struct notifier_block * nb,
list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list) {
bool schedule_destroy_work = false;
- bool schedule_scan_stop = false;
struct cfg80211_sched_scan_request *sched_scan_req =
rcu_dereference(rdev->sched_scan_req);
if (sched_scan_req && notify->portid &&
- sched_scan_req->owner_nlportid == notify->portid)
- schedule_scan_stop = true;
+ sched_scan_req->owner_nlportid == notify->portid) {
+ sched_scan_req->owner_nlportid = 0;
+
+ if (rdev->ops->sched_scan_stop &&
+ rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
+ schedule_work(&rdev->sched_scan_stop_wk);
+ }
list_for_each_entry_rcu(wdev, &rdev->wiphy.wdev_list, list) {
cfg80211_mlme_unregister_socket(wdev, notify->portid);
@@ -14539,12 +14543,6 @@ 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) {
- sched_scan_req->owner_nlportid = 0;
-
- if (rdev->ops->sched_scan_stop &&
- rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
- schedule_work(&rdev->sched_scan_stop_wk);
}
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v4] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2017-01-05 9:56 UTC (permalink / raw)
To: Andrew Zaborowski; +Cc: linux-wireless
In-Reply-To: <CAOq732KZrpsjK96uwwe3hna2DPkwbN3CvJz+LFxJzXxWw08wRA@mail.gmail.com>
On Wed, 2017-01-04 at 15:35 -0500, Andrew Zaborowski wrote:
> On 4 January 2017 at 10:40, Johannes Berg <johannes@sipsolutions.net>
> 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.
Right, makes sense.
> > > @@ -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?
>
> 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.
Yes, this part is only syntactic, but you added something new
afterwards, and that new thing should happen even if another interface
is going to be scheduled for destruction.
I actually think that the code right now is already wrong though, since
schedule_destroy_work and schedule_scan_stop shouldn't be mutually
exclusive, a single socket could own both a sched scan and a different
interface.
I'll fix that bug, and we'll have to deal with the conflicts when
merging this.
johannes
^ permalink raw reply
* Re: [PATCH] brcmfmac: check if we can support used firmware API version
From: Arend Van Spriel @ 2017-01-05 9:50 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: <20170103161140.16919-1-zajec5@gmail.com>
On 3-1-2017 17:11, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Every new firmware API will most likely require changes in our code to
> support it. Right now we support 2 versions only. Refuse to init if we
> detect newer version.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Hi Arend,
>
> I think you were concerned about possible firmware API changes. Please
> review this patch, I hope it's a proper check for running unsupported
> firmware version which could result in broken communication between host
> driver and a device.
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 0babfc7..c69ae84 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -6816,6 +6816,11 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> brcmf_err("Failed to get D11 version (%d)\n", err);
> goto priv_out;
> }
> + if (io_type > BRCMU_D11AC_IOTYPE) {
> + brcmf_err("Unsupported IO version %d\n", io_type);
> + goto priv_out;
> + }
I prefer to have this in brcmu_d11_attach() and have it return an error.
Now this actually does not cover the API version in its entirety. The
broadcom firmware API is a bit more complicated. Firmware commands may
use structured data where we may only add fields at the end to keep it
backward compatible and they are versioned if that can not be avoided.
As example see recent patch [1].
Regards,
Arend
[1] https://patchwork.kernel.org/patch/9442873/
> cfg->d11inf.io_type = (u8)io_type;
> brcmu_d11_attach(&cfg->d11inf);
>
>
^ permalink raw reply
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Arend Van Spriel @ 2017-01-05 9:31 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Johannes Berg, linux-wireless@vger.kernel.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree@vger.kernel.org, Rob Herring,
Rafał Miłecki
In-Reply-To: <CACna6rxay9sC+QT2G4AfjWT1ERsNDzFQ620hNhVhr0FRHXJu=w@mail.gmail.com>
On 4-1-2017 22:19, Rafał Miłecki wrote:
> On 4 January 2017 at 21:07, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 4-1-2017 18:58, 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 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 <rafal@milecki.pl>
>>> ---
>>> 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
What do you mean by current country. There is none that we are aware off
in the driver. So we obtain the channels for the current
country/revision in the firmware and enable those before
wiphy_register(). This all is within the probe/init sequence so I do not
really see an issue. As the wiphy object is not yet registered there is
no user-space awareness
> orig_flags of channels that may become available later, after country
> change. Please note it happens even right now, without this patch.
Nope. As stated earlier the country setting in firmware is not updated
unless you provide a *proper* mapping of user-space country code to
firmware country code/revision. That is the reason, ie. firmware simply
returns the same list of channels as nothing changed from its
perspective. We may actually drop 11d support.
> 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.
Please care to explain your ideas before putting any effort in this
"feature". As the author of the code that makes you unhappy and as
driver maintainer I would like to get a clearer picture of your point of
view. What exactly is the issue that makes you unhappy.
> 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.
Now who is *we*? We as Broadcom can, because we know how to map the ISO
3166-1 country code to firmware country code/revision for a specific
firmware release. Firmware uses its own regulatory rules which may
differ from what regdb has. Now I know Intel submitted a mechanism to
export firmware rules to regdb so maybe we should consider switching to
that api if that has been upstreamed. Need to check.
> So I hope problem with channels in brcmfmac doesn't mean we need to
> postpone patches 1-3.
I do not see any reason to postpone.
Regards,
Arend
^ permalink raw reply
* [PATCH] staging: wilc1000: Connect to highest RSSI value for required SSID
From: Aditya Shankar @ 2017-01-05 7:33 UTC (permalink / raw)
To: Ganesh Krishna, Greg Kroah-Hartman
Cc: linux-wireless, devel, linux-kernel, 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 <aditya.shankar@microchip.com>
---
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 = &last_scanned_shadow[i];
+ if (sel_bssi_idx < last_scanned_cnt) {
+ pstrNetworkInfo = &last_scanned_shadow[sel_bssi_idx];
} else {
s32Error = -ENOENT;
wilc_connecting = 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-04 21:19 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, linux-wireless@vger.kernel.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree@vger.kernel.org, Rob Herring,
Rafał Miłecki
In-Reply-To: <3fc87224-7f08-e365-7bbb-a4b8b5746e4f@broadcom.com>
On 4 January 2017 at 21:07, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 4-1-2017 18:58, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <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 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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> 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_c=
fg80211_info *cfg,
>> band->band);
>> channel[index].hw_value =3D 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.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Dmitry Shmidt @ 2017-01-04 20:32 UTC (permalink / raw)
To: Johannes Berg; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <1483536510.7312.5.camel@sipsolutions.net>
On Wed, Jan 4, 2017 at 5:28 AM, Johannes Berg <johannes@sipsolutions.net> 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
^ permalink raw reply
* Re: [PATCH v4] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Andrew Zaborowski @ 2017-01-04 20:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1483544433.7312.13.camel@sipsolutions.net>
On 4 January 2017 at 10:40, Johannes Berg <johannes@sipsolutions.net> 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(&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?
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
^ permalink raw reply
* Re: [PATCH v2 3/4] cfg80211: Accept multiple RSSI thresholds for CQM
From: Andrew Zaborowski @ 2017-01-04 20:19 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1483545210.7312.17.camel@sipsolutions.net>
Hi,
On 4 January 2017 at 10:53, Johannes Berg <johannes@sipsolutions.net> 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
^ permalink raw reply
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Arend Van Spriel @ 2017-01-04 20:07 UTC (permalink / raw)
To: Rafał Miłecki, Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rob Herring, Rafał Miłecki
In-Reply-To: <20170104175832.25996-4-zajec5@gmail.com>
On 4-1-2017 18:58, 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 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 <rafal@milecki.pl>
> ---
> 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)
>
^ permalink raw reply
* Re: [1/2] ath10k: add accounting for the extended peer statistics
From: Christian Lamparter @ 2017-01-04 20:06 UTC (permalink / raw)
To: Mohammed Shafi Shajakhan; +Cc: Kalle Valo, linux-wireless, ath10k
In-Reply-To: <20170103052827.GA8204@atheros-ThinkPad-T61>
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 <kvalo@qca.qualcomm.com> wrote:
> > > Christian Lamparter <chunkeey@googlemail.com> 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 <chunkeey@googlemail.com>
> > >
> > > 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] <https://www.mail-archive.com/ath10k@lists.infradead.org/msg06066.html>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Arend Van Spriel @ 2017-01-04 19:53 UTC (permalink / raw)
To: Rafał Miłecki, 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: <CACna6rxx-8499ZQKekdORF0UWQQazwGu2On1aq-fwA4oCX7pgw@mail.gmail.com>
On 4-1-2017 15:53, Rafał Miłecki wrote:
> On 4 January 2017 at 15:32, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafał Miłecki <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ł Miłecki wrote:
>>>>> From: Rafał Miłecki <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?
>
> 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
^ permalink raw reply
* Re: [PATCH V6 1/3] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rob Herring @ 2017-01-04 19:46 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Johannes Berg, linux-wireless, Martin Blumenstingl, Felix Fietkau,
Arend van Spriel, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <20170104175832.25996-1-zajec5@gmail.com>
On Wed, Jan 4, 2017 at 11:58 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This new file should be used for properties that apply to all wireless
> devices.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> 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 <robh@kernel.org>
^ permalink raw reply
* [PATCH V6 3/3] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-04 17:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rob Herring, Rafał Miłecki
In-Reply-To: <20170104175832.25996-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
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 <rafal@milecki.pl>
---
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 0000000..de221f0
--- /dev/null
+++ b/net/wireless/of.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 2017 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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 <linux/of.h>
+#include <net/cfg80211.h>
+#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 ieee80211_freq_range *limit = &freq_limits[i];
+
+ if (cfg80211_does_bw_fit_range(limit,
+ MHZ_TO_KHZ(chan->center_freq),
+ bw))
+ return true;
+ }
+
+ return false;
+}
+
+static void wiphy_freq_limits_apply(struct wiphy *wiphy,
+ struct ieee80211_freq_range *freq_limits,
+ unsigned int n_freq_limits)
+{
+ enum nl80211_band band;
+ int i;
+
+ if (WARN_ON(!n_freq_limits))
+ return;
+
+ for (band = 0; band < NUM_NL80211_BANDS; band++) {
+ struct ieee80211_supported_band *sband = wiphy->bands[band];
+
+ if (!sband)
+ continue;
+
+ for (i = 0; i < sband->n_channels; i++) {
+ struct ieee80211_channel *chan = &sband->channels[i];
+
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ if (!wiphy_freq_limits_valid_chan(wiphy, freq_limits,
+ n_freq_limits,
+ chan)) {
+ pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+ chan->center_freq);
+ chan->flags |= IEEE80211_CHAN_DISABLED;
+ }
+ }
+ }
+}
+
+void wiphy_read_of_freq_limits(struct wiphy *wiphy)
+{
+ struct device *dev = wiphy_dev(wiphy);
+ struct device_node *np;
+ struct property *prop;
+ struct ieee80211_freq_range *freq_limits;
+ unsigned int n_freq_limits;
+ const __be32 *p;
+ int len, i;
+ int err = 0;
+
+ if (!dev)
+ return;
+ np = dev_of_node(dev);
+ if (!np)
+ return;
+
+ prop = of_find_property(np, "ieee80211-freq-limit", &len);
+ if (!prop)
+ return;
+
+ if (!len || len % sizeof(u32) || len / sizeof(u32) % 2) {
+ dev_err(dev, "ieee80211-freq-limit wrong format");
+ return;
+ }
+ n_freq_limits = len / sizeof(u32) / 2;
+
+ freq_limits = kcalloc(n_freq_limits, sizeof(*freq_limits), GFP_KERNEL);
+ if (!freq_limits) {
+ err = -ENOMEM;
+ goto out_kfree;
+ }
+
+ p = NULL;
+ for (i = 0; i < n_freq_limits; i++) {
+ struct ieee80211_freq_range *limit = &freq_limits[i];
+
+ p = of_prop_next_u32(prop, p, &limit->start_freq_khz);
+ if (!p) {
+ err = -EINVAL;
+ goto out_kfree;
+ }
+
+ p = of_prop_next_u32(prop, p, &limit->end_freq_khz);
+ if (!p) {
+ err = -EINVAL;
+ goto out_kfree;
+ }
+
+ if (!limit->start_freq_khz ||
+ !limit->end_freq_khz ||
+ limit->start_freq_khz >= limit->end_freq_khz) {
+ err = -EINVAL;
+ goto out_kfree;
+ }
+ }
+
+ wiphy_freq_limits_apply(wiphy, freq_limits, n_freq_limits);
+
+out_kfree:
+ kfree(freq_limits);
+ if (err)
+ dev_err(dev, "Failed to get limits: %d\n", err);
+}
+EXPORT_SYMBOL(wiphy_read_of_freq_limits);
--
2.10.1
^ permalink raw reply related
* [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-04 17:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rob Herring, Rafał Miłecki
In-Reply-To: <20170104175832.25996-1-zajec5@gmail.com>
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 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 <rafal@milecki.pl>
---
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
^ permalink raw reply related
* [PATCH V6 1/3] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rafał Miłecki @ 2017-01-04 17:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rob Herring, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This new file should be used for properties that apply to all wireless
devices.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
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 0000000..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 = <0x0000 0 0 0 0>;
+ wifi@0,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <2402000 2482000>,
+ <5170000 5250000>;
+ };
+};
--
2.10.1
^ permalink raw reply related
* [PATCH V6 2/3] cfg80211: move function checking range fit to util.c
From: Rafał Miłecki @ 2017-01-04 17:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rob Herring, Rafał Miłecki
In-Reply-To: <20170104175832.25996-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
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 <rafal@milecki.pl>
---
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
^ permalink raw reply related
* 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