Re: [RFC v4 06/21] ath10k: sdio support

2017-03-10 Thread Valo, Kalle
"Valo, Kalle"  writes:

> Erik Stromdahl  writes:
>
>> sdio/mailbox HIF implementation.
>>
>> Signed-off-by: Erik Stromdahl 
>
> I'm looking at this more carefully now and noticed this:
>
>> +static int ath10k_sdio_bmi_credits(struct ath10k *ar)
>> +{
>> +int ret;
>> +u32 addr, *cmd_credits;
>> +unsigned long timeout;
>> +
>> +cmd_credits = kzalloc(sizeof(*cmd_credits), GFP_KERNEL);
>> +if (!cmd_credits) {
>> +ret = -ENOMEM;
>> +goto err;
>> +}
>> +
>> +/* Read the counter register to get the command credits */
>> +addr = MBOX_COUNT_DEC_ADDRESS + ATH10K_HIF_MBOX_NUM_MAX * 4;
>> +
>> +timeout = jiffies + BMI_COMMUNICATION_TIMEOUT_HZ;
>> +while (time_before(jiffies, timeout) && !*cmd_credits) {
>> +/* Hit the credit counter with a 4-byte access, the first byte
>> + * read will hit the counter and cause a decrement, while the
>> + * remaining 3 bytes has no effect. The rationale behind this
>> + * is to make all HIF accesses 4-byte aligned.
>> + */
>> +ret = ath10k_sdio_read_write_sync(ar, addr,
>> +  (u8 *)cmd_credits,
>> +  sizeof(*cmd_credits),
>> +  HIF_RD_SYNC_BYTE_INC);
>> +if (ret) {
>> +ath10k_warn(ar,
>> + "Unable to decrement the command credit count register: %d\n",
>> +ret);
>> +goto err_free;
>> +}
>> +
>> +/* The counter is only 8 bits.
>> + * Ignore anything in the upper 3 bytes
>> + */
>> +*cmd_credits &= 0xFF;
>> +}
>> +
>> +if (!*cmd_credits) {
>> +ath10k_warn(ar, "bmi communication timeout\n");
>> +ret = -ETIMEDOUT;
>> +goto err_free;
>> +}
>> +
>> +return 0;
>> +err_free:
>> +kfree(cmd_credits);
>> +err:
>> +return ret;
>> +}
>
> AFAICS we are leaking cmd_credits if there's no error. Or is the buffer
> freed somewhere within the mmc stack or something? The reason why I ask
> is that I saw the same pattern in multiple functions so I'm curious.

Also I'm worried about endianness. We are reading from the device
directly to an u32 variable and not converting the bytes. Is the MMC
subsystem doing the conversion, I guess not?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v4 06/21] ath10k: sdio support

2017-03-10 Thread Valo, Kalle
Erik Stromdahl  writes:

> sdio/mailbox HIF implementation.
>
> Signed-off-by: Erik Stromdahl 

I'm looking at this more carefully now and noticed this:

> +static int ath10k_sdio_bmi_credits(struct ath10k *ar)
> +{
> + int ret;
> + u32 addr, *cmd_credits;
> + unsigned long timeout;
> +
> + cmd_credits = kzalloc(sizeof(*cmd_credits), GFP_KERNEL);
> + if (!cmd_credits) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + /* Read the counter register to get the command credits */
> + addr = MBOX_COUNT_DEC_ADDRESS + ATH10K_HIF_MBOX_NUM_MAX * 4;
> +
> + timeout = jiffies + BMI_COMMUNICATION_TIMEOUT_HZ;
> + while (time_before(jiffies, timeout) && !*cmd_credits) {
> + /* Hit the credit counter with a 4-byte access, the first byte
> +  * read will hit the counter and cause a decrement, while the
> +  * remaining 3 bytes has no effect. The rationale behind this
> +  * is to make all HIF accesses 4-byte aligned.
> +  */
> + ret = ath10k_sdio_read_write_sync(ar, addr,
> +   (u8 *)cmd_credits,
> +   sizeof(*cmd_credits),
> +   HIF_RD_SYNC_BYTE_INC);
> + if (ret) {
> + ath10k_warn(ar,
> + "Unable to decrement the command credit 
> count register: %d\n",
> + ret);
> + goto err_free;
> + }
> +
> + /* The counter is only 8 bits.
> +  * Ignore anything in the upper 3 bytes
> +  */
> + *cmd_credits &= 0xFF;
> + }
> +
> + if (!*cmd_credits) {
> + ath10k_warn(ar, "bmi communication timeout\n");
> + ret = -ETIMEDOUT;
> + goto err_free;
> + }
> +
> + return 0;
> +err_free:
> + kfree(cmd_credits);
> +err:
> + return ret;
> +}

AFAICS we are leaking cmd_credits if there's no error. Or is the buffer
freed somewhere within the mmc stack or something? The reason why I ask
is that I saw the same pattern in multiple functions so I'm curious.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Please update board-2.bin for QCA9888

2017-03-09 Thread Valo, Kalle
Ben Greear  writes:

> On 03/08/2017 11:21 PM, Valo, Kalle wrote:
>> Ben Greear  writes:
>>
>>> Newer 9886 (which use the 9888 firmware it seems) NICs will not
>>> boot unless you use the board-2.bin from the codeaura repo.
>>>
>>> Kalle:  Could you update your repo to the latest from codeaura so that it is
>>> easy to pull the latest board file into LEDE?
>>>
>>> If you have reason not to, please let me know.
>>
>> I'm not involved with codeaurora repositories so I don't know what file
>> you are talking about. What's the URL?
>>
>
> git://codeaurora.org/quic/qsdk/oss/firmware/ath10k-firmware
>
> [greearb@ben-dt3 ath10k-firmware]$ ls -l ath10k/QCA9888/hw2.0/board-2.bin
> -rw-rw-r--. 1 greearb greearb 60660 Mar  2 16:20 
> ath10k/QCA9888/hw2.0/board-2.bin
>
> [greearb@ben-dt3 ath10k-firmware]$ md5sum ath10k/QCA9888/hw2.0/board-2.bin
> 4fffcdf246cea4a4fd5ba67730262563  ath10k/QCA9888/hw2.0/board-2.bin
>
> I actually tested an older version of that file, but I assume the more
> recent one works
> fine too.  I don't have a 9886/8 system available for testing at the moment.

Ok, thanks. I'll ask internally why nobody has submitted that to me.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA4019: calibration files and board files

2017-03-09 Thread Valo, Kalle
Sven Eckelmann  writes:

> On Dienstag, 7. März 2017 08:30:54 CET Adrian Chadd wrote:
>> "use your own bmi ids". :-)
>> 
>> (yeah, this is going to make open source firmware for these things
>> more painful than it should be. :( )
>
> Thanks for the reply. I was just informed that the firmware binary will 
> behave 
> differently depending on the bmi-board-id (even when the rest of the content 
> is the same).
>
> The selection of an own bmi id is therefore also not an actual option. Ok, 
> maybe in some cases but not in general.
>
> The method to have a single board-2.bin for QCA4019 and to let ath10k select 
> the right one using bmi-board-id, bmi-chip-id and bus is simply not working.
>
> Either the board-2.bin is simply not used and only the pre-cal data is used 
> (as seen in Christian Lamparter's/Michal Kazior's patch [1]) or each board 
> has 
> to come with its own board-2.bin. 

I haven't followed the discussion very closely, so I might be way off,
but for laptop SMBIOS implementations Waldemar added a variant field to
board-2.bin so that we can have multiple images for the same subsystem
id. Could it help here also?

https://git.kernel.org/cgit/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=1657b8f84ed9fc1d2a100671f1d42d6286f20073

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Driver support for QCA6174 chip using SDIO interface

2017-03-08 Thread Valo, Kalle
Hi,

sorry for the late reply, I was on vacation and catching up now.

Erik Stromdahl  writes:

> Firmware and board files are normally distributed by the module vendor
> (MuRata, Silex, etc.).
>
> Firmware is depending on chipset, so this is something QCA should be able
> to provide.
>
> Board files are module dependent and contains calibration etc. and should
> definitely be obtained from the module vendor.
>
> If you have a discrete design you would most likely have to create your own
> board file (using ART etc.)
>
> Kalle:
> Perhaps you (or someone else at QCA) can initiate the work to make the sdio
> firmware publicly available?

I'll send a request forward internally, but that might take a while as
I'm heavily backlogged at the moment.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Please update board-2.bin for QCA9888

2017-03-08 Thread Valo, Kalle
Ben Greear  writes:

> Newer 9886 (which use the 9888 firmware it seems) NICs will not
> boot unless you use the board-2.bin from the codeaura repo.
>
> Kalle:  Could you update your repo to the latest from codeaura so that it is
> easy to pull the latest board file into LEDE?
>
> If you have reason not to, please let me know.

I'm not involved with codeaurora repositories so I don't know what file
you are talking about. What's the URL?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [OpenWrt-Devel] ATH10K VLAN firmware issue

2017-02-21 Thread Valo, Kalle
voncken  writes:

> Do you know if the firmware team planned to fix the VLAN issue on ath10k
> firmware?

I reported it forward only this week.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: 9984 VHT

2017-02-20 Thread Valo, Kalle
Sebastian Gottschall  writes:

> Am 14.02.2017 um 11:30 schrieb Valo, Kalle:
>> (you forgot to CC ath10k list, adding it back)
>>
>> Sebastian Gottschall  writes:
>>
>>> Am 10.02.2017 um 07:50 schrieb Valo, Kalle:
>>>> Sebastian Gottschall  writes:
>>>>
>>>>> Am 07.02.2017 um 13:14 schrieb Valo, Kalle:
>>>>>> Ben Greear  writes:
>>>>>>
>>>>>>> On 02/02/2017 10:42 AM, Sebastian Gottschall wrote:
>>>>>>>> Am 02.02.2017 um 19:24 schrieb Ben Greear:
>>>>>>>>
>>>>>>>>> I hacked ath10k to enable radar detection on 160Mhz bandwidths. Now 
>>>>>>>>> hostapd
>>>>>>>>> starts.
>>>>>>>>>
>>>>>>>>> I can reproduce the FW failure. It is because FW 3.3-25 release
>>>>>>>>> started asserting
>>>>>>>>> if the freq2 was zero in VHT160 mode. It seems to use both freq1
>>>>>>>>> and freq2, and not
>>>>>>>>> how the driver or linux seems to normally use them.
>>>>>>>> its even worse. starting from 3.3 it uses freq2 instead of freq1.
>>>>>>>> but i made a patch for it. but it still wont work. vdev_start still
>>>>>>>> fails
>>>>>>> Well it would have been helpful from the start to have known about this 
>>>>>>> patch.
>>>>>>>
>>>>>>> Could you post it?
>>>>>>>
>>>>>>> Kalle: Since the firmware API changed, how do you want to handle
>>>>>>> the differences here?
>>>>>> The firmware API shouldn't change like that, but if it has I guess a
>>>>>> firmware feature flag to enable a workaround in ath10k sounds like the
>>>>>> easiest solution.
>>>>> the more recent firmware have some new wmi services enabled which can
>>>>> be used as indicator as well.
>>>> I don't know what flag exactly you are referring to, but using a WMI
>>>> service flag to detect this is not really reliable. They can be enabled
>>>> or disabled between branches, or even between builds.
>>> valid argument. but these flags should than be also introduced in
>>> codeaurora images
>> I'm not involved with codeaurora images so I can't help with that.
>>
>> But the firmware is not supposed to break the firmware interface like
>> this. Can someone please write a detailed bug report as a reply to this
>> mail (what version, from which location, how exactly the interface is
>> broken) and I'll try to report the issue internally.
>
> i'm not good in this. but let me try to describe whats wrong

Thanks, this was good. I sent this forward now.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: fix block comments style

2017-02-20 Thread Valo, Kalle
"Shajakhan, Mohammed Shafi (Mohammed Shafi)" 
writes:

> Hello Marcin,
>
> To: ath10k@lists.infradead.org
> Cc: Marcin Rokicki
> Subject: [PATCH] ath10k: fix block comments style
>
> Fix output from checkpatch.pl like:
>  Block comments use a trailing */ on a separate lin
>
> [shafi] good to cc : linux-wireless mailing list as well ?

Yes, please do that. And read:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ath10k: MESH points are associated ,but not pinging each other

2017-02-20 Thread Valo, Kalle
kavitam  writes:

> I have  used  backport-4.4.2-1. I have downloaded it from the
> following link(16 Feb 2016):
>
> https://www.kernel.org/pub/linux/kernel/projects/backports/stable/v4.4.2/
>
> I thought this is the latest stable version. Is this correct?

Yeah, there hasn't been a new release for a while. Hopefully that
changes soon. You could of course always build a new package from git.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-02-19 Thread Valo, Kalle
Erik Stromdahl  writes:

> Ok, I'll do another round of checkpatch before I submit anything.
> I couldn't find the script you mentioned though (ath10k-check).

Did you check the link I gave you:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/codingstyle#checking_code

> Is it some kind of checkpatch wrapper?

It runs various tests (gcc, sparse, checkpatch), sets some checkpatch
settings (like line length) and filters out warnings we don't care
about.

> Anyway, I have a few warnings related to 'line over 80 chars' that
> is really hard to get rid of (without breaking indentation etc.) so
> I won't do anything about those for now.
>
> Then there are some other warnings about the BIT macro being preferred
> over (1 << x). I have used (1 << x) in some files despite the checkpatch
> warning in order to keep the patches consistent with the existing code.
> I think the best approach is to have a separate round of cleanup-patches
> replacing all (1 << x) with BIT(x).

These are all disabled by ath10k-check. I think it's easiest that you
forget ath10k-check for now and let me fix those in the next round.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-02-19 Thread Valo, Kalle
Erik Stromdahl  writes:

> I just noticed that Ryan Hsu submitted two patches containing similar
> functionality to what I have in my tree on github. They have not been
> integrated in the ath master yet, but they have been added to master-pending.
>
> Below are the commits I am talking about
>
> bc1054a15be3c962c83aa22476a3e9c1266da792 ath10k: improve the firmware
> download time for QCA9377
> db5af2a25bd8c1410ec4456d1d4f4e6ded2649ca ath10k: improve the firmware
> download time for QCA6174
>
> Since some of my patches overlap with these I think it would be best if
> I add these to my tree and rewrite my patches.
>
> If they haven't made it into master before I submit my updated patch series
> I will include them in my set.
>
> Is this OK?
>
> If they will be added to master soon there is no issue since I will of course
> rebase to the latest ath-MMDDHHmm before I submit anything.

That's ok, just mention this in the cover letter.

Even better is that if you can use master-pending from ath.git as the
baseline (compared to the master branch normally recommended), Ryan's
patches are there now. That way you don't need to apply anything extra
to your tree. But I'm hoping to apply Ryan's patches soon anyway.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-02-19 Thread Valo, Kalle
Kalle Valo  writes:

> Erik Stromdahl  writes:
>
>> I was actually about to email you about this.
>>
>> I have made a few more updates to the sdio code so I think it would be
>> best if I could submit a new series of patches based on this code (v4).
>>
>> Then you can tweak it (v5).
>>
>> It is only minor updates to the HIF layer (added QCA9377 support) and
>> a setup of some pll registers.
>
> Ok, that sounds good. I'll wait for v4.

Forgot to mention can you run ath10k-check before submitting them, it
shows few problems:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/codingstyle#checking_code

But if you wonder what this is about it's checkpatch warning about camel
case (doesn't like the lower case x character):

drivers/net/wireless/ath/ath10k/sdio.h:68: 

I guess I have a bug in the script when it parses checkpatch output.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-02-18 Thread Valo, Kalle
Erik Stromdahl  writes:

> I was actually about to email you about this.
>
> I have made a few more updates to the sdio code so I think it would be
> best if I could submit a new series of patches based on this code (v4).
>
> Then you can tweak it (v5).
>
> It is only minor updates to the HIF layer (added QCA9377 support) and
> a setup of some pll registers.

Ok, that sounds good. I'll wait for v4.

> btw, should I still mark them as RFC or should it be PATCH this time?
>
> If I go for PATCH, should the version be v4 or should I start from v1?

v4 would be fine. Thanks.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-02-18 Thread Valo, Kalle
Hej,

Erik Stromdahl  writes:

> This is the third version of the sdio RFC patch series.
> The actual sdio code (patch 6) has been subject to a massive overhaul,
> mainly as a result of Kalle's review comments.
> It no longer has any strong resemblance of the original ath6kl code from
> which it was originally based upon.
>
> Previous pathes 6 to 10 have been merged into one single patch.
>
> The previous series had a rework of the "HTC fake service" connect
> (ep 0 connect) that introduced a race between the actual endpoint
> connect and the HTC ready message. This issue has been addressed,
> and the current patches (3 and 4) has been rewritten accordingly.
>
> * overview of patches *
>
> Patches 1 to 4 are more or less identical to the previous RFC, with an
> exception for patch 3 that changes the "HTC fake service" connect
> (mentioned above).
>
> Patch 5 is a squashed version of previous patches 6 to 10.
>
> Patch 6 is the actual sdio patch
>
> Patches 7 to 8 are new and adds special sdio versions of BMI get
> target info and HTC ready.
>
> The new version was built and tested against:
> tag: ath-201701121109
>
> Erik Stromdahl (8):
>   ath10k: htc: made static function public
>   ath10k: htc: rx trailer lookahead support
>   ath10k: htc: move htc ctrl ep connect to htc_init
>   ath10k: htc: refactorization
>   ath10k: various sdio related definitions
>   ath10k: sdio support
>   ath10k: sdio get target info
>   ath10k: htc: ready_ext msg support

Sorry for not getting back to you earlier, haven't found time to look
this in detail during the last few weeks.

This is looking quite good now. I have some nitpicks (build warnings,
maybe reorder some patches etc) still but I think it's faster if I fix
those and send v4 as a proper patch (no RFC anymore), naturally
attributing you as the author. Is that ok for you?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ath10k: MESH points are associated ,but not pinging each other

2017-02-17 Thread Valo, Kalle
kavitam  writes:

> Hi All,
>
> I have created two mesh points on two different IEEE802.11
> system(t1024 based WiFi AP) using following steps:
>
> $sudo iw phy phy0 interface add mesh0 type mp
> $sudo ifconfig mesh0 0.0.0.0 up
> $sudo wpa_supplicant -Dnl80211 -B -i mesh0 -c 
> /mnt/jffs2/conf/mesh0_encp.conf
> $sleep 5
> $sudo brctl addif br0 mesh0
>
> Both Mesh points are associated.But  these points are not pinging from
> each other.Follwoing error message has been seen on the serial prompt
> while executing station dump on either of the system :
>
> $sudo iw mesh0 station dump
>
> WARNING: at faaf9548 [verbose debug info unavailable]
> Modules linked in: ath10k_pci(O) ath10k_core(O) ath(O) mac80211(O) 
> cfg80211(O) compat(O)
> uio_pdrv_genirq
> CPU: 0 PID: 2712 Comm: wpa_supplicant Tainted: G   O 3.12.37-rt51+ #23
> task: e29764c0 ti: e38d4000 task.ti: e38d4000
> NIP: faaf9548 LR: faafddc8 CTR: c06663a0
> REGS: e38d5880 TRAP: 0700   Tainted: G   O  (3.12.37-rt51+)
> MSR: 00029002   CR: 2204  XER: 2000
>
> GPR00: faafddc8 e38d5930 e29764c0 ef1e6d20 efa06624 000f 0005803c 0020
> GPR08:  0005  05be4900 8208 10107770  0001
> GPR16: 0004 00f0 000b 0021 101199ee 0001 e38d5e68 0008
> GPR24: e2cf5c00 ef1e6d20 c5a59600 ef1e6c20 efa06618 efa06624 c5a59600 
> NIP [faaf9548] ath10k_install_peer_wep_keys+0x28/0x250 [ath10k_core]
> LR [faafddc8] ath10k_station_assoc+0x258/0x430 [ath10k_core]
> Call Trace:
> [e38d5930] [fab0f658] ath10k_wmi_op_gen_peer_set_param+0x28/0x90 
> [ath10k_core] (unreliable)
> [e38d5960] [faafddc8] ath10k_station_assoc+0x258/0x430 [ath10k_core]
> [e38d5ae0] [faafe184] ath10k_sta_state+0x1e4/0x7b0 [ath10k_core]
> [e38d5b30] [fa4f87b4] sta_info_move_state+0x274/0x340 [mac80211]
> [e38d5b40] [fa5114b8] sta_apply_auth_flags.isra.42+0x88/0x1e0 [mac80211]
> [e38d5b60] [fa511c6c] ieee80211_change_station+0x11c/0x320 [mac80211]
> [e38d5b90] [f999dcf0] nl80211_set_station+0x290/0x2e0 [cfg80211]
> [e38d5c00] [c052148c] genl_rcv_msg+0x23c/0x3f0
> [e38d5c50] [c05207f0] netlink_rcv_skb+0x130/0x150
> [e38d5c70] [c0521238] genl_rcv+0x38/0x50
> [e38d5c80] [c051ffc0] netlink_unicast+0x170/0x1f0
> [e38d5cb0] [c0520400] netlink_sendmsg+0x290/0x3b0
> [e38d5d00] [c04b8910] sock_sendmsg+0x70/0xb0
> [e38d5da0] [c04ba2d0] ___sys_sendmsg.part.29+0x2c0/0x2d0
> [e38d5ec0] [c04bb644] __sys_sendmsg+0x54/0xa0
> [e38d5f10] [c04bbe78] SyS_socketcall+0x178/0x2b0
> [e38d5f40] [c000fb04] ret_from_syscall+0x0/0x3c
> --- Exception: c01 at 0xfbf4f6c
> LR = 0xffac1a0
> Instruction dump:
> 91210028 4bfffee4 9421ffd0 7c0802a6 90010034 bf210014 81230038 83430034
> 8129 552907fa 2b890001 41de002c <0fe0> 3bc0ffea 80010034 7fc3f378
> ---[ end trace 56617f0f443657ab ]---
> 0 ath10k_pci :01:00.0: failed to install peer wep keys for vdev 0: -22
> ath10k_pci :01:00.0: failed to associate station 04:f0:21:1d:77:68 for 
> vdev 0: -22
> station dump
> Station 04:f0:21:1d:77:68 (on mesh0)
> inactive time:  351 ms
> rx bytes:   2122
> rx packets: 24
> tx bytes:   1196
> tx packets: 8
> tx retries: 0
> tx failed:  0
> signal: -54 dBm
> signal avg: -52 dBm
> Toffset:642048384 us
> tx bitrate: 1.0 MBit/s
> rx bitrate: 1.0 MBit/s
> mesh llid:  0
> mesh plid:  0
> mesh plink: ESTAB
> mesh local PS mode: ACTIVE
> mesh peer PS mode:  UNKNOWN
> mesh non-peer PS mode:  ACTIVE
>
> System Details:
>
> Kernel : 3.12 rt 51+ (SDK 1.9)
>
> ath10K driver: backport-4.4.2-1

Your backports release is old, which means that the version of ath10k is
also old. I would try to build the latest backports release from git and
see how that works:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/backports

But do note that the backports project has become active again only
recently so there might be some build issues.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Suspend resume broken since 4.9

2017-02-14 Thread Valo, Kalle
Enrico Tagliavini  writes:

> Hello everybody,
>
> few days ago Fedora 24 pushed kernel 4.9.4 as a stable update, moving
> from the last release of the 4.8 series. I use suspend to ram very
> often and I noticed it doesn't work anymore with 4.9.4 (and also
> 4.9.5). Screen goes black (but still showing mouse pointer) for 5-10
> seconds and then the desktop comes back instead of going to sleep. It
> worked without issues on kernel 4.8.16 and earlier series.
>
> I think it might be a fault of ath10k for two reasons: 1. when I try
> to suspend firmware crashes, 2. if I modprobe -r ath10k_pci before
> attempting the suspend, then I suspend to ram it works!
>
> I've posted a bug report on the kernel bugzilla [1], with some log,
> which I also report here:
>
> [   61.104169] ath10k_pci :03:00.0: firmware crashed! (uuid
> 7f9700df-93ab-4d1b-8c6d-aea24b60d170)
> [   61.104175] ath10k_pci :03:00.0: qca6174 hw2.1 target
> 0x0501 chip_id 0x003405ff sub 1a56:1525
> [   61.104176] ath10k_pci :03:00.0: kconfig debug 0 debugfs 1
> tracing 0 dfs 0 testmode 0
> [   61.104450] ath10k_pci :03:00.0: firmware ver
> SW_RM.1.1.1-00157-QCARMSWPZ-1 api 5 features ignore-otp,no-4addr-pad
> crc32 10bf8e08
> [   61.104596] ath10k_pci :03:00.0: board_file api 2 bmi_id N/A
> crc32 ae2e275a
> [   61.104598] ath10k_pci :03:00.0: htt-ver 3.1 wmi-op 4 htt-op 3
> cal otp max-sta 32 raw 0 hwcrypto 1
> [   61.105151] ath10k_pci :03:00.0: firmware register dump:
> [   61.105151] ath10k_pci :03:00.0: [00]: 0x0501 0x
> 0x0092E4DC 0x365591B9
> [   61.105151] ath10k_pci :03:00.0: [04]: 0x0092E4DC 0x00060130
> 0x0018 0x0041A760
> [   61.105151] ath10k_pci :03:00.0: [08]: 0x365591A5 0x0040
> 0x 0x000A5C88
> [   61.105151] ath10k_pci :03:00.0: [12]: 0x0009 0x
> 0x0096C09C 0x0096C0A7
> [   61.105151] ath10k_pci :03:00.0: [16]: 0x0096BDBC 0x009BFC42
> 0x 0x009287BD
> [   61.105151] ath10k_pci :03:00.0: [20]: 0x4092E4DC 0x0041A710
> 0x 0x0F00
> [   61.105151] ath10k_pci :03:00.0: [24]: 0x809432A7 0x0041A770
> 0x0040D400 0xC092E4DC
> [   61.105151] ath10k_pci :03:00.0: [28]: 0x80942BC4 0x0041A790
> 0x365591A5 0x0040
> [   61.105151] ath10k_pci :03:00.0: [32]: 0x80947BA7 0x0041A7B0
> 0x00404D88 0x00413980
> [   61.105151] ath10k_pci :03:00.0: [36]: 0x809BDECC 0x0041A7D0
> 0x00404D88 0x00413980
> [   61.105151] ath10k_pci :03:00.0: [40]: 0x8099638C 0x0041A7F0
> 0x00404D88 0x
> [   61.105151] ath10k_pci :03:00.0: [44]: 0x80992076 0x0041A810
> 0x004084F0 0x00405244
> [   61.105151] ath10k_pci :03:00.0: [48]: 0x80996BD3 0x0041A830
> 0x004084F0 0x
> [   61.105151] ath10k_pci :03:00.0: [52]: 0x800B4405 0x0041A850
> 0x00422318 0x5002
> [   61.105151] ath10k_pci :03:00.0: [56]: 0x809A6C34 0x0041A8E0
> 0x0042932C 0x0042CA20
> [   61.111014] ath10k_pci :03:00.0: could not suspend target (-108)
> [   61.178604] ath10k_pci :03:00.0: cannot restart a device that
> hasn't been started
>
> there are some more in the bug report, but I don't think they add
> anything useful so I wont repost them here.
>
> Needless to say I'm seeking some assistance here, since I rely on
> suspend to ram for a few things that would make it annoying otherwise.
> To start with, is my assumption correct in believing this is a fault
> within ath10k?

The firmware shouldn't crash, obviously, but I cannot see what's the
reason for the crash. Most helpful would be to find the change which
broke your setup. For example, try different kernel versions but don't
change anything else, especially not the firmware files. And then keep
the kernel version same but try different firmware versions (if that was
updated on your system).

If it's a kernel problem the best is that if you can build your own
kernels and use 'git bisect' to find the exact commit which broke this.
Then the changes to get the problem fixed is very high. There should be
plenty of instructions on the web how to do that.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: All '0' in agg_status

2017-02-14 Thread Valo, Kalle
Kwang Min Kim  writes:

> I have two questions.
>
> 1. As a result of debugging 'ath10k' and 'mac80211', if there is a problem, 
> ADDBA request and DELBA request are receive at 'station' side at the same 
> time.
>Could it be this?
>
>
> 2. The code below seems to fix the problem. (I did not consider the impact 
> elsewhere.)
>Is it the correct code? How do I fix the incorrect code?
>
> --- net/mac80211/iface.c
> +++ net/mac80211/iface.c

For mac80211 changes you should ask about that in linux-wireless mailing
list, but better also CC the ath10k list.

https://wireless.wiki.kernel.org/en/developers/mailinglists

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: add support for controlling tx power to a station

2017-02-14 Thread Valo, Kalle
kbuild test robot  writes:

> Hi Ashok,
>
> [auto build test ERROR on v4.9-rc8]
> [cannot apply to ath6kl/ath-next next-20170210]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Ashok-Raj-Nagarajan/ath10k-add-support-for-controlling-tx-power-to-a-station/20170208-203305
> config: x86_64-randconfig-in0-02120025 (attached as .config)
> compiler: gcc-4.6 (Debian 4.6.4-7) 4.6.4
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
>
> All errors (new ones prefixed by >>):
>
>drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_sta_set_txpwr':
>>> drivers/net/wireless/ath/ath10k/mac.c:5900:10: error: 'struct
>>> ieee80211_sta' has no member named 'txpwr'
>drivers/net/wireless/ath/ath10k/mac.c: At top level:
>>> drivers/net/wireless/ath/ath10k/mac.c:7469:2: error: unknown field
>>> 'sta_set_txpwr' specified in initializer
>drivers/net/wireless/ath/ath10k/mac.c:7469:2: warning:
> initialization from incompatible pointer type [enabled by default]
>drivers/net/wireless/ath/ath10k/mac.c:7469:2: warning: (near
> initialization for 'ath10k_ops.sta_pre_rcu_remove') [enabled by
> default]
>drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_mac_register':
>>> drivers/net/wireless/ath/ath10k/mac.c:8008:11: error:
>>> 'NL80211_EXT_FEATURE_STA_TX_PWR' undeclared (first use in this
>>> function)

These are expected as this depends on cfg80211 patch not yet applied.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: 9984 VHT

2017-02-14 Thread Valo, Kalle
(you forgot to CC ath10k list, adding it back)

Sebastian Gottschall  writes:

> Am 10.02.2017 um 07:50 schrieb Valo, Kalle:
>> Sebastian Gottschall  writes:
>>
>>> Am 07.02.2017 um 13:14 schrieb Valo, Kalle:
>>>> Ben Greear  writes:
>>>>
>>>>> On 02/02/2017 10:42 AM, Sebastian Gottschall wrote:
>>>>>> Am 02.02.2017 um 19:24 schrieb Ben Greear:
>>>>>>
>>>>>>> I hacked ath10k to enable radar detection on 160Mhz bandwidths. Now 
>>>>>>> hostapd
>>>>>>> starts.
>>>>>>>
>>>>>>> I can reproduce the FW failure. It is because FW 3.3-25 release
>>>>>>> started asserting
>>>>>>> if the freq2 was zero in VHT160 mode. It seems to use both freq1
>>>>>>> and freq2, and not
>>>>>>> how the driver or linux seems to normally use them.
>>>>>> its even worse. starting from 3.3 it uses freq2 instead of freq1.
>>>>>> but i made a patch for it. but it still wont work. vdev_start still
>>>>>> fails
>>>>> Well it would have been helpful from the start to have known about this 
>>>>> patch.
>>>>>
>>>>> Could you post it?
>>>>>
>>>>> Kalle: Since the firmware API changed, how do you want to handle
>>>>> the differences here?
>>>> The firmware API shouldn't change like that, but if it has I guess a
>>>> firmware feature flag to enable a workaround in ath10k sounds like the
>>>> easiest solution.
>>> the more recent firmware have some new wmi services enabled which can
>>> be used as indicator as well.
>> I don't know what flag exactly you are referring to, but using a WMI
>> service flag to detect this is not really reliable. They can be enabled
>> or disabled between branches, or even between builds.
>
> valid argument. but these flags should than be also introduced in
> codeaurora images

I'm not involved with codeaurora images so I can't help with that.

But the firmware is not supposed to break the firmware interface like
this. Can someone please write a detailed bug report as a reply to this
mail (what version, from which location, how exactly the interface is
broken) and I'll try to report the issue internally.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-02-10 Thread Valo, Kalle
Kalle Valo  writes:

> BTW, just curious but why do you have "during rmmod" in the title? I
> think I was able to reproduce this crash by removing all firmware files
> and didn't use rmmod at all.

Nevermind, I was blind again. It was my script which calls rmmod and I
failed to realise that. Sorry for the noise.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-02-10 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> the change suggested by you helps, and the device probe, scan
> is successful as well. Still good to have this change part of your
> basic sanity and regression testing !

Sure. I was sort of expecting that you would send v4 but I haven't seen
one so I guess you assumed I send that? :) I'll then submit v4.

BTW, just curious but why do you have "during rmmod" in the title? I
think I was able to reproduce this crash by removing all firmware files
and didn't use rmmod at all.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/3] ath10k: remove ath10k_vif_to_arvif()

2017-02-09 Thread Valo, Kalle
Ben Greear  writes:

> On 02/07/2017 01:14 AM, Valo, Kalle wrote:
>> Adrian Chadd  writes:
>>
>>> Removing this method makes the diff to FreeBSD larger, as "vif" in
>>> FreeBSD is a different pointer.
>>>
>>> (Yes, I have ath10k on freebsd working and I'd like to find a way to
>>> reduce the diff moving forward.)
>>
>> I don't like this "(void *) vif->drv_priv" style that much either but
>> apparently it's commonly used in Linux wireless code and already parts
>> of ath10k. So this patch just unifies the coding style.
>
> Surely the code compiles to the same thing, so why add a patch that
> makes it more difficult for Adrian and makes the code no easier to read
> for the rest of us?

Because that's the coding style used already in Linux. It's great to see
that parts of ath10k can be used also in other systems but in principle
I'm not very fond of the idea starting to reject valid upstream patches
because of driver forks.

I think backports project is doing it right, it's not limiting upstream
development in any way and handles all the API changes internally. Maybe
FreeBSD could do something similar?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: 9984 VHT

2017-02-09 Thread Valo, Kalle
Sebastian Gottschall  writes:

> Am 07.02.2017 um 13:14 schrieb Valo, Kalle:
>> Ben Greear  writes:
>>
>>> On 02/02/2017 10:42 AM, Sebastian Gottschall wrote:
>>>> Am 02.02.2017 um 19:24 schrieb Ben Greear:
>>>>
>>>>> I hacked ath10k to enable radar detection on 160Mhz bandwidths. Now 
>>>>> hostapd
>>>>> starts.
>>>>>
>>>>> I can reproduce the FW failure. It is because FW 3.3-25 release
>>>>> started asserting
>>>>> if the freq2 was zero in VHT160 mode. It seems to use both freq1
>>>>> and freq2, and not
>>>>> how the driver or linux seems to normally use them.
>>>> its even worse. starting from 3.3 it uses freq2 instead of freq1.
>>>> but i made a patch for it. but it still wont work. vdev_start still
>>>> fails
>>> Well it would have been helpful from the start to have known about this 
>>> patch.
>>>
>>> Could you post it?
>>>
>>> Kalle: Since the firmware API changed, how do you want to handle
>>> the differences here?
>>
>> The firmware API shouldn't change like that, but if it has I guess a
>> firmware feature flag to enable a workaround in ath10k sounds like the
>> easiest solution.
>
> the more recent firmware have some new wmi services enabled which can
> be used as indicator as well.

I don't know what flag exactly you are referring to, but using a WMI
service flag to detect this is not really reliable. They can be enabled
or disabled between branches, or even between builds.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: 9984 VHT

2017-02-07 Thread Valo, Kalle
Ben Greear  writes:

> On 02/02/2017 10:42 AM, Sebastian Gottschall wrote:
>> Am 02.02.2017 um 19:24 schrieb Ben Greear:
>>> On 02/02/2017 08:18 AM, Ben Greear wrote:
 On 02/01/2017 10:45 AM, Sebastian Gottschall wrote:
> Am 01.02.2017 um 17:48 schrieb Ben Greear:
>> On 01/30/2017 02:28 AM, Sebastian Gottschall wrote:
>>> Hello
>>>
>>> with recent 9984 firmares vht160 seem to crash the firmware itself for 
>>> no reason i see.
>>> is there any internal structure change in these newer firmwares we need 
>>> to consider?
>>> i also tested the even more recent firmware
>>> firmware-5.bin_10.4-3.4-00068 from codeaurora. this one doesnt
>>> crash but the vdev_start returns with a error
>>>
>>> i compared the more recent wmi headers from the qca drivers,
>>> but wasnt able to find anything which could explain the
>>> behaviour
>>>
>>> Sebastian
>>>
>>>
>>
>> Hello Sebastian,
>>
>> Could you share the hostapd/supplicant config file you are using?
>>
>> And, what kernel (or backports kernel) are you using?  I'd like to
>> see how 160Mhz works on my firmware
> always state of art for sure

 Using a somewhat hacked 4.9.2+ kernel and my CT firmware, I get failure to 
 start CAC:

 1486052184.818710: Mode: IEEE 802.11a  Channel: 100  Frequency: 5500 MHz
 1486052184.818714: DFS 8 channels required radar detection
 1486052184.818717: DFS all channels available, (SKIP CAC): no
 1486052184.818719: DFS 0 chans unavailable - choose other channel: no
 1486052184.818722: vap0: interface state COUNTRY_UPDATE->DFS
 1486052184.818725: DFS start CAC on 5500 MHz
 1486052184.818733: vap0: DFS-CAC-START freq=5500 chan=100 sec_chan=1, 
 width=2, seg0=114, seg1=0, cac_time=60s
 1486052184.818737: nl80211: Start radar detection (CAC) 5500 MHz
 (ht_enabled=1, vht_enabled=1, bandwidth=160 MHz, cf1=5570 MHz,
 cf2=0 MHz)
 1486052184.818742:   * freq=5500
 1486052184.818745:   * vht_enabled=1
 1486052184.818747:   * ht_enabled=1
 1486052184.818749:   * bandwidth=160
 1486052184.818751:   * channel_width=5
 1486052184.818754:   * center_freq1=5570
 1486052184.818756:   * center_freq2=0
 1486052184.823437: nl80211: Failed to start radar detection: -16 (Device 
 or resource busy)
 1486052184.823444: DFS start_dfs_cac() failed, -1


 I'll go dig around to see if I can figure out why...
>>>
>>> I hacked ath10k to enable radar detection on 160Mhz bandwidths. Now hostapd
>>> starts.
>>>
>>> I can reproduce the FW failure.  It is because FW 3.3-25 release started 
>>> asserting
>>> if the freq2 was zero in VHT160 mode.  It seems to use both freq1 and 
>>> freq2, and not
>>> how the driver or linux seems to normally use them.
>> its even worse. starting from 3.3 it uses freq2 instead of freq1.
>> but i made a patch for it. but it still wont work. vdev_start still
>> fails
>
> Well it would have been helpful from the start to have known about this patch.
>
> Could you post it?
>
> Kalle:  Since the firmware API changed, how do you want to handle the 
> differences here?

The firmware API shouldn't change like that, but if it has I guess a
firmware feature flag to enable a workaround in ath10k sounds like the
easiest solution.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/3] ath10k: remove ath10k_vif_to_arvif()

2017-02-07 Thread Valo, Kalle
Adrian Chadd  writes:

> Removing this method makes the diff to FreeBSD larger, as "vif" in
> FreeBSD is a different pointer.
>
> (Yes, I have ath10k on freebsd working and I'd like to find a way to
> reduce the diff moving forward.)

I don't like this "(void *) vif->drv_priv" style that much either but
apparently it's commonly used in Linux wireless code and already parts
of ath10k. So this patch just unifies the coding style.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Recent driver changes destabilized QCA9377 connection quality

2017-02-01 Thread Valo, Kalle
Tobias Predel  writes:

> I just want to report a negative development I have been experiencing
> for a week now.
>
> Recent changes in ath10k driver code seem to destabilize QCA9377
> connection quality in 802.11b/g networks, as hardware and software
> configuration (wpa_supplicant) hasn't changed. Before, I have managed
> to have a fair connection quality for around three months. At the
> moment I encounter multiple connection losses.
>
> dmesg reports usual losses and timeouts on both linux-lts 4.4.44 and
> linux 4.8.13 (Arch Linux).
>
> Please restore former functionality.

You mention recent changes, so what kernel exact version works without
problems? Has the firmware version remained the same all the time?

If it's a regression in kernel, can you bisect to find the commit which
broke it? You would need to compile your own kernels to do that.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2] ath10k: add support for controlling tx power to a station

2017-02-01 Thread Valo, Kalle
kbuild test robot  writes:

> Hi Ashok,
>
> [auto build test WARNING on ath6kl/ath-next]
> [also build test WARNING on v4.10-rc6 next-20170131]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Ashok-Raj-Nagarajan/ath10k-add-support-for-controlling-tx-power-to-a-station/20170201-032529
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
> config: m68k-allyesconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 4.9.0
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=m68k 
>
> All warnings (new ones prefixed by >>):
>
>drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_sta_set_txpwr':
>drivers/net/wireless/ath/ath10k/mac.c:5997:13: error: 'struct 
> ieee80211_sta' has no member named 'txpwr'
>  txpwr = sta->txpwr;
> ^
>drivers/net/wireless/ath/ath10k/mac.c: At top level:
>drivers/net/wireless/ath/ath10k/mac.c:7581:2: error: unknown field 
> 'sta_set_txpwr' specified in initializer
>  .sta_set_txpwr   = ath10k_sta_set_txpwr,
>  ^

This is expected as this patch depends on a mac80211 feature not yet
applied.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA988X firmware pull-push support

2017-01-30 Thread Valo, Kalle
Adrian Chadd  writes:

> On 29 January 2017 at 17:20, Sergey Ryazanov  wrote:
>> Yep. Looks like so.
>
> Which is good, because my FreeBSD ath10k port only works with 10.1 /
> 10.2 firmware on earlier chips, and I haven't figured out the
> intermediate software queue support that came with 10.4. :-)
>
> But yeah. I'll wake for Kalle to verify, but I think it's a 10.4
> thing

So there are two parts here, ieee80211_ops::wake_tx_queue() support in
ath10k and ATH10K_FW_FEATURE_PEER_FLOW_CONTROL in firmware. We did have
wake_tx_queue() support enabled for all hw/fw combinations but it was
changed because some people claimed there are throughput regressions in
certain cases. So for now we only enable wake_tx_queue() support only if
firmware supports ATH10K_FW_FEATURE_PEER_FLOW_CONTROL:

if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL,
  ar->running_fw->fw_file.fw_features))
ar->ops->wake_tx_queue = NULL;

I remember that there were some improvements with wake_tx_queue() even
if firmware didn't support ATH10K_FW_FEATURE_PEER_FLOW_CONTROL. I'm
starting to wonder should we have a module parameter to force use of
wake_tx_queue()?

And IIRC ATH10K_FW_FEATURE_PEER_FLOW_CONTROL was supported in some of
10.4 releases, but not all of them.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Incomplete scan results after rf(un)kill

2017-01-30 Thread Valo, Kalle
Daniel J Blueman  writes:

> On 27 January 2017 at 23:44, Valo, Kalle  wrote:
>> Daniel J Blueman  writes:
>>
>>> On 4.9.5 and previous, I've noticed that 20% of the time after
>>> rfunkilling, AP scan results would often have only 1-2 out of the
>>> previous 10 APs, and the situation would persist until removing and
>>> reinserting the ath10k_pci module.
>>>
>>> This is on my Dell XPS 13 9360 (Kaby Lake) with current BIOS 1.2.3 on
>>> Ubuntu (ElementaryOS) 16.04 with updates.
>>>
>>> Could this have any relation to the missing firmware [1]?
>>>
>>> What state can I capture to help diagnose this?
>>
>> Do you see any pattern what APs are visible when the bug happens? For
>> example, are those 1-2 APs always the same one? And are they ones with
>> strongest signal strength or maybe related to certain channels?
>>
>> After the bug happens how does the device work otherwise? Have you
>> tested performance (iperf) or signal strength? Is there any packet loss
>> etc?
>
> Many thanks for following up Kalle! Interestingly, I do see the same
> subset of APs (but not the strongest) in the Networkmanager GUI,
> however when I scan from the CLI, nothing:
>
> # nmcli dev wifi
> *  SSID  MODE  CHAN  RATE  SIGNAL  BARS  SECURITY
> #

Better to use 'sudo iw wlan0 scan' (or whatever is the ath10k network
interface name in your setup, systemd has made guessing the name
difficult). That provides more information and communicates directly
with kernel.

> The issue also occurs when coming out of suspend. I didn't check dmesg
> other times, but this may or may not be related:
>
> [  204.454815] WARNING: CPU: 3 PID: 0 at
> /home/kernel/COD/linux/net/core/dev.c:5161 net_rx_action+0x26e/0x380

It may very well be. Do you know exactly to what warning that line 5161
points to in your kernel version? With latest ath.git master branch line
5182 in dev.c is this warning napi_poll():

WARN_ON_ONCE(work > weight);

But I do not know if you are seeing that warning or something else
because my kernel sources doesn't match what you use.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Incomplete scan results after rf(un)kill

2017-01-27 Thread Valo, Kalle
Daniel J Blueman  writes:

> On 4.9.5 and previous, I've noticed that 20% of the time after
> rfunkilling, AP scan results would often have only 1-2 out of the
> previous 10 APs, and the situation would persist until removing and
> reinserting the ath10k_pci module.
>
> This is on my Dell XPS 13 9360 (Kaby Lake) with current BIOS 1.2.3 on
> Ubuntu (ElementaryOS) 16.04 with updates.
>
> Could this have any relation to the missing firmware [1]?
>
> What state can I capture to help diagnose this?

Do you see any pattern what APs are visible when the bug happens? For
example, are those 1-2 APs always the same one? And are they ones with
strongest signal strength or maybe related to certain channels?

After the bug happens how does the device work otherwise? Have you
tested performance (iperf) or signal strength? Is there any packet loss
etc?

> ath10k_pci :3a:00.0: enabling device ( -> 0002)
> ath10k_pci :3a:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0
> ath10k_pci :3a:00.0: Direct firmware load for
> ath10k/pre-cal-pci-:3a:00.0.bin failed with error -2
> ath10k_pci :3a:00.0: Direct firmware load for
> ath10k/cal-pci-:3a:00.0.bin failed with error -2
> ath10k_pci :3a:00.0: Direct firmware load for
> ath10k/QCA6174/hw3.0/firmware-5.bin failed with error -2
> ath10k_pci :3a:00.0: could not fetch firmware file
> 'ath10k/QCA6174/hw3.0/firmware-5.bin': -2

You can ignore these failed with error -2 messages, it just means that
ath10k is trying to find the correct firmware image and calibration
data. We know it's confusing users and have a patch pending:

https://patchwork.kernel.org/patch/9237095/

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Question on QCA9377 and SDIO 3.0

2017-01-27 Thread Valo, Kalle
Ann Lo  writes:

> Does ath10k support QCA9377 that uses SDIO 3.0 interface?

Not yet, but Erik Stromdahl is working on adding SDIO support to ath10k:

https://patchwork.kernel.org/patch/9516477/

It's still WIP.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Driver support for QCA6174 chip using SDIO interface

2017-01-27 Thread Valo, Kalle
Akesh M Chacko  writes:

> We are using QCA6174 chip on our embedded platform using SDIO
> interface. could you please help us to get the SDIO support for
> QCA6174 driver.

Erik Stromdahl is working on adding SDIO support to ath10k:

https://patchwork.kernel.org/patch/9516477/

But it's still WIP.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-01-25 Thread Valo, Kalle
Kalle Valo  writes:

> Mohammed Shafi Shajakhan  writes:
>
>> From: Mohammed Shafi Shajakhan 
>>
>> This fixes the below crash when ath10k probe firmware fails,
>> NAPI polling tries to access a rx ring resource which was never
>> allocated, fix this by disabling NAPI right away once the probe
>> firmware fails by calling 'ath10k_hif_stop'. Its good to note
>> that the error is never propogated to 'ath10k_pci_probe' when
>> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
>> PCI related things seems to be ok
>>
>> BUG: unable to handle kernel NULL pointer dereference at (null)
>> IP:  __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
>> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
>>
>> Call Trace:
>>
>> [] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
>> [ath10k_core]
>> [] ath10k_htt_txrx_compl_task+0x433/0x17d0
>> [ath10k_core]
>> [] ? __wake_up_common+0x4d/0x80
>> [] ? cpu_load_update+0xdc/0x150
>> [] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
>> [] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
>> [] net_rx_action+0x20f/0x370
>>
>> Reported-by: Ben Greear 
>> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
>> Signed-off-by: Mohammed Shafi Shajakhan 
>
> Is there an easy way to reproduce this bug? I don't see it on my x86
> laptop with qca988x and I call rmmod all the time. I would like to test
> this myself.
>
>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
>>  ath10k_core_free_firmware_files(ar);
>>  
>>  err_power_down:
>> +ath10k_hif_stop(ar);
>>  ath10k_hif_power_down(ar);
>>  
>>  return ret;
>
> This breaks the symmetry, we should not be calling ath10k_hif_stop() if
> we haven't called ath10k_hif_start() from the same function. This can
> just create a bigger mess later, for example with other bus support like
> sdio or usb. In theory it should enough that we call
> ath10k_hif_power_down() and pci.c does the rest correctly "behind the
> scenes".
>
> I investigated this a bit and I think the real cause is that we call
> napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from
> ath10k_pci_hif_stop(). Does anyone remember why?
>
> I was expecting that we would call napi_enable()/napi_disable() either
> in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not
> mixed like it's currently.

So below is something I was thinking of, now napi_enable() is called
from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would
that work?

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
 
+   napi_enable(&ar->napi);
+
ath10k_pci_irq_enable(ar);
ath10k_pci_rx_post(ar);
 
@@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
goto err_ce;
}
-   napi_enable(&ar->napi);
 
return 0;

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails

2017-01-25 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> From: Mohammed Shafi Shajakhan 
>
> This fixes the below crash when ath10k probe firmware fails,
> NAPI polling tries to access a rx ring resource which was never
> allocated, fix this by disabling NAPI right away once the probe
> firmware fails by calling 'ath10k_hif_stop'. Its good to note
> that the error is never propogated to 'ath10k_pci_probe' when
> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
> PCI related things seems to be ok
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP:  __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
>
> Call Trace:
>
> [] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
> [ath10k_core]
> [] ath10k_htt_txrx_compl_task+0x433/0x17d0
> [ath10k_core]
> [] ? __wake_up_common+0x4d/0x80
> [] ? cpu_load_update+0xdc/0x150
> [] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
> [] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
> [] net_rx_action+0x20f/0x370
>
> Reported-by: Ben Greear 
> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
> Signed-off-by: Mohammed Shafi Shajakhan 

Is there an easy way to reproduce this bug? I don't see it on my x86
laptop with qca988x and I call rmmod all the time. I would like to test
this myself.

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
>   ath10k_core_free_firmware_files(ar);
>  
>  err_power_down:
> + ath10k_hif_stop(ar);
>   ath10k_hif_power_down(ar);
>  
>   return ret;

This breaks the symmetry, we should not be calling ath10k_hif_stop() if
we haven't called ath10k_hif_start() from the same function. This can
just create a bigger mess later, for example with other bus support like
sdio or usb. In theory it should enough that we call
ath10k_hif_power_down() and pci.c does the rest correctly "behind the
scenes".

I investigated this a bit and I think the real cause is that we call
napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from
ath10k_pci_hif_stop(). Does anyone remember why?

I was expecting that we would call napi_enable()/napi_disable() either
in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not
mixed like it's currently.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()

2017-01-24 Thread Valo, Kalle
Joe Perches  writes:

> On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
>> Joe Perches  writes:
>> 
>> > On Mon, 2017-01-23 at 15:04 +, Srinivas Kandagatla wrote:
>> > > use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
>> > 
>> > []
>> > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c
>> > > b/drivers/net/wireless/ath/ath10k/pci.c
>> > 
>> > []
>> > > @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k 
>> > > *ar, u32 address, void *data,
>> > >   */
>> > >  alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
>> > >  
>> > > -data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
>> > > +data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
>> > > alloc_nbytes,
>> > > &ce_data_base,
>> > > GFP_ATOMIC);
>> > 
>> > trivia:
>> > 
>> > Nicer to realign arguments and remove the unnecessary cast.
>> > 
>> > Perhaps:
>> > 
>> >data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
>> >   GFP_ATOMIC);
>> 
>> Sure, but that should be in a separate patch.
>
> I don't think so, trivial patches can be combined.
>
> It's also nicer to realign all modified multiline
> arguments when performing these changes.
>
> Coccinelle generally does it automatically.

A matter of preference really. I prefer keeping style and functional
changes in separate patches, keeps the review simple. And style changes
can hide bugs.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()

2017-01-23 Thread Valo, Kalle
Joe Perches  writes:

> On Mon, 2017-01-23 at 15:04 +, Srinivas Kandagatla wrote:
>> use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
> []
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c 
>> b/drivers/net/wireless/ath/ath10k/pci.c
> []
>> @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, 
>> u32 address, void *data,
>>   */
>>  alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
>>  
>> -data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
>> +data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
>> alloc_nbytes,
>> &ce_data_base,
>> GFP_ATOMIC);
>
> trivia:
>
> Nicer to realign arguments and remove the unnecessary cast.
>
> Perhaps:
>
>   data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
>  GFP_ATOMIC);

Sure, but that should be in a separate patch.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v3 0/8] ath10k sdio support

2017-01-23 Thread Valo, Kalle
Erik Stromdahl  writes:

> This is the third version of the sdio RFC patch series.
> The actual sdio code (patch 6) has been subject to a massive overhaul,
> mainly as a result of Kalle's review comments.
> It no longer has any strong resemblance of the original ath6kl code from
> which it was originally based upon.
>
> Previous pathes 6 to 10 have been merged into one single patch.
>
> The previous series had a rework of the "HTC fake service" connect
> (ep 0 connect) that introduced a race between the actual endpoint
> connect and the HTC ready message. This issue has been addressed,
> and the current patches (3 and 4) has been rewritten accordingly.
>
> * overview of patches *
>
> Patches 1 to 4 are more or less identical to the previous RFC, with an
> exception for patch 3 that changes the "HTC fake service" connect
> (mentioned above).
>
> Patch 5 is a squashed version of previous patches 6 to 10.
>
> Patch 6 is the actual sdio patch
>
> Patches 7 to 8 are new and adds special sdio versions of BMI get
> target info and HTC ready.
>
> The new version was built and tested against:
> tag: ath-201701121109
>
> Erik Stromdahl (8):
>   ath10k: htc: made static function public
>   ath10k: htc: rx trailer lookahead support
>   ath10k: htc: move htc ctrl ep connect to htc_init
>   ath10k: htc: refactorization
>   ath10k: various sdio related definitions
>   ath10k: sdio support
>   ath10k: sdio get target info
>   ath10k: htc: ready_ext msg support

I pushed now this, and your usb series as well, to ath.git
master-pending branch. Let's see if kbuild bot finds any problems.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2017-01-11 Thread Valo, Kalle
Christian Lamparter  writes:

> 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  wrote:
>> > > 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 
>> > >
>> > > 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? 

I think I just read the discussion to hastily as it was rather long,
sorry about that.

After really long or confusin discussions, just to help the maintainers
and also avoid miscommunication between participants, it's usually a
good idea to summarise the conclusion. If us maintainers need to figure
out the conclusion ourselves from a long discussion we are bound to make
mistakes, just like I did here.

So something like this would help me a lot:

"Kalle, please drop these patches. I need to work on these a bit more."

Or: 

"Kalle, me and John came to agreement about foo. So these should be good
to apply."

> 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?

If you both are happy with the patch, I'm happy to take it :)

I actived the patch again in my queue by moving the state from Deferred
to New:

https://patchwork.kernel.org/patch/9461631/

If all goes well I'm expecting to apply it later this week.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2016-12-30 Thread Valo, Kalle
Christian Lamparter  writes:

> On Thu, Dec 29, 2016 at 3:11 PM, Kalle Valo  wrote:
>> 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 
>>
>> 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).

I had problems following the discussion so the conclusion was not clear
for me.

>> 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?

There is a state called "Deferred" which I use whenever I need to
revisit a patch later time. I moved this patch to that state now:

https://patchwork.kernel.org/patch/9461631/

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 11/11] ath10k: Added sdio support

2016-12-16 Thread Valo, Kalle
Erik Stromdahl  writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl 

I know most of this coming from ath6kl but I think we should still
improve the code. Lots of comments will follow, don't get scared :)

> +#define CALC_TXRX_PADDED_LEN(ar_sdio, len) \
> + (__ALIGN_MASK((len), (ar_sdio)->mbox_info.block_mask))

I think this could be a proper static inline function. Andrew Morton
once said: "Write in C, not CPP" (or something like that), I think
that's a really good point.

> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
> +u32 len, u32 request);
> +static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void 
> *buf,
> +  size_t buf_len);
> +static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
> +u32 *value);

We prefer to avoid forward declarations if at all possible. I didn't
check but if there's a clean way to avoid these please remove them.

> +/* HIF mbox interrupt handling */
> +
> +static int ath10k_sdio_mbox_rx_process_packet(struct ath10k_sdio *ar_sdio,
> +   struct ath10k_sdio_rx_data *pkt,
> +   u32 *lookaheads,
> +   int *n_lookaheads)
> +{

So the style in ath10k is that all functions (of course this doesn't
apply to callbacks etc) have "struct ath10k *ar" as the first parameter.
This way there's no need to check if a function takes ar or ar_sdio.

> + int status = 0;

In ath10k we prefer to use ret. And avoid initialising it, please.

> + struct ath10k_htc *htc = &ar_sdio->ar->htc;
> + struct sk_buff *skb = pkt->skb;
> + struct ath10k_htc_hdr *htc_hdr = (struct ath10k_htc_hdr *)skb->data;
> + bool trailer_present = htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESENT;
> + u16 payload_len;
> +
> + payload_len = le16_to_cpu(htc_hdr->len);
> +
> + if (trailer_present) {
> + u8 *trailer;
> + enum ath10k_htc_ep_id eid;
> +
> + trailer = skb->data + sizeof(struct ath10k_htc_hdr) +
> +   payload_len - htc_hdr->trailer_len;
> +
> + eid = (enum ath10k_htc_ep_id)htc_hdr->eid;

A some kind of mapping function for ep id would be nice, it makes it
more visible how it works.

> +static int ath10k_sdio_mbox_rx_process_packets(struct ath10k_sdio *ar_sdio,
> +u32 lookaheads[],
> +int *n_lookahead)
> +{
> + struct ath10k *ar = ar_sdio->ar;
> + struct ath10k_htc *htc = &ar->htc;
> + struct ath10k_sdio_rx_data *pkt;
> + int status = 0, i;
> +
> + for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
> + struct ath10k_htc_ep *ep;
> + enum ath10k_htc_ep_id id;
> + u32 *lookaheads_local = lookaheads;
> + int *n_lookahead_local = n_lookahead;
> +
> + id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
> +
> + if (id >= ATH10K_HTC_EP_COUNT) {
> + ath10k_err(ar, "Invalid endpoint in look-ahead: %d\n",
> +id);

In ath10k we use ath10k_err() for errors from which can't survive and
ath10k_warn() for errors where we try to continue. So ath10k_warn()
would be more approriate here and most of other cases in sdio.c

> + status = -ENOMEM;
> + goto out;
> + }
> +
> + ep = &htc->endpoint[id];
> +
> + if (ep->service_id == 0) {
> + ath10k_err(ar, "ep %d is not connected !\n", id);
> + status = -ENOMEM;
> + goto out;
> + }
> +
> + pkt = &ar_sdio->rx_pkts[i];
> +
> + if (pkt->part_of_bundle && !pkt->last_in_bundle) {
> + /* Only read lookahead's from RX trailers
> +  * for the last packet in a bundle.
> +  */
> + lookaheads_local = NULL;
> + n_lookahead_local = NULL;
> + }
> +
> + status = ath10k_sdio_mbox_rx_process_packet(ar_sdio,
> + pkt,
> + lookaheads_local,
> + n_lookahead_local);
> + if (status)
> + goto out;
> +
> + ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
> + /* The RX complete handler now owns the skb...*/
> + pkt->skb = NULL;
> + pkt->alloc_len = 0;
> + }
> +
> +out:
> + /* Free all packets that was not passed on to the RX completion
> +  * handler...
> +  */
> + for (; i < ar_sdio->n_rx_pkts; i++)
> + ath10k_sdio_

Re: [RFC v2 06/11] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB

2016-12-16 Thread Valo, Kalle
Erik Stromdahl  writes:

> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/htc.h |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h 
> b/drivers/net/wireless/ath/ath10k/htc.h
> index f94b25a..2963694 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
>   ATH10K_HTC_FLAG_BUNDLE_MASK = 0xF0
>  };
>  
> +#define ATH10K_HTC_FLAG_BUNDLE_LSB 4
> +

I think you could fold patches from 6 to 11 into one patch. They are all
about adding code, and most of the commit logs are empty anyway.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 11/11] ath10k: Added sdio support

2016-12-15 Thread Valo, Kalle
Erik Stromdahl  writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/Kconfig  |6 +
>  drivers/net/wireless/ath/ath10k/Makefile |3 +
>  drivers/net/wireless/ath/ath10k/sdio.c   | 1860 
> ++
>  drivers/net/wireless/ath/ath10k/sdio.h   |  276 +
>  4 files changed, 2145 insertions(+)
>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.c
>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.h

AFAIK the sdio firmware binary is different from pci and usb. And as all
the firmware images need to coexist in the same repository I suspect we
can't continue to use firmware-N.bin for both pcie and sdio (and in
future usb). So should we somehow rename the sdio firmware filename to
something else, like firmware-sdio-N.bin?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 05/11] ath10k: htc: refactorization

2016-12-14 Thread Valo, Kalle
Michal Kazior  writes:

>> I have made a few updates since I submitted the original RFC and created
>> a repo on github:
>>
>> https://github.com/erstrom/linux-ath
>>
>> I have a bunch of branches that are all based on the tags on the ath master.
>>
>> As of this moment the latest version is:
>>
>> ath-201612131156-ath10k-sdio
>>
>> This branch contains the original RFC patches plus some addons/fixes.
>>
>> In the above mentioned branch there are a few commits related to this
>> race condition. Perhaps you can have a look at them?
>>
>> The commits are:
>> 821672913328cf737c9616786dc28d2e4e8a4a90
>
> I would avoid if(bus==xx) checks.

Very much agreed. For example to enable HTT high latency support you
could add an enum to ath10k_core_create() with values for both high and
low latency. This way sdio.c and pci.c can enable the correct mode
during initialisation.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 05/11] ath10k: htc: refactorization

2016-12-14 Thread Valo, Kalle
Erik Stromdahl  writes:

> I have made a few updates since I submitted the original RFC and created
> a repo on github:
>
> https://github.com/erstrom/linux-ath
>
> I have a bunch of branches that are all based on the tags on the ath master.
>
> As of this moment the latest version is:
>
> ath-201612131156-ath10k-sdio
>
> This branch contains the original RFC patches plus some addons/fixes.

Good, this makes it easier to follow the development. So what's the
current status with this branch? What works and what doesn't?

Especially I'm interested about the state of the HTT high latency
support. How much work is to add that? It would also make it easier to
add USB support to ath10k.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 05/11] ath10k: htc: refactorization

2016-12-13 Thread Valo, Kalle
Michal Kazior  writes:

> On 13 December 2016 at 14:44, Valo, Kalle  wrote:
>> Erik Stromdahl  writes:
>>
>>> Code refactorization:
>>>
>>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>>> to ath10k_htc_control_rx_complete.
>>>
>>> This eases the implementation of SDIO/mbox significantly since
>>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>>> hif layer.
>>>
>>> Since the ath10k_htc_control_rx_complete already is present
>>> (only containing a warning message) there is no reason for not
>>> using it (instead of having a special case for ep 0 in
>>> ath10k_htc_rx_completion_handler).
>>>
>>> Signed-off-by: Erik Stromdahl 
>>
>> I tested this on QCA988X PCI board just to see if there are any
>> regressions. It crashes immediately during module load, every time, and
>> bisected that the crashing starts on this patch:
>>
>> [ 1239.715325] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 
>> 0 reset_mode 0
>> [ 1239.885125] ath10k_pci :02:00.0: Direct firmware load for 
>> ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
>> [ 1239.885260] ath10k_pci :02:00.0: Direct firmware load for 
>> ath10k/cal-pci-:02:00.0.bin failed with error -2
>> [ 1239.885687] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c 
>> chip_id 0x043202ff sub :
>> [ 1239.885699] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 
>> dfs 1 testmode 1
>> [ 1239.885899] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
>> features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
>> [ 1239.941836] ath10k_pci :02:00.0: Direct firmware load for 
>> ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
>> [ 1239.941993] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
>> bebc7c08
>> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   
>> (null)
>> [ 1241.136738] IP: [<  (null)>]   (null)
>> [ 1241.136759] *pdpt =  *pde = f0002a55f0002a55 [ 
>> 1241.136781]
>> [ 1241.136793] Oops: 0010 [#1] SMP
>>
>> What's odd is that when I added some printks on my own and enabled both
>> boot and htc debug levels it doesn't crash anymore. After everything
>> works normally after that, I can start AP mode and connect to it. Is it
>> a race somewhere?
>
> Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
> is set in htc_wait_target() [changed patch 4, but still too late].
>
> ep_rx_complete must be set prior to calling hif_start(). You probably
> crash on end of ath10k_htc_rx_completion_handler() when trying to call
> ep->ep_ops.ep_rx_complete(ar, skb).

Yeah, just checked and ep->ep_ops.ep_rx_complete is NULL at the end of
ath10k_htc_rx_completion_handler().

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 05/11] ath10k: htc: refactorization

2016-12-13 Thread Valo, Kalle
Erik Stromdahl  writes:

> Code refactorization:
>
> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
> to ath10k_htc_control_rx_complete.
>
> This eases the implementation of SDIO/mbox significantly since
> the ep_rx_complete cb is invoked directly from the SDIO/mbox
> hif layer.
>
> Since the ath10k_htc_control_rx_complete already is present
> (only containing a warning message) there is no reason for not
> using it (instead of having a special case for ep 0 in
> ath10k_htc_rx_completion_handler).
>
> Signed-off-by: Erik Stromdahl 

I tested this on QCA988X PCI board just to see if there are any
regressions. It crashes immediately during module load, every time, and
bisected that the crashing starts on this patch:

[ 1239.715325] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 
reset_mode 0
[ 1239.885125] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
[ 1239.885260] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/cal-pci-:02:00.0.bin failed with error -2
[ 1239.885687] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c chip_id 
0x043202ff sub :
[ 1239.885699] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 
1 testmode 1
[ 1239.885899] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
[ 1239.941836] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
[ 1239.941993] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
bebc7c08
[ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   (null)
[ 1241.136738] IP: [<  (null)>]   (null)
[ 1241.136759] *pdpt =  *pde = f0002a55f0002a55 [ 1241.136781] 
[ 1241.136793] Oops: 0010 [#1] SMP

What's odd is that when I added some printks on my own and enabled both
boot and htc debug levels it doesn't crash anymore. After everything
works normally after that, I can start AP mode and connect to it. Is it
a race somewhere?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC v2 11/11] ath10k: Added sdio support

2016-12-13 Thread Valo, Kalle
Erik Stromdahl  writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl 

While testing this I noticed few new warnings:

drivers/net/wireless/ath/ath10k/sdio.c: In function ath10k_sdio_probe:
drivers/net/wireless/ath/ath10k/sdio.c:1723:6: warning: 'ret' may be used 
uninitialized in this function [-Wuninitialized]
drivers/net/wireless/ath/ath10k/sdio.c:375:5: warning: symbol 
'ath10k_sdio_mbox_rxmsg_pending_handler' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1018:5: warning: symbol 
'ath10k_sdio_hif_tx_sg' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1415:5: warning: symbol 
'ath10k_sdio_hif_exchange_bmi_msg' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1555:5: warning: symbol 
'ath10k_sdio_hif_map_service_to_pipe' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1635:6: warning: symbol 
'ath10k_sdio_hif_get_default_pipe' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/htc.c:265: line over 90 characters
drivers/net/wireless/ath/ath10k/htc.c:355: line over 90 characters

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: free host-mem with DMA_BIRECTIONAL flag.

2016-12-12 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> Hopefully this fixes the problem reported by Kalle:
>
> Noticed this in my log, but I don't have time to investigate this in
> detail right now:
>
> [  413.795346] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> [  414.158755] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
> [  477.439659] ath10k_pci :02:00.0: could not get mac80211 beacon
> [  481.30] [ cut here ]
> [  481.69] WARNING: CPU: 0 PID: 1978 at lib/dma-debug.c:1155 
> check_unmap+0x320/0x8e0
> [  481.88] ath10k_pci :02:00.0: DMA-API: device driver frees DMA 
> memory with different direction [device address=0x2d13] 
> [size=63800 bytes] [mapped with DMA_BIDIRECTIONAL] [unmapped with 
> DMA_TO_DEVICE]
> [  481.666703] Modules linked in: ctr ccm ath10k_pci(E-) ath10k_core(E) 
> ath(E) mac80211(E) cfg80211(E) snd_hda_codec_hdmi snd_hda_codec_idt 
> snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep 
> snd_pcm snd_seq_midi arc4 snd_rawmidi snd_seq_midi_event snd_seq btusb 
> btintel snd_seq_device joydev coret
> [  481.671468] CPU: 0 PID: 1978 Comm: rmmod Tainted: GE   
> 4.9.0-rc7-wt+ #54
> [  481.671478] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 
> 68CDD Ver. F.04 01/27/2010
> [  481.671489]  ef49dcec c842ee92 c8b5830e ef49dd34 ef49dd20 c80850f5 
> c8b5a13c ef49dd50
> [  481.671560]  07ba c8b5830e 0483 c8461830 c8461830 0483 
> ef49ddcc f34e64b8
> [  481.671641]  c8b58360 ef49dd3c c80851bb 0009  ef49dd34 
> c8b5a13c ef49dd50
> [  481.671716] Call Trace:
> [  481.671731]  [] dump_stack+0x76/0xb4
> [  481.671745]  [] __warn+0xe5/0x100
> [  481.671757]  [] ? check_unmap+0x320/0x8e0
> [  481.671769]  [] ? check_unmap+0x320/0x8e0
> [  481.671780]  [] warn_slowpath_fmt+0x3b/0x40
> [  481.671791]  [] check_unmap+0x320/0x8e0
> [  481.671804]  [] debug_dma_unmap_page+0x84/0xa0
> [  481.671835]  [] ath10k_wmi_free_host_mem+0x9a/0xe0 [ath10k_core]
> [  481.671861]  [] ath10k_core_destroy+0x50/0x60 [ath10k_core]
> [  481.671875]  [] ath10k_pci_remove+0x79/0xa0 [ath10k_pci]
> [  481.671889]  [] pci_device_remove+0x38/0xb0
> [  481.671901]  [] __device_release_driver+0x7b/0x110
> [  481.671913]  [] driver_detach+0x97/0xa0
> [  481.671923]  [] bus_remove_driver+0x4b/0xb0
> [  481.671934]  [] driver_unregister+0x2a/0x60
> [  481.671949]  [] pci_unregister_driver+0x18/0x70
> [  481.671965]  [] ath10k_pci_exit+0xd/0x25f [ath10k_pci]
> [  481.671979]  [] SyS_delete_module+0xf4/0x180
> [  481.671995]  [] ? __might_fault+0x8b/0xa0
> [  481.672009]  [] do_fast_syscall_32+0xa0/0x1e0
> [  481.672025]  [] sysenter_past_esp+0x45/0x74
> [  481.672037] ---[ end trace 3fd23759e17e1622 ]---
> [  481.672049] Mapped at:
> [  481.672060]  [  481.672072] [] 
> debug_dma_map_page.part.25+0x1c/0xf0
> [  481.672083]  [  481.672095] [] debug_dma_map_page+0x99/0xc0
> [  481.672106]  [  481.672132] [] 
> ath10k_wmi_alloc_chunk+0x12c/0x1f0 [ath10k_core]
> [  481.672142]  [  481.672168] [] 
> ath10k_wmi_event_service_ready_work+0x304/0x540 [ath10k_core]
> [  481.672178]  [  481.672190] [] process_one_work+0x1c3/0x670
> [  482.137134] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 
> 0 reset_mode 0
> [  482.313144] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
> [  482.313274] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/cal-pci-:02:00.0.bin failed with error -2
> [  482.313768] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c 
> chip_id 0x043202ff sub :
> [  482.313777] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 
> dfs 0 testmode 1
> [  482.313974] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
> features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
> [  482.369858] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
> [  482.370011] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
> bebc7c08
> [  483.596770] ath10k_pci :02:00.0: htt-ver 2.1 wmi-op 5 htt-op 2 cal otp 
> max-sta 128 raw 0 hwcrypto 1
> [  483.701686] ath: EEPROM regdomain: 0x0
> [  483.701706] ath: EEPROM indicates default country code should be used
> [  483.701713] ath: doing EEPROM country->regdmn map search
> [  483.701721] ath: country maps to regdmn code: 0x3a
> [  483.701730] ath: Country alpha2 being used: US
> [  483.701737] ath: Regpair used: 0x3a
>
> Reported-by: Kalle Valo 
> Signed-off-by: Ben Greear 

Thanks, I don't see the warning anymore so this seems to be fixed. I'll
push this to 4.10.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ath10k: wmi-alloc-chunk should use DMA_BIDIRECTIONAL.

2016-12-05 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> These memory chunks are often used as 'swap' by the NIC,
> so it will be both reading and writing to these areas.
>
> This seems to fix errors like this on my x86-64 machine:
>
> kernel: DMAR: DMAR:[DMA Write] Request device [05:00.0] fault addr ff5de000
> DMAR:[fault reason 05] PTE Write access is not set

Noticed this in my log, but I don't have time to investigate this in
detail right now:

[  413.795346] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[  414.158755] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
[  477.439659] ath10k_pci :02:00.0: could not get mac80211 beacon
[  481.30] [ cut here ]
[  481.69] WARNING: CPU: 0 PID: 1978 at lib/dma-debug.c:1155 
check_unmap+0x320/0x8e0
[  481.88] ath10k_pci :02:00.0: DMA-API: device driver frees DMA memory 
with different direction [device address=0x2d13] [size=63800 bytes] 
[mapped with DMA_BIDIRECTIONAL] [unmapped with DMA_TO_DEVICE]
[  481.666703] Modules linked in: ctr ccm ath10k_pci(E-) ath10k_core(E) ath(E) 
mac80211(E) cfg80211(E) snd_hda_codec_hdmi snd_hda_codec_idt 
snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep 
snd_pcm snd_seq_midi arc4 snd_rawmidi snd_seq_midi_event snd_seq btusb btintel 
snd_seq_device joydev coret
[  481.671468] CPU: 0 PID: 1978 Comm: rmmod Tainted: GE   
4.9.0-rc7-wt+ #54
[  481.671478] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 68CDD 
Ver. F.04 01/27/2010
[  481.671489]  ef49dcec c842ee92 c8b5830e ef49dd34 ef49dd20 c80850f5 c8b5a13c 
ef49dd50
[  481.671560]  07ba c8b5830e 0483 c8461830 c8461830 0483 ef49ddcc 
f34e64b8
[  481.671641]  c8b58360 ef49dd3c c80851bb 0009  ef49dd34 c8b5a13c 
ef49dd50
[  481.671716] Call Trace:
[  481.671731]  [] dump_stack+0x76/0xb4
[  481.671745]  [] __warn+0xe5/0x100
[  481.671757]  [] ? check_unmap+0x320/0x8e0
[  481.671769]  [] ? check_unmap+0x320/0x8e0
[  481.671780]  [] warn_slowpath_fmt+0x3b/0x40
[  481.671791]  [] check_unmap+0x320/0x8e0
[  481.671804]  [] debug_dma_unmap_page+0x84/0xa0
[  481.671835]  [] ath10k_wmi_free_host_mem+0x9a/0xe0 [ath10k_core]
[  481.671861]  [] ath10k_core_destroy+0x50/0x60 [ath10k_core]
[  481.671875]  [] ath10k_pci_remove+0x79/0xa0 [ath10k_pci]
[  481.671889]  [] pci_device_remove+0x38/0xb0
[  481.671901]  [] __device_release_driver+0x7b/0x110
[  481.671913]  [] driver_detach+0x97/0xa0
[  481.671923]  [] bus_remove_driver+0x4b/0xb0
[  481.671934]  [] driver_unregister+0x2a/0x60
[  481.671949]  [] pci_unregister_driver+0x18/0x70
[  481.671965]  [] ath10k_pci_exit+0xd/0x25f [ath10k_pci]
[  481.671979]  [] SyS_delete_module+0xf4/0x180
[  481.671995]  [] ? __might_fault+0x8b/0xa0
[  481.672009]  [] do_fast_syscall_32+0xa0/0x1e0
[  481.672025]  [] sysenter_past_esp+0x45/0x74
[  481.672037] ---[ end trace 3fd23759e17e1622 ]---
[  481.672049] Mapped at:
[  481.672060]  [  481.672072] [] debug_dma_map_page.part.25+0x1c/0xf0
[  481.672083]  [  481.672095] [] debug_dma_map_page+0x99/0xc0
[  481.672106]  [  481.672132] [] ath10k_wmi_alloc_chunk+0x12c/0x1f0 
[ath10k_core]
[  481.672142]  [  481.672168] [] 
ath10k_wmi_event_service_ready_work+0x304/0x540 [ath10k_core]
[  481.672178]  [  481.672190] [] process_one_work+0x1c3/0x670
[  482.137134] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 
reset_mode 0
[  482.313144] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
[  482.313274] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/cal-pci-:02:00.0.bin failed with error -2
[  482.313768] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c chip_id 
0x043202ff sub :
[  482.313777] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 
0 testmode 1
[  482.313974] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
[  482.369858] ath10k_pci :02:00.0: Direct firmware load for 
ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
[  482.370011] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
bebc7c08
[  483.596770] ath10k_pci :02:00.0: htt-ver 2.1 wmi-op 5 htt-op 2 cal otp 
max-sta 128 raw 0 hwcrypto 1
[  483.701686] ath: EEPROM regdomain: 0x0
[  483.701706] ath: EEPROM indicates default country code should be used
[  483.701713] ath: doing EEPROM country->regdmn map search
[  483.701721] ath: country maps to regdmn code: 0x3a
[  483.701730] ath: Country alpha2 being used: US
[  483.701737] ath: Regpair used: 0x3a

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ath10k: Fix soft lockup during firmware crash/hw-restart

2016-12-01 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> On Tue, Nov 29, 2016 at 10:16:50AM -0800, Ben Greear wrote:
>> Is this something for stable?  And if so, how far back should it be applied?
>
> @Kalle,
>
> [shafi] kindly suggest. If i am not wrong this is only needed for 4.9

Correct, commit 3c97f5de1f28 went to 4.9-rc1, it was not in 4.8. I'll
add CC stable to the commit log.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2] ath10k: Fix soft lockup during firmware crash/hw-restart

2016-12-01 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> From: Mohammed Shafi Shajakhan 
>
> During firmware crash (or) user requested manual restart
> the system gets into a soft lock up state because of the
> below root cause.
>
> During user requested hardware restart / firmware crash
> the system goes into a soft lockup state as 'napi_synchronize'
> is called after 'napi_disable' (which sets 'NAPI_STATE_SCHED'
> bit) and it sleeps into infinite loop as it waits for
> 'NAPI_STATE_SCHED' to be cleared. This condition is hit because
> 'ath10k_hif_stop' is called twice as below (resulting in calling
> 'napi_synchronize' after 'napi_disable')
>
> 'ath10k_core_restart' -> 'ath10k_hif_stop' (ATH10K_STATE_ON) ->
> -> 'ieee80211_restart_hw' -> 'ath10k_start' -> 'ath10k_halt' ->
> 'ath10k_core_stop' -> 'ath10k_hif_stop' (ATH10K_STATE_RESTARTING)
>
> Fix this by calling 'ath10k_halt' in ath10k_core_restart itself
> as it makes more sense before informing mac80211 to restart h/w
> Also remove 'ath10k_halt' in ath10k_start for the state of 'restarting'
>
> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
> Signed-off-by: Mohammed Shafi Shajakhan 
> ---
> [v2 Added Fixes ]

I'll also add:

Cc:  # v4.9

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 02/12] ath10k: htc: rx trailer lookahead support

2016-11-16 Thread Valo, Kalle
Erik Stromdahl  writes:

> On 11/15/2016 10:57 AM, Michal Kazior wrote:
>> On 14 November 2016 at 17:33, Erik Stromdahl  
>> wrote:
>>> The RX trailer parsing is now capable of parsing lookahead reports.
>>> This is needed by SDIO/mbox.
>> 
>> It'd be useful to know what exactly lookahead will be used for. What
>> payload does it carry.
>> 
> It carries the 4 first bytes of the next RX message, i.e. the first 4
> bytes of an HTC header.
>
> It is used by the sdio interrupt handler to know if the next packet is a
> part of an RX bundle, which endpoint it belongs to and how long it is
> (so we know how many bytes to allocate).

Please add that to the commit log, it's useful information. Or maybe a
code comment would be better?

>>> +static int
>>> +ath10k_htc_process_lookahead(struct ath10k_htc *htc,
>>> +const struct ath10k_htc_lookahead_report 
>>> *report,
>>> +int len,
>>> +enum ath10k_htc_ep_id eid,
>>> +u32 *next_lk_ahds,
>> 
>> next_lk_ahds should be u8 or void. From what I understand by glancing
>> through the code it is an agnostic buffer that carries payload which
>> is later interpreted either as eid or htc header, right? void is
>> probably most suitable in this case for passing around and u8 for
>> inline-based storage.
>> 
> Sounds reasonable, I'll change to void pointer.
>
>> I'd also avoid silly abbreviations. Probably "lookahead" alone is enough.
>> 
> Ok, this abbreviation was actually taken from the ath6kl code. I think
> the intention was to reduce line lengths.

Most likely that comes from the staging ath6kl driver which again is a
fork of the Atheros internal driver. I agree with Michal, better to
avoid silly abbreviations as much as possible. Readability is most
important.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [OpenWrt-Devel] ATH10K VLAN firmware issue

2016-11-15 Thread Valo, Kalle
Bruno Antunes  writes:

> On 7 November 2016 at 18:06, Valo, Kalle  wrote:
>> Bruno Antunes  writes:
>>
>>> On 4 November 2016 at 21:17, Valo, Kalle  wrote:
>>>
>>>> Can someone file a bug to bugzilla about this so that all the info is
>>>> properly stored? The more comprehensive the report is the better.
>>>>
>>>> https://bugzilla.kernel.org/
>>>
>>> I will file a bug report.
>>
>> Thanks, it's good to store all in one place so that it's easier to find
>> the relevant info.
>
> Just file the bug with the ID 187241 - VLAN support in ATH10k Feel
> free to ask for adicional info. I did not mention any names in the bug
> report fell free to take credit if wanted.

Thanks. I'll report it to the firmware team, let's see what happens. If
there's more information which might help to fix this feel free to
update that to the bug report.

https://bugzilla.kernel.org/show_bug.cgi?id=187241

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 3/4] dt: bindings: add new dt entry for BTCOEX feature in qcom, ath10k.txt

2016-11-15 Thread Valo, Kalle
(Adding devicetree list)

 writes:

> From: Tamizh chelvam 
>
> There two things done in this patch.
>
> 1) 'btcoex_support' flag for BTCOEX feature support by the hardware.
> 2) 'wlan_btcoex_gpio' is used to fill wlan priority pin number for
>BTCOEX priority feature support.
>
> Signed-off-by: Tamizh chelvam 
> ---
>  .../bindings/net/wireless/qcom,ath10k.txt  |4 
>  1 file changed, 4 insertions(+)

As this changes the device tree bindings you need to CC the device tree
list. Please resend the whole patchset (and mark it as v2).

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 10/12] ath10k: Added QCA65XX hw definition

2016-11-15 Thread Valo, Kalle
Michal Kazior  writes:

> On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
>> Signed-off-by: Erik Stromdahl 
>> ---
>>  drivers/net/wireless/ath/ath10k/hw.h |1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/hw.h 
>> b/drivers/net/wireless/ath/ath10k/hw.h
>> index 46142e9..ef45ecf 100644
>> --- a/drivers/net/wireless/ath/ath10k/hw.h
>> +++ b/drivers/net/wireless/ath/ath10k/hw.h
>> @@ -224,6 +224,7 @@ enum ath10k_hw_rev {
>> ATH10K_HW_QCA9377,
>> ATH10K_HW_QCA4019,
>> ATH10K_HW_QCA9887,
>> +   ATH10K_HW_QCA65XX,
>
> Are you 100% positive that you're supporting all QCA65XX chips? The
> rule of thumb is to avoid Xs as much as possible unless totally
> perfectly completely sure.

But the thing is that nobody can't be absolutely sure as we never know
what marketing comes up within few years again. So I would say that XX
in chip names should be completely avoided to avoid any confusion. We
already suffer from this in ath10k with QCA988X and QCA9888 which are
different designs but the names overlap.

I haven't reviewed Erik's patches yet but I'm surprised why even a new
enum value is needed here. I was assuming we could use ATH10K_HW_QCA6174
because AFAIK they share the same design. Any particular reason for
this?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 09/12] ath10k: Mailbox address definitions

2016-11-15 Thread Valo, Kalle
Michal Kazior  writes:

> On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
>> Address definitions for SDIO/mbox based chipsets.
>>
>> Signed-off-by: Erik Stromdahl 
>> ---
>>  drivers/net/wireless/ath/ath10k/hw.h |   53 
>> ++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/hw.h 
>> b/drivers/net/wireless/ath/ath10k/hw.h
>> index 883547f..46142e9 100644
>> --- a/drivers/net/wireless/ath/ath10k/hw.h
>> +++ b/drivers/net/wireless/ath/ath10k/hw.h
>> @@ -814,6 +814,59 @@ struct ath10k_hw_ops {
>>  #define QCA9887_EEPROM_ADDR_LO_MASK0x00ff
>>  #define QCA9887_EEPROM_ADDR_LO_LSB 16
>>
>> +#define MBOX_RESET_CONTROL_ADDRESS 0x
>> +#define MBOX_HOST_INT_STATUS_ADDRESS   0x0800
>> +#define MBOX_HOST_INT_STATUS_ERROR_LSB 7
>> +#define MBOX_HOST_INT_STATUS_ERROR_MASK0x0080
>
> I would again suggest using Jakub's bitfield GET_FIELD stuff to get
> rid of MASK+LSB and just have single define per register field. Kalle,
> thoughts?

Same comment as with the other patch, very much preferred but not a hard
requirement.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 05/12] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB

2016-11-15 Thread Valo, Kalle
Michal Kazior  writes:

> On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
>> Signed-off-by: Erik Stromdahl 
>> ---
>>  drivers/net/wireless/ath/ath10k/htc.h |2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htc.h 
>> b/drivers/net/wireless/ath/ath10k/htc.h
>> index 589800a..df16a04 100644
>> --- a/drivers/net/wireless/ath/ath10k/htc.h
>> +++ b/drivers/net/wireless/ath/ath10k/htc.h
>> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
>> ATH10K_HTC_FLAG_BUNDLE_MASK = 0xF0
>>  };
>>
>> +#define ATH10K_HTC_FLAG_BUNDLE_LSB 4
>
> Just an idea - we could start using FIELD_GET() with
> ATH10K_HTC_FLAG_BUNDLE_MASK alone. I would love to see Jakub's
> bitfield stuff be used more. Kalle, thoughts?

Yeah, the bitfield macros are handy and we should definitely switch to
them (slowly). But definitely not a must for these SDIO patches, more
like a nice bonus.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ATH10K VLAN firmware issue

2016-11-07 Thread Valo, Kalle
Bruno Antunes  writes:

> On 4 November 2016 at 21:17, Valo, Kalle  wrote:
>> Bruno Antunes  writes:
>>
>>> Old thread but I think the issue is still present.
>>>
>>> I'm running a setup with VLANs with WDS and ath10k cards.
>>>
>>> To make it work both cards must be loaded in rawmode, AP
>>> and Sta, and with no security.
>>>
>>> I'm using a OpenWrt trunk r49941 and the most recent firmware,
>>> 10.2.4.70.58, from Kalle ath10k firmware tree.
>>>
>>> Although it works the throughput is very bad.
>>> Are there any alternatives to improve the throughput.
>>
>> Can someone file a bug to bugzilla about this so that all the info is
>> properly stored? The more comprehensive the report is the better.
>>
>> https://bugzilla.kernel.org/
>
> I will file a bug report.

Thanks, it's good to store all in one place so that it's easier to find
the relevant info.

> But since it appears to be a firmware related issue
> under what category can fill in the bug?

You can file it under Drivers/network-wireless, AFAIK we don't have any
separate components for firmware bugs. Here's one example to follow:

https://bugzilla.kernel.org/show_bug.cgi?id=186161

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Connection to WPA-PSK/WPA2-PSK networks doesn't work with QCA9733 rev 30

2016-11-04 Thread Valo, Kalle
Markus Mälkönen  writes:

> It seems that I can't connect to any WPA-PSK/WPA2-PSK network with
> this network adapter. But unsecured connections without any WPA or WEP
> works normally.
>
> 02:00.0 Network controller [0280]: Qualcomm Atheros QCA9377 802.11ac
> Wireless Network Adapter [168c:0042] (rev 30)

You are not providing much information so difficult to guess. I haven't
heard anyone else mentioning anything like this, are you sure it really
is a problem with ath10k?

To understand more what is happening we would need to see how you
configure the network, wpasupplicant debug logs and full dmesg output.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: ATH10K VLAN firmware issue

2016-11-04 Thread Valo, Kalle
Bruno Antunes  writes:

> Old thread but I think the issue is still present.
>
> I'm running a setup with VLANs with WDS and ath10k cards.
>
> To make it work both cards must be loaded in rawmode, AP
> and Sta, and with no security.
>
> I'm using a OpenWrt trunk r49941 and the most recent firmware,
> 10.2.4.70.58, from Kalle ath10k firmware tree.
>
> Although it works the throughput is very bad.
> Are there any alternatives to improve the throughput.

Can someone file a bug to bugzilla about this so that all the info is
properly stored? The more comprehensive the report is the better.

https://bugzilla.kernel.org/

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 0/5] ath6kl: non WMI data service support

2016-10-14 Thread Valo, Kalle
(Adding ath10k list)

Erik Stromdahl  writes:

> On 10/14/2016 09:34 AM, Valo, Kalle wrote:
>> Erik Stromdahl  writes:
>> 
>>> This patch series is intended to prepare the ath6kl driver
>>> for newer chipsets that doesn't use the current WMI data
>>> endpoints for data traffic.
>>>
>>> The chipset I have been working with (and used for testing)
>>> is QCA6584. It is SDIO based (at least the variant I have
>>> been using) with 802.11p WAVE DSRC capabilities.
>>>
>>> This chipset is different from the AR600X family in that
>>> it does not use the WMI data services (service id's 0x101
>>> to 0x104 ) for data traffic.
>>> Instead it uses the HTT data service for data and wmi unified
>>> for control messages.
>>> It is also different when it comes to mailbox addresses
>>> and HTC header format as well, but these differences are not
>>> part of this patch series.
>> 
>> Do you have more patches implemented, like something already working or
>> have just started?
>> 
>
> I have an implementation with all features I need so far, but the other
> patches will require cleanup before I can submit anything.
>
> I have been using the qcacld driver as a basis for the work and some of
> the stuff in that driver is not really compliant with the kernel coding
> style (to say the least).
>
> I have so far mainly been focused on getting all features up and running
> and that has (unfortunately) resulted in some copy-pasting from
> qcacld.

Can you share the current code you have somewhere so that I could take a
quick look? I don't care how ugly it is, I would just like to understand
what kind of changes are needed.

> I will start having a look at ath10k and see how much work it would be
> to add sdio support to it...

Thanks, let me know how it goes or if I can help somehow. My time is
limited but if nothing else I can give some tips.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [RFC 0/5] ath6kl: non WMI data service support

2016-10-14 Thread Valo, Kalle
(Adding ath10k list to CC)

Erik Stromdahl  writes:

>> Exactly what I was thinking. When I saw terms like "HTT" and "unified
>> WMI" my first thought was that is this actually an ath10k based design?
>> The product numbers really don't give any indication what driver
>> supports, the division goes something like this:
>> 
>> * ath9k: "non-mobile" 11n chips
>> * ath6k: mobile 11n chips
>> * ath10k: mobile and "non-mobile" 11ac chips
>> 
>> For example QCA6174 is an 11ac mobile chip supported by ath10k. ath10k
>> only supports PCI bus at the moment, but I'm hoping someone would add
>> USB and SDIO support. Patches are very welcome.
>> 
>> I'm starting to suspect that QCA6584 is actually based on the same
>> design as QCA6174. If that's the case when we should also think if
>> instead we should add SDIO support to ath10k, maybe by taking relevant
>> parts from ath6kl?
>> 
>> I'll investigate more what this QCA6584 is, I haven't heard about it
>> before.
>> 
>
> The reason I have patched to ath6kl driver was that it is the
> only driver with sdio/mbox support.
>
> I was actually thinking of writing a new driver from scratch, but I
> thought that it was less work to modify the existing ath6kl driver.

Ok.

> I just haven't considered the option to add sdio/mbox support to ath10k.
> This is definitely something I will have a look at.
>
> I should mention that I have been using the qcacld2.0 driver as
> "documentation" of the chipset.
>
> The qcacld driver identifies the chipset as AR6320
>
> From the qcacld2.0 driver bmi_msg.h:
>
> #define TARGET_TYPE_AR63208
>
> Perhaps this can shed some light on what kind of chip this is?

I'm pretty sure that this is QCA6174 based design which is already
supported by ath10k. So my suggestion is to first look at adding SDIO
support to ath10k and see if that's feasible. We already have PCI code
split from the core code (ath10k_core.ko and ath10k_pci.ko) just because
I was expecting that we would add SDIO and USB support later. That has
just never happened due to lack of time, hopefully you can fix that now
;)

I haven't studied SDIO support at all yet but hopefully WMI, core.c and
mac.c won't need that much modifications. HTT and HTC most likely need
bigger changes. And then you would need to add sdio.c, similarly like we
have pci.c now. Keep in mind that later we might want to add USB support
also so if there's something which both SDIO and USB need to that would
need to be easily sharable. Actually after adding SDIO I hope USB would
be easier to add.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 2/2] ath10k: add VHT160 support

2016-10-14 Thread Valo, Kalle
Kalle Valo  writes:

> From: Sebastian Gottschall 
>
> This patch adds full VHT160 support for QCA9984 chipsets Tested on Netgear
> R7800. 80+80 is possible, but disabled so far since it seems to contain
> glitches like missing vht station flags (this may be firmware or mac80211
> related).
>
> Signed-off-by: Sebastian Gottschall 
> Patchwork-Id: 9335111

Oops, the patchwork id is a leftover from my patchwork script. I'll
remove it before I apply the patch.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] QCA9984 VHT160 support

2016-10-14 Thread Valo, Kalle
Sebastian Gottschall  writes:

> This patch adds full VHT160 support for QCA9984 chipsets
> Tested on Netgear R7800. 80+80 is possible, but disabled so far since
> it seems to contain glitches like missing vht station flags (this may
> be firmware or mac80211 related)

So how did you disable 80+80 exactly? I can't find it and I would like
to add a comment to the code so that it's easy to enable it later (once
it's working).

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] QCA9984 VHT160 support

2016-10-14 Thread Valo, Kalle
Sebastian Gottschall  writes:

> Am 10.10.2016 um 18:06 schrieb Valo, Kalle:
>> Kalle Valo  writes:
>>
>>> Sebastian Gottschall  writes:
>>>
>>>> This patch adds full VHT160 support for QCA9984 chipsets
>>>> Tested on Netgear R7800. 80+80 is possible, but disabled so far since
>>>> it seems to contain glitches like missing vht station flags (this may
>>>> be firmware or mac80211 related)
>>> There are some compilation and checkpatch warnings. Also you forgot to
>>> CC linux-wireless and sent the patch as an attachment. I'll send v2 to
>>> fix those.
>
> gen_dbglog_cfg? i did not touch anything on this struct in my patch.
> this warning doesnt seem to belong to my patch

According to patchwork you did, see below. Most likely just merge damage
from rebasing, I fixed it in v2.

--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -2468,7 +2468,7 @@  ath10k_wmi_tlv_op_gen_force_fw_hang(struct ath10k *ar,
 }
 
 static struct sk_buff *
-ath10k_wmi_tlv_op_gen_dbglog_cfg(struct ath10k *ar, u64 module_enable,
+ath10k_wmi_tlv_op_gen_dbglog_cfg(struct ath10k *ar, u32 module_enable,
 u32 log_level) {
struct wmi_tlv_dbglog_cmd *cmd;
struct wmi_tlv *tlv;

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: qca6174 missing firmware or subsystem id?

2016-10-13 Thread Valo, Kalle
Jiří Prchal  writes:

> On 13.10.2016 15:00, Valo, Kalle wrote:
>> Jiří Prchal  writes:
>>
>>> $dmesg | grep ath
>>> [1.997246] ath10k_pci :02:00.0: pci irq msi interrupts 1 irq_mode 0 
>>> reset_mode 0
>>> [2.252840] ath10k_pci :02:00.0: Direct firmware load for 
>>> ath10k/cal-pci-:02:00.0.bin failed with error -2
>>> [2.260813] ath10k_pci :02:00.0: Direct firmware load for 
>>> ath10k/QCA6174/hw3.0/firmware-5.bin failed with error -2
>>> [2.260816] ath10k_pci :02:00.0: could not fetch firmware file 
>>> 'ath10k/QCA6174/hw3.0/firmware-5.bin': -2
>>
>> So ignore this firmware-5.bin warning, ath10k will automatically fall
>> back to using firmware-4.bin. I know it's confusing, we just haven't
>> found a good enough way to silence this warning yet.
>
> OK. Maybe temporary fix: write info message what firmware is finally
> loaded?

That's a good idea. Developers can already see that from the "fwapi 4"
print but somekind of explanation in english should reduce the confusion
with normal users.

>>> [4.461429] ath10k_pci :02:00.0: qca6174 hw3.2 (0x0503,
>>> 0x00340aff sub 1043:86cd) fw WLAN.RM.2.0-00180-QCARMSWPZ-1 fwapi 4
>>> bdapi 2 htt-ver 3.26 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0
>>> hwcrypto 1 features wowlan,ignore-otp,no-4addr-pad
>>
>> Your ath10k is too old so that it doesn't print the crc32 checksum of
>> the board file you have so I don't know what version of the board file
>> you exactly have. But I just checked, the latest one from
>> ath10k-firmware.git should support your board with subsystem id
>> 1043:86cd:
>
> Should I try the newest kernel?

That's always a good idea, for example you could try just released v4.8.
Does ubuntu still provide .debs for upstream kernels?

Or you can try backports:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/backports

>> $ strings QCA6174/hw3.0/board-2.bin | grep 86cd
>> bus=pci,vendor=168c,device=003e,subsystem-vendor=1043,subsystem-device=86cdm
>>
>> The checksums of the latest board file are:
>>
>> FileCRC32: 6fc88fe7
>> FileMD5: 4807903c956dbd8b87f5d83c559c2211
>>
>> You can check what you are using like this:
>>
>> md5sum /lib/firmware/ath10k/QCA6174/hw3.0/board-2.bin
>
> Yes, I have the newest: 4807903c956dbd8b87f5d83c559c2211.

Then I suspect this is something else than board file related, but of
course I can't be 100% sure. You could also try enabling debug messages
and see if they reveal anything, for example debug_mask=0x0432 is a
good start:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug

Have you verified that this is not a hardware problem? For example, does
the board work on Windows?

Also make sure that you have killed wpasupplicant, network-manager or
any other entity which might control the wireless device (very unlikely
that would cause this but better to be sure).

What channel is your AP in? Are there multiple APs in the area? This
could be also a channel problem, 'iw list' helps to investigate that (or
changing the channel on AP).

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

2016-10-13 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> From: Mohammed Shafi Shajakhan 
>
> 'ath10k_htt_rx_h_unchain' needs to be called only if the return
> value from 'ath10k_htt_rx_amsdu_pop' is 1('chained msdu's'), this
> change makes it more explicit and avoids doing a skb_peek, fetching
> rx descriptor pointer, checking rx msdu decap format for the case of
> ret = 0 (unchained msdus). Found this change during code walk through,
> not sure if this addresses any issue.
>
> Signed-off-by: Mohammed Shafi Shajakhan 

Applied to ath-next, thanks.

(The SMTP server failed, needed to send this manually)

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: qca6174 missing firmware or subsystem id?

2016-10-13 Thread Valo, Kalle
Jiří Prchal  writes:

> Hi,
> I've got this miniPCIe wifi module which comes with Asus motherboard:
>
> $lspci -vnn
> 02:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac Wireless 
> Network Adapter [168c:003e] (rev 32)
>   Subsystem: ASUSTeK Computer Inc. QCA6174 802.11ac Wireless Network 
> Adapter [1043:86cd]
>   Flags: bus master, fast devsel, latency 0, IRQ 125
>   Memory at f700 (64-bit, non-prefetchable) [size=2M]
>   Capabilities: 
>   Kernel driver in use: ath10k_pci
>   Kernel modules: ath10k_pci
>
> But I have problem to get it working. It look that wlan is there, but doesn't 
> receive any SSID.
>
> $ip a
> 4: wlp2s0:  mtu 1500 qdisc mq state DOWN 
> group default qlen 1000
> link/ether c8:ff:28:0e:5e:e1 brd ff:ff:ff:ff:ff:ff
>
> $iw dev wlp2s0 scan
>  returns nothing.
>
> I have Ubuntu 16.04 with this kernel:
> $uname -a
> Linux dole 4.4.0-42-generic #62-Ubuntu SMP Fri Oct 7 23:11:45 UTC 2016 x86_64 
> x86_64 x86_64 GNU/Linux
>
> I've tried both distro and git FW (probably the same), but it still
> complies about firmware-5.bin. I thing there is missing subsystem ID
> in board-2.bin as I mentioned there was added some other subID 2
> months ago.
>
> $dmesg | grep ath
> [1.997246] ath10k_pci :02:00.0: pci irq msi interrupts 1 irq_mode 0 
> reset_mode 0
> [2.252840] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/cal-pci-:02:00.0.bin failed with error -2
> [2.260813] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/QCA6174/hw3.0/firmware-5.bin failed with error -2
> [2.260816] ath10k_pci :02:00.0: could not fetch firmware file 
> 'ath10k/QCA6174/hw3.0/firmware-5.bin': -2

So ignore this firmware-5.bin warning, ath10k will automatically fall
back to using firmware-4.bin. I know it's confusing, we just haven't
found a good enough way to silence this warning yet.

> [4.461429] ath10k_pci :02:00.0: qca6174 hw3.2 (0x0503,
> 0x00340aff sub 1043:86cd) fw WLAN.RM.2.0-00180-QCARMSWPZ-1 fwapi 4
> bdapi 2 htt-ver 3.26 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0
> hwcrypto 1 features wowlan,ignore-otp,no-4addr-pad

Your ath10k is too old so that it doesn't print the crc32 checksum of
the board file you have so I don't know what version of the board file
you exactly have. But I just checked, the latest one from
ath10k-firmware.git should support your board with subsystem id
1043:86cd:

$ strings QCA6174/hw3.0/board-2.bin | grep 86cd
bus=pci,vendor=168c,device=003e,subsystem-vendor=1043,subsystem-device=86cdm

The checksums of the latest board file are:

FileCRC32: 6fc88fe7
FileMD5: 4807903c956dbd8b87f5d83c559c2211

You can check what you are using like this:

md5sum /lib/firmware/ath10k/QCA6174/hw3.0/board-2.bin

(Unless I made a typo in the path, but you should be able to find it
easily.)

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/2] ath10k: add per peer htt tx stats support for 10.4

2016-10-12 Thread Valo, Kalle
Yeoh Chun-Yeow  writes:

>> My understanding is that 10.2 branch does not have this feature,
>> unfortunately.
>>
>
> Alright, noted.
>
> Is QCA988X going to have firmware 10.4 support?

I wish it would have but I don't know the current status.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Latest stable ath10k driver

2016-10-12 Thread Valo, Kalle
"Bonev, George"  writes:

> Hi,
> I am looking for the latest stable version of the ath10k driver.
>
> Right now I am using v4.4.2 I downloaded from here:
> https://www.kernel.org/pub/linux/kernel/projects/backports/stable/v4.4.2/
> I am rebuilding ath10k driver against my old kernel.
>
> Is there a TAG that represents a stable version in 
> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ?

Best to use official Linux releases (v4.7, v4.8 and so on). If you want,
you can in addition use Linus stable releases (4.8.1 etc). There are no
separate releases for ath10k, if that's what you are asking.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: Very low throughput on QCA9880 ath10k module

2016-10-12 Thread Valo, Kalle
Vijay Kumar R Patel  writes:

> HI all,
> we are testing throughput on our QCA9880 module using iperf command,
> we are getting 10Mbps throughput, tried with all iperf options like
> TCP window size,length but not getting more than 10Mbps.
>
> Test procedure and system info
> iperf b/w AP <> STA
> QCA9880 module on both ap and sta, with backported(backports-3.14-1)
> ath10k driver running on Linux kernel version 2.6.32 with
> firmware-2.bin_10.1.467.3-1

>From ath10k point of view backports-3.14-1 is ancient. Try upgrading to
something recent, preferably 4.8.

The problem is that backports doesn't have had new releases for some
time so you have to build the backports package yourself. There are
instructions on the wiki but I'm not sure if they work anymore, I
haven't tried myself for a long time:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/backports

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA9887 Firmware Memory Usage

2016-10-12 Thread Valo, Kalle
David Hutchison  writes:

> I am using a Mikrotik hAP AC Lite which has a QCA9887 radio. It
> appears to be working; however ath10k ( or the qca9887 firmware ) is
> utilizing 15 - 20mb of memory. I applied the kfree(caldata) patch, to
> insure there is no memory leak.
>
> It doesn't appear to be leaking, and it will run Ok with the high
> memory footprint. However the Mikrotik hAP AC Lite is only a 64mb
> platform; so when I initialize the 2nd radio with ath9k I get OOMs.
>
> It just doesn't seem right that when ath10k + qca9887 is in AP mode
> that it's utilizing as much memory as it is. I don't think the issue
> is with ath10k, I think it's the qca9887 firmware (
> firmware-5.bin_10.2.4-1.0-00013 ).
>
> Is there a newer build available that may potentially fix these memory
> issues? The latest one available on
> https://github.com/kvalo/ath10k-firmware/blob/master/QCA9887/hw1.0/firmware-5.bin_10.2.4-1.0-00013
> is 2 months old. Perhaps there is a newer build to test that may have
> the fix?
>
> Any thoughts as to what it could be? I can provide more information if needed.

64MB of RAM nowadays is not much and ath10k is not really tested in such
setups AFAIK. I guess you could try if reducing TARGET_10X_NUM_MSDU_DESC
helps with the memory consumption, but no idea if that would even work
without further modifications to ath10k.

Also if you have fq-codel&co enabled that might increase memory
consumption. Adding linux-wireless as people there might have better
ideas.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [v2,1/2] ath10k: clean up HTT tx buffer allocation and free

2016-10-12 Thread Valo, Kalle
Mohammed Shafi Shajakhan  writes:

> Hi Kalle,
>
> On Tue, Oct 11, 2016 at 01:36:22PM +0200, Kalle Valo wrote:
>> Mohammed Shafi Shajakhan  wrote:
>> > From: Mohammed Shafi Shajakhan 
>> > 
>> > cleanup 'ath10k_htt_tx_alloc' by introducing the API's
>> > 'ath10k_htt_tx_alloc/free_{cont_txbuf, txdone_fifo} and
>> > re-use them whereever needed
>> > 
>> > Signed-off-by: Mohammed Shafi Shajakhan 
>> 
>> Applies but fails to build:
>
> [shafi] sorry for the trouble again, I just figured out I had a private patch 
> by
> mistake  and it worked for me, I will make sure that v3 is properly rebased
> without any errors.

Thanks. And no worries, I now have a script which does commit and build
tests semi-automatically and sends an email if it fails. Yay! This was a
good real life test for the script :)

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/2] ath10k: add per peer htt tx stats support for 10.4

2016-10-11 Thread Valo, Kalle
Yeoh Chun-Yeow  writes:

> On Fri, Oct 7, 2016 at 10:58 PM,   wrote:
>> From: Anilkumar Kolli 
>>
>> Per peer tx stats are part of 'HTT_10_4_T2H_MSG_TYPE_PEER_STATS'
>> event, Firmware sends one HTT event for every four PPDUs.
>> HTT payload has success pkts/bytes, failed pkts/bytes, retry
>> pkts/bytes and rate info per ppdu.
>> Peer stats are enabled through 'WMI_SERVICE_PEER_STATS',
>> which are nowadays enabled by default.
>>
>> Parse peer stats and update the tx rate information per STA.
>>
>> tx rate, Peer stats are tested on QCA4019 with Firmware version
>> 10.4-3.2.1-00028.
>>
>
> Is QCA988X supported? I have tried to test with
> firmware-5.bin_10.2.4.70.56 but not working.

My understanding is that 10.2 branch does not have this feature,
unfortunately.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 1/2] ath10k: add per peer htt tx stats support for 10.4

2016-10-11 Thread Valo, Kalle
 writes:

> From: Anilkumar Kolli 
>
> Per peer tx stats are part of 'HTT_10_4_T2H_MSG_TYPE_PEER_STATS'
> event, Firmware sends one HTT event for every four PPDUs.
> HTT payload has success pkts/bytes, failed pkts/bytes, retry
> pkts/bytes and rate info per ppdu.
> Peer stats are enabled through 'WMI_SERVICE_PEER_STATS',
> which are nowadays enabled by default.
>
> Parse peer stats and update the tx rate information per STA.
>
> tx rate, Peer stats are tested on QCA4019 with Firmware version
> 10.4-3.2.1-00028.
>
> Signed-off-by: Anilkumar Kolli 

This and patch 2 add few new warnings:

drivers/net/wireless/ath/ath10k/htt_rx.c: In function 
'ath10k_htt_fetch_peer_stats':
drivers/net/wireless/ath/ath10k/htt_rx.c:2279:3: warning: too many arguments 
for format [-Wformat-extra-args]
drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17: warning: incorrect type in 
assignment (different base types)
drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17:expected unsigned int 
[unsigned] [usertype] peer_id
drivers/net/wireless/ath/ath10k/htt_rx.c:2285:17:got restricted __le16 
[usertype] peer_id
drivers/net/wireless/ath/ath10k/debugfs_sta.c:84: space required before the 
open parenthesis '('

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] QCA9984 VHT160 support

2016-10-10 Thread Valo, Kalle
Kalle Valo  writes:

> Sebastian Gottschall  writes:
>
>> This patch adds full VHT160 support for QCA9984 chipsets
>> Tested on Netgear R7800. 80+80 is possible, but disabled so far since
>> it seems to contain glitches like missing vht station flags (this may
>> be firmware or mac80211 related)
>
> There are some compilation and checkpatch warnings. Also you forgot to
> CC linux-wireless and sent the patch as an attachment. I'll send v2 to
> fix those.

Forgot the actual warnings:

drivers/net/wireless/ath/ath10k/wmi-tlv.c:3520:2: warning: initialization from 
incompatible pointer type [enabled by default]
drivers/net/wireless/ath/ath10k/wmi-tlv.c:3520:2: warning: (near initialization 
for 'wmi_tlv_ops.gen_dbglog_cfg') [enabled by default]
drivers/net/wireless/ath/ath10k/wmi-tlv.c:3520:27: warning: incorrect type in 
initializer (incompatible argument 2 (different type sizes))
drivers/net/wireless/ath/ath10k/wmi-tlv.c:3520:27:expected struct sk_buff 
*( *gen_dbglog_cfg )( ... )
drivers/net/wireless/ath/ath10k/wmi-tlv.c:3520:27:got struct sk_buff *( 
static [toplevel] * )( ... )
drivers/net/wireless/ath/ath10k/wmi-tlv.h:546: code indent should use tabs 
where possible
drivers/net/wireless/ath/ath10k/wmi-tlv.h:546: please, no spaces at the start 
of a line
drivers/net/wireless/ath/ath10k/mac.c:2556: braces {} should be used on all 
arms of this statement
drivers/net/wireless/ath/ath10k/mac.c:2560: line over 90 characters
drivers/net/wireless/ath/ath10k/mac.c:4301: line over 90 characters
drivers/net/wireless/ath/ath10k/mac.c:4301: braces {} are not necessary for 
single statement blocks

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] QCA9984 VHT160 support

2016-10-10 Thread Valo, Kalle
Sebastian Gottschall  writes:

> This patch adds full VHT160 support for QCA9984 chipsets
> Tested on Netgear R7800. 80+80 is possible, but disabled so far since
> it seems to contain glitches like missing vht station flags (this may
> be firmware or mac80211 related)

There are some compilation and checkpatch warnings. Also you forgot to
CC linux-wireless and sent the patch as an attachment. I'll send v2 to
fix those.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2] ath10k: cache calibration data when the core is stopped

2016-10-10 Thread Valo, Kalle
Marty Faltesek  writes:

> ath10k: cache calibration data when the core is stopped
>
> Caching calibration data allows it to be accessed when the
> device is not active.
>
> Signed-off-by: Marty Faltesek 

Thanks, I'll now send v3.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2] ath10k: cache calibration data when the core is stopped

2016-10-10 Thread Valo, Kalle
Marty Faltesek  writes:

> Caching calibration data allows it to be accessed when the
> device is not active.
>
> ---

Signed-off-by missing. Can you send it as a reply to this message and
I'll add it to v3?

>  drivers/net/wireless/ath/ath10k/core.h  |  1 +
>  drivers/net/wireless/ath/ath10k/debug.c | 72 
> ++---
>  2 files changed, 31 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> b/drivers/net/wireless/ath/ath10k/core.h
> index 6e5aa2d..7274ebe 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -452,6 +452,7 @@ struct ath10k_debug {
>   u32 nf_cal_period;
>  
>   struct ath10k_fw_crash_data *fw_crash_data;
> + void *cal_data;

To properly document locking I'll move this under the "protected by
conf_mutex" comment.

> -static int ath10k_debug_cal_data_open(struct inode *inode, struct file *file)
> +static int ath10k_debug_cal_data_fetch(struct ath10k *ar)
>  {
> - struct ath10k *ar = inode->i_private;
> - void *buf;
>   u32 hi_addr;
>   __le32 addr;
>   int ret;
>  
> - mutex_lock(&ar->conf_mutex);

I'll add lockdep_assert_held() here.

>  static ssize_t ath10k_debug_cal_data_read(struct file *file,
> -   char __user *user_buf,
> -   size_t count, loff_t *ppos)
> +  char __user *user_buf,
> +  size_t count, loff_t *ppos)
>  {
>   struct ath10k *ar = file->private_data;
> - void *buf = file->private_data;
>  
>   return simple_read_from_buffer(user_buf, count, ppos,
> -buf, ar->hw_params.cal_data_len);
> -}

I'll add locking to this function.

> @@ -2343,7 +2327,9 @@ int ath10k_debug_create(struct ath10k *ar)
>   ar->debug.fw_crash_data = vzalloc(sizeof(*ar->debug.fw_crash_data));
>   if (!ar->debug.fw_crash_data)
>   return -ENOMEM;
> -
> + ar->debug.cal_data = vzalloc(ar->hw_params.cal_data_len);
> + if (!ar->debug.cal_data)
> + return -ENOMEM;
>   INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
>   INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
>   INIT_LIST_HEAD(&ar->debug.fw_stats.peers);
> @@ -2355,7 +2341,9 @@ int ath10k_debug_create(struct ath10k *ar)
>  void ath10k_debug_destroy(struct ath10k *ar)
>  {
>   vfree(ar->debug.fw_crash_data);
> + vfree(ar->debug.cal_data);
>   ar->debug.fw_crash_data = NULL;
> + ar->debug.cal_data = NULL;

Actually I gave you a bad advice, this won't work as
ar->hw_params.cal_data_len is not yet initialised, we initialise it only
after we have probed the capabilities during the first firmware boot. I
changed the allocation to a fixed length buffer for now, it's only few
kBs virtual memory anyway so it shouldn't matter to anyone.

I have now v3 ready and tested. I'll just need your Signed-off-by and I
can then submit it.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: cache calibration data when the core is stopped.

2016-10-06 Thread Valo, Kalle
Marty Faltesek  writes:

> On Mon, Oct 3, 2016 at 3:46 AM, Valo, Kalle  wrote:
>> Marty Faltesek  writes:
>>
>>> Caching calibration data allows it to be accessed when the
>>> device is not active.
>>>
>>> Signed-off-by: Marty Faltesek 
>>
>> No comma in the title, please.
>>
>> What tree did you use as the baseline? This doesn't seem to apply to
>> ath.git:
>
> We use backports 20160122 which has not been updated since earlier this year.
> I can forward port it to your tree, and make sure
> it builds but won't be able to test it. Will that be OK?

Sure, I can test it.

>> Also please note that this patch (which I'm queuing to 4.9) touches the
>> same area:
>>
>> ath10k: fix debug cal data file
>>
>> https://patchwork.kernel.org/patch/9340953/
>
> I've modified this too, and this won't be necessary, so can you drop
> it? If not, let me know and I'll pull it in and make sure I'm based
> off it too.

Actually I was first planning to push 9340953 to 4.9 and take your patch
to 4.10. But your patch is a cleaner approach to this and maybe I should
push that to 4.9 instead? Need to think a bit more.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH 2/2] ath10k: add platform regulatory domain support

2016-10-03 Thread Valo, Kalle
Bartosz Markowski  writes:

> On 14 September 2016 at 09:06, Valo, Kalle  wrote:
>>
>> Bartosz Markowski  writes:
>>
>> > On 12 September 2016 at 17:35, Valo, Kalle  wrote:
>> >
>> >> > +#ifdef CONFIG_ACPI
>> >> > +#define WRD_METHOD "WRDD"
>> >> > +#define WRDD_WIFI  (0x07)
>> >> > +
>> >> > +static u32 ath10k_mac_wrdd_get_mcc(struct ath10k *ar, union 
>> >> > acpi_object *wrdd)
>> >> > +{
>> >>
>> >> I don't think the ifdef is really necessary, acpi.h should handle that
>> >> (hopefully). Also I changed the error handling to use standard error
>> >> values and changed the info messages to dbg, they are too spammy in my
>> >> opinion. Please check carefully my changes in the pending branch:
>> >>
>> >> https://git.kernel.org/cgit/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=fe91745381ec3999d8de6dedb07b396c82539717
>> >
>> > I'm OK with the changes, I have not tried that though, except of
>> > reviewing and compiling it (do not have access to the chromebook for
>> > next few days). If you want to wait with it until I test it, it's fine
>> > too.
>>
>> Ok, I'll wait for few days in case you have time to test it.
>
>
> Sorry, it took so long. I've final check this and can confirm the patch.

Thanks, I'll apply the patch soon. Do note that I changed more of the
warnings messages to debug level.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: cache calibration data when the core is stopped.

2016-10-03 Thread Valo, Kalle
Marty Faltesek  writes:

> Caching calibration data allows it to be accessed when the
> device is not active.
>
> Signed-off-by: Marty Faltesek 

[...]

> +int
> +ath10k_cal_data_alloc(struct ath10k *ar, void **buf)
> +{

[...]

> + ath10k_cal_data_alloc(ar, &ar->cal_data);
[...]

> + ret = ath10k_cal_data_alloc(ar, &file->private_data);

Pointer to pointer parameters can be a source of problems and if we
could use one shared buffer for both of these cases when it would
simplify the code and we would need the buf parameter at all.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: cache calibration data when the core is stopped.

2016-10-03 Thread Valo, Kalle
Marty Faltesek  writes:

> Caching calibration data allows it to be accessed when the
> device is not active.
>
> Signed-off-by: Marty Faltesek 

No comma in the title, please.

What tree did you use as the baseline? This doesn't seem to apply to
ath.git:

Applying: ath10k: cache calibration data when the core is stopped.
fatal: sha1 information is lacking or useless 
(drivers/net/wireless/ath/ath10k/core.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ath10k: cache calibration data when the core is stopped.

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -1227,6 +1227,42 @@ success:
>   return 0;
>  }
>  
> +int
> +ath10k_cal_data_alloc(struct ath10k *ar, void **buf)
> +{
> + u32 hi_addr;
> + __le32 addr;
> + int ret;

I think this function should be in debug.c. That way the code is not
wasting memory if DEBUGFS is disabled. 

> + vfree(*buf);
> + *buf = vmalloc(QCA988X_CAL_DATA_LEN);
> + if (!*buf) {
> + return -EAGAIN;
> + }

Is it really necessary to allocate memory every time? What if allocate
it only once when module is loaded, just like with
ar->debug.fw_crash_data?

Also please note that this patch (which I'm queuing to 4.9) touches the
same area:

ath10k: fix debug cal data file

https://patchwork.kernel.org/patch/9340953/

> + hi_addr = host_interest_item_address(HI_ITEM(hi_board_data));
> +
> + ret = ath10k_hif_diag_read(ar, hi_addr, &addr, sizeof(addr));
> +
> + if (ret) {
> + ath10k_warn(ar, "failed to read hi_board_data address: %d\n", 
> ret);
> + } else {
> + ret = ath10k_hif_diag_read(ar, le32_to_cpu(addr), *buf,
> +QCA988X_CAL_DATA_LEN);
> + if (ret) {
> + ath10k_warn(ar, "failed to read calibration data: 
> %d\n", ret);
> + }
> + }

We try to avoid using else branches so that only error handling is
indented and the main code flow is not.

> + /*
> +  * We are up now, so no need to cache calibration data.
> +  */
> + vfree(ar->cal_data);
> +ar->cal_data = NULL;

Indentation looks wrongs here.

> @@ -1757,6 +1799,11 @@ void ath10k_core_stop(struct ath10k *ar)
>   lockdep_assert_held(&ar->conf_mutex);
>   ath10k_debug_stop(ar);
>  
> + /*
> +  * Cache caclibration data while stopped.
> +  */
> + ath10k_cal_data_alloc(ar, &ar->cal_data);

I like the idea that the calibration data is copied during stop(), but
can you do this from debug.c? For example, add ath10k_debug_stop() and
call it from there? I don't think any of this should be in core.c.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: Fix spinlock use in coverage class hack

2016-09-30 Thread Valo, Kalle
Benjamin Berg  writes:

> ath10k_hw_qca988x_set_coverage_class needs to hold both conf_mutex and
> the data_lock spin lock for parts of the function. However, data_lock
> is only needed while storing the coverage_class to store the value that
> the card is configured to.
>
> Fix the locking issue by only holding data_lock for the required duration.
>
> Signed-off-by: Benjamin Berg 

Thanks, I also folded this with the patch in the pending branch.

> And yes, I fully agree with your points of it being rather fragile. But as
> you said, it should be entirely safe if not used.

That's good.

> Obviously a firmware implementation would be preferential.

That's a shame as this feature is quite often requested. But if the
firmware ever starts supporting the featrue we can then remove this hack
from ath10k.

> This locking issue was pretty unnecessary. Lets see if any more issues show
> up in a closer review.

I can't see the locking problem anymore so it seems to be fixed. I'll
fix some minor things and send v2. I'll also CC linux-wireless.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] ath10k: fix debug cal data file

2016-09-28 Thread Valo, Kalle
Nikolay Martynov  writes:

> It got broken by 0b8e3c4ca29fe2c0efd3d41a76e34a657b9f17a4
>
> Signed-off-by: Nikolay Martynov 

Good catch, I'll queue this to 4.9.

There was one checkpatch warning I fixed:

drivers/net/wireless/ath/ath10k/debug.c:1477: Prefer vmalloc(sizeof(*data)...) 
over vmalloc(sizeof(struct ath10k_debug_cal_data)...)

The commit log is quite short so added more information about the bug.
The full patch is in the pending branch.

Author: Nikolay Martynov 
Date:   Wed Sep 28 15:11:52 2016 +0300

ath10k: fix debug cal data file

Commit 0b8e3c4ca29f ("ath10k: move cal data len to hw_params") broke 
retrieving
the calibration data from cal_data debugfs file. The length of file was 
always
zero. The reason is:

static ssize_t ath10k_debug_cal_data_read(struct file *file,
  char __user *user_buf,
  size_t count, loff_t *ppos)
{
struct ath10k *ar = file->private_data;
void *buf = file->private_data;


This is obviously bogus, private_data cannot contain both struct ath10k and 
the
buffer. Fix it by introducing a new temporary structure for storing both the
length of the buffer and the actual buffer, then struct ath10k is not needed
anymore.

Fixes: 0b8e3c4ca29f ("ath10k: move cal data len to hw_params")
Cc: sta...@vger.kernel.org # 4.7+
Signed-off-by: Nikolay Martynov 
[kv...@qca.qualcomm.com: improve commit log, fix a checkpatch warning]
    Signed-off-by: Kalle Valo 

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA6174 Performance

2016-09-16 Thread Valo, Kalle
Hi,

please don't email me privately, I'm adding the mailing list back so
that everyone can participate.

Daniel Holz  writes:

> Do you mean by board file board.bin? I downloaded all files related to the
> QCA6174 from your Github repository.

I meant board-2.bin actually:

$ md5sum QCA6174/hw2.1/board-2.bin 
11920f7c30a1b1dd009928baef1021dc  QCA6174/hw2.1/board-2.bin

This file should support your board with subsystem id 17aa:3044.

> Here is the output of lspci:
>
> 02:00.0 Network controller [0280]: Qualcomm Atheros QCA6174 802.11ac Wireless
> Network Adapter [168c:003e] (rev 20)
> Subsystem: Lenovo QCA6174 802.11ac Wireless Network Adapter [17aa:3044]
> Flags: bus master, fast devsel, latency 0, IRQ 49
> Memory at c100 (64-bit, non-prefetchable) [size=2M]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable+ Count=8/8 Maskable+ 64bit-
> Capabilities: [70] Express Endpoint, MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [148] Virtual Channel
> Capabilities: [168] Device Serial Number 00-00-00-00-00-00-00-00
> Capabilities: [178] Latency Tolerance Reporting
> Capabilities: [180] L1 PM Substates
> Kernel driver in use: ath10k_pci
> Kernel modules: ath10k_pci
>
> And of dmesg | grep -i ath10k:
>
> [3.717494] ath10k_pci :02:00.0: pci irq msi-x interrupts 8 irq_mode 0
> reset_mode 0
> [3.953007] ath10k_pci :02:00.0: Direct firmware load for ath10k/
> cal-pci-:02:00.0.bin failed with error -2
> [5.232572] ath10k_pci :02:00.0: qca6174 hw2.1 (0x0501, 0x003405ff
> sub 17aa:3044) fw SW_RM.1.1.1-00157-QCARMSWPZ-1 fwapi 5 bdapi 2 htt-ver 3.1
> wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1 features
> ignore-otp,no-4addr-pad
> [5.232578] ath10k_pci :02:00.0: debug 0 debugfs 1 tracing 1 dfs 0
> testmode 0
> [5.318496] ath10k_pci :02:00.0 wlp2s0: renamed from wlan0
> [6.879214] ath10k_pci :02:00.0: no channel configured; ignoring frame
> (s)!

>From the logs I can't immeaditely see anything wrong, but you version of
ath10k is so old that it doesn't print the crc32 for the board file so I
can't be 100% sure. So I recommend to check the md5sum above matches
with your file.

>From a quick look I can't immediately come up what else would cause low
throughput, it would involve more detailed analysis with wireless
sniffer etc. It's a wild shot but you could always try latest ath10k
using kernel-backports:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/backports

Do note that backports project hasn't had stable updates for a long time
so use the daily releases instead:

https://www.kernel.org/pub/linux/kernel/projects/backports/2016/03/24/

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA6174 Performance

2016-09-15 Thread Valo, Kalle
Daniel Holz  writes:

> the network performance of the Qualcom QCA6174 in my Lenovo Yoga 3 11
> is only around 7-8 Mbyte/s in tests with iperf when using a wireless
> ac connection. Disabling power management resulted in 10 MByte/s.
> Under the same conditions my Android phone achieved 384 MByte/s.
>
> Currently I'm using Ubuntu 16.04 on my Yoga 3 11 with the most recent
> hw2.1 firmware and kernel 4.4. I already tried kernel 4.8 which
> boosted the speed to 15 MByte/s, but this is still way too slow for
> such a connection.

Might be a problem with the board file, are you using the latest one?
dmesg and 'lspci -vnn' outputs help to analyse the issue more.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 03/21] ath10k: Allow changing ath10k debug mask at runtime.

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:06 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> Using debugfs.  More convenient than module options
>>> in some cases.
>>>
>>> Signed-off-by: Ben Greear 
>>> ---
>>>   drivers/net/wireless/ath/ath10k/debug.c | 62 
>>> +
>>>   1 file changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
>>> b/drivers/net/wireless/ath/ath10k/debug.c
>>> index e251155..d552a4a 100644
>>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>>> @@ -870,6 +870,65 @@ static const struct file_operations fops_reg_addr = {
>>> .llseek = default_llseek,
>>>   };
>>>
>>> +static ssize_t ath10k_read_debug_level(struct file *file,
>>> +  char __user *user_buf,
>>> +  size_t count, loff_t *ppos)
>>> +{
>>> +   int sz;
>>> +   const char buf[] =
>>> +   "To change debug level, set value adding up desired flags:\n"
>>> +   "PCI:0x1\n"
>>> +   "WMI:0x2\n"
>>> +   "HTC:0x4\n"
>>> +   "HTT:0x8\n"
>>> +   "MAC:   0x10\n"
>>> +   "BOOT:  0x20\n"
>>> +   "PCI-DUMP:  0x40\n"
>>> +   "HTT-DUMP:  0x80\n"
>>> +   "MGMT: 0x100\n"
>>> +   "DATA: 0x200\n"
>>> +   "BMI:  0x400\n"
>>> +   "REGULATORY:   0x800\n"
>>> +   "TESTMODE:0x1000\n"
>>> +   "INFO-AS-DBG: 0x4000\n"
>>> +   "FW:  0x8000\n"
>>> +   "ALL: 0x\n";
>>> +   char wbuf[sizeof(buf) + 60];
>>> +   sz = snprintf(wbuf, sizeof(wbuf), "Current debug level: 0x%x\n\n%s",
>>> + ath10k_debug_mask, buf);
>>> +   wbuf[sizeof(wbuf) - 1] = 0;
>>> +
>>> +   return simple_read_from_buffer(user_buf, count, ppos, wbuf, sz);
>>> +}
>>> +
>>> +/* Set logging level.
>>> + */
>>> +static ssize_t ath10k_write_debug_level(struct file *file,
>>> +   const char __user *user_buf,
>>> +   size_t count, loff_t *ppos)
>>> +{
>>> +   struct ath10k *ar = file->private_data;
>>> +   int ret;
>>> +   unsigned long mask;
>>> +
>>> +   ret = kstrtoul_from_user(user_buf, count, 0, &mask);
>>> +   if (ret)
>>> +   return ret;
>>> +
>>> +   ath10k_warn(ar, "Setting debug-mask to: 0x%lx  old: 0x%x\n",
>>> +   mask, ath10k_debug_mask);
>>> +   ath10k_debug_mask = mask;
>>> +   return count;
>>> +}
>>
>> There are already sysfs files for module parameters which seems to work
>> just fine for this case:
>>
>> # echo 0x > /sys/module/ath10k_core/parameters/debug_mask
>
>
> Ok, but it is still nice to have the printout info of what log levels
> means.  Otherwise, you have to go look at firmware source to even know how
> to enable the proper flags.  And as these flags are internal and might change,
> we could change the printout text to match the specific kernel that is 
> running.

The debug log levels are documented in the wiki:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug#debug_log_messages

And they are not supposed to change, there should be only additions.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 15/21] ath10k: support CT firmware flag.

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:30 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> Add placeholder so CT firmware can more easily co-exist with upstream
>>> kernel.  CT firmware should be backwards compatible with existing kernels,
>>> but it also has many new features.  Subsequent patches, if acceptable for
>>> upstream, can enable and further describe those features.
>>>
>>> Signed-off-by: Ben Greear 
>>> ---
>>>   drivers/net/wireless/ath/ath10k/core.c | 1 +
>>>   drivers/net/wireless/ath/ath10k/core.h | 3 +++
>>>   2 files changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
>>> b/drivers/net/wireless/ath/ath10k/core.c
>>> index fa71d57..49c85c3 100644
>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -235,6 +235,7 @@ static const char *const ath10k_core_fw_feature_str[] = 
>>> {
>>> [ATH10K_FW_FEATURE_SUPPORTS_ADAPTIVE_CCA] = "adaptive-cca",
>>> [ATH10K_FW_FEATURE_MFP_SUPPORT] = "mfp",
>>> [ATH10K_FW_FEATURE_PEER_FLOW_CONTROL] = "peer-flow-ctrl",
>>> +   [ATH10K_FW_FEATURE_WMI_10X_CT] = "wmi-10.x-CT",
>>>   };
>>>
>>>   static unsigned int ath10k_core_get_fw_feature_str(char *buf,
>>> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
>>> b/drivers/net/wireless/ath/ath10k/core.h
>>> index cb6aa8c..f9e3b20 100644
>>> --- a/drivers/net/wireless/ath/ath10k/core.h
>>> +++ b/drivers/net/wireless/ath/ath10k/core.h
>>> @@ -566,6 +566,9 @@ enum ath10k_fw_features {
>>>  */
>>> ATH10K_FW_FEATURE_PEER_FLOW_CONTROL = 13,
>>>
>>> +   /* Firmware from Candela Technologies, enables more VIFs, etc */
>>> +   ATH10K_FW_FEATURE_WMI_10X_CT = 31,
>>
>> The idea of firmware feature flags to enable (or disable) one particular
>> feature, not a group of features. This way it's easy to enable certain
>> features on different firmware branches. It also makes the maintenance
>> easier as you don't need to remember all the different features one flag
>> enables.
>
> One potential use for this flag:  Bug reports could be automatically directed 
> to me instead
> of QCA.
>
> I have actually re-written much of my earlier logic so that it uses specific
> feature flags now.  I still like the idea of having the driver (and user)
> know it is using CT firmware, but I can live without this flag if needed.

You can do with firmware version string, add something like "-CT" (maybe
you already have?) and it's clearly visible in boot logs and also when
firmware crashes.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 10/21] ath10k: support logging ath10k_info as KERN_DEBUG

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:19 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> Helps keep messages off of (serial) console when
>>> that is desired.
>>>
>>> Signed-off-by: Ben Greear 
>>
>> Isn't /proc/sys/kernel/print exactly for this purpose? At least I recall
>> using it.
>
> I just wanted to hide some ath10k logs from the console, not all
> system logs. I don't think that /proc/sys/kernel/print has any
> granularity?

It should be based on KERN_ log levels. I don't know what your kernel
does, but ath10k should be printing only very few messages with level
KERN_INFO or above, all of of the debug messages. So you should be
easily able to filter out all ath10k debug messages as they are sent
with KERN_DEBUG.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 09/21] ath10k: print fw debug messages in hex.

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:18 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> This allows user-space tools to decode debug-log
>>> messages by parsing dmesg or /var/log/messages.
>>>
>>> Signed-off-by: Ben Greear 
>>
>> Don't tracing points already provide the same information?
>
> Tracing tools are difficult to set up and may not be available on
> random embedded devices.  And if we are dealing with bug reports from
> the field, most users will not be able to set it up regardless.
>
> There are similar ways to print out hex, but the logic below creates
> specific and parseable logs in the 'dmesg' output and similar.
>
> I have written a tool that can decode these messages into useful 
> human-readable
> text so that I can debug firmware issues both locally and from field reports.
>
> Stock firmware generates similar logs and QCA could write their own decode 
> logic
> for their firmware versions.

Reinventing the wheel by using printk as the delivery mechanism doesn't
sound like a good idea. IIRC Emmanuel talked about some kind of firmware
debugging framework, he might have some ideas.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 08/21] ath10k: make firmware text debug messages more verbose.

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:12 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> There are not many of these messages producted by the
>>> firmware, but they are generally fairly useful, so print
>>> them at info level.
>>>
>>> Signed-off-by: Ben Greear 
>>> ---
>>>   drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
>>> b/drivers/net/wireless/ath/ath10k/wmi.c
>>> index 1758b4a..d9e4b77 100644
>>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>>> @@ -4050,7 +4050,7 @@ void ath10k_wmi_event_debug_print(struct ath10k *ar, 
>>> struct sk_buff *skb)
>>> /* the last byte is always reserved for the null character */
>>> buf[i] = '\0';
>>>
>>> -   ath10k_dbg(ar, ATH10K_DBG_WMI_PRINT, "wmi print '%s'\n", buf);
>>> +   ath10k_info(ar, "wmi print '%s'\n", buf);
>>
>> Useful to whom and how? I understand that for firmware developers this
>> is very valuable information and that's why we have a special debug
>> level for it. But I suspect that for normal users these are just
>> confusing and unnecessarily spam the log.
>
> CT firmare will print out some memory usage info on firmware boot, and that 
> can
> allow a discerning individual to tune their vdev, peer, rate-ctrl, and other
> object usage in order to make best use of resources in the firmware.
>
> These few lines can greatly aid debugging certain types of crashes and 
> performance
> loss in the firmware, so having them readily available in 'dmesg' or similar
> for bug reports from the field helps me.
>
> Stock firmware will also print out some resource usage info.  It is just
> a few lines on firmware boot, but it is quite useful in my experience.

I'm sure it's useful for you, but we have quite a few firmware versions
to support. We do not know what kind of messages they print.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 04/21] ath10k: rate-limit packet tx errors

2016-09-15 Thread Valo, Kalle
Ben Greear  writes:

> On 09/14/2016 07:07 AM, Valo, Kalle wrote:
>> gree...@candelatech.com writes:
>>
>>> From: Ben Greear 
>>>
>>> When firmware crashes, stack can continue to send packets
>>> for a bit, and existing code was spamming logs.
>>>
>>> So, rate-limit the error message for tx failures.
>>>
>>> Signed-off-by: Ben Greear 
>>> ---
>>>   drivers/net/wireless/ath/ath10k/mac.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
>>> b/drivers/net/wireless/ath/ath10k/mac.c
>>> index cd3016d..42cac32 100644
>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>> @@ -3432,8 +3432,9 @@ static int ath10k_mac_tx_submit(struct ath10k *ar,
>>> }
>>>
>>> if (ret) {
>>> -   ath10k_warn(ar, "failed to transmit packet, dropping: %d\n",
>>> -   ret);
>>> +   if (net_ratelimit())
>>> +   ath10k_warn(ar, "failed to transmit packet, dropping: 
>>> %d\n",
>>> +   ret);
>>> ieee80211_free_txskb(ar->hw, skb);
>>> }
>>
>> ath10k_warn() is already rate limited. If there's something wrong then
>> that function should be fixed, not the callers.
>>
>> void ath10k_warn(struct ath10k *ar, const char *fmt, ...)
>> {
>>  struct va_format vaf = {
>>  .fmt = fmt,
>>  };
>>  va_list args;
>>
>>  va_start(args, fmt);
>>  vaf.va = &args;
>>  dev_warn_ratelimited(ar->dev, "%pV", &vaf);
>>  trace_ath10k_log_warn(ar, &vaf);
>>
>>  va_end(args);
>> }
>
> The problem with having the ratelimit here is that you may miss
> rare warnings due to a flood of common warnings.
>
> That is why it is still useful to ratelimit potential floods
> of warnings.

I think this is a common problem in kernel, not specific to ath10k. For
starters you could configure the limits dev_warn_ratelimited() has, not
trying to workaround it in the driver.

> I would like to remove the ratelimit from ath10k_warn eventually.

I think that's not a good idea, it might cause unnecessary host reboots
in problem cases. Rate limitting the messages is much better option.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 12/21] ath10k: Support up to 64 vdevs.

2016-09-14 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> The (1 << x) - 1 trick won't work when you
> are trying to fill up all 64 bits, so add special
> case for that.
>
> And, move the limits to the per-nic structure instead
> of per-driver to allow better dynamic use of the limits.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
> b/drivers/net/wireless/ath/ath10k/core.c
> index 3f1786c..fa71d57 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -1819,7 +1819,10 @@ int ath10k_core_start(struct ath10k *ar, enum 
> ath10k_firmware_mode mode,
>   if (status)
>   goto err_hif_stop;
>  
> - ar->free_vdev_map = (1LL << ar->max_num_vdevs) - 1;
> + if (ar->max_num_vdevs >= 64)
> + ar->free_vdev_map = 0xLL;
> + else
> + ar->free_vdev_map = (1LL << ar->max_num_vdevs) - 1;

The last sentence in the commit log doesn't match the code, I removed
that in the pending branch.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: QCA9984 VHT160 support

2016-09-14 Thread Valo, Kalle
Sebastian Gottschall  writes:

> Am 14.09.2016 um 12:37 schrieb Valo, Kalle:
>> Sebastian Gottschall  writes:
>>
>>> Attached you will find a testing patch for VHT160 support. i tested it
>>> today on a QCA9984 device and it seems to work.
>>> feel free to make any corrections
>> Signed-off-by is missing so I can't take this. Can you resend, please
>
> i can send you a updated version. it tested with vht160 so far.

Great, thanks.

> however. on codeaurore there is a newer firmware for 9984 which
> provides the correct vht flags straight from the firmware but its
> crashing in vht160 mode. can you ask the firmware team to provide a
> bugfixed version for the official ath10k? with 80p80 and vht160
> support? 80p80 works somewhat bug has wrong vht flags per station,
> vht160 works with my quirks so far which are all related to the buggy
> firmware

I can ask but I need more info. What's the exact location of the newer
firmware image? Is there any detailed description about the wrong vht
flags bug?

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 19/21] ath10k: Enable adhoc mode for CT firmware.

2016-09-14 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> CT firmware can support IBSS mode, so allow users to configure this.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/mac.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
> b/drivers/net/wireless/ath/ath10k/mac.c
> index f1bfb3a..3fc9006 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -7482,6 +7482,10 @@ static const struct ieee80211_iface_limit 
> ath10k_10x_ct_if_limits[] = {
>   .max= 7,
>   .types  = BIT(NL80211_IFTYPE_AP)
>   },
> + {
> + .max= 1,
> + .types  = BIT(NL80211_IFTYPE_ADHOC)
> + },
>  };
>  
>  static const struct ieee80211_iface_combination ath10k_if_comb[] = {
> @@ -7862,6 +7866,7 @@ int ath10k_mac_register(struct ath10k *ar)
>   ar->hw->wiphy->iface_combinations = 
> ath10k_10x_ct_if_comb;
>   ar->hw->wiphy->n_iface_combinations =
>   ARRAY_SIZE(ath10k_10x_ct_if_comb);
> + ar->hw->wiphy->interface_modes |= 
> BIT(NL80211_IFTYPE_ADHOC);
>   } else {
>   ar->hw->wiphy->iface_combinations = ath10k_10x_if_comb;
>   ar->hw->wiphy->n_iface_combinations =

There should a feature flag ATH10K_FW_FEATURE_SUPPORTS_ADHOC and we use
that flag as an indication to enable the mode. I wish we had done that
from the beginning, using wmi_op_version to guess that just creates
problems :(

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 15/21] ath10k: support CT firmware flag.

2016-09-14 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> Add placeholder so CT firmware can more easily co-exist with upstream
> kernel.  CT firmware should be backwards compatible with existing kernels,
> but it also has many new features.  Subsequent patches, if acceptable for
> upstream, can enable and further describe those features.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 1 +
>  drivers/net/wireless/ath/ath10k/core.h | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
> b/drivers/net/wireless/ath/ath10k/core.c
> index fa71d57..49c85c3 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -235,6 +235,7 @@ static const char *const ath10k_core_fw_feature_str[] = {
>   [ATH10K_FW_FEATURE_SUPPORTS_ADAPTIVE_CCA] = "adaptive-cca",
>   [ATH10K_FW_FEATURE_MFP_SUPPORT] = "mfp",
>   [ATH10K_FW_FEATURE_PEER_FLOW_CONTROL] = "peer-flow-ctrl",
> + [ATH10K_FW_FEATURE_WMI_10X_CT] = "wmi-10.x-CT",
>  };
>  
>  static unsigned int ath10k_core_get_fw_feature_str(char *buf,
> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> b/drivers/net/wireless/ath/ath10k/core.h
> index cb6aa8c..f9e3b20 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -566,6 +566,9 @@ enum ath10k_fw_features {
>*/
>   ATH10K_FW_FEATURE_PEER_FLOW_CONTROL = 13,
>  
> + /* Firmware from Candela Technologies, enables more VIFs, etc */
> + ATH10K_FW_FEATURE_WMI_10X_CT = 31,

The idea of firmware feature flags to enable (or disable) one particular
feature, not a group of features. This way it's easy to enable certain
features on different firmware branches. It also makes the maintenance
easier as you don't need to remember all the different features one flag
enables.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 11/21] ath10k: add fw-powerup-fail to ethtool stats.

2016-09-14 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> This gives user-space a normal-ish way to detect that
> firmware has failed to start and that a reboot is
> probably required.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/core.h  | 1 +
>  drivers/net/wireless/ath/ath10k/debug.c | 2 ++
>  drivers/net/wireless/ath/ath10k/pci.c   | 2 ++
>  3 files changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> b/drivers/net/wireless/ath/ath10k/core.h
> index 6aa7a14..e7c228a 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -698,6 +698,7 @@ struct ath10k {
>  
>   enum ath10k_hw_rev hw_rev;
>   u16 dev_id;
> + bool fw_powerup_failed; /* If true, might take reboot to recover. */
>   u32 chip_id;
>   u32 target_version;
>   u8 fw_version_major;
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
> b/drivers/net/wireless/ath/ath10k/debug.c
> index 76b5163..05b5ea4 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -1541,6 +1541,7 @@ static const char 
> ath10k_gstrings_stats[][ETH_GSTRING_LEN] = {
>   "d_fw_crash_count",
>   "d_fw_warm_reset_count",
>   "d_fw_cold_reset_count",
> + "d_fw_powerup_failed", /* boolean */
>  };
>  
>  #define ATH10K_SSTATS_LEN ARRAY_SIZE(ath10k_gstrings_stats)
> @@ -1640,6 +1641,7 @@ void ath10k_debug_get_et_stats(struct ieee80211_hw *hw,
>   data[i++] = ar->stats.fw_crash_counter;
>   data[i++] = ar->stats.fw_warm_reset_counter;
>   data[i++] = ar->stats.fw_cold_reset_counter;
> + data[i++] = ar->fw_powerup_failed;
>  
>   spin_unlock_bh(&ar->data_lock);
>  
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c 
> b/drivers/net/wireless/ath/ath10k/pci.c
> index dbf0db8..2adc459 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -2709,10 +2709,12 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
>   goto err_ce;
>   }
>  
> + ar->fw_powerup_failed = false;
>   return 0;
>  
>  err_ce:
>   ath10k_pci_ce_deinit(ar);
> + ar->fw_powerup_failed = true;
>  
>  err_sleep:
>   return ret;

I didn't test myself, but if the firmware boot fails during module load
we should return an error and the module would fail to load (and hence
no network interface available). And if the firmware boot fails during
the interface up call (mac80211 calling ath10k_start()) we should return
an error and the interface would stay down. IMHO that's enough of
indications to the user space, I don't see how providing this boolean
via ethtool stats is really needed.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH v2 10/21] ath10k: support logging ath10k_info as KERN_DEBUG

2016-09-14 Thread Valo, Kalle
gree...@candelatech.com writes:

> From: Ben Greear 
>
> Helps keep messages off of (serial) console when
> that is desired.
>
> Signed-off-by: Ben Greear 

Isn't /proc/sys/kernel/print exactly for this purpose? At least I recall
using it.

-- 
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


  1   2   3   >