* Re: ath10k mesh + ap + encryption?
From: Simon Wunderlich @ 2016-09-13 12:30 UTC (permalink / raw)
To: Valo, Kalle
Cc: ath10k@lists.infradead.org, openwrt-devel@lists.openwrt.org,
linux-wireless@vger.kernel.org, Martin Blumenstingl
In-Reply-To: <87d1k8jb34.fsf@kamboji.qca.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]
On Tuesday, September 13, 2016 11:25:21 AM CEST Valo, Kalle wrote:
> Simon Wunderlich <sw@simonwunderlich.de> writes:
> > On Tuesday, September 13, 2016 10:59:31 AM CEST Valo, Kalle wrote:
> >> Simon Wunderlich <sw@simonwunderlich.de> writes:
> >> > we have done some experiments last week on ath10k, trying to run mesh
> >> > (802.11s) and access point at the same time, both encrypted.
> >> >
> >> > We have tested a recent LEDE (reboot-1519-g42f559e) but with
> >> > firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which
> >> > gave
> >> > us a working encrypted 802.11s network. However, starting an AP at the
> >> > same time didn't work (AP doesn't beacon). This wasn't a problem when
> >> > 802.11s was running unencrypted.
> >> >
> >> > We also tested version 10.2.4.97 (from codeaurora), which is now
> >> > default
> >> > in
> >> > LEDE. However, this version apparently doesn't support 11s mesh at all
> >> > (WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211
> >> > advertises support).
> >> >
> >> > So here are my questions:
> >> > * Did anyone succesfully run AP and mesh, both encrypted at the same
> >> > time?
> >> > * Do you have any pointers how we could fix this? Could it be fixable
> >> > in
> >> > the>
> >> >
> >> > driver (i.e. not in firmware)?
> >> >
> >> > * Does anyone have an idea if 11s will be supported in future
> >> > versions? I
> >> >
> >> > didn't find any changelogs, but having 11s mode no longer in the
> >> > service
> >> > map does not make me optimistic.
> >>
> >> Why is LEDE using 10.2.4.97? It seems to be a quite old release and I
> >> have no knowledge if anyone even tests that firmware branch with ath10k.
> >> I recommend to only use firmware releases from ath10k-firmware.git as we
> >> use those internally with ath10k. In any case, don't make any
> >> assumptions about future from that firmware branch as it's so old.
> >
> > This was introduced in December 25th, 2015 after some firmware-related
> > problems. I'm CC'ing Martin Blumenstingl who suggested this change.
> >
> > Since then, ath10k is pulling firmware from here (unless ct firmware is
> > used):
> >
> > https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/plain
> > /
> > 10.2.4/firmware-5.bin_10.2.4.97-1
> >
> > However, I don't understand the numbering? 10.2.4.97 > 10.2.4.70, but you
> > say 10.2.4.70.42-2 is more recent? I would have assumed otherwise from
> > the numbers. However, 10.2.4.70 has much more sub-revisions.
>
> As I said before, I just deliver the firmware files to the community and
> the firmware team creates the actual releases. But my understanding is
> that these are from different branches which are built independently
> (and might have different features, like in this case the mesh support)
> so I would not make any conclusions if any firmware is "better" just
> from the numbers alone.
you are right ... those numbers are not a good pointer. I found this repo, and
from the checkin dates it looks like 10.2.4.97 is indeed way older (from
September 2015) than 10.2.4.70.42 (April 2016):
https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/log/10.2.4
I would agree that Changelogs would be helpful.
Thanks for the clarification. We will then stick to the 70's branch then.
Does anyone have pointers for the other questions? :) I would believe hat many
people would be interested in running AP + Mesh encrypted at the same time (at
least in the open source community ...).
Thanks,
Simon
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [3/3] ath10k: Improve logging message.
From: Kalle Valo @ 2016-09-13 12:29 UTC (permalink / raw)
To: Ben Greear; +Cc: ath10k, Ben Greear, linux-wireless
In-Reply-To: <1471569995-10028-3-git-send-email-greearb@candelatech.com>
Ben Greear <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Helps to know the sta pointer.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
3040420158c1 ath10k: improve logging message
--
Sent by pwcli
https://patchwork.kernel.org/patch/9289043/
^ permalink raw reply
* [ANNOUNCE] netdev 1.2 tokyo weekly update (13th September, 2016)
From: Hajime Tazaki @ 2016-09-13 12:28 UTC (permalink / raw)
To: netdev, netfilter-devel, linux-wireless, netdev-conference, lwn
Hello folks,
I hope you're fine and ready to trip to Tokyo.
Here is an weekly update of Netdev 1.2 Tokyo.
== Keynote talk ==
We confirmed that David Miller will give a keynote titled
"Fast Programmable Networks & Encapsulated Protocols".
== Newly accepted sessions ==
We also accepted one additional talk in the last week.
- Network interface configuration on a Linux NOS
by Roopa Prabhu
Full list of accepted sessions is available here.
http://netdevconf.org/1.2/accepted-sessions.html
== Our sponsors ==
We got new sponsors, Cisco as a Gold sponsor, LWN.net as a
Media sponsor. Many thanks for your support.
- Platinum
Verizon, Facebook, Cumulus Networks
- Gold
Mojatatu Networks, VMWare, Google, NTT, LinkedIn, Cisco (new)
- Silver
NetApp, IIJ, Netronome, SolarFlare, Mellanox, Sophos
- Bronze
Zen Load Balancer
- Media Sponsor
lwn.net (new)
Twitter: https://twitter.com/netdev01
Web: http://netdevconf.org/1.2/
== Others ==
Be prepared for your travel. Hotel and travel information
are available on the web pages.
http://netdevconf.org/1.2/travel.html
http://netdevconf.org/1.2/hotel.html
The early bird tickets is still available until 15th.
Please be registered - your early registration is always
helpful to organize a great conference.
http://netdevconf.org/1.2/registration.html
Looking forward to seeing you in Tokyo very soon.
-- Hajime
^ permalink raw reply
* Re: ath10k: enable peer stats by default
From: Kalle Valo @ 2016-09-13 12:26 UTC (permalink / raw)
To: Pedersen, Thomas; +Cc: ath10k, Thomas Pedersen, linux-wireless
In-Reply-To: <1473188728-14995-1-git-send-email-twp@qca.qualcomm.com>
"Pedersen, Thomas" <twp@qca.qualcomm.com> wrote:
> IFTYPE_MESH_POINT need to rely on these for accurate path
> selection metrics. Other modes will probably also find
> them useful. Enabling peer stats has the side effect of
> reducing max number of STAs from 128 to 118. There should
> be negligible performance impact.
>
> If users really need 128 STAs and don't mind losing out on
> peer stats, they can still disable them:
>
> echo 0 > debugfs/ieee80211/phyn/ath10k/peer_stats
>
> Signed-off-by: Thomas Pedersen <twp@qca.qualcomm.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
8c1d7fa53166 ath10k: enable peer stats by default
--
Sent by pwcli
https://patchwork.kernel.org/patch/9317865/
^ permalink raw reply
* Re: ath10k: remove unused variable ar_pci
From: Kalle Valo @ 2016-09-13 12:25 UTC (permalink / raw)
To: Chaehyun Lim; +Cc: netdev, linux-wireless, Chaehyun Lim, ath10k
In-Reply-To: <20160905133802.18755-1-chaehyun.lim@gmail.com>
Chaehyun Lim <chaehyun.lim@gmail.com> wrote:
> Trival fix to remove unused variable ar_pci in ath10k_pci_tx_pipe_cleanup
> when building with W=1:
> drivers/net/wireless/ath/ath10k/pci.c:1696:21: warning: variable
> 'ar_pci' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: Chaehyun Lim <chaehyun.lim@gmail.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
214d55394481 ath10k: remove unused variable ar_pci
--
Sent by pwcli
https://patchwork.kernel.org/patch/9313963/
^ permalink raw reply
* Re: [V2] ath10k: fix memory leak on caldata on error exit path
From: Kalle Valo @ 2016-09-13 12:24 UTC (permalink / raw)
To: Colin Ian King; +Cc: ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <20160903163819.13115-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> caldata is not being free'd on the error exit path, causing
> a memory leak and data definitely should not be freed. Free
> caldata instead of data.
>
> Thanks to Kalle Valo for spotting that data should not be
> free'd.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
5f4761dda2ba ath10k: fix memory leak on caldata on error exit path
--
Sent by pwcli
https://patchwork.kernel.org/patch/9312163/
^ permalink raw reply
* FW: Only one of two Intel Wireless 7260 PCI Cards are assigned a logical name (ubuntu 16.04 :: 4.7.0-040700-generic )
From: Merchut, Addison @ 2016-09-13 11:59 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 8712 bytes --]
From: Merchut, Addison
Sent: Monday, September 12, 2016 3:21 PM
To: 'linuxwifi@intel.com' <linuxwifi@intel.com>; 'linux-wireless@vger.kernel.org' <linux-wireless@vger.kernel.org>
Cc: Nudo, Al <Al.Nudo@pdt.com>; Unetich, Rich <Rich.Unetich@pdt.com>; DeCarlo, Bob <Bob.DeCarlo@pdt.com>
Subject: Only one of two Intel Wireless 7260 PCI Cards are assigned a logical name (ubuntu 16.04 :: 4.7.0-040700-generic )
I believe I have an issue with the Intel wireless 7260 pci cards on my linux desktop
I have two wireless cards on a test PC I'm working on. And while I can see both cards, only the top card (in the physical location) is actually getting assigned the logical (interface) name.
This seems to be interment (though I've only had this computer running for two days) because there has been at least one occasion that I have seen both cards assigned an interface name at the same time (via ifconfig and NM). However, when I've had to restart the computer, the names are not always assigned for the 2nd (bottom) card -this is the case most of the time.
Attached is the dmesg for the case where I was able to get both cards to work (dm.txt) and the case where I am not able to get both cards to work (dmbad.txt)
I've switched out the cards (with other 7260 cards), move the cards to other locations, exchanged the locations of the cards, BUT only the top most device is ever assigned the an interface name.
I'm running: 4.7.0-040700-generic GNU/Linux Ubuntu 16.04.1 LTS
The two cards are the same Intel Corporation Wireless 7260:
# cat /sys/bus/pci/devices/0000\:01\:00.0/uevent
DRIVER=iwlwifi
PCI_CLASS=28000
PCI_ID=8086:08B1
PCI_SUBSYS_ID=8086:4070
PCI_SLOT_NAME=0000:01:00.0
MODALIAS=pci:v00008086d000008B1sv00008086sd00004070bc02sc80i00
# cat /sys/bus/pci/devices/0000\:02\:00.0/uevent
DRIVER=iwlwifi
PCI_CLASS=28000
PCI_ID=8086:08B1
PCI_SUBSYS_ID=8086:4070
PCI_SLOT_NAME=0000:02:00.0
MODALIAS=pci:v00008086d000008B1sv00008086sd00004070bc02sc80i00
Only one of the devices is getting assigned a logical name:
#sudo lshw -class network
....
*-network
description: Wireless interface
product: Wireless 7260
vendor: Intel Corporation
physical id: 0
bus info: pci@0000:01:00.0
logical name: wlp1s0
version: 73
serial: 7c:5c:f8:d2:6c:52
width: 64 bits
clock: 33MHz
capabilities: pm msi pciexpress bus_master cap_list ethernet physical wireless
configuration: broadcast=yes driver=iwlwifi driverversion=4.7.0-040700-generic firmware=16.242414.0 latency=0 link=no multicast=yes wireless=IEEE 802.11
resources: irq:32 memory:f7d00000-f7d01fff
*-network
description: Network controller
product: Wireless 7260
vendor: Intel Corporation
physical id: 0
bus info: pci@0000:02:00.0
version: 73
width: 64 bits
clock: 33MHz
capabilities: pm msi pciexpress bus_master cap_list
configuration: driver=iwlwifi latency=0
resources: irq:33 memory:f7c00000-f7c01fff
dmesg does not show that the 2nd device on bus 0000:02:00.0 has been detected by the wifi driver
{Attached as dmbad.txt}
and ifconfig only shows one device (obviously since lshw showed that only one was getting assigned a logical name)
#ifconfig
...
wlp1s0 Link encap:Ethernet HWaddr 7c:5c:f8:d2:6c:52
UP BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)
#ls /lib/firmware | grep 7260
iwlwifi-7260-10.ucode
iwlwifi-7260-12.ucode
iwlwifi-7260-13.ucode
iwlwifi-7260-14.ucode
iwlwifi-7260-16.ucode
iwlwifi-7260-7.ucode
iwlwifi-7260-8.ucode
iwlwifi-7260-9.ucode
#modinfo iwlwifi
filename: /lib/modules/4.7.0-040700-generic/kernel/drivers/net/wireless/intel/iwlwifi/iwlwifi.ko
license: GPL
author: Copyright(c) 2003- 2015 Intel Corporation <linuxwifi@intel.com>
description: Intel(R) Wireless WiFi driver for Linux
firmware: iwlwifi-100-5.ucode
firmware: iwlwifi-1000-5.ucode
firmware: iwlwifi-135-6.ucode
firmware: iwlwifi-105-6.ucode
firmware: iwlwifi-2030-6.ucode
firmware: iwlwifi-2000-6.ucode
firmware: iwlwifi-5150-2.ucode
firmware: iwlwifi-5000-5.ucode
firmware: iwlwifi-6000g2b-IWL6000G2B_UCODE_API_MAX.ucode
firmware: iwlwifi-6000g2a-6.ucode
firmware: iwlwifi-6050-5.ucode
firmware: iwlwifi-6000-6.ucode
firmware: iwlwifi-7265D-21.ucode
firmware: iwlwifi-7265-17.ucode
firmware: iwlwifi-3168-21.ucode
firmware: iwlwifi-3160-17.ucode
firmware: iwlwifi-7260-17.ucode
firmware: iwlwifi-8265-21.ucode
firmware: iwlwifi-8000C--21.ucode
firmware: iwlwifi-9260-th-a0-lc-a0--21.ucode
firmware: iwlwifi-9260-th-a0-jf-a0--21.ucode
firmware: iwlwifi-9000-pu-a0-lc-a0--21.ucode
alias: pci:v00008086d00009DF0sv*sd00002A10bc*sc*i*
alias: pci:v00008086d00009DF0sv*sd00000710bc*sc*i*
....
depends: cfg80211
intree: Y
vermagic: 4.7.0-040700-generic SMP mod_unload modversions
parm: swcrypto:using crypto in software (default 0 [hardware]) (int)
parm: 11n_disable:disable 11n functionality, bitmap: 1: full, 2: disable agg TX, 4: disable agg RX, 8 enable agg TX (uint)
parm: amsdu_size:amsdu size 0:4K 1:8K 2:12K (default 0) (int)
parm: fw_restart:restart firmware in case of error (default true) (bool)
parm: antenna_coupling:specify antenna coupling in dB (default: 0 dB) (int)
parm: nvm_file:NVM file name (charp)
parm: d0i3_disable:disable d0i3 functionality (default: Y) (bool)
parm: lar_disable:disable LAR functionality (default: N) (bool)
parm: uapsd_disable:disable U-APSD functionality bitmap 1: BSS 2: P2P Client (default: 3) (uint)
parm: bt_coex_active:enable wifi/bt co-exist (default: enable) (bool)
parm: led_mode:0=system default, 1=On(RF On)/Off(RF Off), 2=blinking, 3=Off (default: 0) (int)
parm: power_save:enable WiFi power management (default: disable) (bool)
parm: power_level:default power save level (range from 1 - 5, default: 1) (int)
parm: fw_monitor:firmware monitor - to debug FW (default: false - needs lots of memory) (bool)
parm: d0i3_timeout:Timeout to D0i3 entry when idle (ms) (uint)
parm: disable_11ac:Disable VHT capabilities (default: false) (bool)
Please let me know if there's anything I can do or if this is a known issue.
Thank you.
Addison
312.620.2094
Skype: addisonpdt
PDT.com
Email secured by Check Point
[-- Attachment #2: dm.txt.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 27257 bytes --]
[-- Attachment #3: dmbad.txt.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 27011 bytes --]
^ permalink raw reply
* Re: ath10k mesh + ap + encryption?
From: Sebastian Gottschall @ 2016-09-13 11:38 UTC (permalink / raw)
To: Valo, Kalle, Simon Wunderlich
Cc: ath10k@lists.infradead.org, openwrt-devel@lists.openwrt.org,
linux-wireless@vger.kernel.org, Martin Blumenstingl
In-Reply-To: <87d1k8jb34.fsf@kamboji.qca.qualcomm.com>
Am 13.09.2016 um 13:25 schrieb Valo, Kalle:
> Simon Wunderlich <sw@simonwunderlich.de> writes:
>
>> On Tuesday, September 13, 2016 10:59:31 AM CEST Valo, Kalle wrote:
>>> Simon Wunderlich <sw@simonwunderlich.de> writes:
>>>> we have done some experiments last week on ath10k, trying to run mesh
>>>> (802.11s) and access point at the same time, both encrypted.
>>>>
>>>> We have tested a recent LEDE (reboot-1519-g42f559e) but with
>>>> firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which gave
>>>> us a working encrypted 802.11s network. However, starting an AP at the
>>>> same time didn't work (AP doesn't beacon). This wasn't a problem when
>>>> 802.11s was running unencrypted.
>>>>
>>>> We also tested version 10.2.4.97 (from codeaurora), which is now default
>>>> in
>>>> LEDE. However, this version apparently doesn't support 11s mesh at all
>>>> (WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211
>>>> advertises support).
>>>>
>>>> So here are my questions:
>>>> * Did anyone succesfully run AP and mesh, both encrypted at the same
>>>> time?
>>>> * Do you have any pointers how we could fix this? Could it be fixable in
>>>> the>
>>>> driver (i.e. not in firmware)?
>>>>
>>>> * Does anyone have an idea if 11s will be supported in future versions? I
>>>>
>>>> didn't find any changelogs, but having 11s mode no longer in the service
>>>> map does not make me optimistic.
>>> Why is LEDE using 10.2.4.97? It seems to be a quite old release and I
>>> have no knowledge if anyone even tests that firmware branch with ath10k.
>>> I recommend to only use firmware releases from ath10k-firmware.git as we
>>> use those internally with ath10k. In any case, don't make any
>>> assumptions about future from that firmware branch as it's so old.
>> This was introduced in December 25th, 2015 after some firmware-related
>> problems. I'm CC'ing Martin Blumenstingl who suggested this change.
>>
>> Since then, ath10k is pulling firmware from here (unless ct firmware is used):
>>
>> https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/plain/
>> 10.2.4/firmware-5.bin_10.2.4.97-1
>>
>> However, I don't understand the numbering? 10.2.4.97 > 10.2.4.70, but you say
>> 10.2.4.70.42-2 is more recent? I would have assumed otherwise from the
>> numbers. However, 10.2.4.70 has much more sub-revisions.
> As I said before, I just deliver the firmware files to the community and
> the firmware team creates the actual releases. But my understanding is
> that these are from different branches which are built independently
> (and might have different features, like in this case the mesh support)
> so I would not make any conclusions if any firmware is "better" just
> from the numbers alone.
would be good to have changelogs so see what has been changed to test
what they have changed in the fw
>
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Berliner Ring 101, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565
^ permalink raw reply
* Re: ath10k mesh + ap + encryption?
From: Valo, Kalle @ 2016-09-13 11:25 UTC (permalink / raw)
To: Simon Wunderlich
Cc: ath10k@lists.infradead.org, openwrt-devel@lists.openwrt.org,
linux-wireless@vger.kernel.org, Martin Blumenstingl
In-Reply-To: <15353347.MUgkgiNHdu@prime>
Simon Wunderlich <sw@simonwunderlich.de> writes:
> On Tuesday, September 13, 2016 10:59:31 AM CEST Valo, Kalle wrote:
>> Simon Wunderlich <sw@simonwunderlich.de> writes:
>> > we have done some experiments last week on ath10k, trying to run mesh
>> > (802.11s) and access point at the same time, both encrypted.
>> >=20
>> > We have tested a recent LEDE (reboot-1519-g42f559e) but with
>> > firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which g=
ave
>> > us a working encrypted 802.11s network. However, starting an AP at the
>> > same time didn't work (AP doesn't beacon). This wasn't a problem when
>> > 802.11s was running unencrypted.
>> >=20
>> > We also tested version 10.2.4.97 (from codeaurora), which is now defau=
lt
>> > in
>> > LEDE. However, this version apparently doesn't support 11s mesh at all
>> > (WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211
>> > advertises support).
>> >=20
>> > So here are my questions:
>> > * Did anyone succesfully run AP and mesh, both encrypted at the same
>> > time?
>> > * Do you have any pointers how we could fix this? Could it be fixable=
in
>> > the>=20
>> > driver (i.e. not in firmware)?
>> >=20
>> > * Does anyone have an idea if 11s will be supported in future version=
s? I
>> >=20
>> > didn't find any changelogs, but having 11s mode no longer in the servi=
ce
>> > map does not make me optimistic.
>>=20
>> Why is LEDE using 10.2.4.97? It seems to be a quite old release and I
>> have no knowledge if anyone even tests that firmware branch with ath10k.
>> I recommend to only use firmware releases from ath10k-firmware.git as we
>> use those internally with ath10k. In any case, don't make any
>> assumptions about future from that firmware branch as it's so old.
>
> This was introduced in December 25th, 2015 after some firmware-related=20
> problems. I'm CC'ing Martin Blumenstingl who suggested this change.
>
> Since then, ath10k is pulling firmware from here (unless ct firmware is u=
sed):
>
> https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/plai=
n/
> 10.2.4/firmware-5.bin_10.2.4.97-1
>
> However, I don't understand the numbering? 10.2.4.97 > 10.2.4.70, but you=
say=20
> 10.2.4.70.42-2 is more recent? I would have assumed otherwise from the=20
> numbers. However, 10.2.4.70 has much more sub-revisions.
As I said before, I just deliver the firmware files to the community and
the firmware team creates the actual releases. But my understanding is
that these are from different branches which are built independently
(and might have different features, like in this case the mesh support)
so I would not make any conclusions if any firmware is "better" just
from the numbers alone.
--=20
Kalle Valo=
^ permalink raw reply
* Re: ath10k mesh + ap + encryption?
From: Simon Wunderlich @ 2016-09-13 11:13 UTC (permalink / raw)
To: Valo, Kalle
Cc: ath10k@lists.infradead.org, openwrt-devel@lists.openwrt.org,
linux-wireless@vger.kernel.org, Martin Blumenstingl
In-Reply-To: <87h99kjca5.fsf@kamboji.qca.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 2398 bytes --]
On Tuesday, September 13, 2016 10:59:31 AM CEST Valo, Kalle wrote:
> Simon Wunderlich <sw@simonwunderlich.de> writes:
> > we have done some experiments last week on ath10k, trying to run mesh
> > (802.11s) and access point at the same time, both encrypted.
> >
> > We have tested a recent LEDE (reboot-1519-g42f559e) but with
> > firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which gave
> > us a working encrypted 802.11s network. However, starting an AP at the
> > same time didn't work (AP doesn't beacon). This wasn't a problem when
> > 802.11s was running unencrypted.
> >
> > We also tested version 10.2.4.97 (from codeaurora), which is now default
> > in
> > LEDE. However, this version apparently doesn't support 11s mesh at all
> > (WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211
> > advertises support).
> >
> > So here are my questions:
> > * Did anyone succesfully run AP and mesh, both encrypted at the same
> > time?
> > * Do you have any pointers how we could fix this? Could it be fixable in
> > the>
> > driver (i.e. not in firmware)?
> >
> > * Does anyone have an idea if 11s will be supported in future versions? I
> >
> > didn't find any changelogs, but having 11s mode no longer in the service
> > map does not make me optimistic.
>
> Why is LEDE using 10.2.4.97? It seems to be a quite old release and I
> have no knowledge if anyone even tests that firmware branch with ath10k.
> I recommend to only use firmware releases from ath10k-firmware.git as we
> use those internally with ath10k. In any case, don't make any
> assumptions about future from that firmware branch as it's so old.
This was introduced in December 25th, 2015 after some firmware-related
problems. I'm CC'ing Martin Blumenstingl who suggested this change.
Since then, ath10k is pulling firmware from here (unless ct firmware is used):
https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/plain/
10.2.4/firmware-5.bin_10.2.4.97-1
However, I don't understand the numbering? 10.2.4.97 > 10.2.4.70, but you say
10.2.4.70.42-2 is more recent? I would have assumed otherwise from the
numbers. However, 10.2.4.70 has much more sub-revisions. Is there any document
describing the revisions? I don't understand it at least from this wiki page
[1].
Thanks!
Simon
[1] https://wireless.wiki.kernel.org/en/users/Drivers/ath10k/firmware
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: ath10k mesh + ap + encryption?
From: Valo, Kalle @ 2016-09-13 10:59 UTC (permalink / raw)
To: Simon Wunderlich
Cc: ath10k@lists.infradead.org, openwrt-devel@lists.openwrt.org,
linux-wireless@vger.kernel.org
In-Reply-To: <2661831.slnB0LOeVt@prime>
Simon Wunderlich <sw@simonwunderlich.de> writes:
> we have done some experiments last week on ath10k, trying to run mesh=20
> (802.11s) and access point at the same time, both encrypted.=20
>
> We have tested a recent LEDE (reboot-1519-g42f559e) but with=20
> firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which gave=
us a=20
> working encrypted 802.11s network. However, starting an AP at the same ti=
me=20
> didn't work (AP doesn't beacon). This wasn't a problem when 802.11s was=20
> running unencrypted.
>
> We also tested version 10.2.4.97 (from codeaurora), which is now default =
in=20
> LEDE. However, this version apparently doesn't support 11s mesh at all=20
> (WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211=20
> advertises support).
>
> So here are my questions:
>
> * Did anyone succesfully run AP and mesh, both encrypted at the same tim=
e?
> * Do you have any pointers how we could fix this? Could it be fixable in=
the=20
> driver (i.e. not in firmware)?
> * Does anyone have an idea if 11s will be supported in future versions? =
I=20
> didn't find any changelogs, but having 11s mode no longer in the service =
map=20
> does not make me optimistic.
Why is LEDE using 10.2.4.97? It seems to be a quite old release and I
have no knowledge if anyone even tests that firmware branch with ath10k.
I recommend to only use firmware releases from ath10k-firmware.git as we
use those internally with ath10k. In any case, don't make any
assumptions about future from that firmware branch as it's so old.
--=20
Kalle Valo=
^ permalink raw reply
* [PATCH v4] cfg80211: Add support to configure a beacon data rate
From: Purushottam Kushwaha @ 2016-09-13 9:42 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, jouni, usdutt, amarnath, pkushwah
This allows an option to configure a single beacon tx rate (u8) for an AP.
Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
---
include/net/cfg80211.h | 25 +--
net/wireless/nl80211.c | 504 +++++++++++++++++++++++++++----------------------
2 files changed, 296 insertions(+), 233 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d5e7f69..c58afc8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -676,6 +676,18 @@ struct cfg80211_acl_data {
struct mac_address mac_addrs[];
};
+/*
+ * cfg80211_bitrate_mask - masks for bitrate control
+ */
+struct cfg80211_bitrate_mask {
+ struct {
+ u32 legacy;
+ u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
+ u16 vht_mcs[NL80211_VHT_NSS_MAX];
+ enum nl80211_txrate_gi gi;
+ } control[NUM_NL80211_BANDS];
+};
+
/**
* struct cfg80211_ap_settings - AP configuration
*
@@ -700,6 +712,7 @@ struct cfg80211_acl_data {
* MAC address based access control
* @pbss: If set, start as a PCP instead of AP. Relevant for DMG
* networks.
+ * @beacon_rate: masks for setting user configured beacon tx rate.
*/
struct cfg80211_ap_settings {
struct cfg80211_chan_def chandef;
@@ -719,6 +732,7 @@ struct cfg80211_ap_settings {
bool p2p_opp_ps;
const struct cfg80211_acl_data *acl;
bool pbss;
+ struct cfg80211_bitrate_mask beacon_rate;
};
/**
@@ -2001,17 +2015,6 @@ enum wiphy_params_flags {
WIPHY_PARAM_DYN_ACK = 1 << 5,
};
-/*
- * cfg80211_bitrate_mask - masks for bitrate control
- */
-struct cfg80211_bitrate_mask {
- struct {
- u32 legacy;
- u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
- u16 vht_mcs[NL80211_VHT_NSS_MAX];
- enum nl80211_txrate_gi gi;
- } control[NUM_NL80211_BANDS];
-};
/**
* struct cfg80211_pmksa - PMK Security Association
*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7ebad35..f49b7df 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3324,6 +3324,274 @@ static int nl80211_set_mac_acl(struct sk_buff *skb, struct genl_info *info)
return err;
}
+static u32 rateset_to_mask(struct ieee80211_supported_band *sband,
+ u8 *rates, u8 rates_len)
+{
+ u8 i;
+ u32 mask = 0;
+
+ for (i = 0; i < rates_len; i++) {
+ int rate = (rates[i] & 0x7f) * 5;
+ int ridx;
+
+ for (ridx = 0; ridx < sband->n_bitrates; ridx++) {
+ struct ieee80211_rate *srate =
+ &sband->bitrates[ridx];
+ if (rate == srate->bitrate) {
+ mask |= 1 << ridx;
+ break;
+ }
+ }
+ if (ridx == sband->n_bitrates)
+ return 0; /* rate not found */
+ }
+
+ return mask;
+}
+
+static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
+ u8 *rates, u8 rates_len,
+ u8 mcs[IEEE80211_HT_MCS_MASK_LEN])
+{
+ u8 i;
+
+ memset(mcs, 0, IEEE80211_HT_MCS_MASK_LEN);
+
+ for (i = 0; i < rates_len; i++) {
+ int ridx, rbit;
+
+ ridx = rates[i] / 8;
+ rbit = BIT(rates[i] % 8);
+
+ /* check validity */
+ if ((ridx < 0) || (ridx >= IEEE80211_HT_MCS_MASK_LEN))
+ return false;
+
+ /* check availability */
+ if (sband->ht_cap.mcs.rx_mask[ridx] & rbit)
+ mcs[ridx] |= rbit;
+ else
+ return false;
+ }
+
+ return true;
+}
+
+static u16 vht_mcs_map_to_mcs_mask(u8 vht_mcs_map)
+{
+ u16 mcs_mask = 0;
+
+ switch (vht_mcs_map) {
+ case IEEE80211_VHT_MCS_NOT_SUPPORTED:
+ break;
+ case IEEE80211_VHT_MCS_SUPPORT_0_7:
+ mcs_mask = 0x00FF;
+ break;
+ case IEEE80211_VHT_MCS_SUPPORT_0_8:
+ mcs_mask = 0x01FF;
+ break;
+ case IEEE80211_VHT_MCS_SUPPORT_0_9:
+ mcs_mask = 0x03FF;
+ break;
+ default:
+ break;
+ }
+
+ return mcs_mask;
+}
+
+static void vht_build_mcs_mask(u16 vht_mcs_map,
+ u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+{
+ u8 nss;
+
+ for (nss = 0; nss < NL80211_VHT_NSS_MAX; nss++) {
+ vht_mcs_mask[nss] = vht_mcs_map_to_mcs_mask(vht_mcs_map & 0x03);
+ vht_mcs_map >>= 2;
+ }
+}
+
+static bool vht_set_mcs_mask(struct ieee80211_supported_band *sband,
+ struct nl80211_txrate_vht *txrate,
+ u16 mcs[NL80211_VHT_NSS_MAX])
+{
+ u16 tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
+ u16 tx_mcs_mask[NL80211_VHT_NSS_MAX] = {};
+ u8 i;
+
+ if (!sband->vht_cap.vht_supported)
+ return false;
+
+ memset(mcs, 0, sizeof(u16) * NL80211_VHT_NSS_MAX);
+
+ /* Build vht_mcs_mask from VHT capabilities */
+ vht_build_mcs_mask(tx_mcs_map, tx_mcs_mask);
+
+ for (i = 0; i < NL80211_VHT_NSS_MAX; i++) {
+ if ((tx_mcs_mask[i] & txrate->mcs[i]) == txrate->mcs[i])
+ mcs[i] = txrate->mcs[i];
+ else
+ return false;
+ }
+
+ return true;
+}
+
+static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
+ [NL80211_TXRATE_LEGACY] = { .type = NLA_BINARY,
+ .len = NL80211_MAX_SUPP_RATES },
+ [NL80211_TXRATE_HT] = { .type = NLA_BINARY,
+ .len = NL80211_MAX_SUPP_HT_RATES },
+ [NL80211_TXRATE_VHT] = { .len = sizeof(struct nl80211_txrate_vht)},
+ [NL80211_TXRATE_GI] = { .type = NLA_U8 },
+};
+
+static int nl80211_parse_tx_bitrate_mask(struct genl_info *info,
+ struct cfg80211_bitrate_mask *mask)
+{
+ struct nlattr *tb[NL80211_TXRATE_MAX + 1];
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ int rem, i;
+ struct nlattr *tx_rates;
+ struct ieee80211_supported_band *sband;
+ u16 vht_tx_mcs_map;
+
+ memset(mask, 0, sizeof(*mask));
+ /* Default to all rates enabled */
+ for (i = 0; i < NUM_NL80211_BANDS; i++) {
+ sband = rdev->wiphy.bands[i];
+
+ if (!sband)
+ continue;
+
+ mask->control[i].legacy = (1 << sband->n_bitrates) - 1;
+ memcpy(mask->control[i].ht_mcs,
+ sband->ht_cap.mcs.rx_mask,
+ sizeof(mask->control[i].ht_mcs));
+
+ if (!sband->vht_cap.vht_supported)
+ continue;
+
+ vht_tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
+ vht_build_mcs_mask(vht_tx_mcs_map, mask->control[i].vht_mcs);
+ }
+
+ /* if no rates are given set it back to the defaults */
+ if (!info->attrs[NL80211_ATTR_TX_RATES])
+ goto out;
+
+ /* The nested attribute uses enum nl80211_band as the index. This maps
+ * directly to the enum nl80211_band values used in cfg80211.
+ */
+ BUILD_BUG_ON(NL80211_MAX_SUPP_HT_RATES > IEEE80211_HT_MCS_MASK_LEN * 8);
+ nla_for_each_nested(tx_rates, info->attrs[NL80211_ATTR_TX_RATES], rem) {
+ enum nl80211_band band = nla_type(tx_rates);
+ int err;
+
+ if (band < 0 || band >= NUM_NL80211_BANDS)
+ return -EINVAL;
+ sband = rdev->wiphy.bands[band];
+ if (sband == NULL)
+ return -EINVAL;
+ err = nla_parse(tb, NL80211_TXRATE_MAX, nla_data(tx_rates),
+ nla_len(tx_rates), nl80211_txattr_policy);
+ if (err)
+ return err;
+ if (tb[NL80211_TXRATE_LEGACY]) {
+ mask->control[band].legacy = rateset_to_mask(
+ sband,
+ nla_data(tb[NL80211_TXRATE_LEGACY]),
+ nla_len(tb[NL80211_TXRATE_LEGACY]));
+ if ((mask->control[band].legacy == 0) &&
+ nla_len(tb[NL80211_TXRATE_LEGACY]))
+ return -EINVAL;
+ }
+ if (tb[NL80211_TXRATE_HT]) {
+ if (!ht_rateset_to_mask(
+ sband,
+ nla_data(tb[NL80211_TXRATE_HT]),
+ nla_len(tb[NL80211_TXRATE_HT]),
+ mask->control[band].ht_mcs))
+ return -EINVAL;
+ }
+ if (tb[NL80211_TXRATE_VHT]) {
+ if (!vht_set_mcs_mask(
+ sband,
+ nla_data(tb[NL80211_TXRATE_VHT]),
+ mask->control[band].vht_mcs))
+ return -EINVAL;
+ }
+ if (tb[NL80211_TXRATE_GI]) {
+ mask->control[band].gi =
+ nla_get_u8(tb[NL80211_TXRATE_GI]);
+ if (mask->control[band].gi > NL80211_TXRATE_FORCE_LGI)
+ return -EINVAL;
+ }
+
+ if (mask->control[band].legacy == 0) {
+ /* don't allow empty legacy rates if HT or VHT
+ * are not even supported.
+ */
+ if (!(rdev->wiphy.bands[band]->ht_cap.ht_supported ||
+ rdev->wiphy.bands[band]->vht_cap.vht_supported))
+ return -EINVAL;
+
+ for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
+ if (mask->control[band].ht_mcs[i])
+ goto out;
+
+ for (i = 0; i < NL80211_VHT_NSS_MAX; i++)
+ if (mask->control[band].vht_mcs[i])
+ goto out;
+
+ /* legacy and mcs rates may not be both empty */
+ return -EINVAL;
+ }
+ }
+
+out:
+ return 0;
+}
+
+static int validate_beacon_tx_rate(struct cfg80211_ap_settings *params)
+{
+ u32 rate, count_ht, count_vht, i;
+ enum nl80211_band band;
+
+ band = params->chandef.chan->band;
+ rate = params->beacon_rate.control[band].legacy;
+
+ /* Allow only one rate */
+ if (rate && (rate & (rate - 1)))
+ return -EINVAL;
+
+ count_ht = 0;
+ for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
+ if (params->beacon_rate.control[band].ht_mcs[i]) {
+ count_ht++;
+ if (count_ht > 1)
+ return -EINVAL;
+ }
+ }
+
+ count_vht = 0;
+ for (i = 0; i < NL80211_VHT_NSS_MAX; i++) {
+ if (params->beacon_rate.control[band].vht_mcs[i]) {
+ count_vht++;
+ if (count_vht > 1)
+ return -EINVAL;
+ }
+ }
+
+ if ((rate && count_ht) ||
+ (rate && count_vht) ||
+ (count_ht && count_vht) ||
+ (!rate && !count_ht && !count_vht))
+ return -EINVAL;
+
+ return 0;
+}
+
static int nl80211_parse_beacon(struct nlattr *attrs[],
struct cfg80211_beacon_data *bcn)
{
@@ -3553,6 +3821,15 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
wdev->iftype))
return -EINVAL;
+ if (info->attrs[NL80211_ATTR_TX_RATES]) {
+ err = nl80211_parse_tx_bitrate_mask(info, ¶ms.beacon_rate);
+ if (err)
+ return err;
+
+ if (validate_beacon_tx_rate(¶ms))
+ return -EINVAL;
+ }
+
if (info->attrs[NL80211_ATTR_SMPS_MODE]) {
params.smps_mode =
nla_get_u8(info->attrs[NL80211_ATTR_SMPS_MODE]);
@@ -8623,238 +8900,21 @@ static int nl80211_cancel_remain_on_channel(struct sk_buff *skb,
return rdev_cancel_remain_on_channel(rdev, wdev, cookie);
}
-static u32 rateset_to_mask(struct ieee80211_supported_band *sband,
- u8 *rates, u8 rates_len)
-{
- u8 i;
- u32 mask = 0;
-
- for (i = 0; i < rates_len; i++) {
- int rate = (rates[i] & 0x7f) * 5;
- int ridx;
-
- for (ridx = 0; ridx < sband->n_bitrates; ridx++) {
- struct ieee80211_rate *srate =
- &sband->bitrates[ridx];
- if (rate == srate->bitrate) {
- mask |= 1 << ridx;
- break;
- }
- }
- if (ridx == sband->n_bitrates)
- return 0; /* rate not found */
- }
-
- return mask;
-}
-
-static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
- u8 *rates, u8 rates_len,
- u8 mcs[IEEE80211_HT_MCS_MASK_LEN])
-{
- u8 i;
-
- memset(mcs, 0, IEEE80211_HT_MCS_MASK_LEN);
-
- for (i = 0; i < rates_len; i++) {
- int ridx, rbit;
-
- ridx = rates[i] / 8;
- rbit = BIT(rates[i] % 8);
-
- /* check validity */
- if ((ridx < 0) || (ridx >= IEEE80211_HT_MCS_MASK_LEN))
- return false;
-
- /* check availability */
- if (sband->ht_cap.mcs.rx_mask[ridx] & rbit)
- mcs[ridx] |= rbit;
- else
- return false;
- }
-
- return true;
-}
-
-static u16 vht_mcs_map_to_mcs_mask(u8 vht_mcs_map)
-{
- u16 mcs_mask = 0;
-
- switch (vht_mcs_map) {
- case IEEE80211_VHT_MCS_NOT_SUPPORTED:
- break;
- case IEEE80211_VHT_MCS_SUPPORT_0_7:
- mcs_mask = 0x00FF;
- break;
- case IEEE80211_VHT_MCS_SUPPORT_0_8:
- mcs_mask = 0x01FF;
- break;
- case IEEE80211_VHT_MCS_SUPPORT_0_9:
- mcs_mask = 0x03FF;
- break;
- default:
- break;
- }
-
- return mcs_mask;
-}
-
-static void vht_build_mcs_mask(u16 vht_mcs_map,
- u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
-{
- u8 nss;
-
- for (nss = 0; nss < NL80211_VHT_NSS_MAX; nss++) {
- vht_mcs_mask[nss] = vht_mcs_map_to_mcs_mask(vht_mcs_map & 0x03);
- vht_mcs_map >>= 2;
- }
-}
-
-static bool vht_set_mcs_mask(struct ieee80211_supported_band *sband,
- struct nl80211_txrate_vht *txrate,
- u16 mcs[NL80211_VHT_NSS_MAX])
-{
- u16 tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
- u16 tx_mcs_mask[NL80211_VHT_NSS_MAX] = {};
- u8 i;
-
- if (!sband->vht_cap.vht_supported)
- return false;
-
- memset(mcs, 0, sizeof(u16) * NL80211_VHT_NSS_MAX);
-
- /* Build vht_mcs_mask from VHT capabilities */
- vht_build_mcs_mask(tx_mcs_map, tx_mcs_mask);
-
- for (i = 0; i < NL80211_VHT_NSS_MAX; i++) {
- if ((tx_mcs_mask[i] & txrate->mcs[i]) == txrate->mcs[i])
- mcs[i] = txrate->mcs[i];
- else
- return false;
- }
-
- return true;
-}
-
-static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
- [NL80211_TXRATE_LEGACY] = { .type = NLA_BINARY,
- .len = NL80211_MAX_SUPP_RATES },
- [NL80211_TXRATE_HT] = { .type = NLA_BINARY,
- .len = NL80211_MAX_SUPP_HT_RATES },
- [NL80211_TXRATE_VHT] = { .len = sizeof(struct nl80211_txrate_vht)},
- [NL80211_TXRATE_GI] = { .type = NLA_U8 },
-};
-
static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
struct genl_info *info)
{
- struct nlattr *tb[NL80211_TXRATE_MAX + 1];
- struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct cfg80211_bitrate_mask mask;
- int rem, i;
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct net_device *dev = info->user_ptr[1];
- struct nlattr *tx_rates;
- struct ieee80211_supported_band *sband;
- u16 vht_tx_mcs_map;
+ int err;
if (!rdev->ops->set_bitrate_mask)
return -EOPNOTSUPP;
- memset(&mask, 0, sizeof(mask));
- /* Default to all rates enabled */
- for (i = 0; i < NUM_NL80211_BANDS; i++) {
- sband = rdev->wiphy.bands[i];
-
- if (!sband)
- continue;
-
- mask.control[i].legacy = (1 << sband->n_bitrates) - 1;
- memcpy(mask.control[i].ht_mcs,
- sband->ht_cap.mcs.rx_mask,
- sizeof(mask.control[i].ht_mcs));
-
- if (!sband->vht_cap.vht_supported)
- continue;
-
- vht_tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
- vht_build_mcs_mask(vht_tx_mcs_map, mask.control[i].vht_mcs);
- }
-
- /* if no rates are given set it back to the defaults */
- if (!info->attrs[NL80211_ATTR_TX_RATES])
- goto out;
-
- /*
- * The nested attribute uses enum nl80211_band as the index. This maps
- * directly to the enum nl80211_band values used in cfg80211.
- */
- BUILD_BUG_ON(NL80211_MAX_SUPP_HT_RATES > IEEE80211_HT_MCS_MASK_LEN * 8);
- nla_for_each_nested(tx_rates, info->attrs[NL80211_ATTR_TX_RATES], rem) {
- enum nl80211_band band = nla_type(tx_rates);
- int err;
-
- if (band < 0 || band >= NUM_NL80211_BANDS)
- return -EINVAL;
- sband = rdev->wiphy.bands[band];
- if (sband == NULL)
- return -EINVAL;
- err = nla_parse(tb, NL80211_TXRATE_MAX, nla_data(tx_rates),
- nla_len(tx_rates), nl80211_txattr_policy);
- if (err)
- return err;
- if (tb[NL80211_TXRATE_LEGACY]) {
- mask.control[band].legacy = rateset_to_mask(
- sband,
- nla_data(tb[NL80211_TXRATE_LEGACY]),
- nla_len(tb[NL80211_TXRATE_LEGACY]));
- if ((mask.control[band].legacy == 0) &&
- nla_len(tb[NL80211_TXRATE_LEGACY]))
- return -EINVAL;
- }
- if (tb[NL80211_TXRATE_HT]) {
- if (!ht_rateset_to_mask(
- sband,
- nla_data(tb[NL80211_TXRATE_HT]),
- nla_len(tb[NL80211_TXRATE_HT]),
- mask.control[band].ht_mcs))
- return -EINVAL;
- }
- if (tb[NL80211_TXRATE_VHT]) {
- if (!vht_set_mcs_mask(
- sband,
- nla_data(tb[NL80211_TXRATE_VHT]),
- mask.control[band].vht_mcs))
- return -EINVAL;
- }
- if (tb[NL80211_TXRATE_GI]) {
- mask.control[band].gi =
- nla_get_u8(tb[NL80211_TXRATE_GI]);
- if (mask.control[band].gi > NL80211_TXRATE_FORCE_LGI)
- return -EINVAL;
- }
-
- if (mask.control[band].legacy == 0) {
- /* don't allow empty legacy rates if HT or VHT
- * are not even supported.
- */
- if (!(rdev->wiphy.bands[band]->ht_cap.ht_supported ||
- rdev->wiphy.bands[band]->vht_cap.vht_supported))
- return -EINVAL;
-
- for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
- if (mask.control[band].ht_mcs[i])
- goto out;
-
- for (i = 0; i < NL80211_VHT_NSS_MAX; i++)
- if (mask.control[band].vht_mcs[i])
- goto out;
-
- /* legacy and mcs rates may not be both empty */
- return -EINVAL;
- }
- }
+ err = nl80211_parse_tx_bitrate_mask(info, &mask);
+ if (err)
+ return err;
-out:
return rdev_set_bitrate_mask(rdev, dev, NULL, &mask);
}
--
1.9.1
^ permalink raw reply related
* RE: [PATCH v3] cfg80211: Add support to configure a beacon data rate
From: Kushwaha, Purushottam @ 2016-09-13 9:04 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
Undekari, Sunil Dutt, Hullur Subramanyam, Amarnath
In-Reply-To: <1473675973.29016.19.camel@sipsolutions.net>
SGkgSm9oYW5uZXMsDQoNCj4gQWxsIG9mIHRoZXNlIGp1c3QgbW92ZSBhcm91bmQsIHJpZ2h0Pw0K
WWVzLCBqdXN0IG1vdmVkIGFyb3VuZC4gDQoNCj4gSSB0aGluayBpdCB3b3VsZCBiZSBnb29kIHRv
IHNwbGl0IG91dCB0aGlzICJzaW5nbGUgcmF0ZSIgdGhpbmcgaW50byBhIGhlbHBlciBmdW5jdGlv
bi4NCnN1cmUsIHdpbGwgcmFpc2UgYSBuZXcgcGF0Y2hzZXQgd2l0aCB0aGlzIGNoYW5nZS4NCg0K
VGhhbmtzLA0KUHVydXNob3R0YW0NCg==
^ permalink raw reply
* ath10k mesh + ap + encryption?
From: Simon Wunderlich @ 2016-09-13 8:00 UTC (permalink / raw)
To: ath10k@lists.infradead.org; +Cc: linux-wireless, openwrt-devel
[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]
Hi,
we have done some experiments last week on ath10k, trying to run mesh
(802.11s) and access point at the same time, both encrypted.
We have tested a recent LEDE (reboot-1519-g42f559e) but with
firmware-5.bin_10.2.4.70.42-2 and the included wpa_supplicant, which gave us a
working encrypted 802.11s network. However, starting an AP at the same time
didn't work (AP doesn't beacon). This wasn't a problem when 802.11s was
running unencrypted.
We also tested version 10.2.4.97 (from codeaurora), which is now default in
LEDE. However, this version apparently doesn't support 11s mesh at all
(WMI_SERVICE_MESH_11S is disabled in the service map, but cfg/mac80211
advertises support).
So here are my questions:
* Did anyone succesfully run AP and mesh, both encrypted at the same time?
* Do you have any pointers how we could fix this? Could it be fixable in the
driver (i.e. not in firmware)?
* Does anyone have an idea if 11s will be supported in future versions? I
didn't find any changelogs, but having 11s mode no longer in the service map
does not make me optimistic.
Thanks,
Simon
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: Support of rtl8723bs
From: Hanno Zulla @ 2016-09-13 7:47 UTC (permalink / raw)
To: Jes Sorensen, Bastien Nocera; +Cc: Larry Finger, anthony.wong, linux-wireless
In-Reply-To: <wrfj4m5lhzja.fsf@redhat.com>
Hi,
> The issues I am aware of is to start out changing the register access
> macros into function pointers and stick them all into the fileops
> structure. Provide a set of SDIO ones to match the USB ones. Then you
> will need some code to detect the device, as that part will obviously be
> different from the USB device detection.
>
> I know there are a few issues to look out for in the hw register init
> files to look out for too, that does some special casing for SDIO, but
> that should be limited. I am happy to point someone in the right
> direction on that when they get to that.
I had a brief look into the sources of other drivers implementing SDIO,
but not being a kernel-dev, this is out of my league.
If you decide to reprioritize your TODO-List in favour of adding SDIO
sooner, I'll be gladly testing this for you on my hardware (and buy you
a beer).
Kind regards,
Hanno
^ permalink raw reply
* Re: [PATCH v6 1/3] Documentation: dt: net: add ath9k wireless device binding
From: Oleksij Rempel @ 2016-09-13 7:28 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: ath9k-devel, devicetree, linux-wireless, ath9k-devel, mcgrof,
mark.rutland, robh+dt, kvalo, chunkeey, arend.vanspriel,
julian.calaby, bjorn, nbd, arnd
In-Reply-To: <CAFBinCAOQmk3KY1b+D1iKAOEBfasYyei681cae4PbD+201mzbQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 4629 bytes --]
Am 09.09.2016 um 22:57 schrieb Martin Blumenstingl:
> On Fri, Sep 9, 2016 at 9:48 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
>>> +Optional properties:
>>> +- reg: Address and length of the register set for the device.
>>> +- qca,clk-25mhz: Defines that a 25MHz clock is used
>>
>> Some SoCs even Atheros WiSoCs use WiFi clock for System Clock. In this
>> case we need to use clock framework any way, so why not in this case too?
>> Provide dummy static clock in DT and connect it with this node?
>>
>> osc25m: oscillator {
>> compatible = "fixed-clock";
>> #clock-cells = <0>;
>> clock-frequency = <25000000>;
>> clock-accuracy = <30000>;
>> };
>>
>> acc: clock-controller@80040000 {
>> compatible = "some-clock-controller";
>> #clock-cells = <1>;
>> clocks = <&osc25m>;
>> reg = <0x80040000 0x204>;
>> };
>>
>>
>> &pci0 {
>> ath9k@168c,002d {
>> compatible = "pci168c,002d";
>> reg = <0x7000 0 0 0 0x1000>;
>> clocks = <&osc25m>;
>> qca,disable-5ghz;
>> };
>> };
>>
>>
>> driver will need to use clk_get and compare frequency instead of
>> of_property_read_bool(np, "qca,clk-25mhz"). In this case you will need
>> to define source clock only one time and reuse it by all affected DT
>> nodes. If we have 40MHz clock you will only need to change it in
>> fixed-clock.
> that does sound like a good idea, thanks for that input!
> However, I will remove the property for the first version because I am
> trying to get PCI(e) devices supported. OF support for AHB (WiSoC)
> needs more work (in other places, like ahb.c) anyways.
> But this suggestion should be remembered!
>
>>> +- qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>>> + ath9k wireless chip (in this case the calibration /
>>> + EEPROM data will be loaded from userspace using the
>>> + kernel firmware loader).
>>> +- qca,disable-2ghz: Overrides the settings from the EEPROM and disables the
>>> + 2.4GHz band. Setting this property is only needed
>>> + when the RF circuit does not support the 2.4GHz band
>>> + while it is enabled nevertheless in the EEPROM.
>>> +- qca,disable-5ghz: Overrides the settings from the EEPROM and disables the
>>> + 5GHz band. Setting this property is only needed when
>>> + the RF circuit does not support the 5GHz band while
>>> + it is enabled nevertheless in the EEPROM.
>>
>> This option can be reused for every WiFi controller. Not only for qca.
>> So may be instead of adding vendor specific name, make it reusable for
>> all vendors. Instead of qca,disable-5ghz and qca,disable-2ghz make
>> disable-5ghz and disable-2ghz?
> I am personally fine with anything that fits best into the grand
> scheme of things.
> There are three scenarios I can think of which may be influenced by
> devicetree configuration:
> a) let the driver decide automatically whether the 2.4GHz and/or 5GHz
> band is/are enabled
> b) explicitly disable either bands (because the driver thinks due to
> whatever reason that a band is supported while in reality it isn't)
> c) explicitly enable a band (for example because the driver cannot
> automagically detect which band should be enabled)
>
> for ath9k we default to a) but also allow b): the EEPROM (device
> specific calibration data) contains information about which bands are
> enabled (or not). But some manufacturers are shipping devices for
> example with the 5G band enabled, while the actual hardware doesn't
> support it.
>
> Felix' mt76 driver for example defaults to case a) but allows
> overriding (= forcefully enabling or disabling) a specific band.
>
> If we decide how this should look like in the devicetree then I can go
> ahead and implement it accordingly.
To avoid possible conflict with other device-tree bindings i would
suggest this kind of naming:
ieee80211-5ghz-enalbe
ieee80211-5ghz-disable
ieee80211-2.4ghz-enalbe
ieee80211-2.4ghz-disable
i assume at some point we would get even more eeprom overrides. For
example disable/enable some modulation and so on.
we would need 80211 specific functions to read this overrides from
device-tree and/or acpi.
any feedback from DT maintainers?
--
Regards,
Oleksij
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]
^ permalink raw reply
* Re: [PATCH v3] mac80211: Re-structure aqm debugfs output and keep CoDel stats per txq
From: Johannes Berg @ 2016-09-13 7:17 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
In-Reply-To: <20160912135543.26347-1-toke@toke.dk>
On Mon, 2016-09-12 at 15:55 +0200, Toke Høiland-Jørgensen wrote:
> Currently the 'aqm' stats in mac80211 only keeps overlimit drop
> stats,
> not CoDel stats. This moves the CoDel stats into the txqi structure
> to
> keep them per txq in order to show them in debugfs.
>
> In addition, the aqm debugfs output is restructured by splitting it
> up
> into three files: One global per phy, one per netdev and one per
> station, in the appropriate directories. The files are all called
> aqm,
> and are only created if the driver supports the wake_tx_queue op
> (rather
> than emitting an error on open as previously).
>
Applied, thanks.
johannes
^ permalink raw reply
* [PATCH] mac80211: simplify TDLS RA lookup
From: Johannes Berg @ 2016-09-13 6:29 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
smatch pointed out that the second check of "tdls_auth" was
pointless since if it was true, we returned from the function
already. We can further simplify the code by moving the first
check (if it's a TDLS peer at all) into the outer if, to only
handle that inside. This simplifies the control flow here.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/tx.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index ee9e7d60cb78..61d302d97145 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2263,15 +2263,9 @@ static int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
case NL80211_IFTYPE_STATION:
if (sdata->wdev.wiphy->flags & WIPHY_FLAG_SUPPORTS_TDLS) {
sta = sta_info_get(sdata, skb->data);
- if (sta) {
- bool tdls_peer, tdls_auth;
-
- tdls_peer = test_sta_flag(sta,
- WLAN_STA_TDLS_PEER);
- tdls_auth = test_sta_flag(sta,
- WLAN_STA_TDLS_PEER_AUTH);
-
- if (tdls_peer && tdls_auth) {
+ if (sta && test_sta_flag(sta, WLAN_STA_TDLS_PEER)) {
+ if (test_sta_flag(sta,
+ WLAN_STA_TDLS_PEER_AUTH)) {
*sta_out = sta;
return 0;
}
@@ -2283,8 +2277,7 @@ static int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
* after a TDLS sta is removed due to being
* unreachable.
*/
- if (tdls_peer && !tdls_auth &&
- !ieee80211_is_tdls_setup(skb))
+ if (!ieee80211_is_tdls_setup(skb))
return -EINVAL;
}
--
2.8.1
^ permalink raw reply related
* Re: [PATCH] ath10k: Fix rfc1042 header retrieval in QCA4019 with eth decap mode
From: Thiagarajan, Vasanthakumar @ 2016-09-13 4:50 UTC (permalink / raw)
To: Valo, Kalle; +Cc: ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <87fup5m8x5.fsf@kamboji.qca.qualcomm.com>
On Monday 12 September 2016 09:01 PM, Valo, Kalle wrote:
> Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com> writes:
>
>> Chipset from QCA99X0 onwards (QCA99X0, QCA9984, QCA4019 & future)
>> rx_hdr_status is not padded to align in 4-byte boundary. Define a
>> new hw_params field to handle different alignment behaviour between
>> different hw. This patch fixes improper retrieval of rfc1042 header
>> with QCA4019. This patch along with "ath10k: Properly remove padding
>> from the start of rx payload" will fix traffic failure in ethernet
>> decap mode for QCA4019.
>>
>> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>
>
> There were some conflicts, I fixed them in the pending branch. Please
> check my changes:
>
> https://git.kernel.org/cgit/linux/kernel/git/kvalo/ath.git/commit/?h=3Dpe=
nding&id=3Dd0d9e40216e1a8edd2f3b387b96aad62c3a49add
>
Looks fine, thanks.
Vasanth=
^ permalink raw reply
* Re: [PATCH 00/15] drivers: net: use IS_ENABLED() instead of checking for built-in or module
From: David Miller @ 2016-09-13 3:28 UTC (permalink / raw)
To: javier
Cc: linux-kernel, arnd, sony.chacko, peppe.cavallaro,
linux-net-drivers, fw, bkenward, geert, ecree, kvalo, mw, a,
intel-wired-lan, f.fainelli, felipe.balbi, gregory.clement,
Dept-HSGLinuxNICDev, jeffrey.t.kirsher, mugunthanvnm, andrew,
klassert, sgruszka, kda, qiang.zhao, netdev, gerlando.falauto,
linux-wireless, ionut, venza, alexandre.torgue
In-Reply-To: <1473689026-6983-1-git-send-email-javier@osg.samsung.com>
From: Javier Martinez Canillas <javier@osg.samsung.com>
Date: Mon, 12 Sep 2016 10:03:31 -0400
> This trivial series is similar to [0] for net/ that you already merged, but
> for drivers/net. The patches replaces the open coding to check for a Kconfig
> symbol being built-in or module, with IS_ENABLED() macro that does the same.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Julia Lawall @ 2016-09-12 21:11 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Felipe Balbi, Julia Lawall, linux-renesas-soc, joe,
kernel-janitors, Sergei Shtylyov, linux-pm, platform-driver-x86,
linux-media, linux-can, Tatyana Nikolova, Shiraz Saleem,
Mustafa Ismail, Chien Tin Tung, linux-rdma, netdev, devel,
alsa-devel, linux-kernel, linux-fbdev, linux-wireless,
Jason Gunthorpe, tpmdd-devel, linux-scsi, linux-spi, linux-usb,
linux-acpi
In-Reply-To: <20160912201450.GA8889@intel.com>
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> > messages to subsystems based on your findings. And I generally think
> > >> > that if one contributes code one should also at least smoke test changes
> > >> > somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested. And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Hmm... I've been using coccinelle in cyclic basis for some time now.
> My comment was oversized but I didn't mean it to be impolite or attack
> of any kind for that matter.
No problem :) Thanks for the feedback.
julia
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Jarkko Sakkinen @ 2016-09-12 20:14 UTC (permalink / raw)
To: Felipe Balbi
Cc: Julia Lawall, linux-renesas-soc, joe, kernel-janitors,
Sergei Shtylyov, linux-pm, platform-driver-x86, linux-media,
linux-can, Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail,
Chien Tin Tung, linux-rdma, netdev, devel, alsa-devel,
linux-kernel, linux-fbdev, linux-wireless, Jason Gunthorpe,
tpmdd-devel, linux-scsi, linux-spi, linux-usb, linux-acpi
In-Reply-To: <877fah5j35.fsf@linux.intel.com>
On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> > messages to subsystems based on your findings. And I generally think
> >> > that if one contributes code one should also at least smoke test changes
> >> > somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested. And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.
Hmm... I've been using coccinelle in cyclic basis for some time now.
My comment was oversized but I didn't mean it to be impolite or attack
of any kind for that matter.
> --
> balbi
/Jarkko
^ permalink raw reply
* Re: [PATCH] staging: squash lines for simple wrapper functions
From: Jes Sorensen @ 2016-09-12 19:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Greg KH, devel, linux-fbdev, Andreas Dilger, Sumit Semwal,
linux-kernel, linux-wireless, Larry Finger, Teddy Wang,
Sudip Mukherjee, Laura Abbott, Arve Hjønnevåg,
Riley Andrews, Oleg Drokin, lustre-devel
In-Reply-To: <1473706668-24216-1-git-send-email-yamada.masahiro@socionext.com>
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> Remove unneeded variables and assignments.
>
> While we are here, clean up the following as well:
> - refactor rtl8723a_get_bcn_valid() a bit
> - remove unneeded casts in sii164Get{Vendor,Device}ID()
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> drivers/staging/android/ion/ion.c | 6 +-----
> .../staging/lustre/lustre/obdclass/linux/linux-module.c | 5 +----
> drivers/staging/lustre/lustre/ptlrpc/client.c | 5 +----
> drivers/staging/lustre/lustre/ptlrpc/events.c | 5 +----
> drivers/staging/rtl8723au/core/rtw_wlan_util.c | 7 +------
> drivers/staging/rtl8723au/hal/hal_com.c | 6 +-----
> drivers/staging/sm750fb/ddk750_sii164.c | 16 ++++------------
> 7 files changed, 10 insertions(+), 40 deletions(-)
1) Do not submit one giant patch modifying multiple drivers in one go
2) drivers/staging/rtl8723au is gone - had you followed 1), you wouldn't
necessarily have had to respin this patch.
3) Consider if your changes, even if technically correct, actually
improve the code (see below)
Jes
> diff --git a/drivers/staging/rtl8723au/core/rtw_wlan_util.c b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> index 694cf17..7ab47f0 100644
> --- a/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> +++ b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> @@ -1202,12 +1202,7 @@ unsigned int update_supported_rate23a(unsigned char *ptn, unsigned int ptn_sz)
>
> unsigned int update_MSC_rate23a(struct ieee80211_ht_cap *pHT_caps)
> {
> - unsigned int mask;
> -
> - mask = pHT_caps->mcs.rx_mask[0] << 12 |
> - pHT_caps->mcs.rx_mask[1] << 20;
> -
> - return mask;
> + return pHT_caps->mcs.rx_mask[0] << 12 | pHT_caps->mcs.rx_mask[1] << 20;
> }
The use of the mask variable didn't cause any harm, and it was certainly
a lot nicer to read the way it was.
> int support_short_GI23a(struct rtw_adapter *padapter,
> diff --git a/drivers/staging/rtl8723au/hal/hal_com.c b/drivers/staging/rtl8723au/hal/hal_com.c
> index 9d7b11b..dfbeebe 100644
> --- a/drivers/staging/rtl8723au/hal/hal_com.c
> +++ b/drivers/staging/rtl8723au/hal/hal_com.c
> @@ -708,11 +708,7 @@ void rtl8723a_bcn_valid(struct rtw_adapter *padapter)
>
> bool rtl8723a_get_bcn_valid(struct rtw_adapter *padapter)
> {
> - bool retval;
> -
> - retval = (rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0)) ? true : false;
> -
> - return retval;
> + return !!(rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0));
> }
One word: Yuck!
Talk about obfuscating the code for the sake of obfuscation!
> void rtl8723a_set_beacon_interval(struct rtw_adapter *padapter, u16 interval)
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
> index 67f36e7..28818e1 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -36,12 +36,8 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
> */
> unsigned short sii164GetVendorID(void)
> {
> - unsigned short vendorID;
> -
> - vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_LOW);
> -
> - return vendorID;
> + return (i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_HIGH) << 8) |
> + i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_LOW);
> }
>
> /*
> @@ -53,12 +49,8 @@ unsigned short sii164GetVendorID(void)
> */
> unsigned short sii164GetDeviceID(void)
> {
> - unsigned short deviceID;
> -
> - deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_LOW);
> -
> - return deviceID;
> + return (i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_HIGH) << 8) |
> + i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_LOW);
> }
Getting rid of the casts would be nice, merging them into a giant
multi-line return line certainly isn't an improvement in my book. That
said, I will leave that to the maintainer of that driver to decide what
is preferred.
Jes
^ permalink raw reply
* [PATCH] staging: squash lines for simple wrapper functions
From: Masahiro Yamada @ 2016-09-12 18:57 UTC (permalink / raw)
To: Greg KH, devel
Cc: Masahiro Yamada, linux-fbdev, Andreas Dilger, Sumit Semwal,
linux-kernel, linux-wireless, Larry Finger, Teddy Wang,
Sudip Mukherjee, Laura Abbott, Arve Hjønnevåg,
Jes Sorensen, Riley Andrews, Oleg Drokin, lustre-devel
Remove unneeded variables and assignments.
While we are here, clean up the following as well:
- refactor rtl8723a_get_bcn_valid() a bit
- remove unneeded casts in sii164Get{Vendor,Device}ID()
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/staging/android/ion/ion.c | 6 +-----
.../staging/lustre/lustre/obdclass/linux/linux-module.c | 5 +----
drivers/staging/lustre/lustre/ptlrpc/client.c | 5 +----
drivers/staging/lustre/lustre/ptlrpc/events.c | 5 +----
drivers/staging/rtl8723au/core/rtw_wlan_util.c | 7 +------
drivers/staging/rtl8723au/hal/hal_com.c | 6 +-----
drivers/staging/sm750fb/ddk750_sii164.c | 16 ++++------------
7 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index a2cf93b..a07fc03 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -389,11 +389,7 @@ static void ion_handle_get(struct ion_handle *handle)
static int ion_handle_put_nolock(struct ion_handle *handle)
{
- int ret;
-
- ret = kref_put(&handle->ref, ion_handle_destroy);
-
- return ret;
+ return kref_put(&handle->ref, ion_handle_destroy);
}
static int ion_handle_put(struct ion_handle *handle)
diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
index 33342bf..3f2dd95 100644
--- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
+++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
@@ -152,10 +152,7 @@ EXPORT_SYMBOL(obd_ioctl_getdata);
int obd_ioctl_popdata(void __user *arg, void *data, int len)
{
- int err;
-
- err = copy_to_user(arg, data, len) ? -EFAULT : 0;
- return err;
+ return copy_to_user(arg, data, len) ? -EFAULT : 0;
}
EXPORT_SYMBOL(obd_ioctl_popdata);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index d4463d7..38cbf4a 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -417,10 +417,7 @@ void ptlrpc_request_cache_fini(void)
struct ptlrpc_request *ptlrpc_request_cache_alloc(gfp_t flags)
{
- struct ptlrpc_request *req;
-
- req = kmem_cache_zalloc(request_cache, flags);
- return req;
+ return kmem_cache_zalloc(request_cache, flags);
}
void ptlrpc_request_cache_free(struct ptlrpc_request *req)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/events.c b/drivers/staging/lustre/lustre/ptlrpc/events.c
index b1ce725..8821f4d 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/events.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/events.c
@@ -525,10 +525,7 @@ static void ptlrpc_ni_fini(void)
static lnet_pid_t ptl_get_pid(void)
{
- lnet_pid_t pid;
-
- pid = LNET_PID_LUSTRE;
- return pid;
+ return LNET_PID_LUSTRE;
}
static int ptlrpc_ni_init(void)
diff --git a/drivers/staging/rtl8723au/core/rtw_wlan_util.c b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
index 694cf17..7ab47f0 100644
--- a/drivers/staging/rtl8723au/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
@@ -1202,12 +1202,7 @@ unsigned int update_supported_rate23a(unsigned char *ptn, unsigned int ptn_sz)
unsigned int update_MSC_rate23a(struct ieee80211_ht_cap *pHT_caps)
{
- unsigned int mask;
-
- mask = pHT_caps->mcs.rx_mask[0] << 12 |
- pHT_caps->mcs.rx_mask[1] << 20;
-
- return mask;
+ return pHT_caps->mcs.rx_mask[0] << 12 | pHT_caps->mcs.rx_mask[1] << 20;
}
int support_short_GI23a(struct rtw_adapter *padapter,
diff --git a/drivers/staging/rtl8723au/hal/hal_com.c b/drivers/staging/rtl8723au/hal/hal_com.c
index 9d7b11b..dfbeebe 100644
--- a/drivers/staging/rtl8723au/hal/hal_com.c
+++ b/drivers/staging/rtl8723au/hal/hal_com.c
@@ -708,11 +708,7 @@ void rtl8723a_bcn_valid(struct rtw_adapter *padapter)
bool rtl8723a_get_bcn_valid(struct rtw_adapter *padapter)
{
- bool retval;
-
- retval = (rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0)) ? true : false;
-
- return retval;
+ return !!(rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0));
}
void rtl8723a_set_beacon_interval(struct rtw_adapter *padapter, u16 interval)
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 67f36e7..28818e1 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -36,12 +36,8 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
*/
unsigned short sii164GetVendorID(void)
{
- unsigned short vendorID;
-
- vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_HIGH) << 8) |
- (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_LOW);
-
- return vendorID;
+ return (i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_HIGH) << 8) |
+ i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_LOW);
}
/*
@@ -53,12 +49,8 @@ unsigned short sii164GetVendorID(void)
*/
unsigned short sii164GetDeviceID(void)
{
- unsigned short deviceID;
-
- deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_HIGH) << 8) |
- (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_LOW);
-
- return deviceID;
+ return (i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_HIGH) << 8) |
+ i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_LOW);
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 0/9] rsi: code clean-up
From: Joe Perches @ 2016-09-12 18:50 UTC (permalink / raw)
To: Kalle Valo, Prameela Rani Garnepudi
Cc: linux-wireless, johannes.berg, hofrat, prameela.garnepudi
In-Reply-To: <87sht5mg86.fsf@kamboji.qca.qualcomm.com>
On Mon, 2016-09-12 at 15:53 +0300, Kalle Valo wrote:
> Also try to get more familiar how development is done, here are few
> links:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> https://www.kernel.org/doc/Documentation/SubmittingPatches
> https://kernelnewbies.org/FirstKernelPatch
>
> Also mailing list archives and git logs are a great way to learn from
> how others do it.
And only working on drivers/staging files is a good way to start.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox