* Re: [PATCH 1/2] ath10k: add accounting for the extended peer statistics
From: Mohammed Shafi Shajakhan @ 2016-12-16 5:24 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, ath10k, Kalle Valo
In-Reply-To: <2970240.ih558OrmaK@debian64>
Hi Christian,
> On Thursday, December 15, 2016 10:13:39 PM CET Mohammed Shafi Shajakhan wrote:
> > I am also thinking, as of now there is not much use in appending
> > the extended peer stats (which gets periodically ) to the linked list
> > '&ar->debug.fw_stats.peers_extd)' and should we get rid of the below
> > (and the required cleanup as well)
> >
> > list_splice_tail_init(&stats.peers_extd,
> > &ar->debug.fw_stats.peers_extd);
> >
> >
> > since rx_duration is getting updated periodically to the per sta
> > information. Kindly let me know your thoughts about this.
>
> Yes, of course. I see what your are trying to do and I think it's much better
> to get rid of peers_extd and ath10k_fw_extd_stats_peers_free.
>
[shafi] Feel free to post the change and I can test the same for you(next week) !
Let me know if you are busy on something else, I can take this up.
As discussed, the fix to free 'extd stats' when number of peers exceeds the range
is definitely needed. Thank you for looking into this.
thanks,
shafi
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Thiagarajan, Vasanthakumar @ 2016-12-16 5:37 UTC (permalink / raw)
To: Johannes Berg, nbd@nbd.name; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481809984.2582.5.camel@sipsolutions.net>
T24gVGh1cnNkYXkgMTUgRGVjZW1iZXIgMjAxNiAwNzoyMyBQTSwgSm9oYW5uZXMgQmVyZyB3cm90
ZToNCj4NCj4+PiBJIGFncmVlLiBEeW5hbWljIHN3aXRjaCBwYXJ0IGlzIGJ1Z2d5LCB3ZSBjYW4g
c3RhcnQgd2l0aCBub3QNCj4+PiBhbGxvd2luZyBpbnRlcmZhY2VzIHJlc3VsdGluZyBpbiBkeW5h
bWljIHN3aXRjaC4NCj4+DQo+PiBEb2VzIHRoaXMgbWVhbiB0aGF0IHdoZW4gYnJpbmdpbmcgdXAg
bXVsdGlwbGUgaW50ZXJmYWNlcywgdXNlcnMgd291bGQNCj4+IG5lZWQgdG8gZmlndXJlIG91dCB0
aGUgJ21hZ2ljJyBvcmRlciB0aGF0IHdvcmtzPw0KPg0KPiBJIHRoaW5rIHdlIG5lZWQgdG8gdGFs
ayBhYm91dCBoYXJkd2FyZSBjYXBhYmlsaXRpZXMgYXQgdGhpcyBwb2ludC4NCg0KUUNBOTg4WCBk
b2VzIG5vdCBoYXZlIGNhcGFiaWxpdHkgdG8gY29uZmlndXJlIHZpZiBzcGVjaWZpYyBkZWNhcCBt
b2RlLiBFbmNhcCBtb2RlDQppcyBjb25maWd1cmFibGUgcGVyIHBhY2tldCBmb3IgYWxsIHRoZSBh
dGgxMGsgYmFzZWQgY2hpcHMgc28gdGhpcyBwYXJ0IHNob3VsZCBiZQ0KZmluZSB0byBzdXBwb3J0
IHBlciB2aWYgY29uZmlndXJhdGlvbi4gTmV3ZXIgUUNBIGNoaXBzIGxpa2UgUUNBOTk4NCwgUUNB
NDAxOSwgUUNBOTg4OA0KYW5kIFFDQTk5WDAgc3VwcG9ydHMgZGVjYXAgbW9kZSBjb25maWd1cmF0
aW9uIHBlciB2aWYuIFRvIHJlZHVjZSB0aGUgY29tcGxleGl0eSwNCndlIGNhbiBwcm9iYWJseSBt
YWtlIHBlciB2aWYgZW5jYXAvZGVjYXAgY29uZmlndXJhdGlvbiBtYW5kYXRvcnkgdG8gZW5hYmxl
IGV0aGVybmV0DQpmcmFtZSBmb3JtYXQsIG5vdCBzdXJlIGhvdyB0aGlzIHdpbGwgd29yayB3aXRo
IG5vbi1RQ0EgY2FwYWJsZSBoYXJkd2FyZS4NCg0KPg0KPiBJIHdhcyBhc3N1bWluZyB0aGF0IGl0
IHdvdWxkIGFjdHVhbGx5IGJlIHBvc3NpYmxlIHRvIHJ1biB0d28gaW50ZXJmYWNlcw0KPiB3aXRo
IGRpZmZlcmVudCBwYXRocyBoZXJlIGNvbmN1cnJlbnRseSAtIGlzIHRoYXQgbm90IHRydWU/IElm
IHRoYXQncw0KPiBub3QgdHJ1ZSwgdGhlbiB3ZSBhYnNvbHV0ZWx5IF9uZWVkXyBkeW5hbWljIHN3
aXRjaGluZywgSSBhZ3JlZSB3aXRoDQo+IEZlbGl4LCBidXQgdGhlbiB3ZSBoYXZlIGEgcHJldHR5
IGJpZyBjb21wbGljYXRpb24gdG8gZmlndXJlIG91dC4gQnV0IHdlDQo+IGNhbid0IGxldCB0aGlz
IG9wdGltaXNhdGlvbiBhZmZlY3QgdXNlciBleHBlcmllbmNlLg0KDQpTdXJlLg0KDQpWYXNhbnRo
^ permalink raw reply
* Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
From: Tamizh chelvam @ 2016-12-16 5:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: c_traja, linux-wireless, ath10k
In-Reply-To: <1481645351.20412.34.camel@sipsolutions.net>
Hi Johannes,
Thanks for the comments
On 2016-12-13 21:39, Johannes Berg wrote:
>> > > /**
>> > > + * wiphy_btcoex_support_flags
>> > > + * This enum has the driver supported frame types for
>> > > BTCOEX.
>> > > + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
>> > > + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
>> > > + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
>> > > BTCOEX.
>> > > + */
>> >
>> > That's not making much sense to me?
>> >
>>
>> is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
>
> It's not really clear to me what you intend to do this - if it's really
> support flags then you really should name those better.
>
This is support flags and it used by the driver to intimate driver
supported frame type
for the BTCOEX to cfg like "wiphy_wowlan_support_flags" implementation.
Please suggest if this is ok ? I will be thankful if you can suggest a
better one if this is not ok
"WIPHY_BTCOEX_SUPPORTS_BE"
>> > > +/**
>> > > + * enum wiphy_btcoex_priority - BTCOEX priority level
>> > > + * This enum defines priority level for BTCOEX
>> > > + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT
>> > > traffic
>> > > + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT
>> > > traffic
>> > > + */
>> > > +
>> > > +enum wiphy_btcoex_priority {
>> > > + WIPHY_WLAN_PREFERRED_LOW = false,
>> > > + WIPHY_WLAN_PREFERRED_HIGH = true,
>> > > +};
>> >
>> > That false/true seems just strange.
>> >
>>
>> I will just use as a enum without assigning false/true.
>
> What do you even need this enum for though?
>
Ok. I will directly assign true for the flag.
>> > > +enum nl80211_btcoex_priority {
>> > > + __NL80211_WLAN_PREFERRED_INVALID,
>> > > + NL80211_WLAN_BE_PREFERRED,
>> > > + NL80211_WLAN_BK_PREFERRED,
>> > > + NL80211_WLAN_VI_PREFERRED,
>> > > + NL80211_WLAN_VO_PREFERRED,
>> > > + NL80211_WLAN_BEACON_PREFERRED,
>> > > + NL80211_WLAN_MGMT_PREFERRED,
>> > > + __NL80211_WLAN_PREFERRED_LAST,
>> > > + NL80211_WLAN_PREFERRED_MAX =
>> > > + __NL80211_WLAN_PREFERRED_LAST - 1,
>> > > +};
>> >
>> > Wouldn't a bitmap be easier?
>> >
>> since this is to distinguish between different btcoex priorities and
>> we
>> are not going to do any manipulations on these parameters.
>> It is just used as flag attribute.
>
> But why the (parsing) complexity, when a single bitmap would do?
>
Do you mean to say, sending a value from user space and parse that in
the driver?
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Thiagarajan, Vasanthakumar @ 2016-12-16 6:47 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481794695.31776.7.camel@sipsolutions.net>
T24gVGh1cnNkYXkgMTUgRGVjZW1iZXIgMjAxNiAwMzowOCBQTSwgSm9oYW5uZXMgQmVyZyB3cm90
ZToNCj4NCj4+IFRoaXMgcnggcGF0aCBvbmx5IGNoZWNrcyBpZiB0aGUgZHJpdmVyIGhhcyBhZHZl
cnRpc2VkDQo+PiBpdCdzIHN1cHBvcnQgb2YgODAyLjExIGhlYWRlciBlbmNhcC9kZWNhcCBvZmZs
b2FkIGZvcg0KPj4gZGF0YSBmcmFtZXMuDQo+DQo+IEknbSBub3QgZXZlbiBzdXJlIEkgc2VlIHRo
ZSBwb2ludCBpbiB0aGF0PyBPdGhlciB0aGFuIHRoYXQgKGFuZCB0aGUNCj4gdmFyaW91cyBvdGhl
ciBvZmZsb2FkIHJlcXVpcmVtZW50cyksIGl0IHNlZW1zIHRoYXQgZW5jYXAvZGVjYXAgY291bGQg
YmUNCj4gY29uc2lkZXJlZCBvcnRob2dvbmFsLg0KDQpPay4gU28gaXQgaXMgbWFpbmx5IGZvciBl
bmNhcCB0aGVyZSBuZWVkcyB0byBiZSBzb21lIGNhcGFiaWxpdHkgYWR2ZXJ0aXNlbWVudA0KZnJv
bSBkcml2ZXIgYmVjYXVzZSBmb3IgZGVjYXAgZHJpdmVyIHdvdWxkIHVzZSBhcHByb3ByaWF0ZSBt
YWM4MDIxMSBSeCBmdW5jdGlvbg0KdG8gaW5kaWNhdGU/DQoNCj4NCj4+ICsJICogQWRob2MgaW50
ZXJmYWNlIGRlcGVuZHMgb24gYnNzaWQgdG8gdWRwYXRlIGxhc3RfcnguDQo+DQo+IHR5cGUgLSB1
cGRhdGUNCj4NCj4+ICsJaWYgKCEoc3RhdHVzLT5mbGFnICYgUlhfRkxBR19NQ0FTVCkpIHsNCj4+
ICsJCXN0YS0+cnhfc3RhdHMubGFzdF9yeCA9IGppZmZpZXM7DQo+PiArCQlzdGEtPnJ4X3N0YXRz
Lmxhc3RfcmF0ZSA9DQo+PiBzdGFfc3RhdHNfZW5jb2RlX3JhdGUoc3RhdHVzKTsNCj4+ICsJfQ0K
Pg0KPiBZb3Ugc2hvdWxkIHByb2JhYmx5IHJlbmFtZSB0aGF0IGZsYWcgdG8gc29tZXRoaW5nIGxp
a2UNCj4gUlhfRkxBR184MDIxMV9NQ0FTVCBzaW5jZSBvdGhlcndpc2UgaXQncyBjb25mdXNpbmcg
d2l0aCB0aGUgbmV4dA0KPiBtdWx0aWNhc3QgZXRoZXIgYWRkciBjaGVjazoNCg0KU3VyZS4NCg0K
Pg0KPj4gKwlpZiAoc2RhdGEtPnZpZi50eXBlID09IE5MODAyMTFfSUZUWVBFX1NUQVRJT04gJiYN
Cj4+ICsJICAgICFpc19tdWx0aWNhc3RfZXRoZXJfYWRkcihlaGRyLT5oX2Rlc3QpKQ0KPj4gKwkJ
aWVlZTgwMjExX3N0YV9yZXNldF9jb25uX21vbml0b3Ioc2RhdGEpOw0KPg0KPiBCdXQgdGhpcyBj
b3VsZCBqdXN0IGFsc28gdXNlIHRoZSBmbGFnLCBzaW5jZSBpbiBzdGF0aW9uIG1vZGUgdGhlIHR3
bw0KPiBhcmUgZXF1aXZhbGVudCwgYW5kIGl0J2QgYmUgZWFzaWVyIHRvIGZpZ3VyZSBvdXQgaWYg
dGhpcyB3YXMgImVsc2UgaWYNCj4gKHN0YXRpb24gbW9kZSkiPw0KDQpBaCwgY29ycmVjdC4NCg0K
Pg0KPj4gKwltZW1zZXQoJnJ4LCAwLCBzaXplb2YocngpKTsNCj4NCj4gVGhhdCBzZWVtcyBhIGJp
dCBwb2ludGxlc3M/DQoNCkhtbW0sIGlzIGl0IG5vdCB0aGF0IGFsbCBtZW1iZXIgb3RoZXIgdGhh
biB0aGUgb25lcyBpbml0aWFsaXplZCB3aWxsIGJlIGp1bmsgYW5kIG1pZ2h0DQpyZXN1bHQgaW4g
Y3Jhc2ggd2hlbiBhY2Nlc3NpbmcgdW5pbml0aWFsaXplZCBwb2ludGVyIG1lbWJlciBsaWtlIG5h
cGk/DQoNCj4NCj4+ICsJcnguc2tiID0gc2tiOw0KPj4gKwlyeC5zZGF0YSA9IHNkYXRhOw0KPj4g
KwlyeC5sb2NhbCA9IGxvY2FsOw0KPj4gKwlyeC5zdGEgPSBzdGE7DQo+PiArDQo+PiArCWlmICh2
aWYtPnR5cGUgPT0gTkw4MDIxMV9JRlRZUEVfQVBfVkxBTiAmJiBzZGF0YS0+YnNzICYmDQo+PiAr
CSAgICB1bmxpa2VseShlaGRyLT5oX3Byb3RvID09IHNkYXRhLT5jb250cm9sX3BvcnRfcHJvdG9j
b2wpKQ0KPj4gew0KPj4gKwkJc2RhdGEgPSBjb250YWluZXJfb2Yoc2RhdGEtPmJzcywgc3RydWN0
DQo+PiBpZWVlODAyMTFfc3ViX2lmX2RhdGEsDQo+PiArCQkJCSAgICAgdS5hcCk7DQo+PiArCQlk
ZXYgPSBzZGF0YS0+ZGV2Ow0KPj4gKwkJcnguc2RhdGEgPSBzZGF0YTsNCj4+ICsJfQ0KPj4gKw0K
Pj4gKwlyeC5za2ItPmRldiA9IGRldjsNCj4+ICsNCj4+ICsJLyogWFhYOiBTaW5jZSByeC5zZXFu
b19pZHggaXMgbm90IGF2YWlsYWJsZSBmb3IgZGVjYXANCj4+IG9mZmxvYWRlZCBmcmFtZXMNCj4+
ICsJICogcnggbXNkdSBzdGF0cyB1cGRhdGUgYXQgdGhlIHNlcW5vX2lkeCBpbg0KPj4gaWVlZTgw
MjExX2RlbGl2ZXJfc2tiKCkNCj4+ICsJICogd2lsbCBhbHdheXMgYmUgdXBkYXRlZCBhdCBpbmRl
eCAwIGFuZCB3aWxsIG5vdCBiZSB2ZXJ5DQo+PiB1c2VmdWwuDQo+PiArCSAqLw0KPj4gKwlpZWVl
ODAyMTFfZGVsaXZlcl9za2IoJnJ4KTsNCj4NCj4gWWVhaCwgdGhhdCdzIG5vdCBuaWNlIC0gcGVy
aGFwcyB3ZSBjYW4gcHJvdmlkZSB0aGUgVElEIG91dCBvZiBiYW5kPw0KDQpSaWdodC4gVGhpcyBp
cyBwb3NzaWJsZSB3aXRoIGF0aDEwayB3aGVyZSA4MDIuMTEgaGVhZGVyIGlzIGFsc28gYXZhaWxh
YmxlIGluIFJ4DQpkZXNjIG9mIHRoZSBmcmFtZSBpcnJlc3BlY3RpdmUgb2YgdGhlIGZvcm1hdCBv
ZiB0aGUgUnggcGF5bG9hZA0KDQogIElmDQo+IG5vdCwgd2UnbGwgaGF2ZSB0byBkaXNhYmxlIHRo
b3NlIHN0YXRpc3RpY3MgKmFsbCB0aGUgd2F5KiwgaS5lLiBub3QNCj4gZXZlbiByZXBvcnQgdGhl
bSB0byB1c2Vyc3BhY2Ugd2hlbiBmaWxsaW5nIHNpbmZvLg0KDQpZZXMsIHRoaXMgbWF5IGJlIHRo
ZSBvcHRpb24gd2hlbiBkcml2ZXIgaXMgdW5hd2FyZSBvZiByeCBwYWNrZXQgdGlkLg0KDQo+DQo+
PiArCXJldHVybjsNCj4+ICsNCj4+ICttaWNfZmFpbDoNCj4+ICsJY2ZnODAyMTFfbWljaGFlbF9t
aWNfZmFpbHVyZShzZGF0YS0+ZGV2LCBzdGEtPmFkZHIsDQo+PiArCQkJCSAgICAgKHN0YXR1cy0+
ZmxhZyAmIFJYX0ZMQUdfTUNBU1QpDQo+PiA/DQo+PiArCQkJCSAgICAgTkw4MDIxMV9LRVlUWVBF
X0dST1VQIDoNCj4+ICsJCQkJICAgICBOTDgwMjExX0tFWVRZUEVfUEFJUldJU0UsDQo+PiArCQkJ
CSAgICAga2V5ID8ga2V5LT5jb25mLmtleWlkeCA6IC0xLA0KPj4gKwkJCQkgICAgIE5VTEwsIEdG
UF9BVE9NSUMpOw0KPg0KPiBEbyB3ZSByZWFsbHkgd2FudCB0byBoYW5kbGUgdGhhdCBpbmxpbmUg
aGVyZT8gVGhlIGRyaXZlciBwcm9iYWJseSBoYXMgYQ0KPiBkaWZmZXJlbnQgY2hlY2sgdG8gZXZl
biBzZXQgUlhfRkxBR19NTUlDX0VSUk9SLCBzbyB3ZSBjb3VsZCBqdXN0IGFzayBpdA0KPiB0byBj
YWxsIGNmZzgwMjExX21pY2hhZWxfbWljX2ZhaWx1cmUoKSBbb3IgYSB3cmFwcGVyIHRvIGdldCBz
ZGF0YS0+ZGV2XQ0KPiBpbnN0ZWFkPyBJIGd1ZXNzIHRoaXMgd29ya3MgdG9vIHRob3VnaCwgYW5k
IG1pZ2h0IGJlIGVhc2llciB0bw0KPiB1bmRlcnN0YW5kLg0KDQpZZWFoLCBkcml2ZXIgZGlyZWN0
bHkgcmVwb3J0aW5nIE1JQyBmYWlsdXJlIHdpbGwgYmUgZmluZS4gSSB0aGluayBhIHdyYXBwZXIN
Cm1heSBiZSByZXF1aXJlZCByYXRoZXIgdGhhbiBtYWM4MDIxMSBiYXNlZCBkcml2ZXIgZGlyZWN0
bHkgY2FsbGluZyBjZmc4MDIxMQ0KZnVuY3Rpb24/DQoNClZhc2FudGg=
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Daniel Wagner @ 2016-12-16 7:25 UTC (permalink / raw)
To: Luis R. Rodriguez, Arend Van Spriel, Tom Gundersen, Johannes Berg,
Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki
Cc: Pali Rohár, Kalle Valo, Sebastian Reichel, Pavel Machek,
Michal Kazior, Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren,
linux-wireless, Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <CAB=NE6VHF+U7DzapENvLMKtN4Dgi9J0qMoeUJ8o4vSwc2-rRFQ@mail.gmail.com>
On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:
> For the new API a solution for "fallback mechanisms" should be clean
> though and I am looking to stay as far as possible from the existing
> mess. A solution to help both the old API and new API is possible for
> the "fallback mechanism" though -- but for that I can only refer you
> at this point to some of Daniel Wagner and Tom Gunderson's firmwared
> deamon prospect. It should help pave the way for a clean solution and
> help address other stupid issues.
The firmwared project is hosted here
https://github.com/teg/firmwared
As Luis pointed out, firmwared relies on FW_LOADER_USER_HELPER_FALLBACK,
which is not enabled by default. I don't see any reason why firmwared
should not also support loading calibration data. If we find a sound way
to do this.
As you can see from the commit history it is a pretty young project and
more ore less reanimation of the old udev firmware loader feature. We
are getting int into shape, adding integration tests etc.
The main motivation for this project is the get movement back in stuck
discussion on the firmware loader API. Luis was very busy writing up all
the details on the current situation and purely from the amount of
documentation need to describe the API you can tell something is awry.
Thanks,
Daniel
^ permalink raw reply
* [PATCH 2/2] mwifiex: Enable dynamic bandwidth signalling
From: Amitkumar Karwar @ 2016-12-16 7:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Cathy Luo, Nishant Sarmukadam, Amitkumar Karwar
In-Reply-To: <1481873145-5381-1-git-send-email-akarwar@marvell.com>
Enable dynamic bandwidth signalling by setting the corresponding
bit in MAC control register.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
drivers/net/wireless/marvell/mwifiex/init.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index f48247c..55db158f 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -441,6 +441,7 @@ enum mwifiex_channel_flags {
#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE BIT(7)
#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE BIT(8)
#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON BIT(13)
+#define HostCmd_ACT_MAC_DYNAMIC_BW_ENABLE BIT(16)
#define HostCmd_BSS_MODE_IBSS 0x0002
#define HostCmd_BSS_MODE_ANY 0x0003
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 87cda4f..7569483 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -92,7 +92,8 @@ int mwifiex_init_priv(struct mwifiex_private *priv)
for (i = 0; i < ARRAY_SIZE(priv->wep_key); i++)
memset(&priv->wep_key[i], 0, sizeof(struct mwifiex_wep_key));
priv->wep_key_curr_index = 0;
- priv->curr_pkt_filter = HostCmd_ACT_MAC_RX_ON | HostCmd_ACT_MAC_TX_ON |
+ priv->curr_pkt_filter = HostCmd_ACT_MAC_DYNAMIC_BW_ENABLE |
+ HostCmd_ACT_MAC_RX_ON | HostCmd_ACT_MAC_TX_ON |
HostCmd_ACT_MAC_ETHERNETII_ENABLE;
priv->beacon_period = 100; /* beacon interval */
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] mwifiex: change width of MAC control variable
From: Amitkumar Karwar @ 2016-12-16 7:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Cathy Luo, Nishant Sarmukadam, Amitkumar Karwar
Firmware has started making use of reserved field.
Accordingly change curr_pkt_filter from u16 to u32.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 18 ++++++++----------
drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ++--
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index ea45594..f48247c 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -434,14 +434,13 @@ enum mwifiex_channel_flags {
#define HostCmd_ACT_BITWISE_SET 0x0002
#define HostCmd_ACT_BITWISE_CLR 0x0003
#define HostCmd_RESULT_OK 0x0000
-
-#define HostCmd_ACT_MAC_RX_ON 0x0001
-#define HostCmd_ACT_MAC_TX_ON 0x0002
-#define HostCmd_ACT_MAC_WEP_ENABLE 0x0008
-#define HostCmd_ACT_MAC_ETHERNETII_ENABLE 0x0010
-#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE 0x0080
-#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE 0x0100
-#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON 0x2000
+#define HostCmd_ACT_MAC_RX_ON BIT(0)
+#define HostCmd_ACT_MAC_TX_ON BIT(1)
+#define HostCmd_ACT_MAC_WEP_ENABLE BIT(3)
+#define HostCmd_ACT_MAC_ETHERNETII_ENABLE BIT(4)
+#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE BIT(7)
+#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE BIT(8)
+#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON BIT(13)
#define HostCmd_BSS_MODE_IBSS 0x0002
#define HostCmd_BSS_MODE_ANY 0x0003
@@ -1084,8 +1083,7 @@ struct host_cmd_ds_802_11_mac_address {
};
struct host_cmd_ds_mac_control {
- __le16 action;
- __le16 reserved;
+ __le32 action;
};
struct host_cmd_ds_mac_multicast_adr {
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 5f7a010..5c82972 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -529,7 +529,7 @@ struct mwifiex_private {
u8 tx_timeout_cnt;
struct net_device *netdev;
struct net_device_stats stats;
- u16 curr_pkt_filter;
+ u32 curr_pkt_filter;
u32 bss_mode;
u32 pkt_tx_ctrl;
u16 tx_power_level;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index ad2dfd8..2f1f4d1 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -76,7 +76,7 @@
*/
static int mwifiex_cmd_mac_control(struct mwifiex_private *priv,
struct host_cmd_ds_command *cmd,
- u16 cmd_action, u16 *action)
+ u16 cmd_action, u32 *action)
{
struct host_cmd_ds_mac_control *mac_ctrl = &cmd->params.mac_ctrl;
@@ -89,7 +89,7 @@ static int mwifiex_cmd_mac_control(struct mwifiex_private *priv,
cmd->command = cpu_to_le16(HostCmd_CMD_MAC_CONTROL);
cmd->size =
cpu_to_le16(sizeof(struct host_cmd_ds_mac_control) + S_DS_GEN);
- mac_ctrl->action = cpu_to_le16(*action);
+ mac_ctrl->action = cpu_to_le32(*action);
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH mac80211-next] rfkill: hide unused goto label
From: Arnd Bergmann @ 2016-12-16 8:44 UTC (permalink / raw)
To: Johannes Berg
Cc: Arnd Bergmann, David S. Miller, João Paulo Rechi Vita,
Michał Kępień, linux-wireless, netdev,
linux-kernel
A cleanup introduced a harmless warning in some configurations:
net/rfkill/core.c: In function 'rfkill_init':
net/rfkill/core.c:1350:1: warning: label 'error_input' defined but not used [-Wunused-label]
This adds another #ifdef around the label to match that around the
caller.
Fixes: 6124c53edeea ("rfkill: Cleanup error handling in rfkill_init()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/rfkill/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f6b01f3616e5..b7adaee69756 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1347,8 +1347,10 @@ static int __init rfkill_init(void)
return 0;
+#ifdef CONFIG_RFKILL_INPUT
error_input:
rfkill_any_led_trigger_unregister();
+#endif
error_led_trigger:
misc_deregister(&rfkill_miscdev);
error_misc:
--
2.9.0
^ permalink raw reply related
* Re: [PATCH mac80211-next] rfkill: hide unused goto label
From: Johannes Berg @ 2016-12-16 9:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, João Paulo Rechi Vita,
Michał Kępień, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20161216084425.1692828-1-arnd@arndb.de>
On Fri, 2016-12-16 at 09:44 +0100, Arnd Bergmann wrote:
> A cleanup introduced a harmless warning in some configurations:
>
> net/rfkill/core.c: In function 'rfkill_init':
> net/rfkill/core.c:1350:1: warning: label 'error_input' defined but
> not used [-Wunused-label]
>
> This adds another #ifdef around the label to match that around the
> caller.
Applied, thanks.
johannes
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Johannes Berg @ 2016-12-16 9:13 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <58538DFA.1090808@qti.qualcomm.com>
> Ok. So it is mainly for encap there needs to be some capability
> advertisement from driver
Obviously, since mac80211 needs to pick the ndo struct to use.
> because for decap driver would use appropriate mac80211 Rx function
> to indicate?
That should be sufficient, no?
In fact, for RX, assuming that the relevant things are offloaded (and
we should ensure that in the RX function(s)), the driver could be
allowed to switch around as dynamically as it wants to (as long as it's
not mixing in _irqsafe calls or doing other things that might cause
reordering)
> > > + memset(&rx, 0, sizeof(rx));
> >
> > That seems a bit pointless?
>
> Hmmm, is it not that all member other than the ones initialized will
> be junk and might result in crash when accessing uninitialized
> pointer member like napi?
Oh, ok. I thought it was actually all filled.
You really should support NAPI though.
> > Yeah, that's not nice - perhaps we can provide the TID out of band?
>
> Right. This is possible with ath10k where 802.11 header is also
> available in Rx desc of the frame irrespective of the format of the
> Rx payload
We could go with that for now then. Leave a comment indicating that if
it can't be provided, we need to disable statistics for it?
> >
> > > + return;
> > > +
> > > +mic_fail:
> > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr,
> > > + (status->flag &
> > > RX_FLAG_MCAST)
> > > ?
> > > + NL80211_KEYTYPE_GROUP :
> > > + NL80211_KEYTYPE_PAIRWISE,
> > > + key ? key->conf.keyidx :
> > > -1,
> > > + NULL, GFP_ATOMIC);
> >
> > Do we really want to handle that inline here? The driver probably
> > has a different check to even set RX_FLAG_MMIC_ERROR, so we could
> > just ask it to call cfg80211_michael_mic_failure() [or a wrapper to
> > get sdata->dev] instead? I guess this works too though, and might
> > be easier to understand.
>
> Yeah, driver directly reporting MIC failure will be fine. I think a
> wrapper may be required rather than mac80211 based driver directly
> calling cfg80211 function?
It would be, because the driver can't get sdata->dev (although I think
there's now a hidden path to do this?)
johannes
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Johannes Berg @ 2016-12-16 9:25 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar, nbd@nbd.name; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <58537DA3.1090901@qti.qualcomm.com>
On Fri, 2016-12-16 at 05:37 +0000, Thiagarajan, Vasanthakumar wrote:
> QCA988X does not have capability to configure vif specific decap
> mode. Encap mode is configurable per packet for all the ath10k based
> chips so this part should be fine to support per vif configuration.
Ok, that's good.
> Newer QCA chips like QCA9984, QCA4019, QCA9888 and QCA99X0 supports
> decap mode configuration per vif.
Also good.
> To reduce the complexity, we can probably make per vif encap/decap
> configuration mandatory to enable ethernet frame format, not sure how
> this will work with non-QCA capable hardware.
I don't know either, nobody else has talked about it yet :-)
Anyway, if we (for now) we can assume that TX can be constant 802.3
encap mode once enabled, we don't have to deal with the messy dynamic
switching you had there.
RX can switch more dynamically independent of all the mac80211 code
since it basically just means the driver calls one function or the
other - if everything is offloaded correctly there shouldn't really be
a difference.
Then also things like IEEE80211_CONF_CHANGE_80211_HDR_OFFL etc. can go
away entirely because you don't have to switch anything dynamically at
a global level. That will simplify everything a lot.
johannes
^ permalink raw reply
* Re: [RFC 1/3] mac80211: Add provision for 802.11 encap/decap offload
From: Johannes Berg @ 2016-12-16 9:30 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <585273D4.7030100@qti.qualcomm.com>
On Thu, 2016-12-15 at 10:43 +0000, Thiagarajan, Vasanthakumar wrote:
> On Thursday 15 December 2016 02:46 PM, Johannes Berg wrote:
> >
> > > Drivers advertising this capability should also implement other
> > > functionalities which deal with 802.11 frame format like below
> > > - ADDBA/DELBA offload
> >
> > This shouldn't be necessary.
>
> Ok. Since driver/hw needs to implement Tx/Rx aggregation related
> functionalities I thought ADDBA/DELBA processing will be little
> important to mac80211. May be I'm missing something.
It needs to do the aggregation of the data flows, but I don't think the
control flows will need to be offloaded entirely? We'd need to have
some feedback mechanism for the SN used etc., so that might be up to
the driver to implement and might not be easy.
Anyway, I'm only suggesting to drop this from the documentation since
it doesn't seem strictly necessary, assuming the driver can report the
correct SSN in the callback all the A-MPDU setup should work more or
less as-is (except I think mac80211 won't buffer frames while doing it,
so also that would have to be done by the driver).
> > > - Powersave offload
> >
> > That's ambiguous - you do need to handle sleeping stations (and PS-
> > Poll/U-APSD) in AP mode in the device with this,
>
> I did not dig deep into PS requirement with ethernet frame format
> because frame control is not available to mac80211, so I thought to
> mention PS offload is a requirement. May be there is already an
> infrastructure in mac80211 to learn PS state of client which was
> notified in the current data frame (other than 802.11 frame control).
>
> but I don't see a deep
> > technical reason to require it for client mode. OTOH, client mode
> > is
> > almost always offloaded anyway.
>
> Ok.
Actually, come to think of it, the whole client-side powersave stuff is
so broken that I can't recommend using it - perhaps just clarify this
and say:
* AP: offloaded support for powersaving clients
* non-AP: offloaded support for powersave (if desired)
> > I think you may have forgotten one important item,
> >
> > - control port handling
>
> Hmmm, I'm getting WPA-PSK working with this. May be there are other
> control port handling which will not work with ethernet frame format?
I think I later saw control port handling in your 802.3 RX code - my
review at this point was tainted by some thoughts I had how I thought
it should work, but it's different :)
> > > + int (*get_vif_80211_hdr_offload)(struct ieee80211_hw
> > > *hw,
> > > + struct ieee80211_vif
> > > *vif,
> > > + bool is_4addr, bool
> > > *supported);
> >
> > Why are you not simply returning "supported"?
> >
> > I don't like passing the vif pointer here. At this point, the vif
> > pointer isn't known to the driver yet (through drv_add_interface)
> > so it's a dead pointer as far as the sequencing is concerned.
> >
> > Is there a possibility that drivers need to switch off ethernet
> > format handling entirely when an incompatible interface is added?
> > For example, if you add a mesh interface, is there a chance that
> > the AP interface might no longer be able to handle this?
>
> >
> > I'd hope this doesn't happen because I think that would
> be extremely
> > complicated to handle safely.
>
> Unfortunately "[RFC 2/3] mac80211: Implement data xmit for 802.11
> encap offload" tries to implement this but more likely buggy :(. You
> are right, it is very complex to get that right. May be we should not
> allow interface needing dynamic switch?
We discussed this over in the other part of the thread :)
johannes
^ permalink raw reply
* Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
From: Johannes Berg @ 2016-12-16 9:37 UTC (permalink / raw)
To: Tamizh chelvam; +Cc: c_traja, linux-wireless, ath10k
In-Reply-To: <134cc8e58ecb804b6dda0137c4c37be8@codeaurora.org>
> > > is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
> >
> > It's not really clear to me what you intend to do this - if it's
> > really support flags then you really should name those better.
> >
>
> This is support flags and it used by the driver to intimate driver
> supported frame type for the BTCOEX to cfg like
> "wiphy_wowlan_support_flags" implementation. Please suggest if this
> is ok ? I will be thankful if you can suggest a better one if this
> is not ok "WIPHY_BTCOEX_SUPPORTS_BE"
Well, I see a few things here:
1) does it even make sense to split it out per AC? wouldn't it be weird
if you supported this only for VO and BK, and not the others, or
something like that?
2) Wouldn't it make more sense to define this in nl80211 and just pass
the bitmap through to userspace? That would save quite a bit of netlink
mangling complexity.
3) I think the naming is confusing - "WIPHY_BTCOEX_SUPPORTS_BE_PREF" or
so might be more appropriate?
> Do you mean to say, sending a value from user space and parse that
> in the driver?
I was more thinking of the capability advertisement, but yeah, both
ways seems reasonable.
johannes
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Johannes Berg @ 2016-12-16 9:14 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481879623.27953.4.camel@sipsolutions.net>
> > > > + return;
> > > > +
> > > > +mic_fail:
> > > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr,
> > > > + (status->flag &
> > > > RX_FLAG_MCAST)
> > > > ?
> > > > + NL80211_KEYTYPE_GROUP :
> > > > + NL80211_KEYTYPE_PAIRWISE,
> > > > + key ? key->conf.keyidx :
> > > > -1,
> > > > + NULL, GFP_ATOMIC);
> > >
> > > Do we really want to handle that inline here? The driver probably
> > > has a different check to even set RX_FLAG_MMIC_ERROR, so we could
> > > just ask it to call cfg80211_michael_mic_failure() [or a wrapper
> > > to
> > > get sdata->dev] instead? I guess this works too though, and might
> > > be easier to understand.
> >
> > Yeah, driver directly reporting MIC failure will be fine. I think a
> > wrapper may be required rather than mac80211 based driver directly
> > calling cfg80211 function?
>
> It would be, because the driver can't get sdata->dev (although I
> think there's now a hidden path to do this?)
However, we can do both ways, I don't really care that much. It seems
possible though that a driver would not even report the frame, but only
the necessary info, in this case - so that we might need an out-of-band
path for it anyway?
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Johannes Berg @ 2016-12-16 9:56 UTC (permalink / raw)
To: Malinen, Jouni
Cc: Arend Van Spriel, Vamsi, Krishna, linux-wireless@vger.kernel.org
In-Reply-To: <20161215110635.GA11840@jouni.qca.qualcomm.com>
On Thu, 2016-12-15 at 11:06 +0000, Malinen, Jouni wrote:
> On Tue, Dec 13, 2016 at 04:56:55PM +0100, Johannes Berg wrote:
> > Regarding reusing attributes, we have (for the BSS selection thing)
> > the attribute NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, which is really
> > quite similar to your
> > new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since while
> > connected (which BSS_SELECT_ATTR_* assumes) the current BSS is
> > always part of the considered BSSes, I'd think.
>
> It seems to have somewhat similar meaning, but it looks more generic
> by not being tied to just two bands (2.4 and 5). And now that I
> actually looked again at this, it does not look like
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST actually allows more than a
> single band,delta pair to be provided and as such, it actually would
> not work very well with more than two bands even if it might be a bit
> more generic by allowing band to be set to something else than 2.4 or
> 5.
Agree, it wouldn't work well with more than 2 bands. Not that we have
them now (60GHz doesn't really count here).
> By the way, nl80211.h does not seem to document what values struct
> nl82011_bss_select_rssi_adjust band uses.. Is this enum
> nl80211_band?
I believe so, we should fix that.
> Neither does it say that delta is in dB (is it?).
We should also fix that, but let's assume so for the sake of this
discussion.
> > OTOH, the new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF doesn't
> > make this quite clear - is the current BSS to be adjusted before
> > comparing, if it's 5 GHz? If so, the semantics are equivalent. If
> > not, it doesn't actually make much sense ;-)
>
> Maybe the nl80211.h description was not clear enough, but the
> comments in cfg80211.h should be quite clear on how this was designed
> to work at the implementation level:
>
> "If the current connected BSS is in the 2.4 GHz band, other BSSs in
> the 2.4 GHz band to be reported should have better RSSI by
> @relative_rssi and other BSSs in the 5 GHz band to be reported should
> have better RSSI by (@relative_rssi - @relative_rssi_5g_pref).
> If the current connected BSS is in the 5 GHz band, other BSSs in the
> 2.4 GHz band to be reported should have better RSSI by
> (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> band to be reported should have better RSSI by by @relative_rssi."
Oh, right. Should probably be in nl80211.h too, to set expectations
from userspace.
> I'm not sure I'd describe this as adjusting the current BSS RSSI, but
> more like adjusting the RSSI threshold value if roaming would be from
> one band to another and doing that adjustment by adding or
> decrementing based on whether the roam would be from 2.4 to 5 or from
> 5 to 2.4.
It's functionally equivalent to doing the following adjustment
compare_rssi = current_bss_rssi
if current_bss_is_5ghz:
compare_rssi += relative_rssi_5g_pref
and then comparing all values (again adjusted for 5 GHz BSSes) to this.
(which is what I meant by "adjusting the current BSS RSSI").
> > So assuming that it is in fact taken into account after the same
> > adjustment, the two attributes are equivalent, and then perhaps it
> > would make sense to use struct nl80211_bss_select_rssi_adjust for
> > the new attribute. If a driver doesn't support arbitrary bands, but
> > just 5 GHz as in your example, it can just flip it around to 2.4
> > GHz by switching the sign.
> >
> > Perhaps we should even consider doing that in cfg80211 and
> > adjusting the internal API for both that way?
>
> I'm not completely sure I understood. One thing to note about
> differences here is that NL80211_BSS_SELECT_ATTR_* seems to be
> defining some preferences for BSS selection based on RSSI with an
> additional band preference, but it does not seem to define the
> threshold by how many dB the new candidate BSS should be better.
It's defining more than what we're talking about here, absolutely.
Clearly somebody thought a pure band preference might be useful (to
never connect to a 2.4GHz AP if a 5 GHz AP is available, regardless of
their relative RSSI), but that's not what we're looking at here.
(That could probably also be achieved by setting the adjustment to
90dB, but whatever)
> At minimum, we would need to clearly document struct
> nl80211_bss_select_rssi_adjust, but even if we do, I'm not sure it
> really is ideal mechanism to move to now that I realized it is not an
> array, but a single band,delta pair.
We can move to an array easily in the future by extending the attribute
length and advertising the number of array entries that are supported,
if that's your biggest concern? I don't see it as being very useful
right now since I don't think we'll see offloaded roaming between 2.4/5
and 60 GHz anytime soon. This may change when we add more bands later,
I suppose.
> Arend:
> > > I am not saying it should be avoided. Just looking at it
> > > conceptually
> > > the scheduled scan request holds so-called matchsets that specify
> > > the
> > > constraints to determine whether a BSS was found that is worth
> > > notifying the host/user-space about. As such I would expect the
> > > relative RSSI attribute(s) to be part of the matchset. That way
> > > you
> > > can specify it together with the currently connected SSID in a
> > > single
> > > matchset.
> >
> > I think this makes a lot of sense.
>
> If we are talking only about roaming within an ESS (a single SSID),
> that would sound clear, but which relative RSSI rules would apply if
> there are match sets for both the currently connected SSID and
> another SSID that the candidate BSS is for?
Right, I see how this might become a problem. I generally see no issue
with supporting multiple matchsets with different SSIDs but all having
the "relative to connected BSS RSSI filter" (which would disregard the
SSID specified in the matchset), but this then would become a problem
when multiple matchsets are specified with *different* such RSSI
filters, e.g. one matchset would specify that you want a 5G preference
of 10dB, but the other would specify a preference of only 5dB.
Conceptually in this approach, that would be supported, but firmware
likely would not be able to express this, I suppose?
> > We already have NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, which asks to
> > be reporting only networks that have an *absolute* RSSI value above
> > the value of the attribute - a new attribute to make it relative to
> > the current network instead would make sense.
>
> When you say "current network", do you mean the current BSS? This
> gets complex when thinking about multiple SSIDs (which some call
> "networks") and there being NL80211_SCHED_SCAN_MATCH_ATTR_SSID and
> multiple match sets..
Yes, I did mean "current BSS". And yes - it does seem complex, see
above.
> > That would indeed be equivalent to NL80211_BSS_SELECT_ATTR_RSSI
> > then.
>
> NL80211_BSS_SELECT_ATTR_RSSI is a flag attribute.. BSS select
> mechanism does not provide an absolute RSSI value or threshold; it
> just seems to indicate use of RSSI-based selection mechanism without
> defining what exactly is better. There is
> NL80211_BSS_SELECT_ATTR_BAND_PREF that gives a preference to a
> specific band (without defining what that preference is) and then the
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST that can actually give a specific
> RSSI adjustment value (in dB?).
>
> > Now, if we consider this, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
> > actually is equivalent to NL80211_BSS_SELECT_ATTR_RSSI (a flag
> > attribute indicating whether or not RSSI-based selection/matching
> > is done) and NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF is
> > equivalent to NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, both need to be
> > given with the flag and affect operation.
>
> Hmm.. So you did notice it is a flag attribute.. So how would this
> match NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI which provides the
> threshold value for how many dB better the new BSS needs to be for it
> to be reported?
Yeah, umm, I think I got confused. There's no threshold in the
BSS_SELECT case, and maybe there doesn't need to be (at least not to
avoid host wakeups, though you'd want some hysteresis to avoid swapping
around too much).
> > So, how about we move these into NL80211_SCHED_SCAN_MATCH_ATTR_* as
> > suggested by Arend, and define them with the same content as the
> > corresponding NL80211_BSS_SELECT_ATTR_*?
>
> I think I'm missing something here.. Where would the threshold value
> (how much better new BSS needs to be) be stored? And do we really
> want something like the combination of
> NL80211_BSS_SELECT_ATTR_BAND_PREF and
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST which seems to be two different
> ways of doing band preference (the former without specifying delta
> and the latter with specific delta)? Or am I still not understanding
> how NL80211_BSS_SELECT_ATTR_* really works?
No, you're right, I missed the "better by" threshold.
I think between that (unless we add that, we could technically extend
flag attributes to allow them being an int as well, or add a new one)
and the fact that the device may not support different relative RSSI
matches in different matchsets, I'm almost convinced that adding new
attributes is better.
> The main concern I have for optional features with sched_scan is in
> whether the device ends up being woken up constantly if the driver
> does not understand a constraint that user space is trying to use to
> avoid being notified all the time.
Agree.
johannes
^ permalink raw reply
* Re: [RFC V3 03/11] nl80211: add support for gscan
From: Johannes Berg @ 2016-12-16 10:13 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <f4f83c57-3e46-b017-fa36-b3776d71670a@broadcom.com>
On Wed, 2016-12-14 at 10:01 +0100, Arend Van Spriel wrote:
> Had to look for "> 16" ;-)
Sorry.
> Here an instance of the tab vs. space issue you mentioned. Will go
> over the patch and fix that.
There were a few, not really interesting though - git would probably
flag it anyway, or checkpatch :)
> > + if (num_chans > 16)
> > + return -EINVAL;
>
> I suspect this is the restriction you were referring to.
Yes.
> There is no
> reason for this although the android wifi hal has max 16 channels in
> a bucket so I might have picked that up.
I thought I saw something with a u16 bitmap that seemed related, but I
don't see that now so I'm probably just confused.
> So could a driver have a similar limit and should we add such to the
> gscan capabilities? For instance our firmware api has a nasty
> restriction of 64 channels for all buckets together, eg. can do 4
> buckets of 16 channels each.
We do have a limit of the maximum scan buckets, which seems to be 16
right now. We also have a limit on the number of channels per bucket,
which is also 16, but no combined limit afaict (so 16x16 seems fine).
Maybe we do need some advertisement in that area then? Right now,
wifihal seems to be able to read as capabilities the number of buckets
(wifi_gscan_capabilities), but assumes the number of channels:
const unsigned MAX_CHANNELS = 16;
const unsigned MAX_BUCKETS = 16;
I guess we took that and combined it, and you had more negotiation with
Google ;-)
We may then have to actually advertise the limit you have ("64 channels
combined over all buckets"), unless you can get away with just
advertising 4 buckets (and us saying 16 channels per bucket is enough?)
I'm a bit tempted to make this more forward compatible though and not
hard-limit the number of channels per bucket in the code.
johannes
^ permalink raw reply
* Re: [RFC v2 06/11] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB
From: Valo, Kalle @ 2016-12-16 10:23 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <1479496971-19174-7-git-send-email-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> ---
> drivers/net/wireless/ath/ath10k/htc.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless=
/ath/ath10k/htc.h
> index f94b25a..2963694 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
> ATH10K_HTC_FLAG_BUNDLE_MASK =3D 0xF0
> };
> =20
> +#define ATH10K_HTC_FLAG_BUNDLE_LSB 4
> +
I think you could fold patches from 6 to 11 into one patch. They are all
about adding code, and most of the commit logs are empty anyway.
--=20
Kalle Valo=
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-12-16 10:26 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Sebastian Reichel, Pavel Machek, Michal Kazior,
Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren, linux-wireless,
Network Development, linux-kernel, Luis R. Rodriguez
In-Reply-To: <76365770-f5ba-9565-3fca-710518f64d81@broadcom.com>
[-- Attachment #1: Type: Text/Plain, Size: 3509 bytes --]
On Thursday 15 December 2016 21:12:47 Arend Van Spriel wrote:
> On 15-12-2016 16:33, Pali Rohár wrote:
> > On Thu Dec 15 09:18:44 2016 Kalle Valo <kvalo@codeaurora.org> wrote:
> >> (Adding Luis because he has been working on request_firmware()
> >> lately)
> >>
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >>>>> So no, there is no argument against... request_firmware() in
> >>>>> fallback mode with userspace helper is by design blocking and
> >>>>> waiting for userspace. But waiting for some change in DTS in
> >>>>> kernel is just nonsense.
> >>>>
> >>>> I would just mark the wlan device with status = "disabled" and
> >>>> enable it in the overlay together with adding the NVS & MAC
> >>>> info.
> >>>
> >>> So if you think that this solution make sense, we can wait what
> >>> net wireless maintainers say about it...
> >>>
> >>> For me it looks like that solution can be:
> >>>
> >>> extending request_firmware() to use only userspace helper
> >>
> >> I haven't followed the discussion very closely but this is my
> >> preference what drivers should do:
> >>
> >> 1) First the driver should do try to get the calibration data and
> >> mac
> >>
> >> address from the device tree.
> >
> > Ok, but there is no (dynamic, device specific) data in DTS for
> > N900. So 1) is noop.
>
> Uhm. What do you mean? You can propose a patch to the DT bindings [1]
> to get it in there and create your N900 DTB or am I missing
> something here. Are there hardware restrictions that do not allow
> you to boot with your own DTB.
What is [1]?
N900's bootloader does not support DTB and it does not pass any DTB. It
boots kernel in ATAGs mode. What we are doing is appending DTB compiled
from kernel sources to end of zImage.
But that appended DTB cannot contains device specific nodes (e.g.
calibration data for device) as zImage is there for any N900 device, not
just only one.
> >> 2) If they are not in DT the driver should retrieve the
> >> calibration data
> >>
> >> with request_firmware(). BUT with an option for user space
> >> to implement that with a helper script so that the data
> >> can be created dynamically, which I believe openwrt does
> >> with ath10k calibration data right now.
> >
> > Currently there is flag for request_firmware() that it should
> > fallback to user helper if direct VFS access not find needed
> > firmware.
> >
> > But this flag is not suitable as /lib/firmware already provides
> > default (not device specific) calibration data.
> >
> > So I would suggest to add another flag/function which will primary
> > use user helper.
>
> I recall Luis saying that user-mode helper (fallback) should be
> discouraged, because there is no assurance that there is a user-mode
> helper so you might just be pissing in the wind. The idea was to have
> a dedicated API call that explicitly does the request towards
> user-space.
>
> By the way, are we talking here about wl1251 device or driver as you
> also mentioned wl12xx? I did not read the entire thread.
Yes, we are talking about wl1251 device, which is in Nokia N900 and
wl1251.ko and wl1251_spi.ko drivers.
I mentioned wl12xx as it already uses similar approach with mac address
via request_firmware(). And as those drivers are very similar plus from
one manufactor and in same kernel folder I mentioned similar API for
consistency...
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-12-16 10:35 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Arend Van Spriel, Tom Gundersen, Daniel Wagner, Johannes Berg,
Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki,
Kalle Valo, Sebastian Reichel, Pavel Machek, Michal Kazior,
Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren, linux-wireless,
Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <CAB=NE6VHF+U7DzapENvLMKtN4Dgi9J0qMoeUJ8o4vSwc2-rRFQ@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 8476 bytes --]
On Friday 16 December 2016 03:03:19 Luis R. Rodriguez wrote:
> On Thu, Dec 15, 2016 at 2:12 PM, Arend Van Spriel
>
> <arend.vanspriel@broadcom.com> wrote:
> > On 15-12-2016 16:33, Pali Rohár wrote:
> >> On Thu Dec 15 09:18:44 2016 Kalle Valo <kvalo@codeaurora.org>
> >> wrote:
> >>> (Adding Luis because he has been working on request_firmware()
> >>> lately)
> >>>
> >>> Pali Rohár <pali.rohar@gmail.com> writes:
> >>>>>> So no, there is no argument against... request_firmware() in
> >>>>>> fallback mode with userspace helper is by design blocking and
> >>>>>> waiting for userspace. But waiting for some change in DTS in
> >>>>>> kernel is just nonsense.
> >>>>>
> >>>>> I would just mark the wlan device with status = "disabled" and
> >>>>> enable it in the overlay together with adding the NVS & MAC
> >>>>> info.
> >>>>
> >>>> So if you think that this solution make sense, we can wait what
> >>>> net wireless maintainers say about it...
> >>>>
> >>>> For me it looks like that solution can be:
> >>>>
> >>>> extending request_firmware() to use only userspace helper
> >>>
> >>> I haven't followed the discussion very closely but this is my
> >>> preference what drivers should do:
> >>>
> >>> 1) First the driver should do try to get the calibration data and
> >>> mac
> >>>
> >>> address from the device tree.
> >>
> >> Ok, but there is no (dynamic, device specific) data in DTS for
> >> N900. So 1) is noop.
> >
> > Uhm. What do you mean? You can propose a patch to the DT bindings
> > [1] to get it in there and create your N900 DTB or am I missing
> > something here. Are there hardware restrictions that do not allow
> > you to boot with your own DTB.
> >
> >>> 2) If they are not in DT the driver should retrieve the
> >>> calibration data
> >>>
> >>> with request_firmware(). BUT with an option for user space
> >>> to implement that with a helper script so that the data
> >>> can be created dynamically, which I believe openwrt does
> >>> with ath10k calibration data right now.
> >>
> >> Currently there is flag for request_firmware() that it should
> >> fallback to user helper if direct VFS access not find needed
> >> firmware.
> >>
> >> But this flag is not suitable as /lib/firmware already provides
> >> default (not device specific) calibration data.
> >>
> >> So I would suggest to add another flag/function which will primary
> >> use user helper.
> >
> > I recall Luis saying that user-mode helper (fallback) should be
> > discouraged, because there is no assurance that there is a
> > user-mode helper so you might just be pissing in the wind.
>
> There's tons of issues with the current status quo of the so called
> "firmware usermode helper". To start off with its a complete
> misnomer, the kernel's usermode helper infrastructure is implemented
> in lib/kmod.c and it provides an API to help call out to userspace
> some helper for you. That's the real kernel usermode helper. In so
> far as the firmware_class.c driver is concerned -- it only makes use
> of the kernel user mode helper as an option if you have
> CONFIG_UEVENT_HELPER_PATH set to help address kobject uevents. Most
> distributions do not use this, and back in the day systemd udev, and
> prior to that hotplug used to process firmware kobject uevents.
> systemd udev is long gone. Gone. This kobject uevents really are a
> "fallback mechanism" for firmware only -- if cached firmware,
> built-in firmware, or direct filesystem lookup fails. These each are
> their own beast. Finally kobject uevents are only one of the
> fallback
> mechanisms, "custom fallback mechanisms" are the other option and its
> what you and others seem to describe to want for these sorts of
> custom things.
>
> There are issues with the existing request_firmware() API in that
> everyone and their mother keeps extending it with stupid small API
> extensions to do yet-another-tweak, and then having to go and change
> tons of drivers. Or a new API call added for just one custom knob.
> Naturally this is all plain dumb, so yet-another-API call or new
> argument is not going to help us. We don't have "flags" persi in this
> API, that was one issue with the design of the API, but there are
> much more. The entire change in mechanism of "firmware fallback
> mechanism" depending on which kernel config options you have is
> another. Its a big web of gum put together. All this needs to end.
>
> I've recently proposed a new patch to first help with understanding
> each of the individual items I've listed above with as much new
> information as is possible. I myself need to re-read it to just keep
> tabs on what the hell is going on at times. After this I have a new
> API proposal which I've been working on for a long time now which
> adds an extensible API with only 2 calls: sync, async, and lets us
> modify attributes through a parameters struct. This is what we
> should use in the future for further extensions.
>
> For the new API a solution for "fallback mechanisms" should be clean
> though and I am looking to stay as far as possible from the existing
> mess. A solution to help both the old API and new API is possible for
> the "fallback mechanism" though -- but for that I can only refer you
> at this point to some of Daniel Wagner and Tom Gunderson's firmwared
> deamon prospect. It should help pave the way for a clean solution and
> help address other stupid issues.
>
> I'll note -- the "custom fallback mechanism", part of the old API is
> just a terrible idea for many reasons. I've documented issues in my
> documentation revamp, I suggest to read that, refer to this thread:
>
> https://lkml.kernel.org/r/20161213030828.17820-4-mcgrof@kernel.org
>
> > The idea was to have a
> > dedicated API call that explicitly does the request towards
> > user-space.
>
> In so far as this driver example that was mentioned in this thread my
> recommendation is to use the default existing MAC address /
> calibration data on /lib/firmware/ and then use another request to
> help override for the custom thing. The only issue of course is that
> the timeout for the custom thing is 60 seconds, but if your platforms
> are controlled -- just reduce this to 1 second or so given that udev
> is long gone and it would seem you'd only use a custom fw request to
> depend on. You could also wrap this thing into a kconfig option for
> now, and have the filename specified, if empty then no custom request
> is sent. If set then you have it request it.
>
> Please note the other patches in my series for the custom fallback
> though. We have only 2 users upstream -- and from recent discussions
> it would seem it might be possible to address what you need with
> firmwared in a clean way without this custom old fallback crap thing.
> Johannes at least has a similar requirement and is convinced a
> "custom" thing can be done without even adding an extra custom
> kobject uevent attribute as from what I recall he hinted, drivers
> have no business to be involved in this. If you do end up using the
> custom fallback mechanism be sure to add super crystal clear
> documentation for it (see my other patches for the declarer for this
> documentation). Since we have only 2 drivers using this custom thing
> its hard to get folks to commit to writing the docs, write it now
> and be sure you think of the future as well.
>
> Oh also the "custom firmware fallback" was also broken on recent
> kernels for many kernel revisions, it just did not work until
> recently a fix which went in, so your users wills need this fix
> cherry picked. Hey I tell you, the custom fw thing was a terrible
> incarnation. Only 2 drivers use it. There are good reasons to not
> like it (be sure to read the suspend issue I documented).
>
> > By the way, are we talking here about wl1251 device or driver as
> > you also mentioned wl12xx? I did not read the entire thread.
>
> Luis
Hi Luis! wl1251 already uses request_firmware for loading calibration
data from VFS. And because /lib/firmware/ contains preinstalled default
calibration data it uses it -- which is wrong as wireless has bad
quality.
In my setup I'm going to use udev with firmware loading support
(probably fork eudev), so it should be systemd-independent.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [RFC V3 04/11] nl80211: add driver api for gscan notifications
From: Johannes Berg @ 2016-12-16 10:02 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <69ec7e9f-cd46-c46d-140c-0b30343cc0f7@broadcom.com>
> Not sure what is meant by "through the buckets".
TBH, I was handwaving because I don't understand this part of gscan
well :-)
> Referring to your
> remark/question in the "Unversal scan proposal" thread:
>
> """
> 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?
> """
>
> So this is exactly the dilemma I considered so I decided to stick
> with the full BSS reporting model for gscan as well merely to get it
> discussed so glad you brought it up ;-).
Heh.
Ok, so I missed that. Somehow I thought hidden in the buckets was a
partial reporting :-)
> The problem here is that gscan is a vehicle that serves a number of
> use-cases. So ignoring hotlists, ePNO, etc. the gscan configuration
> still hold several notification types:
>
> - report after completing M scans capturing N best APs or a
> percentage of (M * N).
> - report after completing a scan include a specific bucket.
> - report full scan results.
>
> The first two notification trigger retrieval of gscan results which
> are historic, ie. partial scan results for max M scans.
>
> As said earlier the universal scan proposal has some similarities to
> gscan. Guess you share that as you are using the term "bucket
> reporting" in that discussion ;-). The historic results are needed
> for location (if I am not mistaken) so the full BSS reporting model
> does not fit that. Question is what particular attribute in the
> historic results is needed for location (suspecting only rssi and
> possibly the timestamp, but would like to see that confirmed). I was
> thinking about have historic storage in cfg80211 so we do not need a
> per-driver solution.
Ok, now I'm starting to understand this better, I guess.
As far as I can tell from our code, for cached results we're reporting
the following data:
* some flags
* a scan ID
* and for each AP:
* RSSI
* timestamp
* channel
* BSSID
* SSID (which internally we even have a separate table for and each
AP just has an index to it, to save memory I guess)
* beacon period
* capability field
No IEs and similar things like differentiating probe response/beacon,
so we can't use the full reporting for this.
I have no problem introducing a common storage for this, if necessary
with some fields/nl attributes being optional, but I suspect this is
actually a necessary part of gscan, otherwise you're not able to report
all the necessary data?
johannes
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-12-16 10:40 UTC (permalink / raw)
To: Daniel Wagner
Cc: Luis R. Rodriguez, Arend Van Spriel, Tom Gundersen, Johannes Berg,
Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki,
Kalle Valo, Sebastian Reichel, Pavel Machek, Michal Kazior,
Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren, linux-wireless,
Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <07be7fc8-6c7f-6e03-379d-fa781b248259@monom.org>
[-- Attachment #1: Type: Text/Plain, Size: 2061 bytes --]
On Friday 16 December 2016 08:25:44 Daniel Wagner wrote:
> On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:
> > For the new API a solution for "fallback mechanisms" should be
> > clean though and I am looking to stay as far as possible from the
> > existing mess. A solution to help both the old API and new API is
> > possible for the "fallback mechanism" though -- but for that I can
> > only refer you at this point to some of Daniel Wagner and Tom
> > Gunderson's firmwared deamon prospect. It should help pave the way
> > for a clean solution and help address other stupid issues.
>
> The firmwared project is hosted here
>
> https://github.com/teg/firmwared
>
> As Luis pointed out, firmwared relies on
> FW_LOADER_USER_HELPER_FALLBACK, which is not enabled by default.
I know. But it does not mean that I cannot enable this option at kernel
compile time.
Bigger problem is that currently request_firmware() first try to load
firmware directly from VFS and after that (if fails) fallback to user
helper.
So I would need to extend kernel firmware code with new function (or
flag) to not use VFS and try only user mode helper.
> I
> don't see any reason why firmwared should not also support loading
> calibration data. If we find a sound way to do this.
It can, but why should I use another daemon for firmware loading as
non-systemd version of udev (and eudev fork) support firmware loading?
I think I stay with udev/eudev.
> As you can see from the commit history it is a pretty young project
> and more ore less reanimation of the old udev firmware loader
> feature. We are getting int into shape, adding integration tests
> etc.
>
> The main motivation for this project is the get movement back in
> stuck discussion on the firmware loader API. Luis was very busy
> writing up all the details on the current situation and purely from
> the amount of documentation need to describe the API you can tell
> something is awry.
>
> Thanks,
> Daniel
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* wl1251 NVS calibration data format
From: Pali Rohár @ 2016-12-16 11:01 UTC (permalink / raw)
To: Gery Kahn, Shahar Lev, Kalle Valo, linux-wireless, linux-kernel,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov
[-- Attachment #1: Type: Text/Plain, Size: 831 bytes --]
Hi! Do you know format of wl1251 NVS calibration data file?
I found that there is tool for changing NVS file for wl1271 and newer
chips (so not for wl1251!) at: https://github.com/gxk/ti-utils
And wl1271 has in NVS data already place for MAC address. And in wlcore
(for wl1271 and newer) there is really kernel code which is doing
something with MAC address in NVS, see:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ti/wlcore/boot.c#n352
So... I would like to know if in wl1251 NVS calibration file is also
some place for MAC address or not.
Default wl1251 NVS calibration file is available in linux-firmware:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/ti-connectivity/wl1251-nvs.bin
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: wl1251 NVS calibration data format
From: Pali Rohár @ 2016-12-16 11:12 UTC (permalink / raw)
To: Gery Kahn, Shahar Lev, Kalle Valo, linux-wireless, linux-kernel,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov
In-Reply-To: <201612161201.48356@pali>
[-- Attachment #1: Type: Text/Plain, Size: 1133 bytes --]
Resending email to new Gery's address...
On Friday 16 December 2016 12:01:48 Pali Rohár wrote:
> Hi! Do you know format of wl1251 NVS calibration data file?
>
> I found that there is tool for changing NVS file for wl1271 and newer
> chips (so not for wl1251!) at: https://github.com/gxk/ti-utils
>
> And wl1271 has in NVS data already place for MAC address. And in wlcore
> (for wl1271 and newer) there is really kernel code which is doing
> something with MAC address in NVS, see:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ti/wlcore/boot.c#n352
Also, there is parsing MAC address from NVS wl1271 data:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ti/wlcore/main.c#n6009
> So... I would like to know if in wl1251 NVS calibration file is also
> some place for MAC address or not.
>
> Default wl1251 NVS calibration file is available in linux-firmware:
> https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/ti-connectivity/wl1251-nvs.bin
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [RFC v2 11/11] ath10k: Added sdio support
From: Valo, Kalle @ 2016-12-16 11:21 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <1479496971-19174-12-git-send-email-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
I know most of this coming from ath6kl but I think we should still
improve the code. Lots of comments will follow, don't get scared :)
> +#define CALC_TXRX_PADDED_LEN(ar_sdio, len) \
> + (__ALIGN_MASK((len), (ar_sdio)->mbox_info.block_mask))
I think this could be a proper static inline function. Andrew Morton
once said: "Write in C, not CPP" (or something like that), I think
that's a really good point.
> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *=
buf,
> + u32 len, u32 request);
> +static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, voi=
d *buf,
> + size_t buf_len);
> +static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
> + u32 *value);
We prefer to avoid forward declarations if at all possible. I didn't
check but if there's a clean way to avoid these please remove them.
> +/* HIF mbox interrupt handling */
> +
> +static int ath10k_sdio_mbox_rx_process_packet(struct ath10k_sdio *ar_sdi=
o,
> + struct ath10k_sdio_rx_data *pkt,
> + u32 *lookaheads,
> + int *n_lookaheads)
> +{
So the style in ath10k is that all functions (of course this doesn't
apply to callbacks etc) have "struct ath10k *ar" as the first parameter.
This way there's no need to check if a function takes ar or ar_sdio.
> + int status =3D 0;
In ath10k we prefer to use ret. And avoid initialising it, please.
> + struct ath10k_htc *htc =3D &ar_sdio->ar->htc;
> + struct sk_buff *skb =3D pkt->skb;
> + struct ath10k_htc_hdr *htc_hdr =3D (struct ath10k_htc_hdr *)skb->data;
> + bool trailer_present =3D htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESE=
NT;
> + u16 payload_len;
> +
> + payload_len =3D le16_to_cpu(htc_hdr->len);
> +
> + if (trailer_present) {
> + u8 *trailer;
> + enum ath10k_htc_ep_id eid;
> +
> + trailer =3D skb->data + sizeof(struct ath10k_htc_hdr) +
> + payload_len - htc_hdr->trailer_len;
> +
> + eid =3D (enum ath10k_htc_ep_id)htc_hdr->eid;
A some kind of mapping function for ep id would be nice, it makes it
more visible how it works.
> +static int ath10k_sdio_mbox_rx_process_packets(struct ath10k_sdio *ar_sd=
io,
> + u32 lookaheads[],
> + int *n_lookahead)
> +{
> + struct ath10k *ar =3D ar_sdio->ar;
> + struct ath10k_htc *htc =3D &ar->htc;
> + struct ath10k_sdio_rx_data *pkt;
> + int status =3D 0, i;
> +
> + for (i =3D 0; i < ar_sdio->n_rx_pkts; i++) {
> + struct ath10k_htc_ep *ep;
> + enum ath10k_htc_ep_id id;
> + u32 *lookaheads_local =3D lookaheads;
> + int *n_lookahead_local =3D n_lookahead;
> +
> + id =3D ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
> +
> + if (id >=3D ATH10K_HTC_EP_COUNT) {
> + ath10k_err(ar, "Invalid endpoint in look-ahead: %d\n",
> + id);
In ath10k we use ath10k_err() for errors from which can't survive and
ath10k_warn() for errors where we try to continue. So ath10k_warn()
would be more approriate here and most of other cases in sdio.c
> + status =3D -ENOMEM;
> + goto out;
> + }
> +
> + ep =3D &htc->endpoint[id];
> +
> + if (ep->service_id =3D=3D 0) {
> + ath10k_err(ar, "ep %d is not connected !\n", id);
> + status =3D -ENOMEM;
> + goto out;
> + }
> +
> + pkt =3D &ar_sdio->rx_pkts[i];
> +
> + if (pkt->part_of_bundle && !pkt->last_in_bundle) {
> + /* Only read lookahead's from RX trailers
> + * for the last packet in a bundle.
> + */
> + lookaheads_local =3D NULL;
> + n_lookahead_local =3D NULL;
> + }
> +
> + status =3D ath10k_sdio_mbox_rx_process_packet(ar_sdio,
> + pkt,
> + lookaheads_local,
> + n_lookahead_local);
> + if (status)
> + goto out;
> +
> + ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
> + /* The RX complete handler now owns the skb...*/
> + pkt->skb =3D NULL;
> + pkt->alloc_len =3D 0;
> + }
> +
> +out:
> + /* Free all packets that was not passed on to the RX completion
> + * handler...
> + */
> + for (; i < ar_sdio->n_rx_pkts; i++)
> + ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
I got a bit fooled by not initialising i here and only then realised
why. I guess it's ok but a bit of so and so
> +
> + return status;
> +}
> +
> +static int alloc_pkt_bundle(struct ath10k *ar,
> + struct ath10k_sdio_rx_data *rx_pkts,
> + struct ath10k_htc_hdr *htc_hdr,
> + size_t full_len, size_t act_len, size_t *bndl_cnt)
> +{
> + int i, status =3D 0;
> +
> + *bndl_cnt =3D (htc_hdr->flags & ATH10K_HTC_FLAG_BUNDLE_MASK) >>
> + ATH10K_HTC_FLAG_BUNDLE_LSB;
We recently got FIELD_GET() and FIELD_PREP() to kernel for handling
bitmasks. ath10k is not yet converted (patches welcome!) but it would be
good to use those already in sdio.c. Also SM() could be replaced with
those.
> +int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k_sdio *ar_sdio,
> + u32 msg_lookahead, bool *done)
> +{
> + struct ath10k *ar =3D ar_sdio->ar;
> + int status =3D 0;
> + u32 lookaheads[ATH10K_SDIO_MAX_RX_MSGS];
> + int n_lookaheads =3D 1;
> +
> + *done =3D true;
> +
> + /* Copy the lookahead obtained from the HTC register table into our
> + * temp array as a start value.
> + */
> + lookaheads[0] =3D msg_lookahead;
> +
> + for (;;) {
Iternal loops in kernel can be dangerous. Better to add some sort of
timeout check with a warning message, something like:
while ((time_before(jiffies, timeout)) {
}
if (timed out)
ath10k_warn("timeout in foo");
> + /* Try to allocate as many HTC RX packets indicated by
> + * n_lookaheads.
> + */
> + status =3D ath10k_sdio_mbox_rx_alloc(ar_sdio, lookaheads,
> + n_lookaheads);
> + if (status)
> + break;
> +
> + if (ar_sdio->n_rx_pkts >=3D 2)
> + /* A recv bundle was detected, force IRQ status
> + * re-check again.
> + */
> + *done =3D false;
> +
> + status =3D ath10k_sdio_mbox_rx_fetch(ar_sdio);
> +
> + /* Process fetched packets. This will potentially update
> + * n_lookaheads depending on if the packets contain lookahead
> + * reports.
> + */
> + n_lookaheads =3D 0;
> + status =3D ath10k_sdio_mbox_rx_process_packets(ar_sdio,
> + lookaheads,
> + &n_lookaheads);
> +
> + if (!n_lookaheads || status)
> + break;
> +
> + /* For SYNCH processing, if we get here, we are running
> + * through the loop again due to updated lookaheads. Set
> + * flag that we should re-check IRQ status registers again
> + * before leaving IRQ processing, this can net better
> + * performance in high throughput situations.
> + */
> + *done =3D false;
> + }
> +
> + if (status && (status !=3D -ECANCELED))
> + ath10k_err(ar, "failed to get pending recv messages: %d\n",
> + status);
> +
> + if (atomic_read(&ar_sdio->stopping)) {
> + ath10k_warn(ar, "host is going to stop. Turning of RX\n");
> + ath10k_sdio_hif_rx_control(ar_sdio, false);
> + }
I'm always skeptic when I use atomic variables used like this, I doubt
it's really correct.
> +
> + return status;
> +}
> +
> +static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k_sdio *ar_sdio)
> +{
> + int ret;
> + u32 dummy;
> + struct ath10k *ar =3D ar_sdio->ar;
> +
> + ath10k_warn(ar, "firmware crashed\n");
We have firmware crash dump support in ath10k. You could add a "TODO:"
comment about implementing that later.
> +static int ath10k_sdio_mbox_proc_err_intr(struct ath10k_sdio *ar_sdio)
> +{
> + int status;
> + u8 error_int_status;
> + u8 reg_buf[4];
> + struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> + struct ath10k *ar =3D ar_sdio->ar;
> +
> + ath10k_dbg(ar, ATH10K_DBG_SDIO, "error interrupt\n");
> +
> + error_int_status =3D irq_data->irq_proc_reg.error_int_status & 0x0F;
> + if (!error_int_status) {
> + WARN_ON(1);
> + return -EIO;
> + }
> +
> + ath10k_dbg(ar, ATH10K_DBG_SDIO,
> + "valid interrupt source(s) in ERROR_INT_STATUS: 0x%x\n",
> + error_int_status);
> +
> + if (MS(error_int_status, MBOX_ERROR_INT_STATUS_WAKEUP))
> + ath10k_dbg(ar, ATH10K_DBG_SDIO, "error : wakeup\n");
> +
> + if (MS(error_int_status, MBOX_ERROR_INT_STATUS_RX_UNDERFLOW))
> + ath10k_err(ar, "rx underflow\n");
> +
> + if (MS(error_int_status, MBOX_ERROR_INT_STATUS_TX_OVERFLOW))
> + ath10k_err(ar, "tx overflow\n");
> +
> + /* Clear the interrupt */
> + irq_data->irq_proc_reg.error_int_status &=3D ~error_int_status;
> +
> + /* set W1C value to clear the interrupt, this hits the register first *=
/
> + reg_buf[0] =3D error_int_status;
> + reg_buf[1] =3D 0;
> + reg_buf[2] =3D 0;
> + reg_buf[3] =3D 0;
> +
> + status =3D ath10k_sdio_read_write_sync(ar,
> + MBOX_ERROR_INT_STATUS_ADDRESS,
> + reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
> +
> + WARN_ON(status);
This is a bit dangerous, in worst case it can spam the kernel log and
force a host reboot due watchdog timing out etc.
Better to replace with standard warning message:
ret =3D ath10k_sdio_read_write_sync(ar,
MBOX_ERROR_INT_STATUS_ADDRESS,
reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
if (ret) {
ath10k_warn("failed to read interrupr status: %d", ret);
return ret;
}
> +static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k_sdio *ar_sdio)
> +{
> + int status;
> + struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> + struct ath10k *ar =3D ar_sdio->ar;
> + u8 cpu_int_status, reg_buf[4];
> +
> + cpu_int_status =3D irq_data->irq_proc_reg.cpu_int_status &
> + irq_data->irq_en_reg.cpu_int_status_en;
> + if (!cpu_int_status) {
> + WARN_ON(1);
> + return -EIO;
> + }
Ditto about WARN_ON(), ath10k_warn() is much better.
> +/* process pending interrupts synchronously */
> +static int ath10k_sdio_mbox_proc_pending_irqs(struct ath10k_sdio *ar_sdi=
o,
> + bool *done)
> +{
> + struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> + struct ath10k *ar =3D ar_sdio->ar;
> + struct ath10k_sdio_irq_proc_registers *rg;
> + int status =3D 0;
> + u8 host_int_status =3D 0;
> + u32 lookahead =3D 0;
> + u8 htc_mbox =3D 1 << ATH10K_HTC_MAILBOX;
> +
> + /* NOTE: HIF implementation guarantees that the context of this
> + * call allows us to perform SYNCHRONOUS I/O, that is we can block,
> + * sleep or call any API that can block or switch thread/task
> + * contexts. This is a fully schedulable context.
> + */
> +
> + /* Process pending intr only when int_status_en is clear, it may
> + * result in unnecessary bus transaction otherwise. Target may be
> + * unresponsive at the time.
> + */
> + if (irq_data->irq_en_reg.int_status_en) {
> + /* Read the first sizeof(struct ath10k_irq_proc_registers)
> + * bytes of the HTC register table. This
> + * will yield us the value of different int status
> + * registers and the lookahead registers.
> + */
> + status =3D ath10k_sdio_read_write_sync(
> + ar,
> + MBOX_HOST_INT_STATUS_ADDRESS,
> + (u8 *)&irq_data->irq_proc_reg,
> + sizeof(irq_data->irq_proc_reg),
> + HIF_RD_SYNC_BYTE_INC);
> + if (status)
> + goto out;
> +
> + /* Update only those registers that are enabled */
> + host_int_status =3D irq_data->irq_proc_reg.host_int_status &
> + irq_data->irq_en_reg.int_status_en;
> +
> + /* Look at mbox status */
> + if (host_int_status & htc_mbox) {
> + /* Mask out pending mbox value, we use look ahead as
> + * the real flag for mbox processing.
> + */
> + host_int_status &=3D ~htc_mbox;
> + if (irq_data->irq_proc_reg.rx_lookahead_valid &
> + htc_mbox) {
> + rg =3D &irq_data->irq_proc_reg;
> + lookahead =3D le32_to_cpu(
> + rg->rx_lookahead[ATH10K_HTC_MAILBOX]);
> + if (!lookahead)
> + ath10k_err(ar, "lookahead is zero!\n");
> + }
> + }
> + }
> +
> + if (!host_int_status && !lookahead) {
> + *done =3D true;
> + goto out;
> + }
> +
> + if (lookahead) {
> + ath10k_dbg(ar, ATH10K_DBG_SDIO,
> + "pending mailbox msg, lookahead: 0x%08X\n",
> + lookahead);
> +
> + status =3D ath10k_sdio_mbox_rxmsg_pending_handler(ar_sdio,
> + lookahead,
> + done);
> + if (status)
> + goto out;
> + }
> +
> + /* now, handle the rest of the interrupts */
> + ath10k_dbg(ar, ATH10K_DBG_SDIO,
> + "valid interrupt source(s) for other interrupts: 0x%x\n",
> + host_int_status);
> +
> + if (MS(host_int_status, MBOX_HOST_INT_STATUS_CPU)) {
> + /* CPU Interrupt */
> + status =3D ath10k_sdio_mbox_proc_cpu_intr(ar_sdio);
> + if (status)
> + goto out;
> + }
> +
> + if (MS(host_int_status, MBOX_HOST_INT_STATUS_ERROR)) {
> + /* Error Interrupt */
> + status =3D ath10k_sdio_mbox_proc_err_intr(ar_sdio);
> + if (status)
> + goto out;
> + }
> +
> + if (MS(host_int_status, MBOX_HOST_INT_STATUS_COUNTER))
> + /* Counter Interrupt */
> + status =3D ath10k_sdio_mbox_proc_counter_intr(ar_sdio);
> +
> +out:
> + /* An optimization to bypass reading the IRQ status registers
> + * unecessarily which can re-wake the target, if upper layers
> + * determine that we are in a low-throughput mode, we can rely on
> + * taking another interrupt rather than re-checking the status
> + * registers which can re-wake the target.
> + *
> + * NOTE : for host interfaces that makes use of detecting pending
> + * mbox messages at hif can not use this optimization due to
> + * possible side effects, SPI requires the host to drain all
> + * messages from the mailbox before exiting the ISR routine.
> + */
> +
> + ath10k_dbg(ar, ATH10K_DBG_SDIO,
> + "%s: (done:%d, status=3D%d)\n", __func__, *done, status);
We try to follow this kind of format for debug messages:
"sdio pending irqs done %d status %d"
So start with the debug level name followed by the debug separated with spa=
ces.
And IIRC no need for "\n", the macro adds that automatically.
> +
> + return status;
> +}
> +
> +/* Macro to check if DMA buffer is WORD-aligned and DMA-able.
> + * Most host controllers assume the buffer is DMA'able and will
> + * bug-check otherwise (i.e. buffers on the stack). virt_addr_valid
> + * check fails on stack memory.
> + */
> +static inline bool buf_needs_bounce(u8 *buf)
> +{
> + return ((unsigned long)buf & 0x3) || !virt_addr_valid(buf);
> +}
IS_ALIGNED()? And this is super ugly, do we really need this? I would
much prefer that we would directly use struct sk_buff, more of that
later.
> +static inline enum ath10k_htc_ep_id pipe_id_to_eid(u8 pipe_id)
> +{
> + return (enum ath10k_htc_ep_id)pipe_id;
> +}
Oh, we already have a this kind of mapping function? Can't this be used
earlier?
> +static void ath10k_sdio_set_mbox_info(struct ath10k_sdio *ar_sdio)
> +{
> + struct ath10k_mbox_info *mbox_info =3D &ar_sdio->mbox_info;
> + u16 device =3D ar_sdio->func->device;
> +
> + mbox_info->htc_addr =3D ATH10K_HIF_MBOX_BASE_ADDR;
> + mbox_info->block_size =3D ATH10K_HIF_MBOX_BLOCK_SIZE;
> + mbox_info->block_mask =3D ATH10K_HIF_MBOX_BLOCK_SIZE - 1;
> + mbox_info->gmbox_addr =3D ATH10K_HIF_GMBOX_BASE_ADDR;
> + mbox_info->gmbox_sz =3D ATH10K_HIF_GMBOX_WIDTH;
> +
> + mbox_info->ext_info[0].htc_ext_addr =3D ATH10K_HIF_MBOX0_EXT_BASE_ADDR;
> +
> + if ((device & ATH10K_MANUFACTURER_ID_REV_MASK) < 4)
> + mbox_info->ext_info[0].htc_ext_sz =3D ATH10K_HIF_MBOX0_EXT_WIDTH;
> + else
> + /* from rome 2.0(0x504), the width has been extended
> + * to 56K
> + */
> + mbox_info->ext_info[0].htc_ext_sz =3D
> + ATH10K_HIF_MBOX0_EXT_WIDTH_ROME_2_0;
> +
> + mbox_info->ext_info[1].htc_ext_addr =3D
> + mbox_info->ext_info[0].htc_ext_addr +
> + mbox_info->ext_info[0].htc_ext_sz +
> + ATH10K_HIF_MBOX_DUMMY_SPACE_SIZE;
> + mbox_info->ext_info[1].htc_ext_sz =3D ATH10K_HIF_MBOX1_EXT_WIDTH;
> +}
> +
> +static inline void ath10k_sdio_set_cmd52_arg(u32 *arg, u8 write, u8 raw,
> + unsigned int address,
> + unsigned char val)
> +{
> + const u8 func =3D 0;
> +
> + *arg =3D ((write & 1) << 31) |
> + ((func & 0x7) << 28) |
> + ((raw & 1) << 27) |
> + (1 << 26) |
> + ((address & 0x1FFFF) << 9) |
> + (1 << 8) |
> + (val & 0xFF);
> +}
Quite ugly. FIELD_PREP() & co?
> +
> +static int ath10k_sdio_func0_cmd52_wr_byte(struct mmc_card *card,
> + unsigned int address,
> + unsigned char byte)
> +{
> + struct mmc_command io_cmd;
> +
> + memset(&io_cmd, 0, sizeof(io_cmd));
> + ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 1, 0, address, byte);
> + io_cmd.opcode =3D SD_IO_RW_DIRECT;
> + io_cmd.flags =3D MMC_RSP_R5 | MMC_CMD_AC;
> +
> + return mmc_wait_for_cmd(card->host, &io_cmd, 0);
> +}
> +
> +static int ath10k_sdio_func0_cmd52_rd_byte(struct mmc_card *card,
> + unsigned int address,
> + unsigned char *byte)
> +{
> + int ret;
> + struct mmc_command io_cmd;
> +
> + memset(&io_cmd, 0, sizeof(io_cmd));
> + ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 0, 0, address, 0);
> + io_cmd.opcode =3D SD_IO_RW_DIRECT;
> + io_cmd.flags =3D MMC_RSP_R5 | MMC_CMD_AC;
> +
> + ret =3D mmc_wait_for_cmd(card->host, &io_cmd, 0);
> + if (!ret)
> + *byte =3D io_cmd.resp[0];
> +
> + return ret;
> +}
> +
> +static int ath10k_sdio_io(struct ath10k_sdio *ar_sdio, u32 request, u32 =
addr,
> + u8 *buf, u32 len)
> +{
> + int ret =3D 0;
Avoid these kind of unnecessary initialisations.
> + struct sdio_func *func =3D ar_sdio->func;
> + struct ath10k *ar =3D ar_sdio->ar;
> +
> + sdio_claim_host(func);
> +
> + if (request & HIF_WRITE) {
> + if (request & HIF_FIXED_ADDRESS)
> + ret =3D sdio_writesb(func, addr, buf, len);
> + else
> + ret =3D sdio_memcpy_toio(func, addr, buf, len);
> + } else {
> + if (request & HIF_FIXED_ADDRESS)
> + ret =3D sdio_readsb(func, buf, addr, len);
> + else
> + ret =3D sdio_memcpy_fromio(func, buf, addr, len);
> + }
> +
> + sdio_release_host(func);
> +
> + ath10k_dbg(ar, ATH10K_DBG_SDIO, "%s addr 0x%x%s buf 0x%p len %d\n",
> + request & HIF_WRITE ? "wr" : "rd", addr,
> + request & HIF_FIXED_ADDRESS ? " (fixed)" : "", buf, len);
> + ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL,
> + request & HIF_WRITE ? "sdio wr " : "sdio rd ",
> + buf, len);
> +
> + return ret;
> +}
> +
> +static struct ath10k_sdio_bus_request
> +*ath10k_sdio_alloc_busreq(struct ath10k_sdio *ar_sdio)
> +{
> + struct ath10k_sdio_bus_request *bus_req;
> +
> + spin_lock_bh(&ar_sdio->lock);
> +
> + if (list_empty(&ar_sdio->bus_req_freeq)) {
> + spin_unlock_bh(&ar_sdio->lock);
> + return NULL;
> + }
> +
> + bus_req =3D list_first_entry(&ar_sdio->bus_req_freeq,
> + struct ath10k_sdio_bus_request, list);
> + list_del(&bus_req->list);
> +
> + spin_unlock_bh(&ar_sdio->lock);
> +
> + return bus_req;
> +}
> +
> +static void ath10k_sdio_free_bus_req(struct ath10k_sdio *ar_sdio,
> + struct ath10k_sdio_bus_request *bus_req)
> +{
> + spin_lock_bh(&ar_sdio->lock);
> + list_add_tail(&bus_req->list, &ar_sdio->bus_req_freeq);
> + spin_unlock_bh(&ar_sdio->lock);
> +}
> +
> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *=
buf,
> + u32 len, u32 request)
> +{
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + u8 *tbuf =3D NULL;
Extra space after u8?
> + int ret;
> + bool bounced =3D false;
> +
> + if (request & HIF_BLOCK_BASIS)
> + len =3D round_down(len, ar_sdio->mbox_info.block_size);
> +
> + if (buf_needs_bounce(buf)) {
> + if (!ar_sdio->dma_buffer)
> + return -ENOMEM;
> + /* FIXME: I am not sure if it is always correct to assume
> + * that the SDIO irq is a "fake" irq and sleep is possible.
> + * (this function will get called from
> + * ath10k_sdio_irq_handler
> + */
> + mutex_lock(&ar_sdio->dma_buffer_mutex);
> + tbuf =3D ar_sdio->dma_buffer;
> +
> + if (request & HIF_WRITE)
> + memcpy(tbuf, buf, len);
> +
> + bounced =3D true;
> + } else {
> + tbuf =3D buf;
> + }
So I really hate that ar_sdio->dma_buffer, do we really need it? What if
we just get rid of it and allocate struct sk_buff and pass that around?
No need to do extra copying then, I hope :)
> +
> + ret =3D ath10k_sdio_io(ar_sdio, request, addr, tbuf, len);
> + if ((request & HIF_READ) && bounced)
> + memcpy(buf, tbuf, len);
> +
> + if (bounced)
> + mutex_unlock(&ar_sdio->dma_buffer_mutex);
> +
> + return ret;
> +}
> +
> +static void __ath10k_sdio_write_async(struct ath10k_sdio *ar_sdio,
> + struct ath10k_sdio_bus_request *req)
> +{
> + int status;
> + struct ath10k_htc_ep *ep;
> + struct sk_buff *skb;
> +
> + skb =3D req->skb;
> + status =3D ath10k_sdio_read_write_sync(ar_sdio->ar, req->address,
> + skb->data, req->len,
> + req->request);
> + ep =3D &ar_sdio->ar->htc.endpoint[req->eid];
> + ath10k_htc_notify_tx_completion(ep, skb);
> + ath10k_sdio_free_bus_req(ar_sdio, req);
> +}
> +
> +static void ath10k_sdio_write_async_work(struct work_struct *work)
> +{
> + struct ath10k_sdio *ar_sdio;
> + struct ath10k_sdio_bus_request *req, *tmp_req;
> +
> + ar_sdio =3D container_of(work, struct ath10k_sdio, wr_async_work);
> +
> + spin_lock_bh(&ar_sdio->wr_async_lock);
> + list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
> + list_del(&req->list);
> + spin_unlock_bh(&ar_sdio->wr_async_lock);
> + __ath10k_sdio_write_async(ar_sdio, req);
> + spin_lock_bh(&ar_sdio->wr_async_lock);
> + }
> + spin_unlock_bh(&ar_sdio->wr_async_lock);
> +}
> +
> +static void ath10k_sdio_irq_handler(struct sdio_func *func)
> +{
> + int status =3D 0;
> + unsigned long timeout;
> + struct ath10k_sdio *ar_sdio;
> + bool done =3D false;
> +
> + ar_sdio =3D sdio_get_drvdata(func);
> + atomic_set(&ar_sdio->irq_handling, 1);
> +
> + /* Release the host during interrupts so we can pick it back up when
> + * we process commands.
> + */
> + sdio_release_host(ar_sdio->func);
> +
> + timeout =3D jiffies + ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ;
> + while (time_before(jiffies, timeout) && !done) {
> + status =3D ath10k_sdio_mbox_proc_pending_irqs(ar_sdio, &done);
> + if (status)
> + break;
> + }
> +
> + sdio_claim_host(ar_sdio->func);
> +
> + atomic_set(&ar_sdio->irq_handling, 0);
> + wake_up(&ar_sdio->irq_wq);
> +
> + WARN_ON(status && status !=3D -ECANCELED);
> +}
Questionable use of an atomic variable again, looks like badly implement
poor man's locking to me. And instead of wake_up() we should workqueues
or threaded irqs (if sdio supports that).
> +
> +static int ath10k_sdio_hif_disable_intrs(struct ath10k_sdio *ar_sdio)
> +{
> + int ret;
> + struct ath10k_sdio_irq_enable_reg regs;
> + struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +
> + memset(®s, 0, sizeof(regs));
> +
> + ret =3D ath10k_sdio_read_write_sync(ar_sdio->ar,
> + MBOX_INT_STATUS_ENABLE_ADDRESS,
> + ®s.int_status_en, sizeof(regs),
> + HIF_WR_SYNC_BYTE_INC);
> + if (ret) {
> + ath10k_err(ar_sdio->ar, "Unable to disable sdio interrupts\n");
> + return ret;
> + }
> +
> + spin_lock_bh(&irq_data->lock);
> + irq_data->irq_en_reg =3D regs;
> + spin_unlock_bh(&irq_data->lock);
> +
> + return 0;
> +}
> +
> +static int ath10k_sdio_hif_power_up(struct ath10k *ar)
> +{
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + struct sdio_func *func =3D ar_sdio->func;
> + int ret =3D 0;
> +
> + if (!ar_sdio->is_disabled)
> + return 0;
> +
> + ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power on\n");
> +
> + sdio_claim_host(func);
> +
> + ret =3D sdio_enable_func(func);
> + if (ret) {
> + ath10k_err(ar, "Unable to enable sdio func: %d)\n", ret);
> + sdio_release_host(func);
> + return ret;
> + }
> +
> + sdio_release_host(func);
> +
> + /* Wait for hardware to initialise. It should take a lot less than
> + * 20 ms but let's be conservative here.
> + */
> + msleep(20);
> +
> + ar_sdio->is_disabled =3D false;
> +
> + ret =3D ath10k_sdio_hif_disable_intrs(ar_sdio);
> +
> + return ret;
> +}
> +
> +static void ath10k_sdio_hif_power_down(struct ath10k *ar)
> +{
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + int ret;
> +
> + if (ar_sdio->is_disabled)
> + return;
> +
> + ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power off\n");
> +
> + /* Disable the card */
> + sdio_claim_host(ar_sdio->func);
> + ret =3D sdio_disable_func(ar_sdio->func);
> + sdio_release_host(ar_sdio->func);
> +
> + if (ret)
> + ath10k_dbg(ar, ATH10K_DBG_BOOT,
> + "Unable to disable sdio: %d\n", ret);
Shouldn't this be ath10k_warn()?
> +
> + ar_sdio->is_disabled =3D true;
> +}
> +
> +int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
> + struct ath10k_hif_sg_item *items, int n_items)
> +{
> + int i;
> + u32 address;
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + struct ath10k_sdio_bus_request *bus_req;
> +
> + bus_req =3D ath10k_sdio_alloc_busreq(ar_sdio);
> +
> + if (WARN_ON_ONCE(!bus_req))
> + return -ENOMEM;
Is ath10k_warn() more approriate?
> + for (i =3D 0; i < n_items; i++) {
> + bus_req->skb =3D items[i].transfer_context;
> + bus_req->request =3D HIF_WRITE;
> + bus_req->eid =3D pipe_id_to_eid(pipe_id);
> + /* Write TX data to the end of the mbox address space */
> + address =3D ar_sdio->mbox_addr[bus_req->eid] +
> + ar_sdio->mbox_size[bus_req->eid] - bus_req->skb->len;
> + bus_req->address =3D address;
> + bus_req->len =3D CALC_TXRX_PADDED_LEN(ar_sdio, bus_req->skb->len);
> +
> + spin_lock_bh(&ar_sdio->wr_async_lock);
> + list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
> + spin_unlock_bh(&ar_sdio->wr_async_lock);
> + }
> +
> + queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
> +
> + return 0;
> +}
> +
> +static int ath10k_sdio_hif_enable_intrs(struct ath10k_sdio *ar_sdio)
> +{
> + struct ath10k_sdio_irq_enable_reg regs;
> + int status;
> + struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +
> + memset(®s, 0, sizeof(regs));
> +
> + /* Enable all but CPU interrupts */
> + regs.int_status_en =3D SM(0x01, MBOX_INT_STATUS_ENABLE_ERROR) |
> + SM(0x01, MBOX_INT_STATUS_ENABLE_CPU) |
> + SM(0x01, MBOX_INT_STATUS_ENABLE_COUNTER);
> +
> + /* NOTE: There are some cases where HIF can do detection of
> + * pending mbox messages which is disabled now.
> + */
> + regs.int_status_en |=3D SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
> +
> + /* Set up the CPU Interrupt status Register */
> + regs.cpu_int_status_en =3D 0;
> +
> + /* Set up the Error Interrupt status Register */
> + regs.err_int_status_en =3D
> + SM(0x01, MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW) |
> + SM(0x01, MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW);
> +
> + /* Enable Counter interrupt status register to get fatal errors for
> + * debugging.
> + */
> + regs.cntr_int_status_en =3D SM(ATH10K_SDIO_TARGET_DEBUG_INTR_MASK,
> + MBOX_COUNTER_INT_STATUS_ENABLE_BIT);
> +
> + status =3D ath10k_sdio_read_write_sync(ar_sdio->ar,
> + MBOX_INT_STATUS_ENABLE_ADDRESS,
> + ®s.int_status_en, sizeof(regs),
> + HIF_WR_SYNC_BYTE_INC);
> + if (status) {
> + ath10k_err(ar_sdio->ar,
> + "failed to update interrupt ctl reg err: %d\n",
> + status);
> + return status;
> + }
> +
> + spin_lock_bh(&irq_data->lock);
> + irq_data->irq_en_reg =3D regs;
> + spin_unlock_bh(&irq_data->lock);
> +
> + return 0;
> +}
> +
> +static int ath10k_sdio_hif_start(struct ath10k *ar)
> +{
> + int ret;
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + u32 addr, val;
> +
> + addr =3D host_interest_item_address(HI_ITEM(hi_acs_flags));
> +
> + ret =3D ath10k_sdio_hif_diag_read32(ar, addr, &val);
> + if (ret) {
> + ath10k_err(ar, "Unable to read diag mem: %d\n", ret);
> + goto out;
> + }
> +
> + if (val & HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_FW_ACK) {
> + ath10k_dbg(ar, ATH10K_DBG_SDIO,
> + "Mailbox SWAP Service is enabled\n");
> + ar_sdio->swap_mbox =3D true;
> + }
> +
> + /* eid 0 always uses the lower part of the extended mailbox address
> + * space (ext_info[0].htc_ext_addr).
> + */
> + ar_sdio->mbox_addr[0] =3D ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
> + ar_sdio->mbox_size[0] =3D ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
> +
> + sdio_claim_host(ar_sdio->func);
> +
> + /* Register the isr */
> + ret =3D sdio_claim_irq(ar_sdio->func, ath10k_sdio_irq_handler);
> + if (ret) {
> + ath10k_err(ar, "Failed to claim sdio irq: %d\n", ret);
> + sdio_release_host(ar_sdio->func);
> + goto out;
> + }
> +
> + sdio_release_host(ar_sdio->func);
> +
> + ret =3D ath10k_sdio_hif_enable_intrs(ar_sdio);
> + if (ret)
> + ath10k_err(ar, "Failed to enable sdio interrupts: %d\n", ret);
> +
> +out:
> + return ret;
> +}
> +
> +static bool ath10k_sdio_is_on_irq(struct ath10k *ar)
> +{
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +
> + return !atomic_read(&ar_sdio->irq_handling);
> +}
Yikes.
> +
> +static void ath10k_sdio_irq_disable(struct ath10k *ar)
> +{
> + struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> + int ret;
> +
> + sdio_claim_host(ar_sdio->func);
> +
> + atomic_set(&ar_sdio->stopping, 1);
> +
> + if (atomic_read(&ar_sdio->irq_handling)) {
> + sdio_release_host(ar_sdio->func);
> +
> + ret =3D wait_event_interruptible(ar_sdio->irq_wq,
> + ath10k_sdio_is_on_irq(ar));
> + if (ret)
> + return;
> +
> + sdio_claim_host(ar_sdio->func);
> + }
There has to be a better way to implement this, now it feels that it's
just reimplementing the wheel. We should have proper way to wait for
interrupt handlers and workqueues to end etc.
> + ret =3D sdio_release_irq(ar_sdio->func);
> + if (ret)
> + ath10k_err(ar, "Failed to release sdio irq: %d\n", ret);
> +
> + sdio_release_host(ar_sdio->func);
> +}
> +
> +static int ath10k_sdio_config(struct ath10k_sdio *ar_sdio)
> +{
> + struct ath10k *ar =3D ar_sdio->ar;
> + struct sdio_func *func =3D ar_sdio->func;
> + unsigned char byte, asyncintdelay =3D 2;
> + int ret;
> +
> + ath10k_dbg(ar, ATH10K_DBG_BOOT, "SDIO configuration\n");
> +
> + sdio_claim_host(func);
> +
> + byte =3D 0;
> + ret =3D ath10k_sdio_func0_cmd52_rd_byte(func->card,
> + SDIO_CCCR_DRIVE_STRENGTH,
> + &byte);
> +
> + byte =3D (byte & (~(SDIO_DRIVE_DTSx_MASK << SDIO_DRIVE_DTSx_SHIFT))) |
> + SDIO_DTSx_SET_TYPE_D;
FIELD_PREP(). There are lots of places where that an be used.
> +static void ath10k_sdio_write32(struct ath10k *ar, u32 offset, u32 value=
)
> +{
> +}
> +
> +static u32 ath10k_sdio_read32(struct ath10k *ar, u32 offset)
> +{
> + return 0;
> +}
Somekind of FIXME/TODO comments for write32() and read32()? What
functionality are we going to miss when we don't implement these?
> +static void ath10k_sdio_hif_send_complete_check(struct ath10k *ar,
> + u8 pipe, int force)
> +{
> +}
> +
> +static u16 ath10k_sdio_hif_get_free_queue_number(struct ath10k *ar, u8 p=
ipe)
> +{
> + return 0;
> +}
Similar comments here also.
> +
> +static const struct ath10k_hif_ops ath10k_sdio_hif_ops =3D {
> + .tx_sg =3D ath10k_sdio_hif_tx_sg,
> + .diag_read =3D ath10k_sdio_hif_diag_read,
> + .diag_write =3D ath10k_sdio_diag_write_mem,
> + .exchange_bmi_msg =3D ath10k_sdio_hif_exchange_bmi_msg,
> + .start =3D ath10k_sdio_hif_start,
> + .stop =3D ath10k_sdio_hif_stop,
> + .map_service_to_pipe =3D ath10k_sdio_hif_map_service_to_pipe,
> + .get_default_pipe =3D ath10k_sdio_hif_get_default_pipe,
> + .send_complete_check =3D ath10k_sdio_hif_send_complete_check,
> + .get_free_queue_number =3D ath10k_sdio_hif_get_free_queue_number,
> + .power_up =3D ath10k_sdio_hif_power_up,
> + .power_down =3D ath10k_sdio_hif_power_down,
> + .read32 =3D ath10k_sdio_read32,
> + .write32 =3D ath10k_sdio_write32,
> +#ifdef CONFIG_PM
> + .suspend =3D ath10k_sdio_hif_suspend,
> + .resume =3D ath10k_sdio_hif_resume,
> +#endif
> + .fetch_cal_eeprom =3D ath10k_sdio_hif_fetch_cal_eeprom,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> +/* Empty handlers so that mmc subsystem doesn't remove us entirely durin=
g
> + * suspend. We instead follow cfg80211 suspend/resume handlers.
> + */
> +static int ath10k_sdio_pm_suspend(struct device *device)
> +{
> + return 0;
> +}
> +
> +static int ath10k_sdio_pm_resume(struct device *device)
> +{
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(ath10k_sdio_pm_ops, ath10k_sdio_pm_suspend,
> + ath10k_sdio_pm_resume);
> +
> +#define ATH10K_SDIO_PM_OPS (&ath10k_sdio_pm_ops)
> +
> +#else
> +
> +#define ATH10K_SDIO_PM_OPS NULL
> +
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static int ath10k_sdio_probe(struct sdio_func *func,
> + const struct sdio_device_id *id)
> +{
> + int ret;
> + struct ath10k_sdio *ar_sdio;
> + struct ath10k *ar;
> + int i;
> + enum ath10k_hw_rev hw_rev;
> +
> + hw_rev =3D ATH10K_HW_QCA6174;
This needs a comment, at least to remind if ever add something else than
QCA6174 based SDIO design.
> +
> + ar =3D ath10k_core_create(sizeof(*ar_sdio), &func->dev, ATH10K_BUS_SDIO=
,
> + hw_rev, &ath10k_sdio_hif_ops);
> + if (!ar) {
> + dev_err(&func->dev, "failed to allocate core\n");
> + return -ENOMEM;
> + }
> +
> + ath10k_dbg(ar, ATH10K_DBG_BOOT,
> + "sdio new func %d vendor 0x%x device 0x%x block 0x%x/0x%x\n",
> + func->num, func->vendor, func->device,
> + func->max_blksize, func->cur_blksize);
> +
> + ar_sdio =3D ath10k_sdio_priv(ar);
> +
> + ar_sdio->dma_buffer =3D kzalloc(ATH10K_HIF_DMA_BUFFER_SIZE, GFP_KERNEL)=
;
> + if (!ar_sdio->dma_buffer) {
> + ret =3D -ENOMEM;
> + goto err_core_destroy;
> + }
> +
> + ar_sdio->func =3D func;
> + sdio_set_drvdata(func, ar_sdio);
> +
> + ar_sdio->is_disabled =3D true;
> + ar_sdio->ar =3D ar;
> +
> + spin_lock_init(&ar_sdio->lock);
> + spin_lock_init(&ar_sdio->wr_async_lock);
> + spin_lock_init(&ar_sdio->irq_data.lock);
> + mutex_init(&ar_sdio->dma_buffer_mutex);
> +
> + INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
> + INIT_LIST_HEAD(&ar_sdio->wr_asyncq);
> +
> + INIT_WORK(&ar_sdio->wr_async_work, ath10k_sdio_write_async_work);
> + ar_sdio->workqueue =3D create_singlethread_workqueue("ath10k_sdio_wq");
> + if (!ar_sdio->workqueue)
> + goto err;
> +
> + init_waitqueue_head(&ar_sdio->irq_wq);
> +
> + for (i =3D 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
> + ath10k_sdio_free_bus_req(ar_sdio, &ar_sdio->bus_req[i]);
> +
> + ar->dev_id =3D id->device;
> + ar->id.vendor =3D id->vendor;
> + ar->id.device =3D id->device;
> +
> + ath10k_sdio_set_mbox_info(ar_sdio);
> +
> + ret =3D ath10k_sdio_config(ar_sdio);
> + if (ret) {
> + ath10k_err(ar, "Failed to config sdio: %d\n", ret);
> + goto err;
> + }
> +
> + ret =3D ath10k_core_register(ar, 0/*chip_id is not applicaple for SDIO*=
/);
> + if (ret) {
> + ath10k_err(ar, "failed to register driver core: %d\n", ret);
> + goto err;
> + }
I would assume that chip_id is applicaple also with SDIO, there has to
be a register where to get it. Also this kind of comment style is
preferred:
/* TODO: don't know yet how to get chip_id with SDIO */
chip_id =3D 0;
ret =3D ath10k_core_register(ar, chip_id);
> +
> + return ret;
> +
> +err:
> + kfree(ar_sdio->dma_buffer);
> +err_core_destroy:
> + ath10k_core_destroy(ar);
> +
> + return ret;
> +}
> +
> +static void ath10k_sdio_remove(struct sdio_func *func)
> +{
> + struct ath10k_sdio *ar_sdio;
> + struct ath10k *ar;
> +
> + ar_sdio =3D sdio_get_drvdata(func);
> + ar =3D ar_sdio->ar;
> +
> + ath10k_dbg(ar, ATH10K_DBG_BOOT,
> + "sdio removed func %d vendor 0x%x device 0x%x\n",
> + func->num, func->vendor, func->device);
> +
> + (void)ath10k_sdio_hif_disable_intrs(ar_sdio);
> + cancel_work_sync(&ar_sdio->wr_async_work);
> + ath10k_core_unregister(ar);
> + ath10k_core_destroy(ar);
> +
> + kfree(ar_sdio->dma_buffer);
> +}
> +
> +static const struct sdio_device_id ath10k_sdio_devices[] =3D {
> + {SDIO_DEVICE(ATH10K_MANUFACTURER_CODE,
> + (ATH10K_MANUFACTURER_ID_AR6005_BASE | 0xA))},
> + {},
> +};
I suspect there's a more sensible way to create the device table than
this, just no time to check it now. Anyone know?
The naming "ath10k manufacturer" is also wrong, it should be QCA or
Qualcomm.
--=20
Kalle Valo=
^ permalink raw reply
* [PATCH] nl80211: rework {sched_,}scan event related functions
From: Arend van Spriel @ 2016-12-16 11:21 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Arend van Spriel
A couple of functions used with scan events were named with
term "send" although they were only preparing the the event
message so renamed those.
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
net/wireless/nl80211.c | 34 ++++++++--------------------------
net/wireless/nl80211.h | 6 ++----
net/wireless/scan.c | 9 +++++----
3 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3df85a7..727ca50 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12810,7 +12810,7 @@ static int nl80211_add_scan_req(struct sk_buff *msg,
return -ENOBUFS;
}
-static int nl80211_send_scan_msg(struct sk_buff *msg,
+static int nl80211_prep_scan_msg(struct sk_buff *msg,
struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev,
u32 portid, u32 seq, int flags,
@@ -12841,7 +12841,7 @@ static int nl80211_send_scan_msg(struct sk_buff *msg,
}
static int
-nl80211_send_sched_scan_msg(struct sk_buff *msg,
+nl80211_prep_sched_scan_msg(struct sk_buff *msg,
struct cfg80211_registered_device *rdev,
struct net_device *netdev,
u32 portid, u32 seq, int flags, u32 cmd)
@@ -12873,7 +12873,7 @@ void nl80211_send_scan_start(struct cfg80211_registered_device *rdev,
if (!msg)
return;
- if (nl80211_send_scan_msg(msg, rdev, wdev, 0, 0, 0,
+ if (nl80211_prep_scan_msg(msg, rdev, wdev, 0, 0, 0,
NL80211_CMD_TRIGGER_SCAN) < 0) {
nlmsg_free(msg);
return;
@@ -12892,7 +12892,7 @@ struct sk_buff *nl80211_build_scan_msg(struct cfg80211_registered_device *rdev,
if (!msg)
return NULL;
- if (nl80211_send_scan_msg(msg, rdev, wdev, 0, 0, 0,
+ if (nl80211_prep_scan_msg(msg, rdev, wdev, 0, 0, 0,
aborted ? NL80211_CMD_SCAN_ABORTED :
NL80211_CMD_NEW_SCAN_RESULTS) < 0) {
nlmsg_free(msg);
@@ -12902,31 +12902,13 @@ struct sk_buff *nl80211_build_scan_msg(struct cfg80211_registered_device *rdev,
return msg;
}
-void nl80211_send_scan_result(struct cfg80211_registered_device *rdev,
- struct sk_buff *msg)
-{
- if (!msg)
- return;
-
- genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
- NL80211_MCGRP_SCAN, GFP_KERNEL);
-}
-
-void nl80211_send_sched_scan_results(struct cfg80211_registered_device *rdev,
- struct net_device *netdev)
+/* send message created by nl80211_build_scan_msg() */
+void nl80211_send_scan_msg(struct cfg80211_registered_device *rdev,
+ struct sk_buff *msg)
{
- struct sk_buff *msg;
-
- msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
- if (nl80211_send_sched_scan_msg(msg, rdev, netdev, 0, 0, 0,
- NL80211_CMD_SCHED_SCAN_RESULTS) < 0) {
- nlmsg_free(msg);
- return;
- }
-
genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
NL80211_MCGRP_SCAN, GFP_KERNEL);
}
@@ -12940,7 +12922,7 @@ void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
if (!msg)
return;
- if (nl80211_send_sched_scan_msg(msg, rdev, netdev, 0, 0, 0, cmd) < 0) {
+ if (nl80211_prep_sched_scan_msg(msg, rdev, netdev, 0, 0, 0, cmd) < 0) {
nlmsg_free(msg);
return;
}
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 7e3821d..75f8252 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -14,12 +14,10 @@ void nl80211_send_scan_start(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev);
struct sk_buff *nl80211_build_scan_msg(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev, bool aborted);
-void nl80211_send_scan_result(struct cfg80211_registered_device *rdev,
- struct sk_buff *msg);
+void nl80211_send_scan_msg(struct cfg80211_registered_device *rdev,
+ struct sk_buff *msg);
void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
struct net_device *netdev, u32 cmd);
-void nl80211_send_sched_scan_results(struct cfg80211_registered_device *rdev,
- struct net_device *netdev);
void nl80211_common_reg_change_event(enum nl80211_commands cmd_id,
struct regulatory_request *request);
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index b5bd58d..76cdd1d 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -176,7 +176,7 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev,
ASSERT_RTNL();
if (rdev->scan_msg) {
- nl80211_send_scan_result(rdev, rdev->scan_msg);
+ nl80211_send_scan_msg(rdev, rdev->scan_msg);
rdev->scan_msg = NULL;
return;
}
@@ -222,7 +222,7 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev,
if (!send_message)
rdev->scan_msg = msg;
else
- nl80211_send_scan_result(rdev, msg);
+ nl80211_send_scan_msg(rdev, msg);
}
void __cfg80211_scan_done(struct work_struct *wk)
@@ -270,7 +270,8 @@ void __cfg80211_sched_scan_results(struct work_struct *wk)
spin_unlock_bh(&rdev->bss_lock);
request->scan_start = jiffies;
}
- nl80211_send_sched_scan_results(rdev, request->dev);
+ nl80211_send_sched_scan(rdev, request->dev,
+ NL80211_CMD_SCHED_SCAN_RESULTS);
}
rtnl_unlock();
@@ -1078,7 +1079,7 @@ struct cfg80211_bss *
else
rcu_assign_pointer(tmp.pub.beacon_ies, ies);
rcu_assign_pointer(tmp.pub.ies, ies);
-
+
memcpy(tmp.pub.bssid, mgmt->bssid, ETH_ALEN);
tmp.pub.channel = channel;
tmp.pub.scan_width = data->scan_width;
--
1.9.1
^ permalink raw reply related
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