* Re: kernel 4.9 iwlwifi startup error
From: Alexander Morozov @ 2017-01-03 15:41 UTC (permalink / raw)
To: Andrew Donnellan; +Cc: Fabio Coatti, LKML, linux-wireless@vger.kernel.org
In-Reply-To: <942b4775-6027-2b99-84c7-37b7cde124b9@au1.ibm.com>
I have a similar problem on Gentoo. But in my case, it just can't load
firmware: "no suitable firmware found". I've tried to reinstall
firmware with no luck. Everything is ok with 4.8.6.
On Mon, Jan 2, 2017 at 6:42 PM, Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
> On 02/01/17 21:12, Fabio Coatti wrote:
>>
>> Hi all,
>> I'm using kernel 4.9 and maybe half of the times I boot my laptop I get
>> the
>> error reported below, and the wifi does not work. I have to remove iwlwifi
>> (like
>> modprobe -r iwldvm iwlwifi) and insert it again to get things workig
>> again.
>> This seems a bit random, it does not happens all the times so it could be
>> a
>> timing issue or even a flaky hardware (unlikely, as I see only this issue
>> with
>> wifi, once it starts it works just fine)
>> I'm pretty sure to have seen the same behaviour at some point in 4.8.X
>> release, but right now I lost the related notes.
>> Environment:
>> Distro: gentoo
>> gcc 5.4.0
>> HW: Hewlett-Packard HP EliteBook Folio 9470m/18DF, BIOS 68IBD Ver. F.63
>> 04/26/2016
>> Intel(R) Core(TM) i5-3427U CPU @ 1.80GHz
>> Network controller: Intel Corporation Centrino Advanced-N 6235 (rev 24)
>>
>> Of course if more info is needed, just drop me a note.
>>
>> I'm not subscribed to mailing lists, so please keep me in CC: for any
>> information request.
>>
>> Many thanks.
>
>
> I've so far seen this once on my laptop, a Samsung NP540U3C (don't have it
> with me right now, so I'm not sure what the wifi chipset is), running with a
> Debian 4.9 kernel.
>
> --
> Andrew Donnellan OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com IBM Australia Limited
>
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-03 15:49 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: <CACna6rzBr0Co2o4Ym-7coVk1v5g1nvgDv6099oJVFr9drgU_Rw@mail.gmail.com>
On 3 January 2017 at 15:14, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wrot=
e:
> On 3 January 2017 at 14:19, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 3-1-2017 12:31, Rafa=C5=82 Mi=C5=82ecki 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 fai=
l.
>>> Well, I could image something like this happening and not being critica=
l.
>>> 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.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2017-01-03 16:33 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <CAO7Z3WJa0goJ-VXc7dvyz8imZtqby6QsC0QNH+uRAE8LhxqU2w@mail.gmail.com>
[Please stop top-posting. Bottom-post only to these lists.]
Hi Geoff & happy new year.
On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
> Mark - I will split this off soon.
OK
> In the meantime - here is some more info about how we use it.
>
> We do use NFC structures. I did find an interesting clue in that
> there are certain bottles that cause neard to segfault, I'm not sure
> what is different about them. We write a string, like
> "coppola_chardonnay_2015" to the bottles.
Off the top of my head, it could be the length of the text.
It would be useful to compare the data that works to the data
that doesn't work. Can you install NXP's 'TagInfo' app on a
smartphone and scan tags with working & non-working data?
You can email the data from the app to yourself, edit out
the cruft, and share here.
> Come to think of it, I
> haven't done anything special to make that an ndef record, just
> assumed that it would happen by default, I'll look into this further.
If you wrote the data using neard, it will be NDEF formatted.
Since it is working this well, it is virtually guaranteed that
the data is NDEF formatted.
> Also, I've been running neard with --plugin nfctype2. Just in case
> the problem was happening due to cycling through other tag types. It
> didn't seem to make any difference, but I have not gone back to
> default.
Good to know, thanks.
Mark
--
^ permalink raw reply
* [PATCH] brcmfmac: check if we can support used firmware API version
From: Rafał Miłecki @ 2017-01-03 16:11 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
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;
+ }
+
cfg->d11inf.io_type = (u8)io_type;
brcmu_d11_attach(&cfg->d11inf);
--
2.10.1
^ permalink raw reply related
* [PATCH next V2] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-03 16:49 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170103083858.6981-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Our code was assigning number of channels to the index variable by
default. If firmware reported channel we didn't predict this would
result in using that initial index value and writing out of array. This
never happened so far (we got a complete list of supported channels) but
it means possible memory corruption so we should handle it anyway.
This patch simply detects unexpected channel and ignores it.
As we don't try to create new entry now, it's also safe to drop hw_value
and center_freq assignment. For known channels we have these set anyway.
I decided to fix this issue by assigning NULL or a target channel to the
channel variable. This was one of possible ways, I prefefred this one as
it also avoids using channel[index] over and over.
Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Add extra comment in code for not-found channel.
Make it clear this problem have never been seen so far
Explain why it's safe to drop extra assignments
Note & reason changing channel variable usage
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++----------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 9c2c128..a16dd7b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
u32 i, j;
u32 total;
u32 chaninfo;
- u32 index;
pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
@@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
ch.bw == BRCMU_CHAN_BW_80)
continue;
- channel = band->channels;
- index = band->n_channels;
+ channel = NULL;
for (j = 0; j < band->n_channels; j++) {
- if (channel[j].hw_value == ch.control_ch_num) {
- index = j;
+ if (band->channels[j].hw_value == ch.control_ch_num) {
+ channel = &band->channels[j];
break;
}
}
- channel[index].center_freq =
- ieee80211_channel_to_frequency(ch.control_ch_num,
- band->band);
- channel[index].hw_value = ch.control_ch_num;
+ if (!channel) {
+ /* It seems firmware supports some channel we never
+ * considered. Something new in IEEE standard?
+ */
+ brcmf_err("Firmware reported unexpected channel %d\n",
+ ch.control_ch_num);
+ continue;
+ }
/* assuming the chanspecs order is HT20,
* HT40 upper, HT40 lower, and VHT80.
*/
if (ch.bw == BRCMU_CHAN_BW_80) {
- channel[index].flags &= ~IEEE80211_CHAN_NO_80MHZ;
+ channel->flags &= ~IEEE80211_CHAN_NO_80MHZ;
} else if (ch.bw == BRCMU_CHAN_BW_40) {
- brcmf_update_bw40_channel_flag(&channel[index], &ch);
+ brcmf_update_bw40_channel_flag(channel, &ch);
} else {
/* enable the channel and disable other bandwidths
* for now as mentioned order assure they are enabled
* for subsequent chanspecs.
*/
- channel[index].flags = IEEE80211_CHAN_NO_HT40 |
- IEEE80211_CHAN_NO_80MHZ;
+ channel->flags = IEEE80211_CHAN_NO_HT40 |
+ IEEE80211_CHAN_NO_80MHZ;
ch.bw = BRCMU_CHAN_BW_20;
cfg->d11inf.encchspec(&ch);
chaninfo = ch.chspec;
@@ -5907,11 +5909,11 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
&chaninfo);
if (!err) {
if (chaninfo & WL_CHAN_RADAR)
- channel[index].flags |=
+ channel->flags |=
(IEEE80211_CHAN_RADAR |
IEEE80211_CHAN_NO_IR);
if (chaninfo & WL_CHAN_PASSIVE)
- channel[index].flags |=
+ channel->flags |=
IEEE80211_CHAN_NO_IR;
}
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Luis R. Rodriguez @ 2017-01-03 17:59 UTC (permalink / raw)
To: Pavel Machek, Daniel Wagner, Tom Gundersen
Cc: Arend Van Spriel, Pali Rohár, Ming Lei, Luis R. Rodriguez,
Greg Kroah-Hartman, Kalle Valo, David Gnedt, Michal Kazior,
Daniel Wagner, Tony Lindgren, Sebastian Reichel, Ivaylo Dimitrov,
Aaro Koskinen, Takashi Iwai, David Woodhouse, Bjorn Andersson,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161226163559.GB27087@amd>
On Mon, Dec 26, 2016 at 05:35:59PM +0100, Pavel Machek wrote:
>
> Right question is "should we solve it without user-space help"?
>
> Answer is no, too. Way too complex. Yes, it would be nice if hardware
> was designed in such a way that getting calibration data from kernel
> is easy, and if you design hardware, please design it like that. But
> N900 is not designed like that and getting the calibration through
> userspace looks like only reasonable solution.
Arend seems to have a better alternative in mind possible for other
devices which *can* probably pull of doing this easily and nicely,
given the nasty history of the usermode helper crap we should not
in any way discourage such efforts.
Arend -- please look at the firmware cache, it not a hash but a hash
table for an O(1) lookups would be a welcomed change, then it could
be repurposed for what you describe, I think the only difference is
you'd perhaps want a custom driver hook to fetch the calibration data
so the driver does whatever it needs.
> Now... how exactly to do that is other question. (But this is looks
> very reasonable. Maybe I'd add request_firmware_with_flags(, ...int
> flags), but.. that's a tiny detail.). But userspace needs to be
> involved.
No, no, we keep adding yet-another-exported symbol for requesting firmware,
instead of just adding a set of parameters possible and easily extending
functionality. Please review the patches posted on my last set which adds
a flexible API with only 2 calls, sync and async, and lets us customize
our requests using a parameter:
https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20161216-drvdata-v3-try3
This also documents the "usermode helper" properly and explains some of
the issues and limitations you will need to consider if you use it, its
one reason I'd highly encourage to consider an alternative as what Arend
is considering. *Iff* you insist on using the (now using the proper term,
as per the documentation update I am providing) "custom fallback mechanism"
I welcome such a change but I ask we *really* think this through well so
we avoid the stupid issues which have historically made the custom fallback
mechanism more of a nuisance for Linux distributions, users and developers.
To this end -- I ask you check out Daniel Wagner and Tom Gundersen's firmwared
work [0] which I referred you to in December. Although the drvdata API does
not yet use a custom fallback mechanism, after and its merged the goal here
would be to *only* support a clean custom fallback mechanism which aligns
itself *well* with firmwared or solutions like it. Your patch set then could
just become a patch set to add the custom fallback mechaism support to drvdata
API with the new options/prefernce you are looking for to be specified in
the new parameters, not a new exported symbol.
One of the cruxes we should consider addressing before the drvdata API gets a
custom fallback mechanism support added is that the usermode helper lock should
be replaced with a generic solution for the races it was intended to address:
use of the API on suspend/resume and implicitly later avoid a race on init. To
this end we should consider the same race for *other* real kernel "user mode
helpers", I've documented this on a wiki [1] which documents the *real*
kernel usermode helpers users, one of which was the kobject uevent which is
one of the fallback mechanisms.
I should also note that the idea of fallback mechanism using kobject uevents
should really suffice, in review with Johannes Berg at least, he seemed
convinced just letting either the upstream firmwared, a custom firmwared or
a custom userspace solution should be able to just monitor for uevents for
drvdata and do the right thing, this whole "custom fallback mechanism"
in retrospect seems not really needed as far as I can tell.
[0] https://github.com/teg/firmwared
[1] https://kernelnewbies.org/KernelProjects/usermode-helper-enhancements
Luis
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2017-01-03 18:35 UTC (permalink / raw)
To: Mark Greer
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <20170103163324.GA3184@animalcreek.com>
On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> [Please stop top-posting. Bottom-post only to these lists.]
Sorry; gmail keeps baiting me to do it...
>
> Hi Geoff & happy new year.
>
> On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
>> Mark - I will split this off soon.
>
> OK
Thanks for the reminder!
>
>> In the meantime - here is some more info about how we use it.
>>
>> We do use NFC structures. I did find an interesting clue in that
>> there are certain bottles that cause neard to segfault, I'm not sure
>> what is different about them. We write a string, like
>> "coppola_chardonnay_2015" to the bottles.
>
> Off the top of my head, it could be the length of the text.
> It would be useful to compare the data that works to the data
> that doesn't work. Can you install NXP's 'TagInfo' app on a
> smartphone and scan tags with working & non-working data?
> You can email the data from the app to yourself, edit out
> the cruft, and share here.
The data is always the same - and the tags are all the same. Only
difference is that the tag is physically different, and perhaps
orientation; distance from antenna to tag is fixed. I can't even
write the tags at all, so reading them will show blank. Also a minor
but significant detail, is that the tags are embedded in such a way
that the phone cannot get close enough to them to connect.
>
>> Come to think of it, I
>> haven't done anything special to make that an ndef record, just
>> assumed that it would happen by default, I'll look into this further.
>
> If you wrote the data using neard, it will be NDEF formatted.
> Since it is working this well, it is virtually guaranteed that
> the data is NDEF formatted.
OK, good.
>
>> Also, I've been running neard with --plugin nfctype2. Just in case
>> the problem was happening due to cycling through other tag types. It
>> didn't seem to make any difference, but I have not gone back to
>> default.
>
> Good to know, thanks.
>
> Mark
> --
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rob Herring @ 2017-01-03 19:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, linux-wireless, Martin Blumenstingl, Felix Fietkau,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20161228155955.25518-1-zajec5@gmail.com>
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?
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
> 1 file changed, 16 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..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.
Not relavent to a binding. Bindings describe h/w.
> +
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> + reg = <0x0000 0 0 0 0>;
> + ieee80211-min-center-freq = <2437000>;
> + ieee80211-max-center-freq = <2457000>;
> +};
> --
> 2.10.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2017-01-03 20:20 UTC (permalink / raw)
To: Rob Herring
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <20170103195514.vf4psb6qw2bd2lcw@rob-hp-laptop>
On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>
> Why is this needed? Where would this data normally come from?
Vendors limit choice of channels at their web user interface level. I
want to do better and report proper channels directly at kernel level
instead of masking them in every possible configuration tool.
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 +++++++++=
+++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a =
proper
>> +net layer and don't require extra driver code.
>
> Not relavent to a binding. Bindings describe h/w.
Yes, Arend pointed it to me and I improved this Documentation file in
further versions. Could you review V4, please?
https://patchwork.kernel.org/patch/9494713/
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Dmitry Shmidt @ 2017-01-03 20:45 UTC (permalink / raw)
To: Johannes Berg; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <1481645205.20412.32.camel@sipsolutions.net>
On Tue, Dec 13, 2016 at 8:06 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> Supporting requests (or more precisely requests and results)
>> differentiated by user-space entity can be tricky. Right now we are
>> not checking current caller pid, right? Maybe it is also good idea -
>> or maybe we can just make result filtering per user-space caller?
>
> Could be done.
>
> You seem to be very worried about the partial results - I'm not too
> worried about that I guess, the connection manager itself will always
> be able to wait for the full scan to finish before making a decision,
> but it may not even want to (see the separate discussion on per-channel
> "done" notifications etc.)
Probably you are right.
> I'm much more worried about the "bucket reporting" since that doesn't
> fit into the current full BSS reporting model at all. What's your
> suggestion for this?
>
> johannes
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.
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2017-01-03 21:21 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <CAO7Z3WK5pF+q0vAoPPie8wXYQPigpGd7W8uQiGSgN79z4=W9uQ@mail.gmail.com>
On Tue, Jan 03, 2017 at 01:35:18PM -0500, Geoff Lansberry wrote:
> On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> > On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
> >> In the meantime - here is some more info about how we use it.
> >>
> >> We do use NFC structures. I did find an interesting clue in that
> >> there are certain bottles that cause neard to segfault, I'm not sure
> >> what is different about them. We write a string, like
> >> "coppola_chardonnay_2015" to the bottles.
> >
> > Off the top of my head, it could be the length of the text.
> > It would be useful to compare the data that works to the data
> > that doesn't work. Can you install NXP's 'TagInfo' app on a
> > smartphone and scan tags with working & non-working data?
> > You can email the data from the app to yourself, edit out
> > the cruft, and share here.
>
> The data is always the same - and the tags are all the same. Only
> difference is that the tag is physically different, and perhaps
> orientation; distance from antenna to tag is fixed.
Interesting... They're all type 2 tags, right?
> I can't even
> write the tags at all, so reading them will show blank. Also a minor
> but significant detail, is that the tags are embedded in such a way
> that the phone cannot get close enough to them to connect.
This section had me completely confused for a couple minutes until I realized
that you mean that you can read & write the tags using the trf7970a with
an attached antenna but not with your phone. Is that correct?
If so, try a tag that isn't embedded in something else and move it around
the back of the phone. Try to find where it works best. The phone
manufacturers are notorius for paying little attention to the NFC antenna
they put on their products. For example, I have a Samsung S5 next to me
and it seems to work best around the center of the phone. I've used others
where I had to use the upper-left or upper-right corner of the phone.
Mark
--
^ permalink raw reply
* [PATCH V5 1/3] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rafał Miłecki @ 2017-01-03 22:57 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, 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
---
.../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 0000000..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.
+
+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 V5 2/3] cfg80211: move function checking range fit to util.c
From: Rafał Miłecki @ 2017-01-03 22:57 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103225715.14072-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
* [PATCH V5 3/3] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-03 22:57 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103225715.14072-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
This patch adds a helper for reading that new property and applying
limitations or supported channels specified this way.
It may be useful for specifying single band devices or devices that
support only some part of the whole band. It's common that tri-band
routers have separated radios for lower and higher part of 5 GHz band.
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.
---
include/net/cfg80211.h | 25 +++++++++
net/wireless/Makefile | 1 +
net/wireless/of.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 164 insertions(+)
create mode 100644 net/wireless/of.c
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ca2ac1c..a58cdc9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -311,6 +311,31 @@ 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 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.
+ */
+#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..70b21e0
--- /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->orig_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->orig_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 V5 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-03 22:57 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103225715.14072-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 on 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 works.
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
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ccae3bb..f95e316 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.
*/
@@ -6477,6 +6480,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
wiphy->bands[NL80211_BAND_5GHZ] = band;
}
}
+ wiphy_read_of_freq_limits(wiphy);
err = brcmf_setup_wiphybands(wiphy);
return err;
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH V4 1/2] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rob Herring @ 2017-01-03 23:08 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Johannes Berg, linux-wireless, Martin Blumenstingl, Felix Fietkau,
Arend van Spriel, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20170103110340.23249-1-zajec5@gmail.com>
On Tue, Jan 03, 2017 at 12:03:38PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This new file should be used for properties that apply to all wireless
> devices.
The commit msg should answer the questions I asked on v1.
> 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.
> ---
> .../devicetree/bindings/net/wireless/ieee80211.txt | 18 ++++++++++++++++++
> 1 file changed, 18 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..0cd1219
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,18 @@
> +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
> +
> +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
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process
From: Brian Norris @ 2017-01-04 2:12 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
rajatja@google.com, dmitry.torokhov@gmail.com, Xinming Hu
In-Reply-To: <a6067f1c99a749e6a0c265c725694fda@SC-EXCH04.marvell.com>
Hi,
On Thu, Dec 01, 2016 at 02:02:43PM +0000, Amitkumar Karwar wrote:
> > > I could not find async version of cancel_work().
> >
> > cancel_work() *is* asynchronous. It does not synchronize with the last
> > event, so you won't have the deadlock. (Remember: the synchronous
> > version is cancel_work_sync().)
>
> My bad! What I meant is "I could not find async version of cancel_work_sync()"
> cancel_work() isn't available in http://lxr.free-electrons.com/source/kernel/workqueue.c
It's in 4.9-rc1 (and it's available at the above link, at least by now).
See:
commit f72b8792d180948b4b3898374998f5ac8c02e539
Author: Jens Axboe <axboe@fb.com>
Date: Wed Aug 24 15:51:50 2016 -0600
workqueue: add cancel_work()
But anyway:
> Anyways, clear_bit() after remove() during card reset would address the problem.
Yes, I think that's OK.
Brian
^ permalink raw reply
* Re: [PATCH V5 1/3] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rafał Miłecki @ 2017-01-04 6:20 UTC (permalink / raw)
To: Johannes Berg, linux-wireless, Rob Herring
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103225715.14072-1-zajec5@gmail.com>
Hi Rob,
On 01/03/2017 11:57 PM, Rafał Miłecki 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
> ---
> .../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 0000000..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?
> +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>;
> + };
> +};
>
^ permalink raw reply
* [PATCH] cfg80211: sysfs: use wiphy_name()
From: Johannes Berg @ 2017-01-04 7:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Instead of open-coding dev_name(), use the wiphy_name() inline
to make the code easier to understand. While at it, clean up
some coding style.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/sysfs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 14b3f007826d..16b6b5988be9 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -39,9 +39,11 @@ SHOW_FMT(address_mask, "%pM", wiphy.addr_mask);
static ssize_t name_show(struct device *dev,
struct device_attribute *attr,
- char *buf) {
+ char *buf)
+{
struct wiphy *wiphy = &dev_to_rdev(dev)->wiphy;
- return sprintf(buf, "%s\n", dev_name(&wiphy->dev));
+
+ return sprintf(buf, "%s\n", wiphy_name(wiphy));
}
static DEVICE_ATTR_RO(name);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Kalle Valo @ 2017-01-04 8:08 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Rafał Miłecki, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <f74a07d7-94a6-f9c6-ba12-adec6f708c3a@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On 3-1-2017 9:38, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>=20
>> 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.
>>=20
>> Fix this by detecting unexpected channel and ignoring it.
>>=20
>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiph=
y bands")
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> I'm not sure what kind of material it is. It fixes possible memory corru=
ption
>> (serious thing?) but this bug was there since Apr 2015, so is it worth f=
ixing
>> 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 w=
orking
>> 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.
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Kalle Valo @ 2017-01-04 8:12 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <CACna6rxYPFWfvA4TyQTjGCrduefiCU-j4vR8PrfbCnN3P8-iyg@mail.gmail.com>
Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:
> On 3 January 2017 at 15:14, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
>> On 3 January 2017 at 14:19, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 3-1-2017 12:31, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>>> + if (!channel) {
>>>>>> + brcmf_err("Firmware reported unexpected channe=
l %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 fa=
il.
>>>> Well, I could image something like this happening and not being critic=
al.
>>>> 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 o=
f 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.
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH next V2] brcmfmac: avoid writing channel out of allocated array
From: Arend Van Spriel @ 2017-01-04 9:39 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: <20170103164930.29989-1-zajec5@gmail.com>
On 3-1-2017 17:49, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Our code was assigning number of channels to the index variable by
> default. If firmware reported channel we didn't predict this would
> result in using that initial index value and writing out of array. This
> never happened so far (we got a complete list of supported channels) but
> it means possible memory corruption so we should handle it anyway.
>
> This patch simply detects unexpected channel and ignores it.
>
> As we don't try to create new entry now, it's also safe to drop hw_value
> and center_freq assignment. For known channels we have these set anyway.
>
> I decided to fix this issue by assigning NULL or a target channel to the
> channel variable. This was one of possible ways, I prefefred this one as
> it also avoids using channel[index] over and over.
>
> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Add extra comment in code for not-found channel.
> Make it clear this problem have never been seen so far
> Explain why it's safe to drop extra assignments
> Note & reason changing channel variable usage
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++----------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 9c2c128..a16dd7b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> u32 i, j;
> u32 total;
> u32 chaninfo;
> - u32 index;
>
> pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>
> @@ -5873,33 +5872,36 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> ch.bw == BRCMU_CHAN_BW_80)
> continue;
>
> - channel = band->channels;
> - index = band->n_channels;
> + channel = NULL;
> for (j = 0; j < band->n_channels; j++) {
> - if (channel[j].hw_value == ch.control_ch_num) {
> - index = j;
> + if (band->channels[j].hw_value == ch.control_ch_num) {
> + channel = &band->channels[j];
> break;
> }
> }
> - channel[index].center_freq =
> - ieee80211_channel_to_frequency(ch.control_ch_num,
> - band->band);
> - channel[index].hw_value = ch.control_ch_num;
> + if (!channel) {
> + /* It seems firmware supports some channel we never
> + * considered. Something new in IEEE standard?
> + */
> + brcmf_err("Firmware reported unexpected channel %d\n",
> + ch.control_ch_num);
Maybe rephrase to "Ignoring unexpected firmware channel %d\n" so
end-users are not alarmed by this error message. I think using
brcmf_err() is justified, but you may even consider chiming down to
brcmf_dbg(INFO, ...).
Regards,
Arend
^ permalink raw reply
* Re: [RFC] nl80211: allow multiple active scheduled scan requests
From: Johannes Berg @ 2017-01-04 9:59 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless, Dmitry Shmidt
In-Reply-To: <ba1cb1a1-3d27-7421-47ea-a5625a886518@broadcom.com>
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
^ permalink raw reply
* Re: [PATCH V5 1/3] dt-bindings: document common IEEE 802.11 frequency limit property
From: Arend Van Spriel @ 2017-01-04 10:02 UTC (permalink / raw)
To: Rafał Miłecki, Johannes Berg, linux-wireless,
Rob Herring
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <d90127bd-6a9e-2773-fdf9-527afdc004a6@gmail.com>
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 <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
>> ---
>> .../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 0000000..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
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Kalle Valo @ 2017-01-04 8:14 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <CACna6rzGRHLCdzxS_+MHXeZwF=BW4S52pDb9g9MuanNGyToEOw@mail.gmail.com>
Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:
>>> @@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcm=
f_cfg80211_info *cfg,
>>> ch.bw =3D=3D BRCMU_CHAN_BW_80)
>>> continue;
>>>
>>> - channel =3D band->channels;
>>> - index =3D band->n_channels;
>>> + channel =3D NULL;
>>> for (j =3D 0; j < band->n_channels; j++) {
>>> - if (channel[j].hw_value =3D=3D ch.control_ch_num)=
{
>>> - index =3D j;
>>> + if (band->channels[j].hw_value =3D=3D ch.control_=
ch_num) {
>>> + channel =3D &band->channels[j];
>>> break;
>>> }
>>> }
>>
>> You could have kept the index construct and simply check if j =3D=3D
>> band->n_channels here to determine something is wrong.
>
> I wanted to simplify code at the same time. Having channel[index]
> repeated 7 times was a hint for me it could be handled better. I
> should have made that clear, I'll fix improve this in V2.
If you are making a patch to stable or -rc releases you should keep the
patch as simple as possible and do all the cleanup later. But I see that
you dropped "cc stable" in this patch so all is good, just a general
remark.
--=20
Kalle Valo
^ permalink raw reply
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