Re: [PATCH 2/2] ath10k: search DT for qcom, ath10k-calibration-variant

[Christian Lamparter]

On Donnerstag, 19. April 2018 16:56:36 CEST Kalle Valo wrote:
> [...]
> I have added them now, please check:
> 
> https://github.com/kvalo/ath10k-firmware/commit/a47bcf1e58c4d8914af0951a80fd8861368b700d
Yes, The RT-AC58U is now working much better. 

The updated RT-AC58U board-data is here:


Regards
Christian

Re: [PATCH] mac80211: don't WARN on bad WMM parameters from buggy APs

[Christian Lamparter]

Hello,

On Monday, 26. March 2018 15:21:04 CEST Emmanuel Grumbach wrote:
> Apparently, some APs are buggy enough to send a zeroed
> WMM IE. Don't WARN on this since this is not caused by a bug
> on the client's system.
> 
> This aligns the condition of the WARNING in drv_conf_tx
> with the validity check in ieee80211_sta_wmm_params.
> We will now pick the default values whenever we get
> a zeroed WMM IE.
> 
> This has been reported here:
> https://bugzilla.kernel.org/show_bug.cgi?id=199161
> 
> Signed-off-by: Emmanuel Grumbach 
> ---
>  net/mac80211/mlme.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 39b660b9a908..a6b628964b84 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -1785,7 +1785,8 @@ static bool ieee80211_sta_wmm_params(struct 
> ieee80211_local *local,
>   params[ac].acm = acm;
>   params[ac].uapsd = uapsd;
>  
> - if (params[ac].cw_min > params[ac].cw_max) {
> + if (params->cw_min == 0 ||
I'm getting:
[  164.503843] wlan0: AP has invalid WMM params (CWmin/max=15/1023 for ACI 0), 
using defaults

The AP is running a recent OpenWrt and is using hostapd 2.7.

iw says the AP has the following WMM IE:
Extended capabilities: Extended Channel Switching, SSID List, 6
WMM: * Parameter version 1
 * u-APSD
 * BE: CW 15-1023, AIFSN 3
 * BK: CW 15-1023, AIFSN 7
 * VI: CW 7-15, AIFSN 2, TXOP 3008 usec
 * VO: CW 3-7, AIFSN 2, TXOP 1504 usec

which looks reasonable.

What seems to be happening is that the code now expects to start
with ac = 0.

802.11-2012's Figure 8-192 EDCA Parameter Set element lists the AC_BE as the
first element in the IE and because of this ac variable gets set to 
IEEE80211_AC_BE [1] in the first round. This would be fine, if 
IEEE80211_AC_BE was 0, but it is 2 [2].

Should this be params[ac].cw_min? or params[aci].cw_min?
Or should the params->cw_min check be placed after the loop?
Can you please sent a fix? Thanks.

Regards,
Christian

[1] 

[2] 

Asus RT-AC58U boardfile (was Re: [PATCH 2/2] ath10k: search DT for qcom, ath10k-calibration-variant)

[Christian Lamparter]

Sven's boarddata for the OpenMesh A42 has been accepted. \o/
So let's try to get the board data for the RT-AC58U merged.

On Friday, December 8, 2017 10:50:12 AM CET Kalle Valo wrote:
> > I've attached the necessary bmi-board-id=16 and bmi-board-id=17 board 
> > files to this mail as well. So, all that needs to be done is to add
> > them to the board-2.bin on your codeaurora / ath10k-firmware project.
> >
> > Kalle: Can you please update the board-2.bin for the IPQ40XX on your
> > ath10k-firmware project on github?

Since some time has passed since the original submission, I just stick to
answering the questions in the the guide from:


> description for what hardware this is
The hardware is the ASUS RT-AC58U [0]. It has a IPQ4018 SoC which
hosts two ath10k wifis. 
It's based on the qcom-ipq4019-ap.dk01.1-c2. machid: 0x8010100 .
the 5GHz and 2.4GHz radio components were drastically changed and
as a result, the device's wifis don't work very well with the reference
board-2.bin values. The 5GHz wifi suffers the most, the max throughput
tops out at 6 Mbps, even in the best possible "lab conditions".

> origin of the board file (did you create it yourself or where you downloaded)

These files are available in ASUS's GPL Source Code release.

The source code can be downloaded from this site:

Just select "Driver & Tools" in the right frame and "Others"
in the "Please select OS". It's the second tab called "Source Code":

|Version 3.0.0.4.380.8119
|2017/10/30  522.32 MBytes
|GPL of ASUS RT-AC58U for firmware 3.0.0.4.380.8119
(Note: If you press "Show all", it will list two older releases as well.
The boarddata files I sent you back then came from the original 
GPL_RT-AC58U_3.0.0.4.380.6516-g6772678 release. But the boarddata_*.bin
files have been updated since. So I'm attaching the new ones this time.)

The zip file contains one big GPL_RT-AC58U_3.0.0.4.380.8119-gdb642b4.tgz
which in turn contain these these two files.
asuswrt/release/src/router/qca-wifi-fw.ipq40xx/RT-AC58U/boarddata_0.bin
asuswrt/release/src/router/qca-wifi-fw.ipq40xx/RT-AC58U/boarddata_1.bin

I took the liberty of renaming the files to:

boarddata_0 -> bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U.bin
boarddata_1 -> bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U.bin

> ids to be used with the board file (ATH10K_BD_IE_BOARD_NAME in ath10k)

Following OpenMesh A42 example of "VENDOR-DEVICE", I choose to go with:
"ASUS-RT-AC58U" as the variant string:

full board name (ATH10K_BD_IE_BOARD_NAME):
bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U
bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U

{
"data": 
"bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U.bin", 
"names": [
"bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U"
]
}, 
{
"data": 
"bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U.bin", 
"names": [
"bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U"
]
},   

(if this is appended at the end of board-2.json, please remove the
 last comma)

> attach the actual board file (board.bin)
attached.

just in case. These are the sha256sum:
d9eb177737983ed320027d50711fd17fadc8057ff0a99d1d455342ef7a7bb1af  \
bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U.bin

24969cd91553fd915c51a8fe51c7b7e0e273679e1349b47d0a6c5ef4b4bb4a1b  \
bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U.bin

I also verified that the ASUS' firmware for the RT-AC58U 
(FW_RT_AC58U_30043808119.ZIP) uses the very same files.

bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=ASUS-RT-AC58U.bin
Description: Binary data


bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=ASUS-RT-AC58U.bin
Description: Binary data

Re: [PATCH v2 15/19] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Friday, January 12, 2018 7:39:50 PM CET Dan Williams wrote:
> On Fri, Jan 12, 2018 at 6:42 AM, Christian Lamparter <chunk...@gmail.com> 
> wrote:
> > On Friday, January 12, 2018 1:47:46 AM CET Dan Williams wrote:
> >> Static analysis reports that 'queue' may be a user controlled value that
> >> is used as a data dependency to read from the 'ar9170_qmap' array. In
> >> order to avoid potential leaks of kernel memory values, block
> >> speculative execution of the instruction stream that could issue reads
> >> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> >> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> >> 'ar->edcf' array.
> >>
> >> Based on an original patch by Elena Reshetova.
> >>
> >> Cc: Christian Lamparter <chunk...@googlemail.com>
> >> Cc: Kalle Valo <kv...@codeaurora.org>
> >> Cc: linux-wireless@vger.kernel.org
> >> Cc: net...@vger.kernel.org
> >> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> >> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> >> ---
> > This patch (and p54, cw1200) look like the same patch?!
> > Can you tell me what happend to:
> >
> > On Saturday, January 6, 2018 5:34:03 PM CET Dan Williams wrote:
> >> On Sat, Jan 6, 2018 at 6:23 AM, Christian Lamparter <chunk...@gmail.com> 
> >> wrote:
> >> > And Furthermore a invalid queue (param->ac) would cause a crash in
> >> > this line in mac80211 before it even reaches the driver [1]:
> >> > |   sdata->tx_conf[params->ac] = p;
> >> > |   
> >> > |   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
> >> > |^^ (this is a wrapper for the *_op_conf_tx)
> >> >
> >> > I don't think these chin-up exercises are needed.
> >>
> >> Quite the contrary, you've identified a better place in the call stack
> >> to sanitize the input and disable speculation. Then we can kill the
> >> whole class of the wireless driver reports at once it seems.
> > <https://www.spinics.net/lists/netdev/msg476353.html>
> 
> I didn't see where ac is being validated against the driver specific
> 'queues' value in that earlier patch.
The link to the check is right there in the earlier post. It's in 
parse_txq_params():
<https://github.com/torvalds/linux/blob/master/net/wireless/nl80211.c#L2070>
|   if (txq_params->ac >= NL80211_NUM_ACS)
|   return -EINVAL;

NL80211_NUM_ACS is 4
<http://elixir.free-electrons.com/linux/v4.15-rc7/source/include/uapi/linux/nl80211.h#L3748>

This check was added ever since mac80211's ieee80211_set_txq_params():
| sdata->tx_conf[params->ac] = p;

For cw1200: the driver just sets the dev->queue to 4.
In carl9170 dev->queues is set to __AR9170_NUM_TXQ and
p54 uses P54_QUEUE_AC_NUM.

Both __AR9170_NUM_TXQ and P54_QUEUE_AC_NUM are 4.
And this is not going to change since all drivers
have to follow mac80211's queue API:
<https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/queues>

Some background:
In the old days (linux 2.6 and early 3.x), the parse_txq_params() function did
not verify the "queue" value. That's why these drivers had to do it.

Here's the relevant code from 2.6.39:
<http://elixir.free-electrons.com/linux/v2.6.39/source/net/wireless/nl80211.c#L879>
You'll notice that the check is missing there.
Here's mac80211's ieee80211_set_txq_params for reference:
<http://elixir.free-electrons.com/linux/v2.6.39/source/net/mac80211/cfg.c#L1197>

However over time, the check in the driver has become redundant.

> > Anyway, I think there's an easy way to solve this: remove the
> > "if (queue < ar->hw->queues)" check altogether. It's no longer needed
> > anymore as the "queue" value is validated long before the driver code
> > gets called.
> > 
> > And from my understanding, this will fix the "In this case
> > the value of 'ar9170_qmap[queue]' is immediately reused as an index to
> > the 'ar->edcf' array." gripe your tool complains about.
> >
> > This is here's a quick test-case for carl9170.:
> > ---
> > diff --git a/drivers/net/wireless/ath/carl9170/main.c 
> > b/drivers/net/wireless/ath/carl9170/main.c
> > index 988c8857d78c..2d3afb15bb62 100644
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1387,13 +1387,8 @@ static int carl9170_op_conf_tx(struct ieee80211_hw 
> > *hw,
> > int r

Re: [PATCH v2 15/19] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Friday, January 12, 2018 1:47:46 AM CET Dan Williams wrote:
> Static analysis reports that 'queue' may be a user controlled value that
> is used as a data dependency to read from the 'ar9170_qmap' array. In
> order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue reads
> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> 'ar->edcf' array.
> 
> Based on an original patch by Elena Reshetova.
> 
> Cc: Christian Lamparter <chunk...@googlemail.com>
> Cc: Kalle Valo <kv...@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
This patch (and p54, cw1200) look like the same patch?! 
Can you tell me what happend to:

On Saturday, January 6, 2018 5:34:03 PM CET Dan Williams wrote:
> On Sat, Jan 6, 2018 at 6:23 AM, Christian Lamparter <chunk...@gmail.com> 
> wrote:
> > And Furthermore a invalid queue (param->ac) would cause a crash in
> > this line in mac80211 before it even reaches the driver [1]:
> > |   sdata->tx_conf[params->ac] = p;
> > |   
> > |   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
> > |^^ (this is a wrapper for the *_op_conf_tx)
> >
> > I don't think these chin-up exercises are needed.
> 
> Quite the contrary, you've identified a better place in the call stack
> to sanitize the input and disable speculation. Then we can kill the
> whole class of the wireless driver reports at once it seems.
<https://www.spinics.net/lists/netdev/msg476353.html>

Anyway, I think there's an easy way to solve this: remove the 
"if (queue < ar->hw->queues)" check altogether. It's no longer needed
anymore as the "queue" value is validated long before the driver code
gets called. And from my understanding, this will fix the "In this case
the value of 'ar9170_qmap[queue]' is immediately reused as an index to
the 'ar->edcf' array." gripe your tool complains about.

This is here's a quick test-case for carl9170.:
---
diff --git a/drivers/net/wireless/ath/carl9170/main.c 
b/drivers/net/wireless/ath/carl9170/main.c
index 988c8857d78c..2d3afb15bb62 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1387,13 +1387,8 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw,
int ret;
 
mutex_lock(>mutex);
-   if (queue < ar->hw->queues) {
-   memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
-   ret = carl9170_set_qos(ar);
-   } else {
-   ret = -EINVAL;
-   }
-
+   memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
+   ret = carl9170_set_qos(ar);
mutex_unlock(>mutex);
return ret;
 }
---
What does your tool say about this? 

(If necessary, the "queue" value could be even sanitized with a
queue %= ARRAY_SIZE(ar9170_qmap); before the mutex_lock.)

Re: [PATCH 08/18] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Saturday, January 6, 2018 4:06:21 PM CET Alan Cox wrote:
> > The only way a user can set this in any meaningful way would be via
> > a NL80211_CMD_SET_WIPHY netlink message. However, the value will get
> > vetted there by cfg80211's parse_txq_params [0]. This is long before
> 
> Far more than a couple of hundred instructions ?
Well, the user would have to send a netlink message each time. So
cfg80211 can parse it (this is where the initial "if queue >= 4 " check
happen). So the CPU would have to continue through and into 
rdev_set_txq_params() to get to mac80211's ieee80211_set_txq_params().
Then pass through that before gets to the driver's op_tx_conf. Once
there the driver code aquires a mutex_lock too before it gets to
check the queue value again.

Is this enough and how would the mutex_lock fit in there? Or can
the CPU skip past this as well? 
> The problem is that the processor will speculate that the parameter
> is valid and continue on that basis until the speculation resolves
> or it hits some other limit that stops it speculating further.
> That can be quite a distance on a modern x86 processor, and for all
> we know could be even more on some of the other CPUs involved.
 
> > it reaches any of the *_op_conf_tx functions.
> > 
> > And Furthermore a invalid queue (param->ac) would cause a crash in 
> > this line in mac80211 before it even reaches the driver [1]:
> > |   sdata->tx_conf[params->ac] = p;
> > |   
> 
> Firstly it might not because the address you get as a result could be
> valid kernel memory. In fact your attackers wants it to be valid kernel
> memory since they are trying to find the value of a piece of that memory.
> 
> Secondly the processor is doing this speculatively so it won't fault. It
> will eventually decide it went the wrong way and throw all the
> speculative work away - leaving footprints. It won't fault unless the
> speculative resolves that was the real path the code took.
> 
> If it's not a performance critical path then it's better to be safe.
Thank you for reading the canary too.

Christian

Re: [PATCH 08/18] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Saturday, January 6, 2018 2:10:37 AM CET Dan Williams wrote:
> Static analysis reports that 'queue' may be a user controlled value that
> is used as a data dependency to read from the 'ar9170_qmap' array. In
> order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue reads
> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> 'ar->edcf' array.
> 
> Based on an original patch by Elena Reshetova.
> 
> Cc: Christian Lamparter <chunk...@googlemail.com>
> Cc: Kalle Valo <kv...@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
>  drivers/net/wireless/ath/carl9170/main.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/main.c 
> b/drivers/net/wireless/ath/carl9170/main.c
> index 988c8857d78c..0ff34cbe2b62 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "hw.h"
> @@ -1384,11 +1385,12 @@ static int carl9170_op_conf_tx(struct ieee80211_hw 
> *hw,
>  const struct ieee80211_tx_queue_params *param)
>  {
>   struct ar9170 *ar = hw->priv;
> + const u8 *elem;
>   int ret;
>  
>   mutex_lock(>mutex);
> - if (queue < ar->hw->queues) {
> - memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
> + if ((elem = nospec_array_ptr(ar9170_qmap, queue, ar->hw->queues))) {
> + memcpy(>edcf[*elem], param, sizeof(*param));
>   ret = carl9170_set_qos(ar);
>   } else {
>   ret = -EINVAL;
> 
> 
About the "queue" in carl9170_op_conf_tx, p54_conf_tx and cw1200_conf_tx:

The only way a user can set this in any meaningful way would be via
a NL80211_CMD_SET_WIPHY netlink message. However, the value will get
vetted there by cfg80211's parse_txq_params [0]. This is long before
it reaches any of the *_op_conf_tx functions.

And Furthermore a invalid queue (param->ac) would cause a crash in 
this line in mac80211 before it even reaches the driver [1]:
|   sdata->tx_conf[params->ac] = p;
|   
|   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
|^^ (this is a wrapper for the *_op_conf_tx)

I don't think these chin-up exercises are needed.

[0] <https://github.com/torvalds/linux/blob/master/net/wireless/nl80211.c#L2070>
[1] <https://github.com/torvalds/linux/blob/master/net/mac80211/cfg.c#L2157>

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Christian Lamparter]

On Friday, December 1, 2017 4:05:10 PM CET Maxime Bizon wrote:
> 
> On Fri, 2017-12-01 at 19:18 +0530, ako...@codeaurora.org wrote:
> 
> > Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
> 
> it wasn't and IMHO it's confusing because tx rate is filled by the other
> drivers without it.
> 
> I now have the following warning:
> 
> [   96.174967] [ cut here ]
> [   96.179640] WARNING: CPU: 0 PID: 609 at net/wireless/util.c:1254 
> cfg80211_calculate_bitrate+0x174/0x220
> [   96.189538] invalid rate bw=1, mcs=9, nss=2
> [   96.219736] CPU: 0 PID: 609 Comm: hostapd Not tainted 
> 4.14.3-00381-gec9756b0f64d #28
> [   96.227910] Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [   96.235450] [<80010124>] (unwind_backtrace) from [<8000d9ec>] 
> (show_stack+0x10/0x14)
> [   96.247180] [<8000d9ec>] (show_stack) from [<8002016c>] (__warn+0xdc/0xf8)
> [   96.254243] [<8002016c>] (__warn) from [<800201bc>] 
> (warn_slowpath_fmt+0x34/0x44)
> [   96.262016] [<800201bc>] (warn_slowpath_fmt) from [<8064dfdc>] 
> (cfg80211_calculate_bitrate+0x174/0x220)
> [   96.272652] [<8064dfdc>] (cfg80211_calculate_bitrate) from [<806692d4>] 
> (nl80211_put_sta_rate+0x44/0x1dc)
> [   96.282509] [<806692d4>] (nl80211_put_sta_rate) from [<8066001c>] 
> (nl80211_send_station+0x388/0xaf0)
> [   96.292261] [<8066001c>] (nl80211_send_station) from [<8066082c>] 
> (nl80211_get_station+0xa8/0xec)
> [   96.304166] [<8066082c>] (nl80211_get_station) from [<80509c20>] 
> (genl_rcv_msg+0x2dc/0x34c)
> [   96.313324] [<80509c20>] (genl_rcv_msg) from [<8050890c>] 
> (netlink_rcv_skb+0x84/0xdc)
> [   96.321880] [<8050890c>] (netlink_rcv_skb) from [<805093c0>] 
> (genl_rcv+0x20/0x34)
> [   96.329668] [<805093c0>] (genl_rcv) from [<80508188>] 
> (netlink_unicast+0x12c/0x1e0)
> [   96.338408] [<80508188>] (netlink_unicast) from [<805085d8>] 
> (netlink_sendmsg+0x2e0/0x304)
> [   96.350736] [<805085d8>] (netlink_sendmsg) from [<804b5f9c>] 
> (sock_sendmsg+0x14/0x24)
> [   96.358656] [<804b5f9c>] (sock_sendmsg) from [<804b66e8>] 
> (___sys_sendmsg+0x1c8/0x20c)
> [   96.367093] [<804b66e8>] (___sys_sendmsg) from [<804b739c>] 
> (__sys_sendmsg+0x40/0x64)
> [   96.375276] [<804b739c>] (__sys_sendmsg) from [<8000a3e0>] 
> (ret_fast_syscall+0x0/0x44)
> [   96.383455] ---[ end trace da8257d6a850e91a ]---
> 
Look for:

"[PATCH] ath10k: fix recent bandwidth conversion bug"

Re: [PATCH] ath10k: fix recent bandwidth conversion bug

[Christian Lamparter]

On Monday, November 20, 2017 11:57:21 AM CET Kalle Valo wrote:
> Christian Lamparter <chunk...@gmail.com> writes:
> 
> > On Wednesday, November 1, 2017 9:37:53 PM CET Sebastian Gottschall wrote:
> >> a additional array bounds check would be good
> >
> > Ah, about that:
> >
> > the bw variable in ath10k_htt_rx_h_rates() is extracted from info2
> > in the following way [0]:
> > |   bw = info2 & 3;
> >
> > the txrate.bw variable in ath10k_update_per_peer_tx_stats() is set by [1]:
> > |   txrate.bw = ATH10K_HW_BW(peer_stats->flags);
> >
> > ATH10K_HW_BW is a macro defined as [2]:
> > |   #define ATH10K_HW_BW(flags) (((flags) >> 3) & 0x3)
> >
> > In both cases the bandwidth values already are limited to 0-3 by
> > the "and 3" operation.
> 
> Until someone changes that part of the code (and the firmware
> interface). IMHO a switch is safer as there we don't have any risk of
> out of bands access.

The kbuild-bot/CI can catch this too. 

For example, it will look like this:
drivers/net/wireless/ath/ath10k//htt_rx.c:710:52: warning: invalid access past 
the end of 'ath10k_bw_to_mac80211' (4 4)

BTW:
Have you noticed:

<https://github.com/lede-project/source/blob/master/package/kernel/mac80211/patches/319-ath10k-fix-recent-bandwidth-conversion-bug.patch>

Is this really your signed-off-by or not?

In any case, you - as the maintainer - can modify the patch as
you see fit. So, please do so.

Re: [PATCH] ath10k: fix recent bandwidth conversion bug

[Christian Lamparter]

On Wednesday, November 1, 2017 9:37:53 PM CET Sebastian Gottschall wrote:
> a additional array bounds check would be good

Ah, about that:

the bw variable in ath10k_htt_rx_h_rates() is extracted from info2
in the following way [0]:
|   bw = info2 & 3;

the txrate.bw variable in ath10k_update_per_peer_tx_stats() is set by [1]:
|   txrate.bw = ATH10K_HW_BW(peer_stats->flags);

ATH10K_HW_BW is a macro defined as [2]:
|   #define ATH10K_HW_BW(flags) (((flags) >> 3) & 0x3)

In both cases the bandwidth values already are limited to 0-3 by
the "and 3" operation.

[0] 


[1] 

[2] 



> > @@ -592,6 +592,9 @@ struct amsdu_subframe_hdr {
> >   
> >   #define GROUP_ID_IS_SU_MIMO(x) ((x) == 0 || (x) == 63)
> >   
> > +static const u8 ath10k_bw_to_mac80211[] = { RATE_INFO_BW_20, 
> > RATE_INFO_BW_40,
> > +   RATE_INFO_BW_80, RATE_INFO_BW_160 };
> > +
> >   static void ath10k_htt_rx_h_rates(struct ath10k *ar,
> >   struct ieee80211_rx_status *status,
> >   struct htt_rx_desc *rxd)
> > @@ -694,23 +697,7 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
> > if (sgi)
> > status->enc_flags |= RX_ENC_FLAG_SHORT_GI;
> >   
> > [...]
> > +   status->bw = ath10k_bw_to_mac80211[bw];
> > status->encoding = RX_ENC_VHT;
> > break;
> > default:
> > @@ -2297,7 +2284,7 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
> > arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
> >   
> > arsta->txrate.nss = txrate.nss;
> > -   arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
> > +   arsta->txrate.bw = ath10k_bw_to_mac80211[txrate.bw];
> >   }
> >   
> >   static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,

[PATCH] ath10k: fix recent bandwidth conversion bug

[Christian Lamparter]

The commit "cfg80211: make RATE_INFO_BW_20 the default" changed
the index of RATE_INFO_BW_20, but the updates to ath10k missed
the special bandwidth calculation case in
ath10k_update_per_peer_tx_stats().

Fixes: 842be75c77cb ("cfg80211: make RATE_INFO_BW_20 the default")
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 23 +--
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a3f5dc78353f..26b0d201a698 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -592,6 +592,9 @@ struct amsdu_subframe_hdr {
 
 #define GROUP_ID_IS_SU_MIMO(x) ((x) == 0 || (x) == 63)
 
+static const u8 ath10k_bw_to_mac80211[] = { RATE_INFO_BW_20, RATE_INFO_BW_40,
+   RATE_INFO_BW_80, RATE_INFO_BW_160 };
+
 static void ath10k_htt_rx_h_rates(struct ath10k *ar,
  struct ieee80211_rx_status *status,
  struct htt_rx_desc *rxd)
@@ -694,23 +697,7 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
if (sgi)
status->enc_flags |= RX_ENC_FLAG_SHORT_GI;
 
-   switch (bw) {
-   /* 20MHZ */
-   case 0:
-   break;
-   /* 40MHZ */
-   case 1:
-   status->bw = RATE_INFO_BW_40;
-   break;
-   /* 80MHZ */
-   case 2:
-   status->bw = RATE_INFO_BW_80;
-   break;
-   case 3:
-   status->bw = RATE_INFO_BW_160;
-   break;
-   }
-
+   status->bw = ath10k_bw_to_mac80211[bw];
status->encoding = RX_ENC_VHT;
break;
default:
@@ -2297,7 +2284,7 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
 
arsta->txrate.nss = txrate.nss;
-   arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
+   arsta->txrate.bw = ath10k_bw_to_mac80211[txrate.bw];
 }
 
 static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
-- 
2.15.0

Re: [PATCH v6 1/3] dt-bindings: net: add mt76 wireless device binding

[Christian Lamparter]

On Saturday, October 14, 2017 9:20:46 AM CEST Felix Fietkau wrote:
> On 2017-10-13 21:07, Rob Herring wrote:
> > On Fri, Oct 06, 2017 at 01:02:47PM +0200, Felix Fietkau wrote:
> >> Add documentation describing how device tree can be used to configure
> >> wireless chips supported by the mt76 driver.
> >> 
> >> Signed-off-by: Felix Fietkau 
> >> ---
> >>  .../bindings/net/wireless/mediatek,mt76.txt| 24 
> >> ++
> >>  1 file changed, 24 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
> >> 
> >> diff --git 
> >> a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt 
> >> b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
> >> new file mode 100644
> >> index ..19522ab97d62
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
> >> @@ -0,0 +1,24 @@
> >> +* MediaTek mt76xx devices
> >> +
> >> +This node provides properties for configuring the MediaTek mt76xx wireless
> >> +device. The node is expected to be specified as a child node of the PCI
> >> +controller to which the wireless chip is connected.
> >> +
> >> +Optional properties:
> >> +
> >> +- mac-address: See ethernet.txt in the parent directory
> >> +- local-mac-address: See ethernet.txt in the parent directory
> >> +- ieee80211-freq-limit: See ieee80211.txt
> >> +- mediatek,mtd-eeprom: Specify a MTD partition + offset containing EEPROM 
> >> data
> > 
> > MTD is a Linuxism. And is an EEPROM the only supported device? I'd 
> > suggest naming if after what the data contains.
> PCI cards with this kind of wireless chip usually come with some form of
> EEPROM or use the on-chip OTP ROM.
> This property is for the case where the chip is built into an embedded
> device and the data that would otherwise be on an EEPROM is stored on a
> MTD partition instead.
> The EEPROM data itself contains multiple things: calibration data,
> hardware capabilities, MAC address, etc.
> I couldn't think of a better name for it, do you have any suggestions?
This sort of reminds me of the failed ath9k nvmem patches:
https://patchwork.kernel.org/patch/9622127/

Which uses the nvmem system.

https://github.com/torvalds/linux/blob/master/Documentation/nvmem/nvmem.txt

Rob, would this be acceptable?

Regards,
Christian

Re: [PATCH v2] p54: don't unregister leds when they are not initialized

[Christian Lamparter]

On Tuesday, September 26, 2017 5:11:33 PM CEST Andrey Konovalov wrote:
> ieee80211_register_hw() in p54_register_common() may fail and leds won't
> get initialized. Currently p54_unregister_common() doesn't check that and
> always calls p54_unregister_leds(). The fix is to check priv->registered
> flag before calling p54_unregister_leds().
> 
> Found by syzkaller.
> 
> [...]
>  process_scheduled_works kernel/workqueue.c:2179
>  worker_thread+0xb2b/0x1850 kernel/workqueue.c:2255
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> 
> Signed-off-by: Andrey Konovalov <andreyk...@google.com>
Cc: sta...@vger.kernel.org
Acked-by: Christian Lamparter <chunk...@googlemail.com>

Thanks for making the patch too!

[RESEND] Re: usb/net/p54: trying to register non-static key in p54_unregister_leds

[Christian Lamparter]

This got rejected by gmail once. Let's see if it works now.

On Thursday, September 21, 2017 8:22:45 PM CEST Andrey Konovalov wrote:
> On Wed, Sep 20, 2017 at 9:55 PM, Johannes Berg
> <johan...@sipsolutions.net> wrote:
> > On Wed, 2017-09-20 at 21:27 +0200, Christian Lamparter wrote:
> >
> >> It seems this is caused as a result of:
> >> -> lock_map_acquire(>lockdep_map);
> >>   lock_map_release(>lockdep_map);
> >>
> >> in flush_work() [0]
> >
> > Agree.
> >
> >> This was added by:
> >>
> >>   commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
> >>   Author: Stephen Boyd <sb...@codeaurora.org>
> >>   Date:   Fri Apr 20 17:28:50 2012 -0700
> >>
> >>   workqueue: Catch more locking problems with flush_work()
> >
> > Yes, but that doesn't matter.
> >
> >> Looking at the Stephen's patch, it's clear that it was made
> >> with "static DECLARE_WORK(work, my_work)" in mind. However
> >> p54's led_work is "per-device", hence it is stored in the
> >> devices context p54_common, which is dynamically allocated.
> >> So, maybe revert Stephen's patch?
> >
> > I disagree - as the lockdep warning says:
> >
> >> > INFO: trying to register non-static key.
> >> > the code is fine but needs lockdep annotation.
> >> > turning off the locking correctness validator.
> >
> > What it needs is to actually correctly go through initializing the work
> > at least once.
> >
> > Without more information, I can't really say what's going on, but I
> > assume that something is failing and p54_unregister_leds() is getting
> > invoked without p54_init_leds() having been invoked, so essentially
> > it's trying to flush a work that was never initialized?
> >
> > INIT_DELAYED_WORK() does, after all, initialize the lockdep map
> > properly via __INIT_WORK().

Ok, thanks. This does indeed explain it.

But this also begs the question: Is this really working then?
>From what I can tell, if CONFIG_LOCKDEP is not set then there's no BUG
no WARN, no other splat or any other odd system behaviour. Does
[cancel | flush]_[delayed_]work[_sync] really "just work" by *accident*,
as long the delayed_work | work_struct is zeroed out? 
And should it work in the future as well?

> Since I'm able to reproduce this, please let me know if you need me to
> collect some debug traces to help with the triage.

Do you want to take a shot at making a patch too? At a quick glance, it
should be enough to move the [#ifdef CONFIG_P54_LEDS ... #endif] block
in p54_unregister_common() into the if (priv->registered) { block
(preferably before the ieee80211_unregister_hw(dev).

Regards,
Christian

Re: usb/net/p54: trying to register non-static key in p54_unregister_leds

[Christian Lamparter]

On Wednesday, September 20, 2017 8:37:08 PM CEST Andrey Konovalov wrote:
> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
> 
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 1 PID: 1404 Comm: kworker/1:1 Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80-dirty #205
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  __dump_stack lib/dump_stack.c:16
>  dump_stack+0x292/0x395 lib/dump_stack.c:52
>  register_lock_class+0x6c4/0x1a00 kernel/locking/lockdep.c:769
>  __lock_acquire+0x27e/0x4550 kernel/locking/lockdep.c:3385
>  lock_acquire+0x259/0x620 kernel/locking/lockdep.c:4002
>  flush_work+0xf0/0x8c0 kernel/workqueue.c:2886
>  __cancel_work_timer+0x51d/0x870 kernel/workqueue.c:2961
>  cancel_delayed_work_sync+0x1f/0x30 kernel/workqueue.c:3081
>  p54_unregister_leds+0x6c/0xc0 drivers/net/wireless/intersil/p54/led.c:160
>  p54_unregister_common+0x3d/0xb0 drivers/net/wireless/intersil/p54/main.c:856
>  p54u_disconnect+0x86/0x120 drivers/net/wireless/intersil/p54/p54usb.c:1073
>  usb_unbind_interface+0x21c/0xa90 drivers/usb/core/driver.c:423
>  __device_release_driver drivers/base/dd.c:861
>  device_release_driver_internal+0x4f4/0x5c0 drivers/base/dd.c:893
>  device_release_driver+0x1e/0x30 drivers/base/dd.c:918
>  bus_remove_device+0x2f4/0x4b0 drivers/base/bus.c:565
>  device_del+0x5c4/0xab0 drivers/base/core.c:1985
>  usb_disable_device+0x1e9/0x680 drivers/usb/core/message.c:1170
>  usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124
>  hub_port_connect drivers/usb/core/hub.c:4754
>  hub_port_connect_change drivers/usb/core/hub.c:5009
>  port_event drivers/usb/core/hub.c:5115
>  hub_event+0x1318/0x3740 drivers/usb/core/hub.c:5195
>  process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
>  process_scheduled_works kernel/workqueue.c:2179
>  worker_thread+0xb2b/0x1850 kernel/workqueue.c:2255
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> 

It seems this is caused as a result of:
-> lock_map_acquire(>lockdep_map);
lock_map_release(>lockdep_map);

in flush_work() [0]

This was added by:

commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
Author: Stephen Boyd 
Date:   Fri Apr 20 17:28:50 2012 -0700

workqueue: Catch more locking problems with flush_work()

Looking at the Stephen's patch, it's clear that it was made
with "static DECLARE_WORK(work, my_work)" in mind. However
p54's led_work is "per-device", hence it is stored in the
devices context p54_common, which is dynamically allocated.
So, maybe revert Stephen's patch?

[0] 

Re: hung task in mac80211

[Christian Lamparter]

On Wednesday, September 6, 2017 1:57:47 PM CEST Matteo Croce wrote:
> Hi,
> 
> I have an hung task on vanilla 4.13 kernel which I haven't on 4.12.
> The problem is present both on my AP and on my notebook,
> so it seems it affects AP and STA mode as well.
> The generated messages are:
> 
> INFO: task kworker/u16:6:120 blocked for more than 120 seconds.
>   Not tainted 4.13.0 #57
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> kworker/u16:6   D0   120  2 0x
> Workqueue: phy0 ieee80211_ba_session_work [mac80211]
> Call Trace:
>  ? __schedule+0x174/0x5b0
>  ? schedule+0x31/0x80
>  ? schedule_preempt_disabled+0x9/0x10
>  ? __mutex_lock.isra.2+0x163/0x480
>  ? select_task_rq_fair+0xb9f/0xc60
>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>  ? try_to_wake_up+0x1f1/0x340
>  ? update_curr+0x88/0xd0
>  ? ieee80211_ba_session_work+0x148/0x230 [mac80211]
> 
> I did a bisect and the offending commit is:
> 
> commit 699cb58c8a52ff39bf659bff7971893ebe111bf2
> Author: Johannes Berg 
> Date:   Tue May 30 16:34:46 2017 +0200
> 
> mac80211: manage RX BA session offload without SKB queue

I looked at this briefly:

ieee80211_ba_session_work acquires:
mutex_lock(>ampdu_mlme.mtx) @


But it now also calls
__ieee80211_start_rx_ba_session() @
http://elixir.free-electrons.com/linux/v4.13/source/net/mac80211/ht.c#L336

which also wants to hold mutex_lock(>ampdu_mlme.mtx) in:
http://elixir.free-electrons.com/linux/v4.13/source/net/mac80211/agg-rx.c#L314

I guess this is where it deadlocks?

Regards,
Christian

Re: [PATCH] p54: add null pointer check before releasing socket buffer

[Christian Lamparter]

On Monday, April 10, 2017 1:54:14 PM CEST Myungho Jung wrote:
> On Mon, Apr 10, 2017 at 02:12:54PM +0200, Christian Lamparter wrote:
> > On Sunday, April 9, 2017 10:23:20 PM CEST Myungho Jung wrote:
> > > Kernel panic is caused by trying to dereference null pointer. Check if
> > > the pointer is null before freeing space.
> >  [...]
> > As for adding if (!skb) checks. I think kfree, kfree_skb, dev_kfree_skb 
> > (aka consume_skb) all check for null pointers already. So the logical
> > thing to do would be to make dev_kfree_skb_irq (which would also fix
> > dev_kfree_skb_any) consistent with kfree, kfree_skb, dev_kfree_skb and
> > add the check there.
> > > ---
> > >  drivers/net/wireless/intersil/p54/txrx.c | 8 ++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/intersil/p54/txrx.c 
> > > b/drivers/net/wireless/intersil/p54/txrx.c
> > > index 1af7da0..8956061 100644
> > > --- a/drivers/net/wireless/intersil/p54/txrx.c
> > > +++ b/drivers/net/wireless/intersil/p54/txrx.c
> > > @@ -503,7 +503,9 @@ static void p54_rx_eeprom_readback(struct p54_common 
> > > *priv,
> > >  
> > >   priv->eeprom = NULL;
> > >   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> > > - dev_kfree_skb_any(tmp);
> > > + if (unlikely(!tmp))
> > > + dev_kfree_skb_any(tmp);
> > > +
> > >   complete(>eeprom_comp);
> > >  }
> > >  
> > > @@ -597,7 +599,9 @@ static void p54_rx_stats(struct p54_common *priv, 
> > > struct sk_buff *skb)
> > >   }
> > >  
> > >   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> > > - dev_kfree_skb_any(tmp);
> > > + if (unlikely(!tmp))
> > > + dev_kfree_skb_any(tmp);
> > > +
> > >   complete(>stat_comp);
> > >  }
> > >  
> > > 
> > 
> > 
> [...] I'm not sure it actually caused kernel panic but guessed from
> a bug report [https://bugzilla.kernel.org/show_bug.cgi?id=195289].
Thank you for bringing this to my attention. Reading the bugreport, it
does sound like there's a bigger issue with the USB Ports. I'll see if
this can be fixed. But it does sound like a hardware issue at this 
point.

> And correct fix will be like this:
>   if (likely(tmp))
>   dev_kfree_skb_any(tmp);
> 
> But, like you said, I think null pointer should be checked in
> dev_kfree_skb_irq although already checking before calling it in many
> other places. I'll try another patch. Thank you for your advice.

Well, the patch could be as simple as this:
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 7869ae3837ca..44f7d5a1c67c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum 
skb_free_reason reason)
 {
unsigned long flags;
 
+   if (!skb)
+   return;
+
if (likely(atomic_read(>users) == 1)) {
smp_rmb();
atomic_set(>users, 0);
---

The question is: would David or Eric support the change. Any comments,
what's the prefered solution? Just patch __dev_kfree_skb_irq to make
it consistent with *kfree*, or patch the driver? I'm fine either way,
but I would prefere patching __dev_kfree_skb_irq.

Regards,
Christian

Re: [PATCH] p54: add null pointer check before releasing socket buffer

[Christian Lamparter]

(Added linux-wireless, since this is a wireless driver)

On Sunday, April 9, 2017 10:23:20 PM CEST Myungho Jung wrote:
> Kernel panic is caused by trying to dereference null pointer. Check if
> the pointer is null before freeing space.
Do you have the kernel panic somewhere?
I think you have an even bigger problem: You see, in order to get EEPROM
readback and rx_stats feedback you need to sent a request to the firmware
and if the response's req_id cookies don't match, you end up filling up 
the very limited device address space.

As for adding if (!skb) checks. I think kfree, kfree_skb, dev_kfree_skb 
(aka consume_skb) all check for null pointers already. So the logical
thing to do would be to make dev_kfree_skb_irq (which would also fix
dev_kfree_skb_any) consistent with kfree, kfree_skb, dev_kfree_skb and
add the check there.

> Signed-off-by: Myungho Jung 
> ---
>  drivers/net/wireless/intersil/p54/txrx.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c 
> b/drivers/net/wireless/intersil/p54/txrx.c
> index 1af7da0..8956061 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -503,7 +503,9 @@ static void p54_rx_eeprom_readback(struct p54_common 
> *priv,
>  
>   priv->eeprom = NULL;
>   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> - dev_kfree_skb_any(tmp);
> + if (unlikely(!tmp))
> + dev_kfree_skb_any(tmp);
> +
>   complete(>eeprom_comp);
>  }
>  
> @@ -597,7 +599,9 @@ static void p54_rx_stats(struct p54_common *priv, struct 
> sk_buff *skb)
>   }
>  
>   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> - dev_kfree_skb_any(tmp);
> + if (unlikely(!tmp))
> + dev_kfree_skb_any(tmp);
> +
>   complete(>stat_comp);
>  }
>  
> 

Re: compile driver for rtl8192su

[Christian Lamparter]

Hello,

On Wednesday, March 29, 2017 2:51:13 PM CEST walter strametz wrote:
> I try to get my D-Link N300 running and compiled the sources on
> github (chunkeey - rtl8192su).  Other trials also failed, because the
> driver-source is not compatible with the (newer) kernel.
the 4.4.y kernel is too old.

Only kernels built from the latest wireless-testing.git are compatible.
If you want to know more about wireless-testing.git then visit the
wireless wiki's Git-Guide:


Regards,
Christian

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 6:41:59 PM CEST Andrew Lunn wrote:
> > Oh, in that case you should probably go "all out" and ask on the 
> > LKML to remove all of the ath9k and ath10k ahb work. From what I
> > know all the "users" are running some sort of OpenWRT/LEDE or a 
> > derivative. This is because Atheros/QCA provided a SDK based on
> > OpenWRT.
> > 
> > Alban has been trying to convert the platform to device-tree
> > and add them to the mainline for a while now:
> >  
> > https://patchwork.kernel.org/patch/6514551/
> > 
> > So, you are questioning this work as well.
> 
> Not at all. Ralph Sennhauser has been doing a great job of getting all
> the Marvell devices into Mainline, and i help as much as i can, being
> one of the Marvell SoC Maintainers.
> 
> I'm just saying, get a few boards which require these facilities into
> the mainline, and then you have a much stronger base to argue from.

I was arguing not to deprecate "qca,no-eeprom" property.

based on this quote from Linus' :
|if a new interface is truly more flexible, then it should be able
|to implement the old interface with no changes, so that drivers
|shouldn't need to be changed/upgraded.

what stronger point to do you want?

Thanks,
Christian

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 5:18:37 PM CEST Andrew Lunn wrote:
> > But LEDE/OpenWRT rely on the firmware loading API more than ever and 
> > currently there is not a replacement for it.
> 
> 
> 
> 
> > I looked into 10-ath9k-eeprom [0] of LEDE's AR71XX target and I noticed
> > that quite a few devices patch the MACs of the wifi.
> > If you look at the code for the Airtight C-55 and C-60, Meraki MR18,
> > Meraki Z1, you'll notice that each one has to add a fixed value (+1,
> > +2, ...) to the extraced MAC-Address. So how would you replicate this,
> > with "nvmem-cell-names = address" without some sort of 
> > nvmem-provider-processor?
> 
> ...
>  
> > https://github.com/lede-project/source/blob/master/target/linux/ar71xx/files/arch/mips/ath79/dev-eth.c#L1204
> > 
> > and grep lists the following devices:
> > mach-dgl-5500-a1.c, mach-dhp-1565-a1.c, mach-dir-505-a1.c, mach-dir-615-c1.c
> > mach-dir-615-i1.c, mach-dir-825-b1.c, mach-dir-825-c1.c, mach-tew-673gru.c
> > mach-tew-712br.c, mach-tew-732br.c, mach-tew-823dru.c
> 
> I would say a big part of the problem is that all of these use cases
> are outside of mainline. Why should mainline support something which
> is not actually used in mainline.
> 
> So i would suggest your first step is to bring some of these devices
> into mainline. Once in mainline, it becomes a mainline issue, and
> people will help get it solved.
>
Oh, in that case you should probably go "all out" and ask on the 
LKML to remove all of the ath9k and ath10k ahb work. From what I
know all the "users" are running some sort of OpenWRT/LEDE or a 
derivative. This is because Atheros/QCA provided a SDK based on
OpenWRT.

Alban has been trying to convert the platform to device-tree
and add them to the mainline for a while now:
 
https://patchwork.kernel.org/patch/6514551/

So, you are questioning this work as well.

Thanks,
Christian

Re: QCA9984 bmi identification failure

[Christian Lamparter]

On Monday, March 27, 2017 1:33:54 PM CEST Sebastian Gottschall wrote:
> i dont know how to prove you that the firmware format is identical without
> simply showing you the hexdump.
We sort of know what is encoded in these calibration files in the flash and
in the board files. I've told you about the BoardData Files on github. If 
you look into the directory, you'll notice that for each of the board.bin
files there, there's a .txt with the same name:

e.g.:


You can use the identifiers from the file and compare the data from the flash
and the boardData file, you'll notice that they are differences. And the
difference is what caused all these problems. We ran into this last year
with the IPQ40XX. You can read all about it on the ath10k ML: This was one
of the threads:


> > which is what I said in the response as well. we both knew that
> > (from the beginning). If you want you can go on about it:
> > Please do. However, you should provide some data to back up your
> > claims and statements (logs, links to code or patches are fine I think).
> > Furthermore, let's keep the discussion civil and not go off on a
> > tangent and start a pissing contest. And finally, let's not forget
> > that the discussion is about the "QCA9984 bmi identification failure".
> ahm. sorry. we stop here. [...]
> i do not claim anything and i dont have you proof anything. [...]
> its up to you if you believe me or not.

Ok. Then let's stop here.

> > If so, then there's the Aerohive HiveAP 121.
> > . It has an AR934x SoC
> > and the internal WMAC is storing its calibration data in the SoC's OTP area.
> > The device is supported by ath9k. The device does have a wifi-cal/art
> > partition but it was empty.
> need to take a look into a flash memory dump so see if i find the 
> calibration data. the partition you see is created by lede as 
> preconfigured layout.
Well, if you are still interested and want to take a look at it.
I can ask Chris Blake, if he's willing to sent the complete flashdump
of the Aerohive HiveAP 121. It's one of those enterprise APs, it has 
a NAND and NOR chip, so from what I remember the image is like 20MiB+
zipped.

Thanks,
Christian

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 10:44:41 AM CEST Alban wrote:
> On Mon, 27 Mar 2017 18:11:15 +0200
> Christian Lamparter <chunk...@googlemail.com> wrote:
> 
> > On Monday, March 13, 2017 10:05:09 PM CEST Alban wrote:
> > > The current binding only cover PCI devices so extend it for SoC devices.
> > > 
> > > Most SoC platforms use an MTD partition for the calibration data
> > > instead of an EEPROM. The qca,no-eeprom property was added to allow
> > > loading the EEPROM content using firmware loading. This new binding
> > > replace this hack with NVMEM cells, so we also mark the qca,no-eeprom
> > > property as deprecated in case anyone ever used it.  
> > 
> > Please don't mark "qca,no-eeprom" as deprecated then.
> > If some devices geniously need to rely on userspace for extracting 
> > and processing the calibration data, it should be stay a
> > optional properties.
> 
> Deprecated just mean that it shouldn't be used for new devices. 
> But as it is not used by any board, misuse the firmware loading API and
> firmware loader user helper are deprecated in udev, I find we could also
> just drop it.
But LEDE/OpenWRT rely on the firmware loading API more than ever and 
currently there is not a replacement for it. From what I know, Luis 
tried to replace it with his sysdata approach:

<https://lkml.org/lkml/2016/12/16/204>

however, this was disliked by Greg KH and Linus for the following reasons.
<https://lkml.org/lkml/2016/6/16/995>:

|So I absolutely abhor "changes for changes sake".
|
|If the existing code works for existing drivers, let them keep it.
|
|And if a new interface is truly more flexible, then it should be able
|to implement the old interface with no changes, so that drivers
|shouldn't need to be changed/upgraded.
|
|Then, drivers that actually _want_ new features, or that can take
|advantage of new interfaces to actually make things *simpler*, can
|choose to make those changes. But those changes should have real
|advantages.
|[...]


your nvmem approach would need to be as universal and
powerful as the "qca,no-eeprom" + userspace solutions
in order to deprecate it.
 
> > For example: A device that can't do easily without "qca,no-eeprom" is
> > the AVM FRITZ!WLAN Repeater 300E. For this device, the caldata 
> > is stored in the flash, however for whatever reason the vendor
> > choose to "reverse" it. (like completely back to front, not byteswapped
> > or something). So an extra "unreversing step" is required. So, it would
> > require some sort of a special nvmem-provider-processor as an 
> > alternative.
> 
> Or just handle this special eeprom format in the ath9k driver. I doubt
> that this case is so common that it would justify adding a whole new
> layer to nvmem.
Well, you'll have to deal with it in nvmem, if you want it to deprecate
"qca,no-eeprom".

I looked into 10-ath9k-eeprom [0] of LEDE's AR71XX target and I noticed
that quite a few devices patch the MACs of the wifi.
If you look at the code for the Airtight C-55 and C-60, Meraki MR18,
Meraki Z1, you'll notice that each one has to add a fixed value (+1,
+2, ...) to the extraced MAC-Address. So how would you replicate this,
with "nvmem-cell-names = address" without some sort of 
nvmem-provider-processor?

Also, there's another usecase of a nvmem-provider-processor. 
For example, one could be convert all the different types of
ascii-macs (Either strings like "00:11:22:33:44:55", 
"00.11.22.33..." or "00112233..." ) to their binary representation.
For AR71XX, this is mostly done by ath79_parse_ascii_mac:

https://github.com/lede-project/source/blob/master/target/linux/ar71xx/files/arch/mips/ath79/dev-eth.c#L1204

and grep lists the following devices:
mach-dgl-5500-a1.c, mach-dhp-1565-a1.c, mach-dir-505-a1.c, mach-dir-615-c1.c
mach-dir-615-i1.c, mach-dir-825-b1.c, mach-dir-825-c1.c, mach-tew-673gru.c
mach-tew-712br.c, mach-tew-732br.c, mach-tew-823dru.c

I did a quick check: All of them use the extracted MACs for ath9k
and/or ethernet.

Note: I think Ralink/MediaTek will have the same issues.

> > >  Optional properties:
> > > +- mac-address: See ethernet.txt in the parent directory
> > > +- local-mac-address: See ethernet.txt in the parent directory
> > > [...]
> > > +
> > > +Deprecated properties:
> > >  - 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).
> > > -- mac-address: See ethernet.txt in the parent directory
> > > -- local-mac-addres

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Monday, March 13, 2017 10:05:09 PM CEST Alban wrote:
> The current binding only cover PCI devices so extend it for SoC devices.
> 
> Most SoC platforms use an MTD partition for the calibration data
> instead of an EEPROM. The qca,no-eeprom property was added to allow
> loading the EEPROM content using firmware loading. This new binding
> replace this hack with NVMEM cells, so we also mark the qca,no-eeprom
> property as deprecated in case anyone ever used it.

Please don't mark "qca,no-eeprom" as deprecated then.
If some devices geniously need to rely on userspace for extracting 
and processing the calibration data, it should be stay a
optional properties.

For example: A device that can't do easily without "qca,no-eeprom" is
the AVM FRITZ!WLAN Repeater 300E. For this device, the caldata 
is stored in the flash, however for whatever reason the vendor
choose to "reverse" it. (like completely back to front, not byteswapped
or something). So an extra "unreversing step" is required. So, it would
require some sort of a special nvmem-provider-processor as an 
alternative.

> Signed-off-by: Alban 
> ---
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt 
> b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> index b7396c8..61f5f6d 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -27,16 +27,34 @@ Required properties:
>   - 0034 for AR9462
>   - 0036 for AR9565
>   - 0037 for AR9485
> + For SoC devices the compatible should be "qca,-wmac"
> + and one of the following fallbacks:
> + - "qca,ar9100-wmac"
> + - "qca,ar9330-wmac"
> + - "qca,ar9340-wmac"
> + - "qca,qca9550-wmac"
> + - "qca,qca9530-wmac"
>  - reg: Address and length of the register set for the device.
>  
> +Required properties for SoC devices:
> +- interrupt-parent: phandle of the parent interrupt controller.
> +- interrupts: Interrupt specifier for the controllers interrupt.
> +
>  Optional properties:
> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +- clock-names: has to be "ref"
> +- clocks: phandle of the reference clock
> +- resets: phandle of the reset line
> +- nvmem-cell-names: has to be "eeprom" and/or "address"
> +- nvmem-cells: phandle to the eeprom nvmem cell and/or to the mac address
> + nvmem cell.
> +
> +Deprecated properties:
>  - 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).
> -- mac-address: See ethernet.txt in the parent directory
> -- local-mac-address: See ethernet.txt in the parent directory
> -
It sounds like you want to deprecate mac-address and local-mac-address as well.
If so you sould add this to the commit as well. From my point of view, people
mostly flat-out patched the eeprom-image if they wanted to set the mac-address.
However, this was an extra step, if nvmem does away with it, I'm completely
fine with deprecating these properties.

Thanks,
Christian

Re: QCA9984 bmi identification failure

[Christian Lamparter]

On Saturday, March 25, 2017 8:24:59 AM CET Sebastian Gottschall wrote:
> Am 24.03.2017 um 16:01 schrieb Christian Lamparter:
> > On Friday, March 24, 2017 11:09:03 AM CET Sebastian Gottschall wrote:
> >> i have a r7800 running. consider to use the board.bin file which is
> >> stored in flash memory of the r7800.
> > Well, this is a bit beside the point. But what makes you think that
> > what is stored in the flash memory of R7800 is the "board.bin"?
> i dont know how to answer this question without getting rude. 
> i'm developing dd-wrt which is a alternate firmware like lede/openwrt for 
> many hundrets of different routers.this is my job since more than 10 years.
> [...]

Well. What you're posting to the ML, is entirely your own decision.
But let's focus again on:

> > On Friday, March 24, 2017 11:09:03 AM CET Sebastian Gottschall wrote:
> > i have a r7800 running. consider to use the board.bin file which is 
> > stored in flash memory of the r7800.
> > there are 2 stored for both cards. you need to patch ath10k to use 
> > different board.bin files for each card.
> > this is a log from a working r7800 running dd-wrt. the failed to fetch 
> > board data error is normal. it will use api1 board files then which i 
> > provide on fs based on the board data stored in flash memory

What makes you think that what you do with the data in the flash is
the "board.bin"? Do you have any documentation to back up your statement?
I know that you have been reporting about the QCA99x0. there even
is a patch with your "reported-by" tag: 
"ath10k: fix calibration init sequence of qca99x0".
<http://lists.infradead.org/pipermail/ath10k/2016-March/007173.html>

Clearly, you must have read it. So you know that:
"[...] whereas calibration file is used by ar9888 and qca99x0 that contains
both board data and caldata. [...]"

which is what I said in the response as well. we both knew that
(from the beginning). If you want you can go on about it:
Please do. However, you should provide some data to back up your
claims and statements (logs, links to code or patches are fine I think).
Furthermore, let's keep the discussion civil and not go off on a
tangent and start a pissing contest. And finally, let's not forget
that the discussion is about the "QCA9984 bmi identification failure".

which seems to be also affecting your unit, since you wrote about the same 
issue in your first mail:
> > On Friday, March 24, 2017 11:09:03 AM CET Sebastian Gottschall wrote:
> > you need to patch ath10k to use different board.bin files for each card.

and

> > the failed to fetch board data error is normal. 

Why do you need to patch the ath10k driver? And why is it, that even
you have patched it, ath10k is still producing an error message? And
why is it normal(save?) to ignore it?

> > I know that Netgear provided a myriad of different board data files
> > with in there GPL drop:
> you can ignore them. use the files stored in flash memory. this board 
> data is the calibration data which is different for each device you buy. 
> its precalibrated and stored in flash memory.
If this was the case, then why does the ath10k-firmware and codeaurora.org
provide the board files in the firmware repositories?
<https://github.com/kvalo/ath10k-firmware/tree/master/QCA9984/hw1.0>
<https://source.codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware/plain/ath10k/QCA9984/hw1.0/>
 
Also, why does ath10k insist on requesting the board-2.bin files, even after
it has loaded the calibration file which is supposed to contain both 
(for the QCA9984 and others)?
Would it be easier just to request "one file" and not two different files
with the same content? 

Note: I know that LEDE currently puts the data from the flash
into the cal-pci-:01:00.0.bin and creates a symlink to "board.bin".
<https://git.lede-project.org/?p=source.git;a=blob;f=package/firmware/ath10k-firmware/Makefile;h=febf7d26794bd8c5b34bd6703e88cf8070e213a1;hb=HEAD#l323>
I expect to see something similar in DD-WRT. If not, please tell us about your
method.

> a normal wifi card has a own eeprom on it which stores this data. but on 
> embedded devices the routers own flash memory is used for storing this data.
> this case is mainly ignored by drivers like ath10k, so patches are 
> required right now until ath10k does officially support these conditions
The QCA9984 support was added by this patch back in August 2016:  
"ath10k: enable support for QCA9984"
https://patchwork.kernel.org/patch/8890981/

At this point, I think ath10k should support the QCA9984 in the R7800
"out of the box" without any need for special or custom patches.
LEDE's compat-wireless is dated 2017-01-31.
> > Here's a link:
> > <https://github.

Re: [PATCH 3/7] ath9k: Add support for reading the EEPROM data using the nvmem API

[Christian Lamparter]

On Thursday, March 23, 2017 3:43:28 PM CET Alban wrote:
> On Tue, 14 Mar 2017 00:53:55 +0100
> Christian Lamparter <chunk...@googlemail.com> wrote:
> 
> > On Monday, March 13, 2017 10:05:11 PM CET Alban wrote:
> > > Currently SoC platforms use a firmware request to get the EEPROM data.
> > > This is mostly a hack and rely on using a user-helper scripts which is
> > > deprecated. A nicer alternative is to use the nvmem API which was
> > > designed for this kind of task.
> > > 
> > > Furthermore we let CONFIG_ATH9K_AHB select CONFIG_NVMEM as such
> > > devices will generally use this method for loading the EEPROM data.
> > > 
> > > Signed-off-by: Alban <al...@free.fr>
> > > ---
> > > @@ -654,6 +656,25 @@ static int ath9k_init_softc(u16 devid, struct 
> > > ath_softc *sc,
> > >   if (ret)
> > >   return ret;
> > >  
> > > + /* If the EEPROM hasn't been retrieved via firmware request
> > > +  * use the nvmem API insted.
> > > +  */
> > > + if (!ah->eeprom_blob) {
> > > + struct nvmem_cell *eeprom_cell;
> > > +
> > > + eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
> > > + if (!IS_ERR(eeprom_cell)) {
> > > + ah->eeprom_data = nvmem_cell_read(
> > > + eeprom_cell, >eeprom_size);
> > > + nvmem_cell_put(eeprom_cell);
> > > +
> > > + if (IS_ERR(ah->eeprom_data)) {
> > > + dev_err(ah->dev, "failed to read eeprom");
> > > + return PTR_ERR(ah->eeprom_data);
> > > + }
> > > + }
> > > + }
> > > +
> > >   if (ath9k_led_active_high != -1)
> > >   ah->config.led_active_high = ath9k_led_active_high == 1;
> > >
> > Are you sure this works with AR93XX SoCs that have the calibration data
> > in the OTP?
> 
> I only tested this on an ar9132 platform, so it might well be that a
> few things are missing for the newer SoC. However this shouldn't break
> anything existing as a platform needs to define an nvmem cell on the
> athk9 device for this code to be used a all.

Yes, I looked at it.
+   eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
+   if (!IS_ERR(eeprom_cell)) {
+   ...
+   }
So if there's a error with the "eeprom" nvmem property,
(i.e.: it's not present) the driver should works as before.

Thanks,
Christian

Re: QCA9984 bmi identification failure

[Christian Lamparter]

On Friday, March 24, 2017 11:09:03 AM CET Sebastian Gottschall wrote:
> i have a r7800 running. consider to use the board.bin file which is 
> stored in flash memory of the r7800.
Well, this is a bit beside the point. But what makes you think that 
what is stored in the flash memory of R7800 is the "board.bin"? 
I know that Netgear provided a myriad of different board data files
with in there GPL drop:

Here's a link:


So, does the data in your flash matches any of those files 1:1 or not?

(Note: From what I know, it's the caldata that's in the flash. 
caldata ≈ cal+board. But I'm asking why ath10k's bmi identification
isn't working for those chips right now. And judging from your logs,
you are using probably a similar WA to the 
936-ath10k_skip_otp_check.patch out of necessity as well.)

> there are 2 stored for both cards. you need to patch ath10k to use 
> different board.bin files for each card.
Exactly. Why do you (or anyone for that matter) need to patch ath10k?
The driver is supposed to support the QCA9984 out of the box, right?

And I know, that the bmi identification is supposed to work, as
somebody posted the following log:


[  379.392210] ath10k_pci 0002:01:00.0: boot upload otp to 0x1234 len 9000 for 
board id
[  379.399945] ath10k_pci 0002:01:00.0: bmi fast download address 0x1234 buffer 
0xe1676038 length 9000
[  379.408977] ath10k_pci 0002:01:00.0: bmi lz stream start address 0x1234
[  379.415603] ath10k_pci 0002:01:00.0: bmi lz data buffer 0xe1676038 length 
9000
[  379.451626] ath10k_pci 0002:01:00.0: bmi lz stream start address 0x0
[  379.457985] ath10k_pci 0002:01:00.0: bmi execute address 0x1234 param 0x10
[  380.857006] ath10k_pci 0002:01:00.0: bmi execute result 0x400
[  380.862749] ath10k_pci 0002:01:00.0: boot get otp board id result 0x0400 
board_id 1 chip_id 0
[  380.871603] ath10k_pci 0002:01:00.0: boot using board name 
'bus=pci,bmi-chip-id=0,bmi-board-id=1'
[  380.880468] ath10k_pci 0002:01:00.0: board name
[  380.884999] ath10k_pci 0002:01:00.0: : 62 75 73 3d 70 63 69 2c 62 6d 
69 2d 63 68 69 70  bus=pci,bmi-chip
[  380.895159] ath10k_pci 0002:01:00.0: 0010: 2d 69 64 3d 30 2c 62 6d 69 2d 
62 6f 61 72 64 2d  -id=0,bmi-board-
[  380.905317] ath10k_pci 0002:01:00.0: 0020: 69 64 3d 31   
   id=1
[  380.914436] ath10k_pci 0002:01:00.0: boot found match for name 
'bus=pci,bmi-chip-id=0,bmi-board-id=1'
[  380.923640] ath10k_pci 0002:01:00.0: boot found board data for 
'bus=pci,bmi-chip-id=0,bmi-board-id=1'
[  380.932845] ath10k_pci 0002:01:00.0: using board api 2
...

The board name for the QCA9984 is supposed to look like
"'bus=pci,bmi-chip-id=0,bmi-board-id=1'"

and not like (from your log):
> bus=pci,vendor=168c,device=0046,subsystem-vendor=168c,subsystem-device=cafe 
> from ath10k/QCA9984/hw1.0/board-2.bin

> the failed to fetch board data error is normal. 
I don't think it is. I think it's a regression.

Thanks,
Christian

QCA9984 bmi identification failure

[Christian Lamparter]

Hannu Nyman reported a issue with the QCA9984 in his Netgear R7800 
and LEDE's ath10k: (This is with 936-ath10k_skip_otp_check.patch removed):

[   12.999550] ath10k_pci :01:00.0: enabling device (0140 -> 0142)
[   12.999637] ath10k_pci :01:00.0: enabling bus mastering
[   13.000105] ath10k_pci :01:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 
reset_mode 0
[   13.130838] ath10k_pci :01:00.0: Direct firmware load for 
ath10k/pre-cal-pci-:01:00.0.bin failed with error -2
[   13.130888] ath10k_pci :01:00.0: Falling back to user helper
[   13.183995] firmware ath10k!pre-cal-pci-:01:00.0.bin: 
firmware_loading_store: map pages failed
[   13.184338] ath10k_pci :01:00.0: Direct firmware load for 
ath10k/cal-pci-:01:00.0.bin failed with error -2
[   13.191960] ath10k_pci :01:00.0: Falling back to user helper
[   13.673417] ath10k_pci :01:00.0: qca9984/qca9994 hw1.0 target 0x0100 
chip_id 0x sub 168c:cafe
[   13.673451] ath10k_pci :01:00.0: kconfig debug 0 debugfs 1 tracing 0 dfs 
1 testmode 1
[   13.684393] ath10k_pci :01:00.0: firmware ver 10.4-3.4-00074 api 5 
features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast crc32 fa32e88e
[   15.728598] ath10k_pci :01:00.0: unable to read from the device
[   15.728621] ath10k_pci :01:00.0: could not execute otp for board id 
check: -110
[   15.733663] ath10k_pci :01:00.0: failed to get board id from otp: -110
[   15.741474] ath10k_pci :01:00.0: could not probe fw (-110)

I requested to test what happens, if the driver ignored -ETIMEDOUT error from
ath10k_core_get_board_id_from_otp() and the device initialized successfully:

[   16.163318] ath10k_pci :01:00.0: enabling device (0140 -> 0142)
[   16.163401] ath10k_pci :01:00.0: enabling bus mastering
[   16.163850] ath10k_pci :01:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 
reset_mode 0
[   16.337294] ath10k_pci :01:00.0: Direct firmware load for 
ath10k/pre-cal-pci-:01:00.0.bin failed with error -2
[   16.337351] ath10k_pci :01:00.0: Falling back to user helper
[   22.837360] firmware ath10k!pre-cal-pci-:01:00.0.bin: 
firmware_loading_store: map pages failed
[   23.212157] ath10k_pci :01:00.0: qca9984/qca9994 hw1.0 target 0x0100 
chip_id 0x sub 168c:cafe
[   23.212211] ath10k_pci :01:00.0: kconfig debug 0 debugfs 1 tracing 0 dfs 
1 testmode 1
[   23.226748] ath10k_pci :01:00.0: firmware ver 10.4-3.4-00074 api 5 
features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast crc32 fa32e88e
[   25.259266] ath10k_pci :01:00.0: unable to read from the device
[   25.259288] ath10k_pci :01:00.0: could not execute otp for board id 
check: -110
[   25.277326] ath10k_pci :01:00.0: failed to fetch board data for 
bus=pci,vendor=168c,device=0046,subsystem-vendor=168c,subsystem-device=cafem...from
 ath10k/QCA9984/hw1.0/board-2.bin
[   25.277588] ath10k_pci :01:00.0: board_file api 1 bmi_id N/A crc32 
dd636801
[   26.800717] ath10k_pci :01:00.0: htt-ver 2.2 wmi-op 6 htt-op 4 cal file 
max-sta 512 raw 0 hwcrypto 1



What seems strange is that only the call bmi_execute with 
BMI_PARAM_GET_EEPROM_BOARD_ID is timing out. So by just 
ignoring the -ETIMEDOUT result from:

ret = ath10k_bmi_execute(ar, address, BMI_PARAM_GET_EEPROM_BOARD_ID,
 );

in ath10k_core_get_board_id_from_otp() the device will initialize and work.
This begs the question, what is so special about the 
BMI_PARAM_GET_EEPROM_BOARD_ID
at that time for the QCA9984? Does the device need some extra msleep time after
the OTP has been uploaded? Or is the BMI_PARAM_GET_EEPROM_BOARD_ID not 
implemented/has a different ID, etc... ?

Thanks,
Christian

Re: [PATCH 3/7] ath9k: Add support for reading the EEPROM data using the nvmem API

[Christian Lamparter]

On Monday, March 13, 2017 10:05:11 PM CET Alban wrote:
> Currently SoC platforms use a firmware request to get the EEPROM data.
> This is mostly a hack and rely on using a user-helper scripts which is
> deprecated. A nicer alternative is to use the nvmem API which was
> designed for this kind of task.
> 
> Furthermore we let CONFIG_ATH9K_AHB select CONFIG_NVMEM as such
> devices will generally use this method for loading the EEPROM data.
> 
> Signed-off-by: Alban 
> ---
> @@ -654,6 +656,25 @@ static int ath9k_init_softc(u16 devid, struct ath_softc 
> *sc,
>   if (ret)
>   return ret;
>  
> + /* If the EEPROM hasn't been retrieved via firmware request
> +  * use the nvmem API insted.
> +  */
> + if (!ah->eeprom_blob) {
> + struct nvmem_cell *eeprom_cell;
> +
> + eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
> + if (!IS_ERR(eeprom_cell)) {
> + ah->eeprom_data = nvmem_cell_read(
> + eeprom_cell, >eeprom_size);
> + nvmem_cell_put(eeprom_cell);
> +
> + if (IS_ERR(ah->eeprom_data)) {
> + dev_err(ah->dev, "failed to read eeprom");
> + return PTR_ERR(ah->eeprom_data);
> + }
> + }
> + }
> +
>   if (ath9k_led_active_high != -1)
>   ah->config.led_active_high = ath9k_led_active_high == 1;
>  
Are you sure this works with AR93XX SoCs that have the calibration data
in the OTP?

I've added Chris to the CC, since he has a HiveAP 121 that has this
configuration, so he can test, whenever this is a problem or not.

But from what I can tell, devices with the calibration data in the
OTP would fail now. This is because the OTP is done by ath9k_hw_init()
which hasn't run yet (it's a bit further down the road in the same
function though). 

Note: the OTP doesn't store the whole caldata. Just a few parts.
A temporary "eeprom" gets constructed with the help of the 
ar9300_eep_templates in ar9003_eeprom.c's
ar9300_eeprom_restore_internal(). But from what I don't think, 
that the eeprom_blob is constructed/set by the functions in
ar9003_eeprom.

I think this will require an additional property like
qca,calibration-in-otp. What's your opinion?

Thanks,
Christian

Re: [PATCH v5] qtnfmac: announcement of new FullMAC driver for Quantenna chipsets

[Christian Lamparter]

On Saturday, March 11, 2017 11:04:17 PM CET Igor Mitsyanko wrote:
> This patch adds support for new FullMAC WiFi driver for Quantenna
> QSR10G chipsets.
> 
> QSR10G (aka Pearl) is Quantenna's 8x8, 160M, 11ac offering.
> QSR10G supports 2 simultaneous WMACs - one 5G and one 2G.
> 5G WMAC supports 160M, 8x8 configuration. FW supports
> up to 8 concurrent virtual interfaces on each WMAC.
> [...]
---

I see a lot of error/warning/info messages the likes of:
 
pr_err("failed to change interface type\n");
pr_err("bss not started\n");
pr_err("failed to configure phy thresholds\n");
pr_err("failed to add key\n");
pr_err("failed to delete key\n");
pr_err("failed to change STA\n");
...

I'm wondering what this will look like if there are multiple VIFs,
multiple PHYs or more than one QSR10G in a System. How - for example -
can you map a error/warning/info message to the individual Client, VIF,
PHY instance or device? From what I can tell, this is bound to get very
confusing very fast.

the kernel and cfg80211 has a ton of predefined logging and debug
functions: dev_[info|warn|err|...], wiphy_[WARN|emerg|alert|crit|err|warn|...]
I think using those functions (and adding information about the client/vif)
would tremendously help with that?

Thanks,
Christian

Re: [PATCH 2/2] ath10k: search DT for qcom,ath10k-calibration-variant

[Christian Lamparter]

On Friday, March 10, 2017 9:06:15 AM CET Sven Eckelmann wrote:
> Board Data File (BDF) is loaded upon driver boot-up procedure. The right
> board data file is identified on QCA4019 using bus, bmi-chip-id and
> bmi-board-id.
> 
> The problem, however, can occur when the (default) board data file cannot
> fulfill with the vendor requirements and it is necessary to use a different
> board data file.
> 
> This problem was solved for SMBIOS by adding a special SMBIOS type 0xF8.
> Something similar has to be provided for systems without SMBIOS but with
> device trees. No solution was specified by QCA and therefore a new one has
> to be found for ath10k.
> 
> The device tree requires addition strings to define the variant name
> 
> wifi@a00 {
>   status = "okay";
>   qcom,ath10k-calibration-variant = "RT-AC58U";
> };
> 
> wifi@a80 {
>   status = "okay";
>   qcom,ath10k-calibration-variant = "RT-AC58U";
> };
> 
> This would create the boarddata identifiers for the board-2.bin search
> 
>  *  bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=RT-AC58U
>  *  bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=RT-AC58U
> 
> Signed-off-by: Sven Eckelmann 
> ---
I've attached a modified board-2.bin for the RT-AC58U variant:

# ath10k-bdencoder -i board-2.bin
FileSize: 48564
FileCRC32: 7ac95dfd
FileMD5: 825c8e7377b0d543024dbf62f2fd29ff
BoardNames[0]: 'bus=ahb,bmi-chip-id=0,bmi-board-id=16,variant=RT-AC58U'
BoardLength[0]: 12064
BoardCRC32[0]: bd216dd3
BoardMD5[0]: a12a4745c775beb5ab6ba1e4d711aea0
BoardNames[1]: 'bus=ahb,bmi-chip-id=0,bmi-board-id=16'
BoardLength[1]: 12064
BoardCRC32[1]: 65a8a5b3
BoardMD5[1]: cb5dcb0337706c313ea3342587b283ae
BoardNames[2]: 'bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=RT-AC58U'
BoardLength[2]: 12064
BoardCRC32[2]: a7d74de3
BoardMD5[2]: 17bbd05ee0cd9099a549c835b7399b3c
BoardNames[3]: 'bus=ahb,bmi-chip-id=0,bmi-board-id=17'
BoardLength[3]: 12064
BoardCRC32[3]: 4fe93ca0
BoardMD5[3]: 1aa45fad7a0d6f1c5774b251c712c67c

And this is a patched ath10k loading it:

[  106.155058] ath10k_ahb a80.wifi: boot get otp board id result 0x4400 
board_id 17 chip_id 0
[  106.155092] ath10k_ahb a80.wifi: SMBIOS bdf variant name not set.
[  106.155154] ath10k_ahb a80.wifi: boot using board name 
'bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=RT-AC58U'
[  106.155570] ath10k_ahb a80.wifi: board name
[  106.155607] ath10k_ahb a80.wifi: : 62 75 73 3d 61 68 62 2c 62 6d 
69 2d 63 68 69 70  bus=ahb,bmi-chip
[  106.155644] ath10k_ahb a80.wifi: 0010: 2d 69 64 3d 30 2c 62 6d 69 2d 
62 6f 61 72 64 2d  -id=0,bmi-board-
[  106.155681] ath10k_ahb a80.wifi: 0020: 69 64 3d 31 36 2c 76 61 72 69 
61 6e 74 3d 52 54  id=16,variant=RT
[  106.155904] ath10k_ahb a80.wifi: 0030: 2d 41 43 35 38 55 
   -AC58U
[  106.155972] ath10k_ahb a80.wifi: board name
[  106.156057] ath10k_ahb a80.wifi: : 62 75 73 3d 61 68 62 2c 62 6d 
69 2d 63 68 69 70  bus=ahb,bmi-chip
[  106.156142] ath10k_ahb a80.wifi: 0010: 2d 69 64 3d 30 2c 62 6d 69 2d 
62 6f 61 72 64 2d  -id=0,bmi-board-
[  106.156211] ath10k_ahb a80.wifi: 0020: 69 64 3d 31 36
   id=16
[  106.156662] ath10k_ahb a80.wifi: board name
[  106.156700] ath10k_ahb a80.wifi: : 62 75 73 3d 61 68 62 2c 62 6d 
69 2d 63 68 69 70  bus=ahb,bmi-chip
[  106.156737] ath10k_ahb a80.wifi: 0010: 2d 69 64 3d 30 2c 62 6d 69 2d 
62 6f 61 72 64 2d  -id=0,bmi-board-
[  106.156773] ath10k_ahb a80.wifi: 0020: 69 64 3d 31 37 2c 76 61 72 69 
61 6e 74 3d 52 54  id=17,variant=RT
[  106.156809] ath10k_ahb a80.wifi: 0030: 2d 41 43 35 38 55 
   -AC58U
[  106.156845] ath10k_ahb a80.wifi: boot found match for name 
'bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=RT-AC58U'
[  106.156876] ath10k_ahb a80.wifi: boot found board data for 
'bus=ahb,bmi-chip-id=0,bmi-board-id=17,variant=RT-AC58U'
[  106.156906] ath10k_ahb a80.wifi: using board api 2
[  106.163704] ath10k_ahb a80.wifi: board_file api 2 bmi_id 0:17 crc32 
f222ba02
[  106.165037] ath10k_ahb a80.wifi: boot push board extended data addr 0x0
[  106.184867] ath10k_ahb a80.wifi: boot cal file downloaded
[  106.184905] ath10k_ahb a80.wifi: boot using calibration mode pre-cal-file
[  106.184945] ath10k_ahb a80.wifi: boot upload otp to 0x1234 len 4582 for 
board id
[  106.205657] ath10k_ahb a80.wifi: boot get otp board id result 0x4400 
board_id 17 chip_id 0
[  106.205811] ath10k_ahb a80.wifi: boot push board extended data addr 0x0
[  106.225685] ath10k_ahb a80.wifi: boot upload otp to 0x1234 len 4582
[  106.252294] ath10k_ahb a80.wifi: boot otp execute result 0

@Aeolus Yang / Kalle / QCA: Would it be possible to assign a variant string to
the Asus RT-AC58U?

I've attached the necessary bmi-board-id=16 and bmi-board-id=17 board 
files to this mail as well. 

[PATCH] ath9k: use correct OTP register offsets for the AR9340 and AR9550

[Christian Lamparter]

This patch fixes the OTP register definitions for the AR934x and AR9550
WMAC SoC.

Previously, the ath9k driver was unable to initialize the integrated
WMAC on an Aerohive AP121:

| ath: phy0: timeout (1000 us) on reg 0x30018: 0xbadc0ffe & 0x0007 != 
0x0004
| ath: phy0: timeout (1000 us) on reg 0x30018: 0xbadc0ffe & 0x0007 != 
0x0004
| ath: phy0: Unable to initialize hardware; initialization status: -5
| ath9k ar934x_wmac: failed to initialize device
| ath9k: probe of ar934x_wmac failed with error -5

It turns out that the AR9300_OTP_STATUS and AR9300_OTP_DATA
definitions contain a typo.

Cc: Gabor Juhos <juh...@openwrt.org>
Cc: sta...@vger.kernel.org
Fixes: add295a4afbdf5852d0 "ath9k: use correct OTP register offsets for AR9550"
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
Signed-off-by: Chris Blake <chrisrblak...@gmail.com>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.h 
b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.h
index 7dc7205dc877..bd2269c7de6b 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.h
@@ -75,13 +75,13 @@
 #define AR9300_OTP_BASE \
((AR_SREV_9340(ah) || AR_SREV_9550(ah)) ? 0x3 : 0x14000)
 #define AR9300_OTP_STATUS \
-   ((AR_SREV_9340(ah) || AR_SREV_9550(ah)) ? 0x30018 : 0x15f18)
+   ((AR_SREV_9340(ah) || AR_SREV_9550(ah)) ? 0x31018 : 0x15f18)
 #define AR9300_OTP_STATUS_TYPE 0x7
 #define AR9300_OTP_STATUS_VALID0x4
 #define AR9300_OTP_STATUS_ACCESS_BUSY  0x2
 #define AR9300_OTP_STATUS_SM_BUSY  0x1
 #define AR9300_OTP_READ_DATA \
-   ((AR_SREV_9340(ah) || AR_SREV_9550(ah)) ? 0x3001c : 0x15f1c)
+   ((AR_SREV_9340(ah) || AR_SREV_9550(ah)) ? 0x3101c : 0x15f1c)
 
 enum targetPowerHTRates {
HT_TARGET_RATE_0_8_16,
-- 
2.11.0

Re: [RFC] [PULL REQUEST] rt2x00 patches from OpenWrt.org

[Christian Lamparter]

On Friday, January 13, 2017 4:46:30 PM CET Daniel Golle wrote:
> On Fri, Jan 13, 2017 at 12:46:56PM +0200, Kalle Valo wrote:
> > Daniel Golle  writes:
> > > ...
> > > Please review and comment, so we can get those patches merged!
> > 
> > No pull requests, please. Instead send these as patches, easier to
> > review and actually also easier for me to merge.
> 
> The advantage of pull requests is that author information can be
> preserved more easily. Running git format-patch results in most patches
> having wrong SMTP sender information due to the assumption that the
> patch author is the same person also submitting the patch.
> So in practise, this would either require changing the From: (and thus
> Author) to myself or having most mails eaten by anti-spam measures due
> to non-matching SPF which prohibits my SMTP to send mail on behalf of
> the original authors of the patches.
> 
> How do you suggest to handle this situation?
> 
>From what I know, git format-patch and send-email [0] will add a second
FROM: in the email's body with the author of the commit automatically
(if author isn't you). This is what it did, when I posted the apm821xx
patches on lede-dev [1] (Look at the additional "FROM: Chris Blake ..."
line in these patches. Whereas the mail came from my address).

The MTA (MDA, ...) will use the first FROM: (your address) whereas git will
use the FROM in the mail body (so the patch will be correctly attributed to
the original patch author). If you don't want to bother the original
authors, you can look at the --suppress-cc=author option and enable --dry-run
on git send-email.

I would say, just give it a "dry".
(Sadly, I didn't find any documentation for this feature.
But I know it worked back then and it should be fine with SPF.)

Regards,
Christian

[0] 
[1] 

[PATCH] ath9k: move RELAY and DEBUG_FS to ATH9K[_HTC]_DEBUGFS

[Christian Lamparter]

Currently, the common ath9k_common module needs to have a
dependency on RELAY and DEBUG_FS in order to built. This
is usually not a problem. But for RAM and FLASH starved
AR71XX devices, every little bit counts.

This patch adds a new symbol CONFIG_ATH9K_COMMON_DEBUG
which makes it possible to drop the RELAY and DEBUG_FS
dependency there and move it to ATH_(HTC)_DEBUGFS.

Note: The shared FFT/spectral code (which is the only user
of the relayfs in ath9k*) needs DEBUG_FS to export the relayfs
interface to dump the data to userspace. So it makes no sense
to have the functions compiled in, if DEBUG_FS is not there.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
Link to the RFC: <https://patchwork.kernel.org/patch/9501361/>.
---
 drivers/net/wireless/ath/ath9k/Kconfig   |  9 ++--
 drivers/net/wireless/ath/ath9k/Makefile  |  5 +++--
 drivers/net/wireless/ath/ath9k/common-debug.h| 27 
 drivers/net/wireless/ath/ath9k/common-spectral.c |  2 +-
 drivers/net/wireless/ath/ath9k/common-spectral.h | 23 
 drivers/net/wireless/ath/ath9k/eeprom_4k.c   |  2 +-
 drivers/net/wireless/ath/ath9k/eeprom_9287.c |  2 +-
 drivers/net/wireless/ath/ath9k/eeprom_def.c  |  2 +-
 8 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig 
b/drivers/net/wireless/ath/ath9k/Kconfig
index 8f231c67dd51..783a38f1a626 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -3,8 +3,8 @@ config ATH9K_HW
 config ATH9K_COMMON
tristate
select ATH_COMMON
-   select DEBUG_FS
-   select RELAY
+config ATH9K_COMMON_DEBUG
+   bool
 config ATH9K_DFS_DEBUGFS
def_bool y
depends on ATH9K_DEBUGFS && ATH9K_DFS_CERTIFIED
@@ -60,12 +60,14 @@ config ATH9K_DEBUGFS
bool "Atheros ath9k debugging"
depends on ATH9K && DEBUG_FS
select MAC80211_DEBUGFS
+   select ATH9K_COMMON_DEBUG
select RELAY
---help---
  Say Y, if you need access to ath9k's statistics for
  interrupts, rate control, etc.
 
  Also required for changing debug message flags at run time.
+ As well as access to the FFT/spectral data and TX99.
 
 config ATH9K_STATION_STATISTICS
bool "Detailed station statistics"
@@ -174,8 +176,11 @@ config ATH9K_HTC
 config ATH9K_HTC_DEBUGFS
bool "Atheros ath9k_htc debugging"
depends on ATH9K_HTC && DEBUG_FS
+   select ATH9K_COMMON_DEBUG
+   select RELAY
---help---
  Say Y, if you need access to ath9k_htc's statistics.
+ As well as access to the FFT/spectral data.
 
 config ATH9K_HWRNG
bool "Random number generator support"
diff --git a/drivers/net/wireless/ath/ath9k/Makefile 
b/drivers/net/wireless/ath/ath9k/Makefile
index 76f9dc37500b..36a40ffdce15 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -60,8 +60,9 @@ obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
 ath9k_common-y:=   common.o \
common-init.o \
common-beacon.o \
-   common-debug.o \
-   common-spectral.o
+
+ath9k_common-$(CONFIG_ATH9K_COMMON_DEBUG) += common-debug.o \
+common-spectral.o
 
 ath9k_htc-y += htc_hst.o \
hif_usb.o \
diff --git a/drivers/net/wireless/ath/ath9k/common-debug.h 
b/drivers/net/wireless/ath/ath9k/common-debug.h
index 7c9788490f7f..3376990d3a24 100644
--- a/drivers/net/wireless/ath/ath9k/common-debug.h
+++ b/drivers/net/wireless/ath/ath9k/common-debug.h
@@ -60,6 +60,7 @@ struct ath_rx_stats {
u32 rx_spectral;
 };
 
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
 void ath9k_cmn_debug_modal_eeprom(struct dentry *debugfs_phy,
  struct ath_hw *ah);
 void ath9k_cmn_debug_base_eeprom(struct dentry *debugfs_phy,
@@ -70,3 +71,29 @@ void ath9k_cmn_debug_recv(struct dentry *debugfs_phy,
  struct ath_rx_stats *rxstats);
 void ath9k_cmn_debug_phy_err(struct dentry *debugfs_phy,
 struct ath_rx_stats *rxstats);
+#else
+static inline void ath9k_cmn_debug_modal_eeprom(struct dentry *debugfs_phy,
+   struct ath_hw *ah)
+{
+}
+
+static inline void ath9k_cmn_debug_base_eeprom(struct dentry *debugfs_phy,
+  struct ath_hw *ah)
+{
+}
+
+static inline void ath9k_cmn_debug_stat_rx(struct ath_rx_stats *rxstats,
+  struct ath_rx_status *rs)
+{
+}
+
+static inline void ath9k_cmn_debug_recv(struct dentry *debugfs_phy,
+   struct ath_rx_stats *rxstats)
+{
+}
+
+static inline void ath9k_cmn_debug_phy_err(struct dentry *debugfs_phy,
+ 

[RFC] ath9k: move RELAY and DEBUG_FS to ATH9K[_HTC]_DEBUGFS

[Christian Lamparter]

Currently, the common ath9k_common module needs to have a
dependency on RELAY and DEBUG_FS in order to built. This
is usually not a problem. But for RAM and FLASH starved
AR71XX devices, every little bit counts.

This patch adds a new symbol CONFIG_ATH9K_COMMON_DEBUG
which makes it possible to drop the RELAY and DEBUG_FS
dependency there and move it to ATH_(HTC)_DEBUGFS.

Note: The shared FFT/spectral code (which is the only user
of the relayfs in ath9k*) needs DEBUG_FS to export the relayfs
interface to dump the data to userspace. So it makes no sense
to have the functions compiled in, if DEBUG_FS is not there.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
Here are some numbers for my WD Range Extender (AR7370 with a AR9300):
For both configurations MAC80211_DEBUGFS and ATH_DEBUG is disabled.
(if they are enabled, there should be no change). All sizes are in
bytes. And I only test with or without the patch applied.

module | file size | .text size |
ath9k_common.ko (w/o)  | 32208 |  12832 |
ath9k_common.ko (with) | 12204 |   3456 |

Note: The kernel with the patch, doesn't need RELAY support anymore.
Therefore it shrinks a bit as well.

  | lzma uimage size | .text size |
kernel (w/o)  |  1181777 |3004592 |
kernel (with) |  1179666 |2999448 |

If anyone wants to play with it, I made a test-patch For LEDE [0].
Just remember to disable CONFIG_PACKAGE_MAC80211_DEBUGFS and
CONFIG_PACKAGE_ATH_DEBUG.

There are more ways to do this. Let's hear if there's support for
it or not. The main motivation was that relayfs can be very costly
on the RAM as well (on ath10k in can eat like 4MiB with VM 
debugging etc...).

[0] 
<https://github.com/chunkeey/apm82181-lede/commit/5ef8d4e6497b0b41f0c562650347251e96d73ec8>
---
---
 drivers/net/wireless/ath/ath9k/Kconfig   |  9 ++--
 drivers/net/wireless/ath/ath9k/Makefile  |  5 +++--
 drivers/net/wireless/ath/ath9k/common-debug.h| 27 
 drivers/net/wireless/ath/ath9k/common-spectral.c |  2 +-
 drivers/net/wireless/ath/ath9k/common-spectral.h | 23 
 drivers/net/wireless/ath/ath9k/eeprom_4k.c   |  2 +-
 drivers/net/wireless/ath/ath9k/eeprom_9287.c |  2 +-
 drivers/net/wireless/ath/ath9k/eeprom_def.c  |  2 +-
 8 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig 
b/drivers/net/wireless/ath/ath9k/Kconfig
index 8f231c67dd51..783a38f1a626 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -3,8 +3,8 @@ config ATH9K_HW
 config ATH9K_COMMON
tristate
select ATH_COMMON
-   select DEBUG_FS
-   select RELAY
+config ATH9K_COMMON_DEBUG
+   bool
 config ATH9K_DFS_DEBUGFS
def_bool y
depends on ATH9K_DEBUGFS && ATH9K_DFS_CERTIFIED
@@ -60,12 +60,14 @@ config ATH9K_DEBUGFS
bool "Atheros ath9k debugging"
depends on ATH9K && DEBUG_FS
select MAC80211_DEBUGFS
+   select ATH9K_COMMON_DEBUG
select RELAY
---help---
  Say Y, if you need access to ath9k's statistics for
  interrupts, rate control, etc.
 
  Also required for changing debug message flags at run time.
+ As well as access to the FFT/spectral data and TX99.
 
 config ATH9K_STATION_STATISTICS
bool "Detailed station statistics"
@@ -174,8 +176,11 @@ config ATH9K_HTC
 config ATH9K_HTC_DEBUGFS
bool "Atheros ath9k_htc debugging"
depends on ATH9K_HTC && DEBUG_FS
+   select ATH9K_COMMON_DEBUG
+   select RELAY
---help---
  Say Y, if you need access to ath9k_htc's statistics.
+ As well as access to the FFT/spectral data.
 
 config ATH9K_HWRNG
bool "Random number generator support"
diff --git a/drivers/net/wireless/ath/ath9k/Makefile 
b/drivers/net/wireless/ath/ath9k/Makefile
index 76f9dc37500b..36a40ffdce15 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -60,8 +60,9 @@ obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
 ath9k_common-y:=   common.o \
common-init.o \
common-beacon.o \
-   common-debug.o \
-   common-spectral.o
+
+ath9k_common-$(CONFIG_ATH9K_COMMON_DEBUG) += common-debug.o \
+common-spectral.o
 
 ath9k_htc-y += htc_hst.o \
hif_usb.o \
diff --git a/drivers/net/wireless/ath/ath9k/common-debug.h 
b/drivers/net/wireless/ath/ath9k/common-debug.h
index 7c9788490f7f..3376990d3a24 100644
--- a/drivers/net/wireless/ath/ath9k/common-debug.h
+++ b/drivers/net/wireless/ath/ath9k/common-debug.h
@@ -60,6 +60,7 @@ struct ath_rx_stats {
u32 rx_spectral;
 };
 
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
 void ath9k_cmn_debug_modal_eeprom(str

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

[Christian Lamparter]

Hello Shafi and Kalle,

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

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

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

Regards,
Christian

[0] <https://www.mail-archive.com/ath10k@lists.infradead.org/msg06066.html>

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

[Christian Lamparter]

On Thu, Dec 29, 2016 at 3:11 PM, Kalle Valo <kv...@qca.qualcomm.com> wrote:
> Christian Lamparter <chunk...@googlemail.com> wrote:
>> The 10.4 firmware adds extended peer information to the
>> firmware's statistics payload. This additional info is
>> stored as a separate data field and the elements are
>> stored in their own "peers_extd" list.
>>
>> These elements can pile up in the same way as the peer
>> information elements. This is because the
>> ath10k_wmi_10_4_op_pull_fw_stats() function tries to
>> pull the same amount (num_peer_stats) for every statistic
>> data unit.
>>
>> Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
>> Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
>
> My understanding is that I should skip this patch 1. Please let me know if
> I misunderstood. But I'm still plannning to apply patch 2.

I added Mohammed (I hope, he can reply to the open question when he
returns), Since I'm not sure what he wants or not.

As far as I'm aware, the "extended" boolean serves no purpose
because it was only used in once place in debugfs_sta which was
removed in the patch. ( "ath10k_sta_update_stats_rx_duration"
and "ath10k_sta_update_extd_stats_rx_duration" have been unified).


> Patch set to Changes Requested.
Isn't there a: "Waiting for Maintainer" state as well?
Otherwise, if nobody has any complains or question:
can you please queue it for the next merge window?

Regards,
Christian
> --
> https://patchwork.kernel.org/patch/9461631/
>
> Documentation about submitting wireless patches and checking status
> from patchwork:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>

Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device

[Christian Lamparter]

On Friday, December 23, 2016 6:12:35 PM CET igor.mitsyanko...@quantenna.com 
wrote:
> From: Igor Mitsyanko 
> 
> QSR10G is Quantenna's 8x8, 160M, 11ac WiFi card.
> 
> Signed-off-by: Kamlesh Rath 
> Signed-off-by: Sergey Matyukevich 
> Signed-off-by: Avinash Patil 
> Signed-off-by: Igor Mitsyanko 
> ---
> This is an RFC patch that we sent to linux-wireless mailing list a while ago,
> and it spawned a discussion regarding whether GPL sources has to be submitted
> together with this firmware. We would like to clarify with linux-firmware 
> maintainers
> to understnad what should be our actions to allow this to be accepted.
> - firmware contains GPL components;
> - LICENCE file states that sources for GPL components will be provided on 
> request
> to osle...@quantenna.com
> 
> There was an opinion that it is not enough: precedent case like this with 
> carl9170
> firmware was resolved by submitting GPL sources into linux-firmware repository
> itself.
The ar9170-usb firmware was released as GPL v2.0 from the get-go.


And this was never a problem. What is THE PROBLEM though:
"if all the patches from ar9170fw to carl9170fw and beyond (i.e new versions)
have to be added to linux-firmware.git." Or not.

That's why there was never a update to the carl9170fw.
So be prepared for this scenario too.

> Does the same applies to qsr10g firmware?
>
> The SDK that we use to build it is quite heavy (> 100MB, but with toolchain). 
> Maybe we can take another approach to place SDK separately on Quantenna 
> website
> (as a compressed archive), and add a link in linux-firmware?
Yes, I would like to know this as well.

Regards,
Christian

Re: [PATCH] ath10k: merge extended peer info data with existing peers info

[Christian Lamparter]

Hello Shafi,

On Thursday, December 22, 2016 9:18:01 PM CET Mohammed Shafi Shajakhan wrote:
> > On Monday, December 19, 2016 10:19:57 PM CET Mohammed Shafi Shajakhan wrote:
> > > On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> > > > The 10.4 firmware adds extended peer information to the
> > > > firmware's statistics payload. This additional info is
> > > > stored as a separate data field. During review of
> > > > "ath10k: add accounting for the extended peer statistics" [0]
> > > > 
> > > > Mohammed Shafi Shajakhan commented that the extended peer statistics
> > > > lists are of little use:"... there is not much use in appending
> > > > the extended peer stats (which gets periodically updated) to the
> > > > linked list '>debug.fw_stats.peers_extd)' and should we get
> > > > rid of the below (and the required cleanup as well)
> > > > 
> > > > list_splice_tail_init(_extd,
> > > > >debug.fw_stats.peers_extd);
> > > > 
> > > > since rx_duration is getting updated periodically to the per sta
> > > > information."
> > > > 
> > > > This patch replaces the extended peers list with a lookup and
> > > > puts the retrieved data (rx_duration) into the existing
> > > > ath10k_fw_stats_peer entry that was created earlier.
> > > 
> > > [shafi] Its good to maintain the extended stats variable
> > > and retain the two different functions to update rx_duration.
> > > 
> > > a) extended peer stats supported - mainly for 10.4
> > > b) extender peer stats not supported - for 10.2
> > Well, I have to ask why you want to retain the two different
> > functions to update the same arsta->rx_duration? I think a
> > little bit of code that helps to explain what's on your mind
> > (or how you would do it) would help immensely in this case.
> > Since I have the feeling that this is the problem here. 
> > So please explain it in C(lang).
> 
> [shafi] I see you prefer to stuff the 'rx_duration' from
> the extended stats to the existing global 'ath10k_peer_stats'
> The idea of extended stats is, ideally its not going to stop
> for 'rx_duration' . Extended stats is maintained as a seperate
> list / entry for addition of new fields as well
I'm guessing you are trying to say here:

replace:

dst->rx_duration = __le32_to_cpu(src->rx_duration); 

with

dst->rx_duration += __le32_to_cpu(src->rx_duration);

in ath10k_wmi_10_4_op_pull_fw_stats()?

Is this correct? If so then can you tell me why 
ath10k_wmi_10_4_op_pull_fw_stats()
is using for (i = 0; i < num_peer_stats; i++) to iterate over the extended peer
stats instead of looking up the number of extended peer items. Because this does
imply that there are the "same" (and every peer stat has a matching extended 
peer stat)... (This will be important for the comment below - so ***).
Of course, if this isn't true then this is a bug and should be fixed because
otherwise the ath10k_wmi_10_4_op_pull_fw_stats functions might return -EPROTO
at some point which causes a "failed to pull fw stats: -71" and unprocessed/lost
stats.
> > 
> > > > [0] 
> > > > <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunk...@googlemail.com>
> > > > Cc: Mohammed Shafi Shajakhan <moham...@codeaurora.org>
> > > > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> > > > ---
> > > >  drivers/net/wireless/ath/ath10k/core.h|  2 --
> > > >  drivers/net/wireless/ath/ath10k/debug.c   | 17 --
> > > >  drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 
> > > > ++---
> > > >  drivers/net/wireless/ath/ath10k/wmi.c | 34 
> > > > ---
> > > >  4 files changed, 28 insertions(+), 57 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> > > > b/drivers/net/wireless/ath/ath10k/core.h
> > > > index 09ff8b8a6441..3fffbbb18c25 100644
> > > > --- a/drivers/net/wireless/ath/ath10k/core.h
> > > > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > > > @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> > > >  };
> > > >  
> > > >  struct ath10k_fw_stats {
> > > > -   bool extended;
> > > > struct list_head pdevs;
> > > > struct list_head vdevs;
> > > > str

Re: [PATCH] ath10k: merge extended peer info data with existing peers info

[Christian Lamparter]

Hello Shafi,

On Monday, December 19, 2016 10:19:57 PM CET Mohammed Shafi Shajakhan wrote:
> On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> > The 10.4 firmware adds extended peer information to the
> > firmware's statistics payload. This additional info is
> > stored as a separate data field. During review of
> > "ath10k: add accounting for the extended peer statistics" [0]
> > 
> > Mohammed Shafi Shajakhan commented that the extended peer statistics
> > lists are of little use:"... there is not much use in appending
> > the extended peer stats (which gets periodically updated) to the
> > linked list '>debug.fw_stats.peers_extd)' and should we get
> > rid of the below (and the required cleanup as well)
> > 
> > list_splice_tail_init(_extd,
> > >debug.fw_stats.peers_extd);
> > 
> > since rx_duration is getting updated periodically to the per sta
> > information."
> > 
> > This patch replaces the extended peers list with a lookup and
> > puts the retrieved data (rx_duration) into the existing
> > ath10k_fw_stats_peer entry that was created earlier.
> 
> [shafi] Its good to maintain the extended stats variable
> and retain the two different functions to update rx_duration.
> 
> a) extended peer stats supported - mainly for 10.4
> b) extender peer stats not supported - for 10.2
Well, I have to ask why you want to retain the two different
functions to update the same arsta->rx_duration? I think a
little bit of code that helps to explain what's on your mind
(or how you would do it) would help immensely in this case.
Since I have the feeling that this is the problem here. 
So please explain it in C(lang).

> > [0] 
> > <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunk...@googlemail.com>
> > Cc: Mohammed Shafi Shajakhan <moham...@codeaurora.org>
> > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> > ---
> >  drivers/net/wireless/ath/ath10k/core.h|  2 --
> >  drivers/net/wireless/ath/ath10k/debug.c   | 17 --
> >  drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 
> > ++---
> >  drivers/net/wireless/ath/ath10k/wmi.c | 34 
> > ---
> >  4 files changed, 28 insertions(+), 57 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> > b/drivers/net/wireless/ath/ath10k/core.h
> > index 09ff8b8a6441..3fffbbb18c25 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.h
> > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> >  };
> >  
> >  struct ath10k_fw_stats {
> > -   bool extended;
> > struct list_head pdevs;
> > struct list_head vdevs;
> > struct list_head peers;
> > -   struct list_head peers_extd;
> >  };
> >  
> >  #define ATH10K_TPC_TABLE_TYPE_FLAG 1
> > diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
> > b/drivers/net/wireless/ath/ath10k/debug.c
> > index 82a4c67f3672..89f7fde77cdf 100644
> > --- a/drivers/net/wireless/ath/ath10k/debug.c
> > +++ b/drivers/net/wireless/ath/ath10k/debug.c
> > @@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct 
> > list_head *head)
> > }
> >  }
> >  
> > -static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
> > -{
> > -   struct ath10k_fw_extd_stats_peer *i, *tmp;
> > -
> > -   list_for_each_entry_safe(i, tmp, head, list) {
> > -   list_del(>list);
> > -   kfree(i);
> > -   }
> > -}
> > -
> >  static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> >  {
> > spin_lock_bh(>data_lock);
> > ar->debug.fw_stats_done = false;
> > -   ar->debug.fw_stats.extended = false;
> 
> [shafi] this looks fine, but not removing the 'extended' variable 
> from ath10k_fw_stats structure, I see the design for 'rx_duration'
> arguably some what convoluted !
I removed extended because it is now a write-only variable.
So I figured, there's no point in keeping it around? I don't have
access to the firmware interface documentation, so I don't know
if there's a reason why it would be good to have it later.
So please tell me, what information do we gain from it?

> *We get periodic events from firmware updating 'ath10k_debug_fw_stats_process'
> *Fetch rx_duration from  'ath10k_wmi_pull_fw_stats(ar, skb, )'
> {certainly 'stats' object is for this particular update only, and freed
> up later)
> *Update immediately using 'ath10k_sta_update

[PATCH] ath10k: merge extended peer info data with existing peers info

[Christian Lamparter]

The 10.4 firmware adds extended peer information to the
firmware's statistics payload. This additional info is
stored as a separate data field. During review of
"ath10k: add accounting for the extended peer statistics" [0]

Mohammed Shafi Shajakhan commented that the extended peer statistics
lists are of little use:"... there is not much use in appending
the extended peer stats (which gets periodically updated) to the
linked list '>debug.fw_stats.peers_extd)' and should we get
rid of the below (and the required cleanup as well)

list_splice_tail_init(_extd,
>debug.fw_stats.peers_extd);

since rx_duration is getting updated periodically to the per sta
information."

This patch replaces the extended peers list with a lookup and
puts the retrieved data (rx_duration) into the existing
ath10k_fw_stats_peer entry that was created earlier.

[0] 
<https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunk...@googlemail.com>
Cc: Mohammed Shafi Shajakhan <moham...@codeaurora.org>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/wireless/ath/ath10k/core.h|  2 --
 drivers/net/wireless/ath/ath10k/debug.c   | 17 --
 drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 ++---
 drivers/net/wireless/ath/ath10k/wmi.c | 34 ---
 4 files changed, 28 insertions(+), 57 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 09ff8b8a6441..3fffbbb18c25 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
 };
 
 struct ath10k_fw_stats {
-   bool extended;
struct list_head pdevs;
struct list_head vdevs;
struct list_head peers;
-   struct list_head peers_extd;
 };
 
 #define ATH10K_TPC_TABLE_TYPE_FLAG 1
diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index 82a4c67f3672..89f7fde77cdf 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct list_head 
*head)
}
 }
 
-static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
-{
-   struct ath10k_fw_extd_stats_peer *i, *tmp;
-
-   list_for_each_entry_safe(i, tmp, head, list) {
-   list_del(>list);
-   kfree(i);
-   }
-}
-
 static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
 {
spin_lock_bh(>data_lock);
ar->debug.fw_stats_done = false;
-   ar->debug.fw_stats.extended = false;
ath10k_fw_stats_pdevs_free(>debug.fw_stats.pdevs);
ath10k_fw_stats_vdevs_free(>debug.fw_stats.vdevs);
ath10k_fw_stats_peers_free(>debug.fw_stats.peers);
-   ath10k_fw_extd_stats_peers_free(>debug.fw_stats.peers_extd);
spin_unlock_bh(>data_lock);
 }
 
@@ -348,7 +336,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
struct sk_buff *skb)
INIT_LIST_HEAD();
INIT_LIST_HEAD();
INIT_LIST_HEAD();
-   INIT_LIST_HEAD(_extd);
 
spin_lock_bh(>data_lock);
ret = ath10k_wmi_pull_fw_stats(ar, skb, );
@@ -411,8 +398,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
struct sk_buff *skb)
 
list_splice_tail_init(, >debug.fw_stats.peers);
list_splice_tail_init(, >debug.fw_stats.vdevs);
-   list_splice_tail_init(_extd,
- >debug.fw_stats.peers_extd);
}
 
complete(>debug.fw_stats_complete);
@@ -424,7 +409,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
struct sk_buff *skb)
ath10k_fw_stats_pdevs_free();
ath10k_fw_stats_vdevs_free();
ath10k_fw_stats_peers_free();
-   ath10k_fw_extd_stats_peers_free(_extd);
 
spin_unlock_bh(>data_lock);
 }
@@ -2347,7 +2331,6 @@ int ath10k_debug_create(struct ath10k *ar)
INIT_LIST_HEAD(>debug.fw_stats.pdevs);
INIT_LIST_HEAD(>debug.fw_stats.vdevs);
INIT_LIST_HEAD(>debug.fw_stats.peers);
-   INIT_LIST_HEAD(>debug.fw_stats.peers_extd);
 
return 0;
 }
diff --git a/drivers/net/wireless/ath/ath10k/debugfs_sta.c 
b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
index fce6f8137d33..bf2d49cbb3bb 100644
--- a/drivers/net/wireless/ath/ath10k/debugfs_sta.c
+++ b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
@@ -18,27 +18,8 @@
 #include "wmi-ops.h"
 #include "debug.h"
 
-static void ath10k_sta_update_extd_stats_rx_duration(struct ath10k *ar,
-struct ath10k_fw_stats 
*stats)
-{
-   struct ath10k_fw_extd_stats_peer *peer;
-   struct ieee80211_sta *sta;
-   struct ath10k_sta *arsta;
-
-   rcu_read_lock(

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

[Christian Lamparter]

Hello Shafi,

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
> '>debug.fw_stats.peers_extd)'  and should we get rid of the below
> (and the required cleanup as well) 
> 
> list_splice_tail_init(_extd,
>   >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.

Regards,
Christian

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

[Christian Lamparter]

On Wednesday, December 14, 2016 1:03:38 PM CET Mohammed Shafi Shajakhan wrote:
> > On Wednesday, December 7, 2016 11:58:24 AM CET Mohammed Shafi Shajakhan 
> > wrote:
> > > On Mon, Dec 05, 2016 at 10:52:45PM +0100, Christian Lamparter wrote:
> > > > @@ -409,10 +410,12 @@ void ath10k_debug_fw_stats_process(struct ath10k 
> > > > *ar, struct sk_buff *skb)
> > > > goto free;
> > > > }
> > > >  
> > > > +   if (!list_empty())
> > > 
> > > [shafi] sorry please correct me if i am wrong, for 'extended peer stats' 
> > > we are checking
> > > for normal 'peer stats' ? Is this the fix intended, i had started a build 
> > > to
> > > check your change and we will keep you posted, does this fix displaying
> > > 'rx_duration' in ath10k fw_stats.
> > The idea is not to queue any "extended peer stats" when there where no 
> > "peer stats" to
> > begin with. Because otherwise, the function is still vulnerable to OOM 
> > since the 
> > extended peers stats will be queued unchecked (not that this is currently a 
> > problem).
> 
> [shafi] list_splice_tail_init should still check for non-empty 'peers_extd' 
> list
> and append if required ? please let me know if i am still missing something
Well, you can also count the entries in peers_extd and delete the old entries
if they start to overflow. If you want to do it differently, please go ahead.

Regards,
Christian

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

[Christian Lamparter]

Hello,

It looks like google put your mail into the spam-can. 
I'm sorry for not answering sooner.

On Wednesday, December 7, 2016 11:58:24 AM CET Mohammed Shafi Shajakhan wrote:
> On Mon, Dec 05, 2016 at 10:52:45PM +0100, Christian Lamparter wrote:
> > The 10.4 firmware adds extended peer information to the
> > firmware's statistics payload. This additional info is
> > stored as a separate data field and the elements are
> > stored in their own "peers_extd" list.
> > 
> > These elements can pile up in the same way as the peer
> > information elements. This is because the
> > ath10k_wmi_10_4_op_pull_fw_stats() function tries to
> > pull the same amount (num_peer_stats) for every statistic
> > data unit.
> > 
> > Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
> > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> > ---
> >  drivers/net/wireless/ath/ath10k/debug.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
> > b/drivers/net/wireless/ath/ath10k/debug.c
> > index 82a4c67f3672..4acd9eb65910 100644
> > --- a/drivers/net/wireless/ath/ath10k/debug.c
> > +++ b/drivers/net/wireless/ath/ath10k/debug.c
> > @@ -399,6 +399,7 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
> > struct sk_buff *skb)
> >  * prevent firmware from DoS-ing the host.
> >  */
> > ath10k_fw_stats_peers_free(>debug.fw_stats.peers);
> > +   
> > ath10k_fw_extd_stats_peers_free(>debug.fw_stats.peers_extd);
> 
> [shafi] thanks for fixing this !
> 
> > ath10k_warn(ar, "dropping fw peer stats\n");
> > goto free;
> > }
> > @@ -409,10 +410,12 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
> > struct sk_buff *skb)
> > goto free;
> > }
> >  
> > +   if (!list_empty())
> 
> [shafi] sorry please correct me if i am wrong, for 'extended peer stats' we 
> are checking
> for normal 'peer stats' ? Is this the fix intended, i had started a build to
> check your change and we will keep you posted, does this fix displaying
> 'rx_duration' in ath10k fw_stats.
The idea is not to queue any "extended peer stats" when there where no "peer 
stats" to
begin with. Because otherwise, the function is still vulnerable to OOM since 
the 
extended peers stats will be queued unchecked (not that this is currently a 
problem).
 
> > +   list_splice_tail_init(_extd,
> > + >debug.fw_stats.peers_extd);
> > +
> > list_splice_tail_init(, >debug.fw_stats.peers);
> > list_splice_tail_init(, >debug.fw_stats.vdevs);
> > -   list_splice_tail_init(_extd,
> > - >debug.fw_stats.peers_extd);
> > }
> >  
> > complete(>debug.fw_stats_complete);

Regards,
Christian

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

[Christian Lamparter]

The 10.4 firmware adds extended peer information to the
firmware's statistics payload. This additional info is
stored as a separate data field and the elements are
stored in their own "peers_extd" list.

These elements can pile up in the same way as the peer
information elements. This is because the
ath10k_wmi_10_4_op_pull_fw_stats() function tries to
pull the same amount (num_peer_stats) for every statistic
data unit.

Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/wireless/ath/ath10k/debug.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index 82a4c67f3672..4acd9eb65910 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -399,6 +399,7 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
struct sk_buff *skb)
 * prevent firmware from DoS-ing the host.
 */
ath10k_fw_stats_peers_free(>debug.fw_stats.peers);
+   
ath10k_fw_extd_stats_peers_free(>debug.fw_stats.peers_extd);
ath10k_warn(ar, "dropping fw peer stats\n");
goto free;
}
@@ -409,10 +410,12 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, 
struct sk_buff *skb)
goto free;
}
 
+   if (!list_empty())
+   list_splice_tail_init(_extd,
+ >debug.fw_stats.peers_extd);
+
list_splice_tail_init(, >debug.fw_stats.peers);
list_splice_tail_init(, >debug.fw_stats.vdevs);
-   list_splice_tail_init(_extd,
- >debug.fw_stats.peers_extd);
}
 
complete(>debug.fw_stats_complete);
-- 
2.11.0

[PATCH 2/2] ath10k: fix potential memory leak in ath10k_wmi_tlv_op_pull_fw_stats()

[Christian Lamparter]

ath10k_wmi_tlv_op_pull_fw_stats() uses tb = ath10k_wmi_tlv_parse_alloc(...)
function, which allocates memory. If any of the three error-paths are
taken, this tb needs to be freed.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index f304f6632c4f..1f6bb9e8bb01 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1105,8 +1105,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k 
*ar,
struct ath10k_fw_stats_pdev *dst;
 
src = data;
-   if (data_len < sizeof(*src))
+   if (data_len < sizeof(*src)) {
+   kfree(tb);
return -EPROTO;
+   }
 
data += sizeof(*src);
data_len -= sizeof(*src);
@@ -1126,8 +1128,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k 
*ar,
struct ath10k_fw_stats_vdev *dst;
 
src = data;
-   if (data_len < sizeof(*src))
+   if (data_len < sizeof(*src)) {
+   kfree(tb);
return -EPROTO;
+   }
 
data += sizeof(*src);
data_len -= sizeof(*src);
@@ -1145,8 +1149,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k 
*ar,
struct ath10k_fw_stats_peer *dst;
 
src = data;
-   if (data_len < sizeof(*src))
+   if (data_len < sizeof(*src)) {
+   kfree(tb);
return -EPROTO;
+   }
 
data += sizeof(*src);
data_len -= sizeof(*src);
-- 
2.11.0

Re: Final 4.8 w-t build is wt-2016-10-03

[Christian Lamparter]

Hello,

On Monday, October 3, 2016 9:45:07 AM CEST Bob Copeland wrote:
> Per the subject, Linus released 4.8 yesterday so, unless I broke something
> in recent conflict resolutions, wireless-testing is on pause until the
> merge window closes in two weeks.  Have a happy (Canadian) thanksgiving!
> 
> https://git.kernel.org/cgit/linux/kernel/git/wireless/wireless-testing.git

Thanks for the timely update. 
I have a request: can you please pull Linus' kernel tags?
Currently the last tag from Linus' is v4.4-rc4 [0] and
everything between v4.4-rc5 and v4.8 is missing?!

Regards,
Christian

[0] 

[PATCH v2] carl9170: fix debugfs crashes

[Christian Lamparter]

Ben Greear reported:
> I see lots of instability as soon as I load up the carl9710 NIC.
> My application is going to be poking at it's debugfs files...
>
> BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> [carl9170] at addr 0x8801bc1208b0
> Read of size 8 by task btserver/5888
> ===
> BUG kmalloc-256 (Tainted: GW  ): kasan: bad access detected
> ---
>
> INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
>...

This breakage was caused by the introduction of intermediate
fops in debugfs by commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Thankfully, the original/real fops are still available in d_fsdata.

Reported-by: Ben Greear <gree...@candelatech.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
Cc: stable <sta...@vger.kernel.org> # 4.7+
---
 drivers/net/wireless/ath/carl9170/debug.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/debug.c 
b/drivers/net/wireless/ath/carl9170/debug.c
index 6808db4..ec3a64e 100644
--- a/drivers/net/wireless/ath/carl9170/debug.c
+++ b/drivers/net/wireless/ath/carl9170/debug.c
@@ -75,7 +75,8 @@ static ssize_t carl9170_debugfs_read(struct file *file, char 
__user *userbuf,
 
if (!ar)
return -ENODEV;
-   dfops = container_of(file->f_op, struct carl9170_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct carl9170_debugfs_fops, fops);
 
if (!dfops->read)
return -ENOSYS;
@@ -127,7 +128,8 @@ static ssize_t carl9170_debugfs_write(struct file *file,
 
if (!ar)
return -ENODEV;
-   dfops = container_of(file->f_op, struct carl9170_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct carl9170_debugfs_fops, fops);
 
if (!dfops->write)
return -ENOSYS;
-- 
2.9.3

Re: [PATCH 2/4] carl9170: fix debugfs crashes

[Christian Lamparter]

On Wednesday, September 21, 2016 12:13:25 PM CEST Greg KH wrote:
> On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> > Ben Greear reported:
> > > I see lots of instability as soon as I load up the carl9710 NIC.
> > > My application is going to be poking at it's debugfs files...
> > >
> > > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > > [carl9170] at addr 8801bc1208b0
> > > Read of size 8 by task btserver/5888
> > > ===
> > > BUG kmalloc-256 (Tainted: GW  ): kasan: bad access detected
> > > ---
> > >
> > > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> > >...
> > 
> > This breakage was caused by the introduction of intermediate
> > fops in debugfs by commit 9fd4dcece43a
> > ("debugfs: prevent access to possibly dead file_operations at file open")
> > 
> > Thankfully, the original/real fops are still available in d_fsdata.
> > 
> > Reported-by: Ben Greear <gree...@candelatech.com>
> > Reviewed-by: Nicolai Stange <nicsta...@gmail.com>
> > Signed-off-by: Christian Lamparter <chunk...@gmail.com>
> > Acked-by: Kalle Valo <kv...@codeaurora.org>
> > Cc: stable <sta...@vger.kernel.org> # 4.7+
> > ---
> >  drivers/net/wireless/ath/carl9170/debug.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/carl9170/debug.c 
> > b/drivers/net/wireless/ath/carl9170/debug.c
> > index 01a0919..ad7ffd5 100644
> > --- a/drivers/net/wireless/ath/carl9170/debug.c
> > +++ b/drivers/net/wireless/ath/carl9170/debug.c
> > @@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, 
> > char __user *userbuf,
> >  
> > if (!ar)
> > return -ENODEV;
> > -   dfops = container_of(file->f_path.dentry->d_fsdata,
> > +   dfops = container_of(debugfs_real_fops(file),
> >  struct carl9170_debugfs_fops, fops);
> >  
> > if (!dfops->read)
> > @@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
> >  
> > if (!ar)
> > return -ENODEV;
> > -   dfops = container_of(file->f_path.dentry->d_fsdata,
> > +   dfops = container_of(debugfs_real_fops(file),
> >  struct carl9170_debugfs_fops, fops);
> > if (!dfops->write)
> > return -ENOSYS;
> 
> What tree is this against?  I can't apply it to 4.8-rc5, or 4.8-rc7, are
> you sure it is still needed?
---
Yes, the patch is needed. That said I screwed this patch up and as a result
it is faulty. I'll send out v2 shortly

Thanks,
Christian

Re: [PATCH 2/4] carl9170: fix debugfs crashes

[Christian Lamparter]

On Sunday, September 18, 2016 6:44:08 PM CEST Greg KH wrote:
> On Sun, Sep 18, 2016 at 02:49:33PM +0200, Christian Lamparter wrote:
> > On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote:
> > > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
> > > > Greg KH <gre...@linuxfoundation.org> writes:
> > > > 
> > > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> > > > >> Ben Greear reported:
> > > > >> > I see lots of instability as soon as I load up the carl9710 NIC.
> > > > >> > My application is going to be poking at it's debugfs files...
> > > > >> >
> > > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > > > >> > [carl9170] at addr 8801bc1208b0
> > > > >> > Read of size 8 by task btserver/5888
> > > > >> > ===
> > > > >> > BUG kmalloc-256 (Tainted: GW  ): kasan: bad access 
> > > > >> > detected
> > > > >> > ---
> > > > >> >
> > > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> > > > >> >...
> > > > >> 
> > > > >> This breakage was caused by the introduction of intermediate
> > > > >> fops in debugfs by commit 9fd4dcece43a
> > > > >> ("debugfs: prevent access to possibly dead file_operations at file 
> > > > >> open")
> > > > >
> > > > > Because of this, these should all be backported to 4.7-stable, and
> > > > > 4.8-stable, right?
> > Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 
> > debugfs support is usually disabled.
> > 
> > Greg, would you take these four patches "as is" for -stable
> > or do you want a "minimal version" which just replaces the
> > 
> > dfops = container_of(file->f_op, ...
> > 
> > with
> > 
> > dfops = container_of(file->f_path.dentry->d_fsdata, ...
> > 
> > in the three drivers for -stable?
> 
> No, I'll take this as is, we want things to remain as close as possible
> to Linus's tree.  When we are not, is when things break.
> 
> > > > Via which tree should these go, Greg's or mine?
> > > 
> > > I'll take it if you ack it, as it's a debugfs issue.
> > For carl9170: Ben Greear has reported:
> > "I have verified this fixes my problem in the 4.7 kernel."
> > 
> > But this was with a preliminary/minimal version so I didn't
> > add the tested-by tag.
> > 
> > As for b43, I'll see if I have a working b43 in my collection
> > somewhere to confirm the issue and the fix. Question is, do
> > you want to wait or not?
> 
> I'll queue these up this week, no rush.

I was able to sucessfully test the b43 patch on my iBook G4's BCM4306.

Thanks,

Christian

Re: [PATCH 2/4] carl9170: fix debugfs crashes

[Christian Lamparter]

On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote:
> On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
> > Greg KH <gre...@linuxfoundation.org> writes:
> > 
> > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> > >> Ben Greear reported:
> > >> > I see lots of instability as soon as I load up the carl9710 NIC.
> > >> > My application is going to be poking at it's debugfs files...
> > >> >
> > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > >> > [carl9170] at addr 8801bc1208b0
> > >> > Read of size 8 by task btserver/5888
> > >> > ===
> > >> > BUG kmalloc-256 (Tainted: GW  ): kasan: bad access detected
> > >> > ---
> > >> >
> > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> > >> >...
> > >> 
> > >> This breakage was caused by the introduction of intermediate
> > >> fops in debugfs by commit 9fd4dcece43a
> > >> ("debugfs: prevent access to possibly dead file_operations at file open")
> > >
> > > Because of this, these should all be backported to 4.7-stable, and
> > > 4.8-stable, right?
Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 
debugfs support is usually disabled.

Greg, would you take these four patches "as is" for -stable
or do you want a "minimal version" which just replaces the

dfops = container_of(file->f_op, ...

with

dfops = container_of(file->f_path.dentry->d_fsdata, ...

in the three drivers for -stable?

> > Via which tree should these go, Greg's or mine?
> 
> I'll take it if you ack it, as it's a debugfs issue.
For carl9170: Ben Greear has reported:
"I have verified this fixes my problem in the 4.7 kernel."

But this was with a preliminary/minimal version so I didn't
add the tested-by tag.

As for b43, I'll see if I have a working b43 in my collection
somewhere to confirm the issue and the fix. Question is, do
you want to wait or not?

Regards,
Christian

[PATCH 3/4] b43: fix debugfs crash

[Christian Lamparter]

This patch fixes a crash that happens because b43's
debugfs code expects file->f_op to be a pointer to
its own b43_debugfs_fops struct. This is no longer
the case since commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Reviewed-by: Nicolai Stange <nicsta...@gmail.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/wireless/broadcom/b43/debugfs.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/debugfs.c 
b/drivers/net/wireless/broadcom/b43/debugfs.c
index b4bcd94..7704638 100644
--- a/drivers/net/wireless/broadcom/b43/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43/debugfs.c
@@ -524,7 +524,8 @@ static ssize_t b43_debugfs_read(struct file *file, char 
__user *userbuf,
goto out_unlock;
}
 
-   dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct b43_debugfs_fops, fops);
if (!dfops->read) {
err = -ENOSYS;
goto out_unlock;
@@ -585,7 +586,8 @@ static ssize_t b43_debugfs_write(struct file *file,
goto out_unlock;
}
 
-   dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct b43_debugfs_fops, fops);
if (!dfops->write) {
err = -ENOSYS;
goto out_unlock;
-- 
2.9.3

[PATCH 4/4] b43legacy: fix debugfs crash

[Christian Lamparter]

This patch fixes a crash that happens because b43legacy's
debugfs code expects file->f_op to be a pointer to its own
b43legacy_debugfs_fops struct. This is no longer the case
since commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Reviewed-by: Nicolai Stange <nicsta...@gmail.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/wireless/broadcom/b43legacy/debugfs.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/debugfs.c 
b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
index 090910e..82ef56e 100644
--- a/drivers/net/wireless/broadcom/b43legacy/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
@@ -221,7 +221,8 @@ static ssize_t b43legacy_debugfs_read(struct file *file, 
char __user *userbuf,
goto out_unlock;
}
 
-   dfops = container_of(file->f_op, struct b43legacy_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct b43legacy_debugfs_fops, fops);
if (!dfops->read) {
err = -ENOSYS;
goto out_unlock;
@@ -287,7 +288,8 @@ static ssize_t b43legacy_debugfs_write(struct file *file,
goto out_unlock;
}
 
-   dfops = container_of(file->f_op, struct b43legacy_debugfs_fops, fops);
+   dfops = container_of(debugfs_real_fops(file),
+struct b43legacy_debugfs_fops, fops);
if (!dfops->write) {
err = -ENOSYS;
goto out_unlock;
-- 
2.9.3

[PATCH 1/4] debugfs: introduce a public file_operations accessor

[Christian Lamparter]

This patch introduces an accessor which can be used
by the users of debugfs (drivers, fs, ...) to get the
original file_operations struct. It also removes the
REAL_FOPS_DEREF macro in file.c and converts the code
to use the public version.

Previously, REAL_FOPS_DEREF was only available within
the file.c of debugfs. But having a public getter
available for debugfs users is important as some
drivers (carl9170 and b43) use the pointer of the
original file_operations in conjunction with container_of()
within their debugfs implementations.

Reviewed-by: Nicolai Stange <nicsta...@gmail.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 fs/debugfs/file.c   | 13 +
 include/linux/debugfs.h | 17 +
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 592059f..309f4e9 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -97,9 +97,6 @@ EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
 
 #define F_DENTRY(filp) ((filp)->f_path.dentry)
 
-#define REAL_FOPS_DEREF(dentry)\
-   ((const struct file_operations *)(dentry)->d_fsdata)
-
 static int open_proxy_open(struct inode *inode, struct file *filp)
 {
const struct dentry *dentry = F_DENTRY(filp);
@@ -112,7 +109,7 @@ static int open_proxy_open(struct inode *inode, struct file 
*filp)
goto out;
}
 
-   real_fops = REAL_FOPS_DEREF(dentry);
+   real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not clean up after itself at exit? */
@@ -143,7 +140,7 @@ static ret_type full_proxy_ ## name(proto)  
\
 {  \
const struct dentry *dentry = F_DENTRY(filp);   \
const struct file_operations *real_fops =   \
-   REAL_FOPS_DEREF(dentry);\
+   debugfs_real_fops(filp);\
int srcu_idx;   \
ret_type r; \
\
@@ -176,7 +173,7 @@ static unsigned int full_proxy_poll(struct file *filp,
struct poll_table_struct *wait)
 {
const struct dentry *dentry = F_DENTRY(filp);
-   const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
+   const struct file_operations *real_fops = debugfs_real_fops(filp);
int srcu_idx;
unsigned int r = 0;
 
@@ -193,7 +190,7 @@ static unsigned int full_proxy_poll(struct file *filp,
 static int full_proxy_release(struct inode *inode, struct file *filp)
 {
const struct dentry *dentry = F_DENTRY(filp);
-   const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
+   const struct file_operations *real_fops = debugfs_real_fops(filp);
const struct file_operations *proxy_fops = filp->f_op;
int r = 0;
 
@@ -241,7 +238,7 @@ static int full_proxy_open(struct inode *inode, struct file 
*filp)
goto out;
}
 
-   real_fops = REAL_FOPS_DEREF(dentry);
+   real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not cleanup after itself at exit? */
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 1438e23..4d3f0d1 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -45,6 +45,23 @@ extern struct dentry *arch_debugfs_dir;
 
 extern struct srcu_struct debugfs_srcu;
 
+/**
+ * debugfs_real_fops - getter for the real file operation
+ * @filp: a pointer to a struct file
+ *
+ * Must only be called under the protection established by
+ * debugfs_use_file_start().
+ */
+static inline const struct file_operations *debugfs_real_fops(struct file 
*filp)
+   __must_hold(_srcu)
+{
+   /*
+* Neither the pointer to the struct file_operations, nor its
+* contents ever change -- srcu_dereference() is not needed here.
+*/
+   return filp->f_path.dentry->d_fsdata;
+}
+
 #if defined(CONFIG_DEBUG_FS)
 
 struct dentry *debugfs_create_file(const char *name, umode_t mode,
-- 
2.9.3

[PATCH 2/4] carl9170: fix debugfs crashes

[Christian Lamparter]

Ben Greear reported:
> I see lots of instability as soon as I load up the carl9710 NIC.
> My application is going to be poking at it's debugfs files...
>
> BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> [carl9170] at addr 8801bc1208b0
> Read of size 8 by task btserver/5888
> ===
> BUG kmalloc-256 (Tainted: GW  ): kasan: bad access detected
> ---
>
> INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
>...

This breakage was caused by the introduction of intermediate
fops in debugfs by commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Thankfully, the original/real fops are still available in d_fsdata.

Reported-by: Ben Greear <gree...@candelatech.com>
Reviewed-by: Nicolai Stange <nicsta...@gmail.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/wireless/ath/carl9170/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/debug.c 
b/drivers/net/wireless/ath/carl9170/debug.c
index 01a0919..ad7ffd5 100644
--- a/drivers/net/wireless/ath/carl9170/debug.c
+++ b/drivers/net/wireless/ath/carl9170/debug.c
@@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, char 
__user *userbuf,
 
if (!ar)
return -ENODEV;
-   dfops = container_of(file->f_path.dentry->d_fsdata,
+   dfops = container_of(debugfs_real_fops(file),
 struct carl9170_debugfs_fops, fops);
 
if (!dfops->read)
@@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
 
if (!ar)
return -ENODEV;
-   dfops = container_of(file->f_path.dentry->d_fsdata,
+   dfops = container_of(debugfs_real_fops(file),
 struct carl9170_debugfs_fops, fops);
if (!dfops->write)
return -ENOSYS;
-- 
2.9.3

Re: regarding RTL8188SU

[Christian Lamparter]

Hello,

On Tuesday, August 16, 2016 5:39:38 PM CEST Sambhu R wrote:
> I was following your--> https://github.com/chunkeey/rtl8192su but i couldn't
> install. When i execute ./install.sh i'm getting the following error.

There's no install.sh file in this repository?! 

> 
> make[1]: Entering directory '/usr/src/linux-headers-3.4.112-sun8i'
> Makefile:568: /usr/src/linux-headers-3.4.112-sun8i/arch/armv7l/Makefile: No 
> such file or directory
> make[1]: *** No rule to make target 
> '/usr/src/linux-headers-3.4.112-sun8i/arch/armv7l/Makefile'.  Stop.
> make[1]: Leaving directory '/usr/src/linux-headers-3.4.112-sun8i'
> Makefile:220: recipe for target 'modules' failed
> make: *** [modules] Error 2
> ##
> Compile make driver error: 2
> Please check error Mesg
> ##
> 
> please help..
Please look at the project's README.md [0].

"This is "a" driver repository for the WIP rtl8192su for any
interested developer.

To build the driver you will need to have a recent and compatible kernel
source. Currently, only kernels built from the latest wireless-testing.git
are supported. If you want to know more about wireless-testing.git then
visit Linux wireless wiki's Git-Guide [1]."

[0] 
[1] 

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Wednesday, August 3, 2016 3:49:26 AM CEST Alan Curry wrote:
> Al Viro wrote:
> > 
> > Which just might mean that we have *three* issues here -
> > (1) buggered __copy_to_user_inatomic() (and friends) on some sparcs
> > (2) your ssl-only corruption
> > (3) Alan's x86_64 corruption on plain TCP read - no ssl *or* sparc
> > anywhere, and no multi-segment recvmsg().  Which would strongly argue in
> > favour of some kind of copy_page_to_iter() breakage triggered when handling
> > a fragmented skb, as in (1).  Except that I don't see anything similar in
> > x86_64 uaccess primitives...
> > 
> 
> I think I've solved (3) at least...
> 
> Using the twin weapons of printk and stubbornness, I have built a working
> theory of the bug. I haven't traced it all the way through, so my explanation
> may be partly wrong. I do have a patch that eliminates the symptom in all my
> tests though. Here's what happens:
> 
> A corrupted packet somehow arrives in skb_copy_and_csum_datagram_msg().
> During downloads at reasonably high speed, about 0.1% of my incoming
> packets are bad. Probably because the access point is that suspicious
> Comcast thing.

Thanks for being very persistent with this. I think I'm able to reproduce
this now (on any hardware... like r8169 ethernet) as long as the following 
"traffic policy" is enacted on the HTTP - Server: 

# tc qdisc add dev eth0 root netem corrupt 0.1%

(This needs the "Network Emulation" Sched CONFIG_NET_SCH_NETEM [0].)

With your tool (changed to point to my apache local server). I'm seeing 
corruptions in the "noselect" case. Running it in "select" mode however
and the resulting files have no corruptions.

About AR9170 corruption issues: I know of one report that the AR9170's
Encryption Engine can cause corruptions [1]. In this case outgoing
data was corrupted which lead to deauths/disassocs since the AP was
basically sending out multicast deauths/disassocs with bad addresses.
However, "nohwcrypt" should have made a difference there since the
software decryption would discard the faulty package due the message
integrety checks.

Another source for corruptions could be the USB-PHY (FUSB200) in the
AR9170 [2]. I know it's causing problems for the ath9k_htc. However
not everyone is affected.

One thing I noticed in your previous post is that you "might" not have
draft-802.11n enabled. Do you see any "disabling HT/VHT due to WEP/TKIP use."
in your dmesg logs? If so, check if you can force your AP to use WPA2
with CCMP/AES only.

Regards,
Christian

[0] 
[1] 
[2] 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Tuesday, July 26, 2016 4:57:03 AM CEST Alan Curry wrote:
> Al Viro wrote:
> > On Sun, Jul 24, 2016 at 07:45:13PM +0200, Christian Lamparter wrote:
> > 
> > > > The symptom is that downloaded files (http, ftp, and probably other
> > > > protocols) have small corrupted segments (about 1-2 kilobytes long) in
> > > > random locations. Only downloads that sustain a high speed for at least 
> > > > a
> > > > few seconds are corrupted. Anything small enough to be received in less
> > > > than about 5 seconds is not affected.
> > 
> > Can that sucker be reproduced with netcat?  That would eliminate all issues
> > with multi-iovec recvmsg(2), narrowing the things down quite bit.
> 
> netcat seems to be immune. Comparing strace results, I didn't see any
> recvmsg() calls in the other programs that have had the problem, but there
> is an interesting difference: netcat calls select() to wait for the socket
> to be ready for reading, where my other test programs just call read() and
> let it block until ready.
> 
> So I wrote a small test program to isolate that difference. It downloads
> a file using only read() and write() and a hardcoded HTTP request. It has
> a select mode (main loop alternates read() and select() on the TCP socket)
> and a noselect mode (main loop just read()s the TCP socket).
> 
> The program is included at the bottom of this message.
> 
> I ran it several times in both modes and got corruption if and only if the
> noselect mode was used.
> 
> > 
> > Another thing (and if that works, it's *NOT* a proper fix - it would be
> > papering over the problem, but at least it would show where to look for
> > it) - try (on top of mainline) the following delta:
> > 
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> 
> Will try that patch soon. Meanwhile, here's my test:
> 
> /* Demonstration program "dlbug".
>Usage: dlbug select > outfile
>   or
>   dlbug noselect > outfile
>outfile will contain the full HTTP response. Edit out the HTTP headers
>and what's left should be a valid gzip if the download worked. */
> [...]
Thanks, I gave the program a try with my WNDA3100 and a WN821N v2 devices.
I did not see any corruptions in any of the tests though. Can you tell me
something about your wireless network too? I would like to know what router
and firmware are you using? Also important: what's your wireless configuration?
(WPA?, CCMP or TKIP? HT40, HT20 or Legacy rates? ...)

Probably the quickest and easiest way to get that information is by running
the following commands as root, when you are connected to your wifi network
and post the results:
# iw dev wlan0 link
# iw dev wlan0 scan dump

(You can of course remove your device's MACs, but please do it consistently).

Regards,
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

Hello,

I added Al Viro to the CC (probably not necessary...)

On Sunday, July 24, 2016 3:35:14 AM CEST Alan Curry wrote:
> [1.] One line summary of the problem:
> network data corruption (bisected to e5a4b0bb803b)
> 
> [2.] Full description of the problem/report:
> Note: although my bisect ended at a commit from before 3.19, I have the
> same symptom in all newer kernels I've tried, up to 4.6.4.
> 
> The commit was:
> 
> >commit e5a4b0bb803b39a36478451eae53a880d2663d5b
> >Author: Al Viro 
> >Date:   Mon Nov 24 18:17:55 2014 -0500
> >
> >switch memcpy_to_msg() and skb_copy{,_and_csum}_datagram_msg() to 
> > primitives
> 
> The symptom is that downloaded files (http, ftp, and probably other
> protocols) have small corrupted segments (about 1-2 kilobytes long) in
> random locations. Only downloads that sustain a high speed for at least a
> few seconds are corrupted. Anything small enough to be received in less
> than about 5 seconds is not affected.
> 
> If I download the same file twice in a row, the corruption is in different
> places in each copy.
> 
> If I try to do a git clone, it fails a few seconds into the "Receiving
> objects" stage with a deflate error.

Thanks for the detailed bug-report. I looked around the web to see if it
was already reported or not. If found that this issue was reported before:
[0], [1] and [2] by the same person (CC'ed). One difference is that the 
reporter had this issue with rsync on multiple SPARC systems. I ran a
git grep on a 4.7.0-rc7+ (wt-2016-07-21-15-g97bd3b0). But it didn't find
any patches directly referencing the commit. I'm not sure if this issue
has been fixed by now or not. I would greatly appreciate any comment
about this from the "people of netdev" (Al Viro? Alex Mcwhirter?).

As for carl9170: I'm not sure what the driver or firmware can do about
this at this time. You can try to disable the hardware crypto by setting
nohwcrypt via the module option. However, this might not do anything at all.

> [3.] Keywords: networking, carl9170
> 
> [4.] Kernel information
> [4.1.] Kernel version (from /proc/version):
> Multiple versions are known to be affected, from 3.19 to 4.6.4
> 
> [4.2.] Kernel .config file:
> For testing I built with make x86_64_defconfig followed by enabling the
> carl9170 driver, which adds these lines:
> CONFIG_ATH_COMMON=m
> CONFIG_ATH_CARDS=m
> CONFIG_CARL9170=m
> CONFIG_CARL9170_LEDS=y
> CONFIG_CARL9170_WPC=y
> 
> [5.] Most recent kernel version which did not have the bug:
> That would be the predecessor of e5a4b0bb803b39a36478451eae53a880d2663d5b
> which is v3.18-rc6-1620-g17836394e578
> 
> [6.] no Oops
> 
> [7.] A small shell script or example program which triggers the
>  problem (if possible)
> 
> This command fails reliably for me when running an affected kernel:
> 
> git clone git://git.kernel.org/pub/scm/git/git.git
> 
> (I'm including all the standard format stuff suggested by REPORTING-BUGS,
> but I think you can skip from here to section 8.7 without missing anything
> relevant)
Yes, I removed it for the most part. If anyone is interested in the details:
Here's a link to the original post @LKML [3].

> 
> [8.] Environment
> [8.1.] Software (add the output of the ver_linux script here)
> 
> Mostly Debian 8.5 stable packages here.
> 
> [8.3.] Module information (from /proc/modules):
> 
> When I tested with the x86_64_defconfig + carl9170 kernel, there were
> hardly any modules built, and I reproduced the problem after booting with
> init=/bin/sh, so no unnecessary modules were loaded. Currently running a
> normal 4.6.4 kernel which is showing the bug.
> 
> [...]
> [8.7.] Other information that might be relevant to the problem
>(please look in /proc and include all information that you
>think to be relevant):
> 
> lsusb identifies my network device as:
> 
> Bus 005 Device 004: ID 0cf3:1002 Atheros Communications, Inc. TP-Link 
> TL-WN821N v2 802.11n [Atheros AR9170]
> 
> I have version 1.9.9 of carl9170-1.fw in /lib/firmware
Just one additional question: Is the TL-WN821N connected to a USB3 port?

Regards,
Christian

[0] 
[1] 
[2] 
[3] 



--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

[Christian Lamparter]

On Wednesday, July 20, 2016 5:06:27 PM CEST Xose Vazquez Perez wrote:
> Arnd Bergmann  wrote:
> 
> > rtlwifi, but I found the older r8712u device to work fine with
> > the staging/rtl8712 driver.
> 
> A replacement for "staging/rtl8712", with MAC80211 support, is
> available at: https://github.com/chunkeey/rtl8192su
> 
> Also a fullmac/cfg80211 driver(r92su) is available at the
> same repository.
> 
Yes, it has its problems. The rtl8712/r92su isn't really
a fullmac device. While the MLME (scan, probe, auth, assoc) is
done by the firmware. The 802.11 frame creation (from 802.3)
frames needs to be done by the driver. The rtl8712 firmware
however has its fair share of issues. Like: no support for
(tx) fragmentation and ERP is a mystery too. monitor 
mode is barely working and limited to 20Mhz wide channels.
There's also limited tx injection support and of course
stability issues (just like with the staging/rtl8712 driver
or FreeBSD's rsu driver).

The rtl8192su driver (based on rtlwifi) in the same repository
has proper fragmentation support. But it uses the firmware from
the windows/mac os x driver, which is similar to rtl8192se softmac
firmware in design. Getting it to work properly in station mode
however either needs "some magic" or help from Realtek's USB group...


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Christian Lamparter]

On Saturday, June 25, 2016 05:08:29 PM Martin Blumenstingl wrote:
> On Sat, Jun 25, 2016 at 2:01 PM, Christian Lamparter
> <chunk...@googlemail.com> wrote:
> > On Friday, June 24, 2016 02:34:30 PM Martin Blumenstingl wrote:
> >> This makes it possible to configure ath9k based devices using
> >> devicetree. That makes some out-of-tree "convert devicetree to
> >> ath9k_platform_data glue"-code obsolete.
> >
> > Hm, what about the embedded ath9k pcie chips that need the early
> > pci-fixup routine for the device to work properly [0], [1]? How
> > will this be handled/integrated? I know that the ar71xx and the
> > lantiq platforms use similar pci-fixup routines that need a few
> > bytes from the eeprom/cal data. So lantiq has a few extra properties:
> > "ath,pci-slot", "ath,device-id" and "ath,eep-flash".
> that is exactly the use-case I want to use your owl-loader for (see
> [0], it's a small kernel module which adds the PCI configuration for
> ath9k devices).
Well, we also cooked up a userspace hack for OpenWRT/LEDE which would
work with the existing code (for the case you mentioned below) [2]. 
Furthermore it also works for other devices, as long as the fw is in
/lib/firmware and not in a subdirectory (But this can be fixed in 5
minutes with better bash foo). One thing that needs to be considered
though: That script interferes with procd firmware loading if the
"timing" is right. However usually procd has already finished all
firmware requests by then.

> This makes ath,pci-slot and ath,eep-flash obsolete.
> As far as I'm aware ath,device-id is a bit of a special case (mtd_read
> issues when the caldata is stored at an unaligned position on NOR
> flash). So this might be obsolete as well when using owl-loader.

The problem with the owl-loader is/was that it sticks around
when it has initialized all the cards. Unloading a module by
itself is tough. One way out would be to add it to ath9k's pci.c.
The question is: will such a feature have support from the ath9k
folks?

> > As an example: the AR9280 in the Cisco Z1 AP is initially
> > 0x168c:0xff1f (<-- ath9k doesn't know about that id). The
> > pci-fixup routine will change it to  0x168c:0x002A. Only
> > then ath9k can take it over and will initialize it.
> > Thing is: this is all currently done by platform code for
> > those architectures... And currently, the request_firmware
> > doesn't work for caldata on UBI partitions.
> request_firmware is working on UBI partitions in many cases.
> It's just not working when request_firmware is called too early (and
> this is not UBI specific, other filesystems might be affected as
> well): if it is called before rootfs is mounted (which is the case
> when you call it from a PCI fixup function) then it's not working
> (like you said).
> The "solution" to this is to compile the driver as kernel module (once
> this is loadable everything else should be readable as well).
> Not only ath9k is affected by this "issue", this is simply a
> limitation of request_firmware and/or the linux boot chain.
> 
> A few words regarding your owl-loader:
> First of all I would like to say "thank you"!
> Mathias and I are working on changing the lantiq target in LEDE to use
> owl-loader for all (ath9k) devices.
> All I had to do was to add another OWL PCI ID, implement a fallback
> for the firmware filename when there is no ath9k_platform_data (I'm
> using the same pattern as in PATCH 3/3 in this series). You can find
> the WIP code here: [1]

I've added lede-dev and Luis since this is relevant for them.
Maybe between the sysloadfw.sh and owl-loader, there's another
solution we overlooked so far? I know Luis has been digging
around in the firmware_class and added the sysdata API. But
from what I can tell, this would ?break? LEDE/OpenWRT's
userspace helper, since the sysfs interface in 
/sys/class/firmware which is used by procd to upload the data
is gone with sysdata or am I wrong?
 
Regards,
Christian

> [0] https://patchwork.ozlabs.org/patch/607682/
> [1] https://github.com/xdarklight/source/commits/ath9k-owl-loader-20160624
[2] 
<https://github.com/riptidewave93/Openwrt-Z1/commit/9a38c60a1206b4010fbfb626fc7b2ec69bbe232a>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC v3 3/3] ath9k: parse the device configuration from an OF node

[Christian Lamparter]

On Friday, June 24, 2016 02:34:30 PM Martin Blumenstingl wrote:
> This makes it possible to configure ath9k based devices using
> devicetree. That makes some out-of-tree "convert devicetree to
> ath9k_platform_data glue"-code obsolete.

Hm, what about the embedded ath9k pcie chips that need the early
pci-fixup routine for the device to work properly [0], [1]? How
will this be handled/integrated? I know that the ar71xx and the
lantiq platforms use similar pci-fixup routines that need a few
bytes from the eeprom/cal data. So lantiq has a few extra properties:
"ath,pci-slot", "ath,device-id" and "ath,eep-flash". 

As an example: the AR9280 in the Cisco Z1 AP is initially 
0x168c:0xff1f (<-- ath9k doesn't know about that id). The
pci-fixup routine will change it to  0x168c:0x002A. Only 
then ath9k can take it over and will initialize it.
Thing is: this is all currently done by platform code for
those architectures... And currently, the request_firmware
doesn't work for caldata on UBI partitions.

Regards,
Christian

[0] 


[1] 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Documentation: dt: net: add ath9k wireless device binding

[Christian Lamparter]

On Thursday, June 23, 2016 05:13:28 PM Martin Blumenstingl wrote:
> Add documentation how devicetree can be used to configure ath9k based
> devices.
> 
> Signed-off-by: Martin Blumenstingl 

You need to CC' the devicetree maintainers:
Mark Rutland 
Rob Herring 
devicet...@vger.kernel.org

for all patches which touch Documentation/devicetree/... .

Also, I know (from experience) that they would prefer it, if you put
the device tree binding patch at the top of the series (i.e. make it:
[PATCH 1/2] dt-bindings...). ;-)

> ---
>  .../devicetree/bindings/net/wireless/ath,ath9k.txt | 40 
> ++
>  1 file changed, 40 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt 
> b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
> new file mode 100644
> index 000..d6f5471
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ath,ath9k.txt
> @@ -0,0 +1,40 @@
> +* Atheros ath9k wireless devices
> +
> +This node provides properties for configuring the ath9k wireless device. The
> +node is expected to be specified as a child node of the PCI controller to
> +which the wireless chip is connected.
> +
> +Required properties:
> +- compatible: Should be "ath,ath9k"
Documentation/devicetree/bindings/vendor-prefixes.txt has an entry for
Qualcomm Atheros, Inc. => qca. I would use that instead, given that this
is a new binding, so there's '"no"' legacy code to worry about.

> +
> +Optional properties:
> +- reg: Address and length of the register set for the device.
> +- ath,gpio-mask: The GPIO mask
> +- ath,gpio-val: The GPIO value
> +- ath,led-pin: The GPIO number to which the LED is connected
> +- ath,led-active-high: The LED is active when the GPIO is HIGH
> +- ath,clk-25mhz: Defines that at 25MHz clock is used
> +- ath,eeprom-name: The name of the file which contains the EEPROM data (which
> + will be loaded via request_firmware)
> +- ath,check-eeprom-endianness: Allow checking the EEPROM endianness and
> + swapping of the EEPROM data if required
> +- ath,disable-2ghz: Disables the 2.4GHz band, even if enabled in the EEPROM
> +- ath,disable-5ghz: Disables the 5GHz band, even if enabled in the EEPROM
> +
> +In this example, the node is defined as child node of the PCI controller.
> +
> +pci {
> + pcie@0 {
> + reg = <0 0 0 0 0>;
> + #interrupt-cells = <1>;
> + #size-cells = <2>;
> + #address-cells = <3>;
> + device_type = "pci";
> +
> + ath9k@0,0 {
compatible = "qca,ath9k"; ?

> + reg = <0 0 0 0 0>;
> + device_type = "pci";
> + ath,disable-5ghz;
> + };
> + };
> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ath9k: parse the device configuration from an OF node

[Christian Lamparter]

On Thursday, June 23, 2016 05:13:27 PM Martin Blumenstingl wrote:
> This makes it possible to configure ath9k based devices using
> devicetree. That makes some out-of-tree "convert devicetree to
> ath9k_platform_data glue"-code obsolete.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/net/wireless/ath/ath9k/init.c | 70 
> +++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/init.c 
> b/drivers/net/wireless/ath/ath9k/init.c
> index a0f4a52..0f76137 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  
> @@ -555,6 +557,70 @@ static int ath9k_init_platform(struct ath_softc *sc)
>   return 0;
>  }
>  
> +static int ath9k_of_init(struct ath_softc *sc)
> +{
> + struct device_node *np = sc->dev->of_node;
> + struct ath_hw *ah = sc->sc_ah;
> + struct ath_common *common = ath9k_hw_common(ah);
> + const char *mac, *eeprom_name;
> + int led_pin, ret;
> + u32 gpio_data;
> +
> + if (!np)
> + return 0;

Can you please add a of_device_is_available check here too? So
we can skip it with the status property?

> +
> + ath_dbg(common, CONFIG, "parsing configuration from OF node\n");
> +
> + if (!of_property_read_u32(np, "ath,led-pin", _pin))
> + ah->led_pin = led_pin;
> +
> + if (!of_property_read_u32(np, "ath,gpio-mask", _data))
> + ah->gpio_mask = gpio_data;
> +
> + if (!of_property_read_u32(np, "ath,gpio-val", _data))
> + ah->gpio_val = gpio_data;
> +
> + if (of_property_read_bool(np, "ath,clk-25mhz"))
> + ah->is_clk_25mhz = true;
> +
> + if (!of_property_read_u32(np, "ath,gpio-mask", _data))
> + ah->gpio_mask = gpio_data;
^^
Duplicated? (see 11 lines above)

> +
> + if (!of_property_read_u32(np, "ath,gpio-val", _data))
> + ah->gpio_val = gpio_data;
^^
Duplicated?

> +
> + if (of_property_read_bool(np, "ath,clk-25mhz"))
> + ah->is_clk_25mhz = true;
^^
Duplicated?

> +
> + if (of_property_read_bool(np, "ath,led-active-high"))
> + ah->config.led_active_high = true;
> +
> + if (of_property_read_bool(np, "ath,disable-2ghz"))
> + ah->disable_2ghz = true;
> +
> + if (of_property_read_bool(np, "ath,disable-5ghz"))
> + ah->disable_5ghz = true;
> +
> + if (of_property_read_bool(np, "ath,check-eeprom-endianness"))
> + ah->ah_flags &= ~AH_NO_EEP_SWAP;
> + else
> + ah->ah_flags |= AH_NO_EEP_SWAP;
> +
> + if (!of_property_read_string(np, "ath,eeprom-name", _name)) {
> + ret = ath9k_eeprom_request(sc, eeprom_name);
> + if (ret)
> + return ret;
> + }
> +
> + mac = of_get_mac_address(np);
> + if (mac)
> + ether_addr_copy(common->macaddr, mac);
> +
> + ah->ah_flags &= ~AH_USE_EEPROM;
> +
> + return 0;
> +}
> +
>  static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
>   const struct ath_bus_ops *bus_ops)
>  {
> @@ -611,6 +677,10 @@ static int ath9k_init_softc(u16 devid, struct ath_softc 
> *sc,
>   if (ret)
>   return ret;
>  
> + ret = ath9k_of_init(sc);
> + if (ret)
> + return ret;
> +
>   if (ath9k_led_active_high != -1)
>   ah->config.led_active_high = ath9k_led_active_high == 1;


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND] Re: updating carl9170-1.fw in linux-firmware.git

[Christian Lamparter]

On Thursday, May 19, 2016 04:08:10 PM Kalle Valo wrote:
> Lauri Kasanen <c...@gmx.com> writes:
> 
> > On Tue, 03 May 2016 15:09:39 +0200
> > Christian Lamparter <chunk...@googlemail.com> wrote:
> >
> >> On Thursday, April 21, 2016 03:22:15 PM Kalle Valo wrote:
> >> > Christian Lamparter <chunk...@googlemail.com> writes:
> >> > 
> >> > > Maintainers. So for those people reading the fw list:
> >> > >
> >> > > What would it take to update the carl9170-1.fw firmware file in your
> >> > > repository to the latest version?
> >> > 
> >> > Thanks for following up on this. It would be good to solve this finally.
> >> Well, this was almost two weeks and no update yet.
> >> Kalle, can you please apply the "[PATCH v2] carl9170: Clarify kconfig text"
> >> patch from "Lauri Kasanen <c...@gmx.com>" [0].
> >> 
> >> [0] <https://marc.info/?l=linux-wireless=146081633405880>
> >
> > Seems we missed 4.7?
> 
> Yeah, I didn't consider this important enough and delayed it due to lack
> of time. I now put the patch back to my queue for 4.8, but please check
> that it's the correct version:
> 
> https://patchwork.kernel.org/patch/8861621/
Yes, this links to v2, which is the correct version.
 
> But we really should figure out how to get the latest version to
> linux-firmware, this is not good in the long run.
Well, none of my firmware patches were added/accepted and nobody
responded on how to add it to linux-firmware. I'm open for ideas.

Regards,
Christan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND] Re: updating carl9170-1.fw in linux-firmware.git

[Christian Lamparter]

On Wednesday, April 20, 2016 10:59:44 AM Kalle Valo wrote:
> Christian Lamparter <chunk...@googlemail.com> writes:
> 
> > On Monday, April 18, 2016 07:42:05 PM Kalle Valo wrote:
> >> Christian Lamparter <chunk...@googlemail.com> writes:
> >> 
> >> > On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> >> >
> >> >> Why even mention anything about a "special firmware" as the firmware is
> >> >> already available from linux-firmware.git? 
> >> >
> >> > Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
> >> > but that failed.
> >> > <http://comments.gmane.org/gmane.linux.kernel.wireless.general/114639>
> >> 
> >> Rick's comment makes sense to me, better just to provide the latest
> >> version. No need to unnecessary confuse the users. And if someone really
> >> wants to use an older version that she can retrieve it from the git
> >> history.
> >
> > Part of the fun here is that firmware is GPLv2. The linux-firmware.git has
> > to point to or add the firmware source to their tree. They have added every
> > single source file to it instead of "packaging" it in a tar.bz2/gz/xz
> > like you normally do for release sources.
> >
> > If you want to read more about it:
> > <http://www.spinics.net/lists/linux-wireless/msg101868.html>
> 
> Yeah, that's more work. I get that. But I'm still not understanding
> what's the actual problem which prevents us from updating carl9170
> firmware in linux-firmware.
I'm not sure, but why not ask? I've added the cc'ed Linux Firmware
Maintainers. So for those people reading the fw list:

What would it take to update the carl9170-1.fw firmware file in your
repository to the latest version?

Who has to sent the firmware update. Does it have to be the person who
sent the first request? (Xose)? The maintainer of the firmware (me)?
someone from Qualcomm Atheros? Or someone else (specific)? (the 
firmware is licensed as GPLv2 - in theory anyone should be able to
do that)

How should the firmware source update be handled? Currently the latest
.tar.xz of the firmware has ~130kb. The formated patches from 1.9.6 to
latest are about ~100kb (182 individual patches).

How does linux-firmware handle new binary firmware images and new 
sources? What if carl9170fw-2.bin is added. Do we need another
source directory for this in the current tree then? Because 
carl9170fw-1.bin will still be needed for backwards compatibility
so we basically need to duplicate parts of the source?

Also, how's the situation with ath9k_htc? The 1.4.0 image contains
some GPLv2 code as well? So, why is there no source in the tree, but 
just the link to it? Because, I would like to do basically the same
for carl9170fw and just add a link to the carl9170fw repository and
save everyone this source update "song and dance".

Regards,
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 06:47:44 PM Xose Vazquez Perez wrote:
> Christian Lamparter wrote:
> 
> > Sure, but this could be a different patch then. I think Intel devices 
> > (iwlwifi, iwlegacy and ipw2x00) have a similar text about "download
> > firmware from this device from our homepage here" too. So if we want,
> > we can remove them altogether?
> 
> linux-firmware.git does not contain firmware for all drivers. _At least_
> zd1211rw [1], atmel [2] and ipw2x00 [3] are out of the tree.

Yeah, don't forget p54. It needs out-of-tree firmwares as well.

But what about iwlegacy? Isn't that another candidate that would fit both
requirements (no recent firmware update, firmware in linux-firmware.git) ?

> [1] http://sf.net/projects/zd1211/files/
> [2] 
> http://web.archive.org/web/20121016132320/http://at76c503a.berlios.de/fw_dl.html
> [3] http://ipw2100.sf.net/firmware.php http://ipw2200.sf.net/firmware.php
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 07:42:05 PM Kalle Valo wrote:
> Christian Lamparter <chunk...@googlemail.com> writes:
> 
> > On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> >> Lauri Kasanen <c...@gmx.com> writes:
> >> 
> >> > --- a/drivers/net/wireless/ath/carl9170/Kconfig
> >> > +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> >> > @@ -5,12 +5,10 @@ config CARL9170
> >> >  select FW_LOADER
> >> >  select CRC32
> >> >  help
> >> > -  This is another driver for the Atheros "otus" 802.11n USB 
> >> > devices.
> >> > +  This is the mainline driver for the Atheros "otus" 802.11n 
> >> > USB devices.
> >> >  
> >> > -  This driver provides more features than the original,
> >> > -  but it needs a special firmware (carl9170-1.fw) to do that.
> >> > -
> >> > -  The firmware can be downloaded from our wiki here:
> >> > +  It needs a special firmware (carl9170-1.fw), which can be 
> >> > downloaded
> >> > +  from our wiki here:
> >> ><http://wireless.kernel.org/en/users/Drivers/carl9170>
> >> 
> >> Why even mention anything about a "special firmware" as the firmware is
> >> already available from linux-firmware.git? 
> >
> > Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
> > but that failed.
> > <http://comments.gmane.org/gmane.linux.kernel.wireless.general/114639>
> 
> Rick's comment makes sense to me, better just to provide the latest
> version. No need to unnecessary confuse the users. And if someone really
> wants to use an older version that she can retrieve it from the git
> history.
Part of the fun here is that firmware is GPLv2. The linux-firmware.git has
to point to or add the firmware source to their tree. They have added every
single source file to it instead of "packaging" it in a tar.bz2/gz/xz
like you normally do for release sources.

If you want to read more about it:
<http://www.spinics.net/lists/linux-wireless/msg101868.html>

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> Lauri Kasanen  writes:
> 
> > The previous text was confusing, leading readers to think this
> > driver was a duplicate, and so didn't need to be enabled.
> >
> > After the removal of the older staging driver, this is the only
> > driver in mainline for these devices.
> >
> > Signed-off-by: Lauri Kasanen 
> > ---
> > v2: Remove the mention of the previous driver, suggested by Christian.
> >
> >  drivers/net/wireless/ath/carl9170/Kconfig | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> > b/drivers/net/wireless/ath/carl9170/Kconfig
> > index 1a796e5..2e34bae 100644
> > --- a/drivers/net/wireless/ath/carl9170/Kconfig
> > +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> > @@ -5,12 +5,10 @@ config CARL9170
> > select FW_LOADER
> > select CRC32
> > help
> > - This is another driver for the Atheros "otus" 802.11n USB devices.
> > + This is the mainline driver for the Atheros "otus" 802.11n USB 
> > devices.
> >  
> > - This driver provides more features than the original,
> > - but it needs a special firmware (carl9170-1.fw) to do that.
> > -
> > - The firmware can be downloaded from our wiki here:
> > + It needs a special firmware (carl9170-1.fw), which can be downloaded
> > + from our wiki here:
> >   
> 
> Why even mention anything about a "special firmware" as the firmware is
> already available from linux-firmware.git? 
Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
but that failed.


> That's default location for all firmware images and I think most, if not all,
> distros should have it available. So wouldn't it be better not to mention 
> anything about firmware at all?
Sure, but this could be a different patch then. I think Intel devices 
(iwlwifi, iwlegacy and ipw2x00) have a similar text about "download
firmware from this device from our homepage here" too. So if we want,
we can remove them altogether?

Regards,
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 07:07:39 PM Lauri Kasanen wrote:
> The previous text was confusing, leading readers to think this
> driver was a duplicate, and so didn't need to be enabled.
> 
> After the removal of the older staging driver, this is the only
> driver in mainline for these devices.
> 
> Signed-off-by: Lauri Kasanen 
> ---
> v3: Remove all firmware mentions.
> 
>  drivers/net/wireless/ath/carl9170/Kconfig | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> b/drivers/net/wireless/ath/carl9170/Kconfig
> index 1a796e5..ca1ae09 100644
> --- a/drivers/net/wireless/ath/carl9170/Kconfig
> +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -5,13 +5,7 @@ config CARL9170
>   select FW_LOADER
>   select CRC32
>   help
> -   This is another driver for the Atheros "otus" 802.11n USB devices.
> -
> -   This driver provides more features than the original,
> -   but it needs a special firmware (carl9170-1.fw) to do that.
> -
> -   The firmware can be downloaded from our wiki here:
> -   
Please do keep the wiki link.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Saturday, April 16, 2016 05:18:56 PM Lauri Kasanen wrote:
> The previous text was confusing, leading readers to think this
> driver was a duplicate, and so didn't need to be enabled.
> 
> After the removal of the older staging driver, this is the only
> driver in mainline for these devices.
> 
> Signed-off-by: Lauri Kasanen <c...@gmx.com>
Acked-by: Christian Lamparter <chunk...@googlemail.com>

> ---
> v2: Remove the mention of the previous driver, suggested by Christian.

Thanks!
 
>  drivers/net/wireless/ath/carl9170/Kconfig | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> b/drivers/net/wireless/ath/carl9170/Kconfig
> index 1a796e5..2e34bae 100644
> --- a/drivers/net/wireless/ath/carl9170/Kconfig
> +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -5,12 +5,10 @@ config CARL9170
>   select FW_LOADER
>   select CRC32
>   help
> -   This is another driver for the Atheros "otus" 802.11n USB devices.
> +   This is the mainline driver for the Atheros "otus" 802.11n USB 
> devices.
>  
> -   This driver provides more features than the original,
> -   but it needs a special firmware (carl9170-1.fw) to do that.
> -
> -   The firmware can be downloaded from our wiki here:
> +   It needs a special firmware (carl9170-1.fw), which can be downloaded
> +   from our wiki here:
> <http://wireless.kernel.org/en/users/Drivers/carl9170>
>  
> If you choose to build a module, it'll be called carl9170.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: carl1970: stops working at random periods on Ubuntu 15.05

[Christian Lamparter]

On Friday, March 11, 2016 09:12:40 AM alexander nekrasov wrote:
> Thanks for an answer, Christian! Adapter is getting hot sometimes
> indeed. Also my adapter is usb2.0 but it was on extender with another
> usb1.1 device. I plugged it into motherboards port directly but issue
> still continue to happens. I  also tried reload host controller but it
> did not help. 
In that case, this might be a problem with the host controller on your
motherboard. I say "might", because it could also be that the ar9170
device is damaged or that something else is going on. Can you test the
device on a different PC and test if it fails in the same way?

Other than that, there's not much you can do (easily). If you want
to investigate the issue further, you would need probe the FUSB200
in the device. The only place you can do that is within the firmware
as the USB subsystem is breaking down. The firmware can be downloaded
from [0]. The register to look at is 0x1E110C (AR9170_USB_REG_DMA_STATUS).

There are a few error bits that can be checked.

Bit 24: Error when the upstream DMA access the data bus.
Bit 25: Error when the upstream DMA access the command bus.
Bit 26: Error when the downstream DMA access the data bus.
Bit 27: Error when the downstream DMA access the command bus.
Bit 28: Error when the CPU access the data bus.
Bit 29: Error when the CPU access the command bus.

If any of those bits are set, it's probably time to issue a
firmware reboot (set fw.reboot to 1 or directly call "reboot();").

That said, I don't have high hopes. In your logs, the carl9170
driver is already trying to reset the device... and failing since
it is unable to communicate with the device.

> However I discovered strange behavior. Here is my
> scenario:
> 1. boot system
> 2. check device is displayed by 'lsusb' and 'lsusb -t'
> 3. unbind and bind root hub where device is plugged into
> 4. this time 'lsusb' does list device and 'lsusb -t' does not
> 
> Is it ok?
> 
> After unbind/bind device definitely is not working - there is led
> indicator and it is off. Should I 'enable' device after binding root
> hub with another command?

I would suggest you also contact the linux-usb mailing list [1].
They may be able to debug why the host controller is not responding
to the bind and unbind.

[0] 
[1] 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: carl1970: stops working at random periods on Ubuntu 15.05

[Christian Lamparter]

Hello,

On Sunday, February 28, 2016 03:06:35 AM alexander nekrasov wrote:
> I'm using TP-LINK TL-WN821N and encountering very annoying behaviour -
> adapter 'hangs' at random period of time - sometimes it is 1 hour,
> sometimes 2. After it hangs Network manager does not see any wireless
> networks and the only way to make it work is to reboot system (also
> tried unplugging/plugging adapter and removing/adding module but it
> did not work)
Did it get hot? Or Have you tried unloading/reloading the ohci/ehci/xhci?
The used USB-PHY FUSB200 (in the AR9170) is known to misbehave. If you
want to read more about the implications and possible solution, you can
visit the ath9k_htc firmware community. They have written a wiki entry
which describes the issues in detail [0].
 
> Heres lsusb -v:
> Bus 004 Device 004: ID 0cf3:1002 Atheros Communications, Inc. TP-Link
> TL-WN821N v2 / TL-WN822N v1 802.11n [Atheros AR9170]
> 
> And here what i have in syslog right before the hang:
> Following messages are also relevant to adapter failure but i guess it
> will be to long to post it here.
> 
> Feb 28 01:46:53 evren kernel: [ 4978.753087] usb 4-1.4: no command
> feedback received (-90).
> Feb 28 01:46:53 evren kernel: [ 4978.753098] usb 4-1.4: restart device (6)
> Feb 28 01:46:54 evren kernel: [ 4979.891093] usb 4-1.4: device
> restarted successfully.
> Feb 28 01:46:54 evren kernel: [ 4979.899273] ieee80211 phy1: Hardware
> restart was requested
Commands are issued on the devices end point #4 And the response the
driver is waiting for doesn't get through. If you want to debug this, 
you'll need to look at the usb data stream and check which party (host
controller (ehci/ohci), device (AR9170) or cable?) is at fault. 

Regards,
Christian

[0] 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND] Re: carl9170 client RTS/CTS option being overridden by WAP's WMM option

[Christian Lamparter]

On Friday, February 05, 2016 04:07:22 PM Eric Hillary wrote:
> Has anyone experienced that RTS/CTS handshaking stops occurring at the
> client if the the access point enables WMM?
yes, the 802.11-draftn device has problems (rx and tx become stuck) with
802.11n and proper WMM (and with RTS/CTS). Sadly, I don't know of any
workaround that works for all the configurations I tested, other than 
going for a ath9k_htc device. 
 
> Using the carl9170 driver with RTS/CTS handshaking enabled on a USB 
> Ubiquiti SR71-USB (Atheros AR9170 based wireless adapter).  RTS/CTS will 
> stop working when WMM is enabled on the WAP.  
You can overwrite carl9170's behavior (per device). You need to have 
DEBUGFS enabled for the driver (CONFIG_CARL9170_DEBUGFS). There is a
control setting in: /sys/kernel/debug/ieee80211/phyX/carl9170/erp

1 - Automatic (default)
2 - Set by mac80211
3 - Force off
4 - Force CTS
5 - Force RTS

You can try echo 2 > erp. And check the setting with cat erp
> The network is running in 802.11n C-band (5GHz) channels in 
> Infrastructure mode with an Atheros AR9370 based Access Point.  We are 
> using RTS/CTS handshaking to reduce “Hidden Node” effects.  Other 
> clients in the network require WMM to be enabled.  For reasons cited in 
> http://www.smallnetbuilder.com/wireless/wireless-features/30938-dont-
> mess-with-wmm, WMM should be enabled to support 802.11e, 
> required for 802.11n to use HT (High Throughput) link rates.
> 
> With the WMM turned OFF at the AP and RTS value = 256 at the Ubiquiti 
> client, the RTS/CTS handshake functions normally.  With WMM turned ON at 
> the AP and RTS value = 256 still on at the client no RTS/CTS handshake 
> occurs.  The carl9170 client stops sending the RTS, and the carl1970 
> client ignoring the CTS messages transmitted by the AP, causing 
> collisions because of my “Hidden Node” condition.
> 
> Had to get scope plots of the RF network traffic between the AP and 
> client to observe this happening.
scope plots from a real analyzer? I would love to have a look at those :).

Regards,
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mac80211: fix oops in ieee80211_beacon_get_tim

[Christian Lamparter]

This patch fixes a crash which is triggered
by __ieee80211_beacon_get returning NULL.
This causes sky_copy to crash later unless
the hardware supports BEACON_TX_STATUS
feature.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
"mac80211: Copy tx'ed beacons to monitor mode" added the skb_copy.
There are few other possibilities to do this. This is just one.
---
 net/mac80211/tx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f7317a7..666e46b 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3530,6 +3530,9 @@ struct sk_buff *ieee80211_beacon_get_tim(struct 
ieee80211_hw *hw,
struct ieee80211_supported_band *sband;
int shift;
 
+   if (!bcn)
+   return bcn;
+
if (tim_offset)
*tim_offset = offs.tim_offset;
 
-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] carl9170: fix bad rssi reading

[Christian Lamparter]

On Thursday, September 10, 2015 06:04:45 PM Hiroaki KAWAI wrote:
> Fix rssi calculation error which was introduced in otus to ar9170
> porting.
> 
> Signed-off-by: Hiroaki KAWAI <hiroaki.ka...@gmail.com>
Acked-by: Christian Lamparter <chunk...@googlemail.com>

For reference: Here's the line in the original otus code:
<http://lxr.free-electrons.com/source/drivers/staging/otus/hal/hpusb.c?v=2.6.33#L818>

This value/result isn't used anywhere, so we could also remove it. 

> ---
>  drivers/net/wireless/ath/carl9170/rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/rx.c 
> b/drivers/net/wireless/ath/carl9170/rx.c
> index 924135b..d66533c 100644
> --- a/drivers/net/wireless/ath/carl9170/rx.c
> +++ b/drivers/net/wireless/ath/carl9170/rx.c
> @@ -453,7 +453,7 @@ static void carl9170_rx_phy_status(struct ar9170 *ar,
>   /* post-process RSSI */
>   for (i = 0; i < 7; i++)
>   if (phy->rssi[i] & 0x80)
> - phy->rssi[i] = ((phy->rssi[i] & 0x7f) + 1) & 0x7f;
> + phy->rssi[i] = ((~phy->rssi[i] & 0x7f) + 1) & 0x7f;
>  
>   /* TODO: we could do something with phy_errors */
>   status->signal = ar->noise[0] + phy->rssi_combined;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel BUG in rtl92s_phy_chk_fwcmd_iodone():0-0 Set FW Cmd fail

[Christian Lamparter]

On Tuesday, July 07, 2015 11:56:21 PM Szőts Ákos wrote:
 I have an RTL8191SU 802.11n USB WLAN Adapter with rtl8192su driver. 
 Unfortunately, I have to turn it off and on sometimes to make it work. I use 
 NetworkManager with KDE 5 nm-applet in which I simply turn off the wireless 
 capability then on again.
 
 Once I did this, I got a kernel BUG. Here is the link for the full text: 
 http://paste.opensuse.org/view/simple/25089935

This message lets you know that a firmware command has timed out.
A call trace is included to let you know what the driver was doing
at the time. If this problem bothers you, the information from the
trace should help to debug it and make a patch.
 
 kernel: rtl8192s_common:rtl92s_phy_chk_fwcmd_iodone():0-0 Set FW Cmd fail!!
 kernel: [ cut here ]
 kernel: WARNING: CPU: 3 PID: 1105 at 
 /home/aki/temp/rtl8192su/rtlwifi/rtl8192s/phy_common.c:1192 
 rtl92s_phy_chk_fwcmd_iodone+0x65/0xb0 [rtl8192s_common]()
 kernel: Modules linked in: rtl8192su(O) rtl8192s_common(O) rtl_usb(O) 
 rtlwifi(O)...
 kernel: Call Trace:
 kernel:  [816d178e] dump_stack+0x4c/0x6e
 kernel:  [8106b4fa] warn_slowpath_common+0x8a/0xc0
 kernel:  [8106b5ea] warn_slowpath_null+0x1a/0x20
 kernel:  [a05ef875] rtl92s_phy_chk_fwcmd_iodone+0x65/0xb0 
 [rtl8192s_common]
 kernel:  [a05f1c31] _rtl92s_phy_set_fwcmd_io+0xe1/0x670 
 [rtl8192s_common]
 kernel:  [a05f2279] rtl92s_phy_set_fw_cmd+0xb9/0x7b0 
 [rtl8192s_common]
 kernel:  [a05f29aa] rtl92s_phy_scan_operation_backup+0x3a/0x90 
 [rtl8192s_common]
 kernel:  [a061b8d9] rtl_op_sw_scan_start+0xa9/0x170 [rtlwifi]

Regards,
Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Realtek RTL8191SU really slow with „N” mode enabled

[Christian Lamparter]

Hello,

On Saturday, June 27, 2015 11:16:21 AM Szőts Ákos wrote:
 I have a 0bda:8172 Realtek RTL8191SU 802.11n WLAN Adapter” working
 - previously with kernel v4.0 staging driver r8712u,
 - and now with wireless kernel v4.1 driver rtl8192su (from 
 https://github.com/chunkeey/rtl8192su).
 
 My problem is when, by default, the 802.11n mode is enabled, the USB dongle's 
 connection speed is only around 1 Mb/s (with both drivers). When I turn it 
 off 
 with options rtl8192su ht_enable=0 in its config file, I get my full 
 internet 
 speed, about 30 Mb/s.

Wait! There isn't any ht_enable option for the rtl8192su(.ko) driver...?!

 I'm asking whether it's a problem in the firmware or somewhere in the driver 
 code? If it's the latter case and you're interested in, I would like to help 
 debugging the problem and help you to fix this issue.
Can't help you with debugging the problem as long as I can't reproduce the
issue. No idea if this is a hardware, firmware, driver, setup or some
kind of interaction issue. In my opinion: you simply disable HT, then you
just did take a feature away rather than dealing with anything. But, if
that's what you want: fine.

Note: I think the simplest solution would be to have a working reference.
If you are looking for a wifi usb-stick, ath9k_htc solutions are usually
the way to go.

Regards,
  Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Wireless Module: Carl9170 driver and firmware communication

[Christian Lamparter]

Hello,

On Tue, Mar 31, 2015 at 2:40 AM, Ashkan Parcham Kashani
ashkan.parchamkash...@mail.utoronto.ca wrote:
 I'm working on a project where I need to communicate some additional
 control information between the carl9170 driver and firmware. I am not
 exactly sure where the best place to add this extra information would be.
 I assumed that the DMA might work if I can extend it and reserve some
 space for the controls. I only need a few bytes for the communication.

Should be doable. Add your fields to the _carl9170_tx_superdesc
and carl9170_tx_superdesc for both the driver and the firmware
and recompile. Watch out: the size of the structs should be a
multiple of 4. (Note: this is only for TX!)

 I also considered using device registers for this, but it's hard to
 find documentation of which registers may be free to use.
This is way easier, as the modified firmware will continue to work
with the standard driver and vice versa. The driver can directly
write into and read from the program/data ram of the firmware.
Add your fields to the firmware_context_struct in carlfw/include/carl9170.h
and add a handle_mywork() into main_loop() in carlfw/src/main.c.

 I'm also wondering how one would go about interrupting the firmware.
 At times, the driver will need to interrupt the firmware, in which
 case the firmware should stop transmitting any packets it is currently
 transmitting.
Depends on how you implement it. There's a watchdog that needs to
be serviced. If you don't want to deal with the dog, you can set (stops
TX) or clear (resumes TX) the AR9170_MAC_VIRTUAL_CCA_QX bits
in the AR9170_MAC_REG_QOS_PRIORITY_VIRTUAL_CCA register.

Note: The firmware doesn't have any real interrupt handling.

Regards,
   Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[Christian Lamparter]

On Saturday, March 14, 2015 06:55:32 PM Florian Westphal wrote:
 Christian Lamparter chunk...@googlemail.com wrote:
  On Friday, March 13, 2015 04:37:25 PM Florian Westphal wrote:
   its embededded inside rate_driver_data of the ieee80211_tx_info struct,
   which in turn is stored in skb-cb[].
   
   In order to shrink cb, we need to shrink ieee80211_tx_info which means
   to downsize all users first.
   
   Alternatively, one might be able to remove kref but
   its less intrusive/simpler to use u32 for timeout handling.
   
   Signed-off-by: Florian Westphal f...@strlen.de
   ---
  u32 jiffies... that's a lot of pointing (well not so much) and really 
  ugly casting (a lot). I guess it would be easier to just use a per-queue
  timeout watchdog like almost everybody else. This way, the driver will
  be ready for the next skb-cb shrink as well.
  
  carl9170_tx_ampdu_timeout can be completely removed.
 
 Maybe, but there is only so much I am willing to do with compile
 tested only patches...
well, reinventing jiffies_32 is just a ugly hack and not intrusive/simpler
than removing kref here.

Please try again. Don't worry about testing, I'm sure this is not a problem
and someone will test the patches with the hw on 32-bit and 64-bit platforms.

Regards,
  Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: carl1970: monitor mode only displaying beacons/probs from APs?

[Christian Lamparter]

Hello,

On Wednesday, March 04, 2015 11:02:25 PM ma...@tampabay.rr.com wrote:
 Quite a while back after doing an upgrade to the latest (back then)
 compat-wireless, I noticed that I was only seeing beacon,prob requests, and
 the occasional data packet when in monitor mode; which at the time I wrote
 off.  After switching to backports-3.18.1-1 recently, I still see the same
 behavior(on x86 and arm).  I'm sure I'm missing something, but I couldn't
 find any references to this behavior in my search.  I also tried iw wlanX
 set monitor control otherbss to no avail.  Switching to a different chipset
 using ath9k-htc showed expected traffic.
 
 Can anyone point me in the right direction?
maybe. Could it be that you are looking for 802.11n(MCS) data frames?
Because by default, carl9170's monitor mode only catches the legacy 
802.11a/b/g frames. If this is the case, then try setting the right
channel mode (HT20/HT40+/HT40-) when selecting the channel. i.e.:  

# iw dev wlanX set channel Y HT20
# iw dev wlanX set channel Y HT40+

I hope this helps. Otherwise, you could try: 

# iw dev wlanX set monitor [...] fcsfail

and see if you are picking up frames this way. This should work, but you
will be picking up mostly damaged stuff, so some extra frame processing
will be needed to filter out the noise. 

Regards,
  Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [IWLMVM] Firmware restart issue with 3.19-rc6-wl with AC 3160, REV=0x164

[Christian Lamparter]

Hello,

On Fri, Jan 30, 2015 at 9:35 PM, Johannes Berg
johan...@sipsolutions.net wrote:
 On Fri, 2015-01-30 at 21:23 +0100, Christian Lamparter wrote:
 [6.274625] iwlwifi :01:00.0: 0x0038 | BAD_COMMAND
 [6.275034] iwlwifi :01:00.0: FW error in SYNC CMD SCD_QUEUE_CFG

 WTF. How did the firmware even announce support for this command??

 Perhaps I missed it in the log, but which firmware are you using? Any
 chance you also upgraded that? Because I can't imagine this code is new
 in the tree...
Uhh, good question: I think got this firmware from one of Emmanuel's firmware
announcements. (ls says 10 January 2015)

iwlwifi :01:00.0: loaded firmware version 23.11.10.0 op_mode iwlmvm
Loaded firmware version: 23.11.10.0

sha1sum iwl-3160-10.ucode cf1a39c1b30f389e12d17c1379a2ac539491b8e2
md5sum iwl-3160-10.ucode 9f744224cee51dac1d11708cade96641

If you need further infos, let me know!

 Anyway, make this change:

 diff --git a/drivers/net/wireless/iwlwifi/mvm/mvm.h 
 b/drivers/net/wireless/iwlwifi/mvm/mvm.h
 index 979ac23522f2..5b8c969f457e 100644
 --- a/drivers/net/wireless/iwlwifi/mvm/mvm.h
 +++ b/drivers/net/wireless/iwlwifi/mvm/mvm.h
 @@ -874,7 +874,7 @@ static inline bool iwl_mvm_is_d0i3_supported(struct 
 iwl_mvm *mvm)

  static inline bool iwl_mvm_is_scd_cfg_supported(struct iwl_mvm *mvm)
  {
 -   return mvm-fw-ucode_capa.capa[0]  IWL_UCODE_TLV_API_SCD_CFG;
 +   return false;
  }

  extern const u8 iwl_mvm_ac_to_tx_fifo[];

 I guess Emmanuel will look at it on Sunday :)
Thanks! That did the trick!

Regards,
   Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [IWLMVM] Firmware restart issue with 3.19-rc6-wl with AC 3160, REV=0x164

[Christian Lamparter]

On Sat, Jan 31, 2015 at 6:36 PM, Grumbach, Emmanuel
emmanuel.grumb...@intel.com wrote:
 On Sat, 2015-01-31 at 15:16 +0100, Christian Lamparter wrote:
  I guess Emmanuel will look at it on Sunday :)

 Or earlier.

 Thanks! That did the trick!


 what about this:
 https://git.kernel.org/cgit/linux/kernel/git/iwlwifi/iwlwifi-next.git/commit/?id=c774905d98badddbeb4ad1bc653aedbd7ab024e4

Works.
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[IWLMVM] Firmware restart issue with 3.19-rc6-wl with AC 3160, REV=0x164

[Christian Lamparter]

Hello,

since upgrading to 3.19-rc6-wl (3.19-rc5-wl is working fine). I can no
longer use the IWL 3160 in my laptop. As soon as the link goes up (the
modules load fine, just ip/ifconfig up), I get this:

[5.960381] iwlwifi :01:00.0: L1 Enabled - LTR Disabled
[5.960722] iwlwifi :01:00.0: L1 Enabled - LTR Disabled
[6.256941] iwlwifi :01:00.0: L1 Enabled - LTR Disabled
[6.257392] iwlwifi :01:00.0: L1 Enabled - LTR Disabled
[6.272858] iwlwifi :01:00.0: Microcode SW error detected.
Restarting 0x8200.
[6.272875] iwlwifi :01:00.0: CSR values:
[6.272886] iwlwifi :01:00.0: (2nd byte of CSR_INT_COALESCING
is CSR_INT_PERIODIC_REG)
[6.272937] iwlwifi :01:00.0:CSR_HW_IF_CONFIG_REG: 0X00c89204
[6.272985] iwlwifi :01:00.0:  CSR_INT_COALESCING: 0X8000ff40
[6.273033] iwlwifi :01:00.0: CSR_INT: 0X
[6.273080] iwlwifi :01:00.0:CSR_INT_MASK: 0X
[6.273127] iwlwifi :01:00.0:   CSR_FH_INT_STATUS: 0X
[6.273173] iwlwifi :01:00.0: CSR_GPIO_IN: 0X
[6.273220] iwlwifi :01:00.0:   CSR_RESET: 0X
[6.273266] iwlwifi :01:00.0:CSR_GP_CNTRL: 0X080403cd
[6.273313] iwlwifi :01:00.0:  CSR_HW_REV: 0X0164
[6.273359] iwlwifi :01:00.0:  CSR_EEPROM_REG: 0X
[6.273406] iwlwifi :01:00.0:   CSR_EEPROM_GP: 0X8000
[6.273452] iwlwifi :01:00.0:  CSR_OTP_GP_REG: 0X803a
[6.273499] iwlwifi :01:00.0: CSR_GIO_REG: 0X00080042
[6.273545] iwlwifi :01:00.0:CSR_GP_UCODE_REG: 0X
[6.273591] iwlwifi :01:00.0:   CSR_GP_DRIVER_REG: 0X
[6.273637] iwlwifi :01:00.0:   CSR_UCODE_DRV_GP1: 0X
[6.273684] iwlwifi :01:00.0:   CSR_UCODE_DRV_GP2: 0X
[6.273730] iwlwifi :01:00.0: CSR_LED_REG: 0X0018
[6.273777] iwlwifi :01:00.0:CSR_DRAM_INT_TBL_REG: 0X880beb28
[6.273823] iwlwifi :01:00.0:CSR_GIO_CHICKEN_BITS: 0X27800200
[6.273869] iwlwifi :01:00.0: CSR_ANA_PLL_CFG: 0Xd5d5
[6.273916] iwlwifi :01:00.0:  CSR_MONITOR_STATUS_REG: 0Xdbb7fff7
[6.273962] iwlwifi :01:00.0:   CSR_HW_REV_WA_REG: 0X0001001a
[6.274009] iwlwifi :01:00.0:CSR_DBG_HPET_MEM_REG: 0X
[6.274018] iwlwifi :01:00.0: FH register values:
[6.274067] iwlwifi :01:00.0:
FH_RSCSR_CHNL0_STTS_WPTR_REG: 0X2921bc00
[6.274114] iwlwifi :01:00.0:
FH_RSCSR_CHNL0_RBDCB_BASE_REG: 0X02921bd0
[6.274161] iwlwifi :01:00.0:
FH_RSCSR_CHNL0_WPTR: 0X0010
[6.274209] iwlwifi :01:00.0:
FH_MEM_RCSR_CHNL0_CONFIG_REG: 0X80801114
[6.274256] iwlwifi :01:00.0:
FH_MEM_RSSR_SHARED_CTRL_REG: 0X00fc
[6.274304] iwlwifi :01:00.0:
FH_MEM_RSSR_RX_STATUS_REG: 0X0703
[6.274351] iwlwifi :01:00.0:
FH_MEM_RSSR_RX_ENABLE_ERR_IRQ2DRV: 0X
[6.274398] iwlwifi :01:00.0:
FH_TSSR_TX_STATUS_REG: 0X07ff0001
[6.274446] iwlwifi :01:00.0:
FH_TSSR_TX_ERROR_REG: 0X
[6.274594] iwlwifi :01:00.0: Start IWL Error Log Dump:
[6.274605] iwlwifi :01:00.0: Status: 0x, count: 6
[6.274614] iwlwifi :01:00.0: Loaded firmware version: 23.11.10.0
[6.274625] iwlwifi :01:00.0: 0x0038 | BAD_COMMAND
[6.274637] iwlwifi :01:00.0: 0x02F0 | uPc
[6.274647] iwlwifi :01:00.0: 0x | branchlink1
[6.274657] iwlwifi :01:00.0: 0x0B3C | branchlink2
[6.274667] iwlwifi :01:00.0: 0x00014960 | interruptlink1
[6.274676] iwlwifi :01:00.0: 0x00060117 | interruptlink2
[6.274685] iwlwifi :01:00.0: 0x0915001D | data1
[6.274695] iwlwifi :01:00.0: 0xDEADBEEF | data2
[6.274704] iwlwifi :01:00.0: 0xDEADBEEF | data3
[6.274713] iwlwifi :01:00.0: 0x003FE973 | beacon time
[6.274722] iwlwifi :01:00.0: 0x168C | tsf low
[6.274731] iwlwifi :01:00.0: 0x | tsf hi
[6.274740] iwlwifi :01:00.0: 0x | time gp1
[6.274749] iwlwifi :01:00.0: 0x168C | time gp2
[6.274758] iwlwifi :01:00.0: 0x | time gp3
[6.274767] iwlwifi :01:00.0: 0x0004170B | uCode version
[6.274776] iwlwifi :01:00.0: 0x0164 | hw version
[6.274785] iwlwifi :01:00.0: 0x00C89204 | board version
[6.274794] iwlwifi :01:00.0: 0x0915001D | hcmd
[6.274803] iwlwifi :01:00.0: 0x00022080 | isr0
[6.274812] iwlwifi :01:00.0: 0x | isr1
[6.274821] iwlwifi :01:00.0: 0x0002 | isr2
[6.274829] iwlwifi :01:00.0: 0x004000C0 | isr3
[6.274838] iwlwifi :01:00.0: 0x | isr4
[6.274847] iwlwifi :01:00.0: 0x01000112 | isr_pref
[6.274856] iwlwifi :01:00.0: 0x | 

Re: [PATCH] wireless: p54: add handling of the signal case

[Christian Lamparter]

On Friday, January 23, 2015 01:27:05 PM Nicholas Mc Guire wrote:
 On Thu, 22 Jan 2015, Christian Lamparter wrote:
  On Tuesday, January 20, 2015 06:25:43 AM Nicholas Mc Guire wrote:
   @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, 
   void *buf,
   + wiphy_err(priv-hw-wiphy,
   + device does not respond or signal received!\n);
 
 Tried fixing this up - but simply no clue what coding style
 rule that might have violated - and my attempts to fix this
 did not succeed - allignment seems righta - the complete siequence is:
 
 timeout = wait_for_completion_interruptible_timeout(
 priv-eeprom_comp, HZ);
 if (timeout = 0) {
 wiphy_err(priv-hw-wiphy,
 device does not respond or signal received!\n);
 ret = -EBUSY;
 }
 
 what am I overlooking ?
Not much, you might be confused simply because your editor and email program
doesn't use a fixed space font (or it is not enabled (yet)).

Fixing this is just a matter of adding a few extra spaces so the text lines
up with the open parenthesis.

original:
+   wiphy_err(priv-hw-wiphy,
+   device does not respond or signal received!\n);

aligned:
+   wiphy_err(priv-hw-wiphy,
+device does not respond or signal 
received!\n);

Regards,
Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wireless: p54: add handling of the signal case

[Christian Lamparter]

On Tuesday, January 20, 2015 06:25:43 AM Nicholas Mc Guire wrote:
 if(!wait_for_completion_interruptible_timeout(...))
 only handles the timeout case - this patch adds handling the
 signal case the same as timeout.
 
 Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
Acked-by: Christian Lamparter chunk...@googlemail.com

 diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c
 index bc065e8..5367d51 100644
 --- a/drivers/net/wireless/p54/fwio.c
 +++ b/drivers/net/wireless/p54/fwio.c
 @@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void 
 *buf,
  
   p54_tx(priv, skb);
  
 - if (!wait_for_completion_interruptible_timeout(
 -  priv-eeprom_comp, HZ)) {
 - wiphy_err(priv-hw-wiphy, device does not respond!\n);
 + timeout = wait_for_completion_interruptible_timeout(
 + priv-eeprom_comp, HZ);
 + if (timeout = 0) {
 + wiphy_err(priv-hw-wiphy,
 + device does not respond or signal received!\n);
   ret = -EBUSY;
   }
   priv-eeprom = NULL;
 

CHECK: Alignment should match open parenthesis
#98: FILE: drivers/net/wireless/p54/fwio.c:257:
+   wiphy_err(priv-hw-wiphy,
+   device does not respond or signal received!\n);

total: 0 errors, 0 warnings, 1 checks, 21 lines checked

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wireless: p54pci: add handling of signal case

[Christian Lamparter]

On Tuesday, January 20, 2015 06:26:17 AM Nicholas Mc Guire wrote:
 if(!wait_for_completion_interruptible_timeout(...))
 only handles the timeout case - this patch adds handling the
 signal case the same as timeout.
 
 Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
Acked-by: Christian Lamparter chunk...@googlemail.com

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFT 3/3] cw1200: use mac80211 to resize outgoing frames for protected

[Christian Lamparter]

The patch mac80211: add feature flag to enable resizing of
outgoing frames for encryption  introduced a feature  flag
which has to be set for cw1200 to re-enable resizing of
outgoing frames for hardware-offloaded CCMP connections.

Cc: Solomon Peachy pi...@shaftnet.org
Signed-off-by: Christian Lamparter chunk...@googlemail.com
---
 drivers/net/wireless/cw1200/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/cw1200/main.c 
b/drivers/net/wireless/cw1200/main.c
index 3e78cc3..69c1f51 100644
--- a/drivers/net/wireless/cw1200/main.c
+++ b/drivers/net/wireless/cw1200/main.c
@@ -281,6 +281,7 @@ static struct ieee80211_hw *cw1200_init_common(const u8 
*macaddr,
hw-flags = IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_SUPPORTS_DYNAMIC_PS |
+   IEEE80211_HW_TAILROOM_CRYPTO |
IEEE80211_HW_REPORTS_TX_ACK_STATUS |
IEEE80211_HW_SUPPORTS_UAPSD |
IEEE80211_HW_CONNECTION_MONITOR |
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3.17 2/3] p54: use mac80211 to resize outgoing frames for protected

[Christian Lamparter]

The patch mac80211: add feature flag to enable resizing of
outgoing frames for encryption  introduced a feature  flag
which has to be set for p54 to re-enable resizing of outgoing
frames for hardware-offloaded CCMP connections.

Cc: sta...@vger.kernel.org # mac80211: add feature flag to enable resizing of 
outgoing frames for encryption
Cc: Kalle Valo kv...@codeaurora.org
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=90331
Reported-by: Christopher Chavez chrischa...@gmx.us
Tested-by: Larry Finger larry.fin...@lwfinger.net
Signed-off-by: Christian Lamparter chunk...@googlemail.com
---
 drivers/net/wireless/p54/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
index 97aeff0..b877c7f 100644
--- a/drivers/net/wireless/p54/main.c
+++ b/drivers/net/wireless/p54/main.c
@@ -751,6 +751,7 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len)
 IEEE80211_HW_SUPPORTS_PS |
 IEEE80211_HW_PS_NULLFUNC_STACK |
 IEEE80211_HW_MFP_CAPABLE |
+IEEE80211_HW_TAILROOM_CRYPTO |
 IEEE80211_HW_REPORTS_TX_ACK_STATUS;
 
dev-wiphy-interface_modes = BIT(NL80211_IFTYPE_STATION) |
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: p54usb kernel panic on recent mainline kernels

[Christian Lamparter]

[Readded Larry to the CC]

On Saturday, December 27, 2014 12:15:58 AM Christopher Chavez wrote:
  My bisection led to a branch commit d17ec4d as the bad commit. 
  Rather than finding out where the bisection went bad, I added 
  code to check skb-tail, skb-end, and the length to be added.
  At the time of the call that panics, there are 6 bytes between
  tail and end with 8 bytes needed.
 
  I will be looking for the place where the driver calculates how
  large the skb should be.

I think this narrows it down. However, I'm not 100% sure yet if the
problem is just because of mac80211: don't resize skbs needlessly.

From looking at a other patch from that time and context. I think: 

commit ca34e3b5c808385b175650605faa29e71e91991b
Author: Ido Yariv i...@wizery.com
Date:   Tue Jul 29 15:38:53 2014 +0300

mac80211: Fix accounting of the tailroom-needed counter [1]

When hw acceleration is enabled, the GENERATE_IV or PUT_IV_SPACE flags
will only require headroom space. Consequently, the tailroom-needed
counter can safely be decremented.

changed/broke things for p54* (note: cw1200 could be affected as well?
This driver also modifies the tailroom for skbs in cw1200_tx_h_crypt).
Previously, the driver didn't need to manage the tailroom. If the 
IEEE80211_KEY_FLAG_GENERATE_IV flag was set, mac80211 would take care of
resizing the skb at the right time and just in one place [of course the
downside was that mac80211 did the resize needlessly].

I can think of several ways of dealing with this issue:

 1. move the expand and trim tailroom into the driver.
AFAICT this would add an additional resize [at a bad time].

 2. add extra IEEE80211_KEY_FLAG_ or HW_FLAG to restore the old behavior.
This should be possible and relatively simple. But we/I have to be
especially careful to differentiate properly between the old and new.
[i.e.: I need to know what the deal is behind: 
IEEE80211_KEY_FLAG_GENERATE_IV_MGMT in this case? Looks like it can
be ignored?]

 3. suggestions?
[No, I'm not going to touch crypto_tx_tailroom_needed_cnt outside of
mac80211 :D]

Regards,
Christian

[1] http://www.spinics.net/lists/linux-wireless/msg125374.html
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: p54usb kernel panic on recent mainline kernels

[Christian Lamparter]

Alright, here's lunch [for the people in CET].

On Saturday, December 27, 2014 11:10:16 AM Christian Lamparter wrote:
 On Saturday, December 27, 2014 12:15:58 AM Christopher Chavez wrote:
   My bisection led to a branch commit d17ec4d as the bad commit. 
   Rather than finding out where the bisection went bad, I added 
   code to check skb-tail, skb-end, and the length to be added.
   At the time of the call that panics, there are 6 bytes between
   tail and end with 8 bytes needed.
  
   I will be looking for the place where the driver calculates how
   large the skb should be.
 
 From looking at a other patch from that time and context. I think: 
 
 commit ca34e3b5c808385b175650605faa29e71e91991b
 Author: Ido Yariv i...@wizery.com
 Date:   Tue Jul 29 15:38:53 2014 +0300
 
 mac80211: Fix accounting of the tailroom-needed counter [1]
 
 [...]
 I can think of several ways of dealing with this issue:
 
  2. add extra IEEE80211_KEY_FLAG_ or HW_FLAG to restore the old behavior.
 This should be possible and relatively simple. But we/I have to be
 especially careful to differentiate properly between the old and new.
 [i.e.: I need to know what the deal is behind: 
 IEEE80211_KEY_FLAG_GENERATE_IV_MGMT in this case? Looks like it can
 be ignored?]
 

---
[Note: for convenience this patch is rolled into one for now -
if it and the concept works, I'll make two parts. one for p54
one for mac80211 [both -stable]. It would be great if someone
could proofread the commit message though - and provide 
tested-by tags if possible]

mac80211: re-enable tailroom resizing when needed for hardware encryption

The patch mac80211: Fix accounting of the tailroom-needed counter reduced
the overhead associated with unnecessary resizing of outgoing frames. 
Unfortunately this change broke the assumption that there is always enough
tailroom and this in turn caused p54* to panic.   

Reported-by: Christopher Chavez chrischa...@gmx.us
Signed-off-by: Christian Lamparter chunk...@googlemail.com
---
diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
index 97aeff0..b877c7f 100644
--- a/drivers/net/wireless/p54/main.c
+++ b/drivers/net/wireless/p54/main.c
@@ -751,6 +751,7 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len)
 IEEE80211_HW_SUPPORTS_PS |
 IEEE80211_HW_PS_NULLFUNC_STACK |
 IEEE80211_HW_MFP_CAPABLE |
+IEEE80211_HW_TAILROOM_CRYPTO |
 IEEE80211_HW_REPORTS_TX_ACK_STATUS;
 
dev-wiphy-interface_modes = BIT(NL80211_IFTYPE_STATION) |
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 58d719d..c04ac04 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1270,8 +1270,11 @@ struct ieee80211_vif *wdev_to_ieee80211_vif(struct 
wireless_dev *wdev);
  *
  * @IEEE80211_KEY_FLAG_GENERATE_IV: This flag should be set by the
  * driver to indicate that it requires IV generation for this
- * particular key. Setting this flag does not necessarily mean that SKBs
- * will have sufficient tailroom for ICV or MIC.
+ * particular key. Setting this flag does not mean that SKBs will
+ * have sufficient tailroom for ICV or MIC. If additional tailroom
+ * tailroom needs to be reserved for the ICV or MIC, the driver
+ * should also set the hardware feature flag:
+ *  %IEEE80211_HW_TAILROOM_CRYPTO.
  * @IEEE80211_KEY_FLAG_GENERATE_MMIC: This flag should be set by
  * the driver for a TKIP key if it requires Michael MIC
  * generation in software.
@@ -1583,6 +1586,10 @@ struct ieee80211_tx_control {
  * @IEEE80211_HW_MFP_CAPABLE:
  * Hardware supports management frame protection (MFP, IEEE 802.11w).
  *
+ * @IEEE80211_HW_TAILROOM_CRYPTO: The driver would like to have sufficient
+ * tailroom for ICV or MIC for outgoing frames in order to perform
+ * hardware encryption without any additional resizing overhead.
+ *
  * @IEEE80211_HW_SUPPORTS_UAPSD:
  * Hardware supports Unscheduled Automatic Power Save Delivery
  * (U-APSD) in managed mode. The mode is configured with
@@ -1673,7 +1680,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_MFP_CAPABLE= 113,
IEEE80211_HW_WANT_MONITOR_VIF   = 114,
IEEE80211_HW_NO_AUTO_VIF= 115,
-   /* free slot */
+   IEEE80211_HW_TAILROOM_CRYPTO= 116,
IEEE80211_HW_SUPPORTS_UAPSD = 117,
IEEE80211_HW_REPORTS_TX_ACK_STATUS  = 118,
IEEE80211_HW_CONNECTION_MONITOR = 119,
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 0bb7038..c3e9a9a 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -140,7 +140,8 @@ static int ieee80211_key_enable_hw_accel(struct 
ieee80211_key *key)
if (!ret) {
key-flags |= KEY_FLAG_UPLOADED_TO_HARDWARE;
 
-   if (!(key

Re: p54usb kernel panic on recent mainline kernels

[Christian Lamparter]

Hello,

On Wednesday, December 24, 2014 10:39:55 PM Christopher Chavez wrote:
 When a device using p54usb joins/connects/associates with an access
 point, a kernel panic occurs.
I've just got out one of two of my p54usb devices. My device is able
scan, connect, receive and pass traffic to and from my WPA2 AP without
causing any panics.

 The AP tested uses WPA2; have not tested whether the issue occurs for
 other security types or ad hoc connections. The specific devices
 tested are 2Wire 802.11g USB v1 (vendor 1630 device 0005).
All I have are p54usb V2 devices [Specifically, Dell 1450 USB V2]. 
I don't know if V1 works or not - So, this might actually be the
culprit right here.

 The firmware used is 2.13.1.0.lm86.arm (a.k.a. isl3886usb 
 recommended on wireless.kernel.org). Tested on Ubuntu 14.10,
 32-bit x86 (have not tested 64-bit or other architectures).
V2 firmware (due to V2 device). I only have 64-Bit arch. 

 Tested on machines with Intel and SiS USB chipsets.
It works with the Intel Z87 chipset I have. Don't know
about other chipsets from different vendors.

 I can try collecting more info (e.g. dmesg output), and am currently 
 bisecting the kernel somewhere around 3.17-rc1.
I'm running 3.19-rc1(-wl). I haven't seen or heard anything wrong
with it in the past. It is supposed to work.

 Should this be reported as a kernel bug or with the driver?
no special mailing list, use the linux wireless list 
yes, linux-wireless@vger.kernel.org is fine for development
and firmware issues.

http://wireless.kernel.org/en/users/Drivers/p54#Contact

I can't really dig into anything specific to v1 [no device],
chipset or usb-subsystem. It would be nice if you can capture
a crash and post it [preferably with the right subsystem in CC].

Regards,

Christian
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: p54usb kernel panic on recent mainline kernels

[Christian Lamparter]

On Friday, December 26, 2014 04:23:21 AM Christopher Chavez wrote:
 Larry Finger Larry.Finger@... writes:
  In any case, file a bug report at bugzilla.kernel.org, mark it as a 
  regression,
  and post the bug number here. If you are able to finish the bisection, that
  would be helpful.
 
 Reported.
 https://bugzilla.kernel.org/show_bug.cgi?id=90331

According to what Larry wrote, this bug should disappear, if
the hw crypto offloading is disabled. 

Christopher, can you please load the p54common kernel module 
with the module parameter nohwcrypt=1 and report back?

Regards,

Christian

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html