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

[Valo, Kalle]

Ben Greear <gree...@candelatech.com> writes:

> On 02/07/2017 01:14 AM, Valo, Kalle wrote:
>> Adrian Chadd <adr...@freebsd.org> 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

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

[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

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

[Valo, Kalle]

Joe Perches <j...@perches.com> writes:

> On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
>> Joe Perches <j...@perches.com> 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,
>> > > _data_base,
>> > > GFP_ATOMIC);
>> > 
>> > trivia:
>> > 
>> > Nicer to realign arguments and remove the unnecessary cast.
>> > 
>> > Perhaps:
>> > 
>> >data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, _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

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

[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,
>> _data_base,
>> GFP_ATOMIC);
>
> trivia:
>
> Nicer to realign arguments and remove the unnecessary cast.
>
> Perhaps:
>
>   data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, _data_base,
>  GFP_ATOMIC);

Sure, but that should be in a separate patch.

-- 
Kalle Valo

Re: regression: ath_tx_edma_tasklet() Illegal idle entry in RCU read-side critical section

[Valo, Kalle]

Gabriel C <nix.or@gmail.com> writes:

> On 18.12.2016 21:14, Paul E. McKenney wrote:
>> On Sun, Dec 18, 2016 at 07:57:42PM +0000, Valo, Kalle wrote:
>>> Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de> writes:
>>>
>>>> A patch for this is already floating on the ML for a while now latest:
>>>> (ath9k: do not return early to fix rcu unlocking)
>>>
>>> It's here:
>>>
>>> https://patchwork.kernel.org/patch/9472709/
>>
>> Feel free to add:
>>
>> Acked-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
>>
>
> This patch fixes the bug for me.
>
> You can add my Tested-by if you wish.
>
> Tested-by: Gabriel Craciunescu <nix.or@gmail.com>

I added both of these to the commit log, thanks.

-- 
Kalle Valo

Re: regression: ath_tx_edma_tasklet() Illegal idle entry in RCU read-side critical section

[Valo, Kalle]

Tobias Klausmann  writes:

> A patch for this is already floating on the ML for a while now latest:
> (ath9k: do not return early to fix rcu unlocking)

It's here:

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

> Hopefully Kalle will include it in one of his upcoming pull requests.

Yes, I'll try to get it to 4.10-rc2.

-- 
Kalle Valo

Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()

[Valo, Kalle]

Hi Dave,

Andy Gross <andy.gr...@linaro.org> writes:

> On 1 December 2016 at 04:17, Valo, Kalle <kv...@qca.qualcomm.com> wrote:
>> Kalle Valo <kv...@qca.qualcomm.com> writes:
>>
>>> It found the same problem. Interestingly I'm also building x86 with 32
>>> bit, maybe it's related?
>>>
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git 
>>> pending
>>> head:   1ea16a1c457939b4564643f7637d5cc639a8d3b7
>>> commit: 5eb09c672b01460804fd49b1c9cc7d1072a102f0 [96/99] wcn36xx: 
>>> Transition driver to SMD client
>>> config: i386-allmodconfig (attached as .config)
>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>> reproduce:
>>> git checkout 5eb09c672b01460804fd49b1c9cc7d1072a102f0
>>> # save the attached .config to linux build tree
>>> make ARCH=i386
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>>> ERROR: "qcom_wcnss_open_channel" 
>>>>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>>
>> Bjorn mentioned me on IRC that this is because of a missing commit in my
>> tree:
>>
>> daa6e41ce2b5 soc: qcom: wcnss_ctrl: Stub wcnss_ctrl API
>>
>> When I pull the tag below (which contains the above commit) wcn36xx
>> builds fine for me:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git 
>> tags/qcom-drivers-for-4.10
>>
>> Andy, is it ok if I pull your tag also to my ath.git tree to solve the
>> wcn36xx build problem? My trees go to Linus via net-next and I don't
>> know when exactly Dave would send a pull request to Linus, before or
>> after the arm trees, but as the tag seems to contain only few patches I
>> hope it doesn't matter.
>
> The qcom-drivers-for-4.10 tag was already merged into arm-soc.  But
> having you pull it as well won't cause issues so long as you are using
> the tag (which you are).  I don't see any issues with this approach.

Andy, thanks for the confirmation.

Dave, how do you suggest to handle depency issues like this? I have
pending important wcn36xx patches (converting the driver to use the
recently introduced proper SMD subsystem) which have a build dependency
on a commit which is in Andy's tag qcom-drivers-for-4.10. The commit in
question is currently in arm-soc tree going to 4.10, but not in your
net-next tree. I assume Linus will pull that during the next merge
window.

What I'm planning to do is to pull tag qcom-drivers-for-4.10 to my tree
and then send the patches to you. This will mean that from my pull
request you would get four new qcom-drivers commits which are not in
your tree, yet. Or do you prefer that I wait the qcom-drivers commits
trickle down from Linux until I send you wcn36xx patches? Or something
else?

$ git pull git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git 
tags/qcom-drivers-for-4.10
>From git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux
 * tag qcom-drivers-for-4.10 -> FETCH_HEAD
Auto-merging MAINTAINERS
Merge made by the 'recursive' strategy.
 MAINTAINERS  |1 +
 drivers/firmware/qcom_scm.c  |4 +++-
 include/dt-bindings/pinctrl/qcom,pmic-gpio.h |4 
 include/dt-bindings/pinctrl/qcom,pmic-mpp.h  |6 ++
 include/linux/soc/qcom/wcnss_ctrl.h  |   13 +
 5 files changed, 27 insertions(+), 1 deletion(-)

$ git log --oneline ORIG_HEAD..
6d0491261ecc Merge tag 'qcom-drivers-for-4.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux into test
bd4760ca0315 firmware: qcom: scm: Use devm_reset_controller_register()
4fb1a4207804 MAINTAINERS: add drivers/pinctrl/qcom to ARM/QUALCOMM SUPPORT
636959fc1232 pinctrl: pm8994: add pad voltage regulator defines
daa6e41ce2b5 soc: qcom: wcnss_ctrl: Stub wcnss_ctrl API
$

-- 
Kalle Valo

Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()

[Valo, Kalle]

Kalle Valo <kv...@qca.qualcomm.com> writes:

> Kalle Valo <kv...@qca.qualcomm.com> writes:
>
>> "Valo, Kalle" <kv...@qca.qualcomm.com> writes:
>>
>>> Bjorn Andersson <bjorn.anders...@linaro.org> writes:
>>>
>>>> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>>>>
>>>>> Bjorn Andersson <bjorn.anders...@linaro.org> wrote:
>>>>> > The correct include file for getting errno constants and ERR_PTR() is
>>>>> > linux/err.h, rather than linux/errno.h, so fix the include.
>>>>> > 
>>>>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled 
>>>>> > smem_state")
>>>>> > Acked-by: Andy Gross <andy.gr...@linaro.org>
>>>>> > Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
>>>>> 
>>>>> For some reason this fails to compile now. Can you take a look, please?
>>>>> 
>>>>> ERROR: "qcom_wcnss_open_channel" 
>>>>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>>>>> make[1]: *** [__modpost] Error 1
>>>>> make: *** [modules] Error 2
>>>>> 
>>>>> 5 patches set to Changes Requested.
>>>>> 
>>>>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>>>>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>>>>
>>>> This patch was updated with the necessary depends in Kconfig to catch
>>>> this exact issue and when I pull in your .config (which has QCOM_SMD=n,
>>>> QCOM_WCNSS_CTRL=n and WCN36XX=y) I can build this just fine.
>>>>
>>>> I've tested the various combinations and it seems to work fine. Do you
>>>> have any other patches in your tree?
>>>
>>> This was with the pending branch of my ath.git tree. There are other
>>> wireless patches (ath10k etc) but I would guess they don't affect here.
>>>
>>>> Any stale objects?
>>>
>>> Not sure what you mean with this question, but I didn't run 'make clean'
>>> if that's what you are asking.
>>>
>>>> Would you mind retesting this, before I invest more time in trying to
>>>> reproduce the issue you're seeing?
>>>
>>> Sure, I'll take a look but that might take few days.
>>
>> I didn't find enough time to look at this in detail. I applied this to
>> my ath.git pending branch, let's see what the kbuild bot finds.
>
> It found the same problem. Interestingly I'm also building x86 with 32
> bit, maybe it's related?
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending
> head:   1ea16a1c457939b4564643f7637d5cc639a8d3b7
> commit: 5eb09c672b01460804fd49b1c9cc7d1072a102f0 [96/99] wcn36xx: Transition 
> driver to SMD client
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> git checkout 5eb09c672b01460804fd49b1c9cc7d1072a102f0
> # save the attached .config to linux build tree
> make ARCH=i386 
>
> All errors (new ones prefixed by >>):
>
>>> ERROR: "qcom_wcnss_open_channel" 
>>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!

Bjorn mentioned me on IRC that this is because of a missing commit in my
tree:

daa6e41ce2b5 soc: qcom: wcnss_ctrl: Stub wcnss_ctrl API

When I pull the tag below (which contains the above commit) wcn36xx
builds fine for me:

git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git 
tags/qcom-drivers-for-4.10

Andy, is it ok if I pull your tag also to my ath.git tree to solve the
wcn36xx build problem? My trees go to Linus via net-next and I don't
know when exactly Dave would send a pull request to Linus, before or
after the arm trees, but as the tag seems to contain only few patches I
hope it doesn't matter.

-- 
Kalle Valo

Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()

[Valo, Kalle]

Kalle Valo <kv...@qca.qualcomm.com> writes:

> "Valo, Kalle" <kv...@qca.qualcomm.com> writes:
>
>> Bjorn Andersson <bjorn.anders...@linaro.org> writes:
>>
>>> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>>>
>>>> Bjorn Andersson <bjorn.anders...@linaro.org> wrote:
>>>> > The correct include file for getting errno constants and ERR_PTR() is
>>>> > linux/err.h, rather than linux/errno.h, so fix the include.
>>>> > 
>>>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled 
>>>> > smem_state")
>>>> > Acked-by: Andy Gross <andy.gr...@linaro.org>
>>>> > Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
>>>> 
>>>> For some reason this fails to compile now. Can you take a look, please?
>>>> 
>>>> ERROR: "qcom_wcnss_open_channel" 
>>>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>>>> make[1]: *** [__modpost] Error 1
>>>> make: *** [modules] Error 2
>>>> 
>>>> 5 patches set to Changes Requested.
>>>> 
>>>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>>>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>>>
>>> This patch was updated with the necessary depends in Kconfig to catch
>>> this exact issue and when I pull in your .config (which has QCOM_SMD=n,
>>> QCOM_WCNSS_CTRL=n and WCN36XX=y) I can build this just fine.
>>>
>>> I've tested the various combinations and it seems to work fine. Do you
>>> have any other patches in your tree?
>>
>> This was with the pending branch of my ath.git tree. There are other
>> wireless patches (ath10k etc) but I would guess they don't affect here.
>>
>>> Any stale objects?
>>
>> Not sure what you mean with this question, but I didn't run 'make clean'
>> if that's what you are asking.
>>
>>> Would you mind retesting this, before I invest more time in trying to
>>> reproduce the issue you're seeing?
>>
>> Sure, I'll take a look but that might take few days.
>
> I didn't find enough time to look at this in detail. I applied this to
> my ath.git pending branch, let's see what the kbuild bot finds.

It found the same problem. Interestingly I'm also building x86 with 32
bit, maybe it's related?

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending
head:   1ea16a1c457939b4564643f7637d5cc639a8d3b7
commit: 5eb09c672b01460804fd49b1c9cc7d1072a102f0 [96/99] wcn36xx: Transition 
driver to SMD client
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 5eb09c672b01460804fd49b1c9cc7d1072a102f0
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "qcom_wcnss_open_channel" 
>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!

-- 
Kalle Valo

Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()

[Valo, Kalle]

"Valo, Kalle" <kv...@qca.qualcomm.com> writes:

> Bjorn Andersson <bjorn.anders...@linaro.org> writes:
>
>> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>>
>>> Bjorn Andersson <bjorn.anders...@linaro.org> wrote:
>>> > The correct include file for getting errno constants and ERR_PTR() is
>>> > linux/err.h, rather than linux/errno.h, so fix the include.
>>> > 
>>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled 
>>> > smem_state")
>>> > Acked-by: Andy Gross <andy.gr...@linaro.org>
>>> > Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
>>> 
>>> For some reason this fails to compile now. Can you take a look, please?
>>> 
>>> ERROR: "qcom_wcnss_open_channel" 
>>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>>> make[1]: *** [__modpost] Error 1
>>> make: *** [modules] Error 2
>>> 
>>> 5 patches set to Changes Requested.
>>> 
>>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>>
>> This patch was updated with the necessary depends in Kconfig to catch
>> this exact issue and when I pull in your .config (which has QCOM_SMD=n,
>> QCOM_WCNSS_CTRL=n and WCN36XX=y) I can build this just fine.
>>
>> I've tested the various combinations and it seems to work fine. Do you
>> have any other patches in your tree?
>
> This was with the pending branch of my ath.git tree. There are other
> wireless patches (ath10k etc) but I would guess they don't affect here.
>
>> Any stale objects?
>
> Not sure what you mean with this question, but I didn't run 'make clean'
> if that's what you are asking.
>
>> Would you mind retesting this, before I invest more time in trying to
>> reproduce the issue you're seeing?
>
> Sure, I'll take a look but that might take few days.

I didn't find enough time to look at this in detail. I applied this to
my ath.git pending branch, let's see what the kbuild bot finds.

-- 
Kalle Valo

Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()

[Valo, Kalle]

Bjorn Andersson  writes:

> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>
>> Bjorn Andersson  wrote:
>> > The correct include file for getting errno constants and ERR_PTR() is
>> > linux/err.h, rather than linux/errno.h, so fix the include.
>> > 
>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled 
>> > smem_state")
>> > Acked-by: Andy Gross 
>> > Signed-off-by: Bjorn Andersson 
>> 
>> For some reason this fails to compile now. Can you take a look, please?
>> 
>> ERROR: "qcom_wcnss_open_channel" 
>> [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>> make[1]: *** [__modpost] Error 1
>> make: *** [modules] Error 2
>> 
>> 5 patches set to Changes Requested.
>> 
>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>
> This patch was updated with the necessary depends in Kconfig to catch
> this exact issue and when I pull in your .config (which has QCOM_SMD=n,
> QCOM_WCNSS_CTRL=n and WCN36XX=y) I can build this just fine.
>
> I've tested the various combinations and it seems to work fine. Do you
> have any other patches in your tree?

This was with the pending branch of my ath.git tree. There are other
wireless patches (ath10k etc) but I would guess they don't affect here.

> Any stale objects?

Not sure what you mean with this question, but I didn't run 'make clean'
if that's what you are asking.

> Would you mind retesting this, before I invest more time in trying to
> reproduce the issue you're seeing?

Sure, I'll take a look but that might take few days.

-- 
Kalle Valo

Re: [PATCH] ath10k: Spelling and miscellaneous neatening

[Valo, Kalle]

Joe Perches  writes:

> Correct some trivial comment typos.
> Remove unnecessary parentheses in a long line.
> Convert a return; before the end of a void function definition to just ;
>
> Signed-off-by: Joe Perches 

[...]

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2118,7 +2118,7 @@ err:
>   /* TODO: It's probably a good idea to release device from the driver
>* but calling device_release_driver() here will cause a deadlock.
>*/
> - return;
> + ;
>  }

I don't think this improves anything, I dropped this part from the patch
in my pending branch.

-- 
Kalle Valo

Re: [PATCH] ath10k: fix memory leak on caldata on error exit path

[Valo, Kalle]

Colin King  writes:

> From: Colin Ian King 
>
> caldata is not being free'd on the error exit path, causing
> a memory leak. kfree it to fix the leak.
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/wireless/ath/ath10k/pci.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c 
> b/drivers/net/wireless/ath/ath10k/pci.c
> index 9a22c47..886337c 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -2725,6 +2725,7 @@ static int ath10k_pci_hif_fetch_cal_eeprom(struct 
> ath10k *ar, void **data,
>   return 0;
>  
>  err_free:
> + kfree(caldata);
>   kfree(data);
>  
>   return -EINVAL;

I don't think we should free data at all:

static int ath10k_download_cal_eeprom(struct ath10k *ar)
{
size_t data_len;
void *data = NULL;
int ret;

ret = ath10k_hif_fetch_cal_eeprom(ar, , _len);

Instead we should free only caldata, right?

-- 
Kalle Valo

Re: [PATCH 1/2] ath10k: remove unused

[Valo, Kalle]

Sergei Shtylyov  writes:

> On 6/21/2016 2:45 PM, Chaehyun Lim wrote:
>
>>  is not used anymore, so just remove it.
>
>s/it/#include/?

Fixed that in the pending branch.

-- 
Kalle Valo

Re: [PATCH V2 2/2] ath6kl: replace semaphore with mutex

[Valo, Kalle]

(Adding ath6k list)

Chaehyun Lim  writes:

> It replaces struct semaphore sem with struct mutex mutex
>
> Reported-by: kbuild test robot 
> Signed-off-by: Chaehyun Lim 
> ---
> V2: fix build failure reported by kbuild test robot

I don't think Reported-by is correct, it should be only used when
reporting the original issue

Have you tested this on a real device? I'm reluctant to apply this big
locking change without some level of testing.

-- 
Kalle Valo

Re: linux-4.7-rc2/drivers/net/wireless/ath/ath6kl/wmi.c:2547]: (style) Redundant condition

[Valo, Kalle]

David Binderman  writes:

> Hello there,
>
> linux-4.7-rc2/drivers/net/wireless/ath/ath6kl/wmi.c:2547]: (style)
> Redundant condition: If 'EXPR <= 7', the comparison 'EXPR < 8' is
> always true.
>
> Source code is
>
> if (!((params->user_pri < 8) &&
>   (params->user_pri <= 0x7) &&
>
> This might be a possible cut'n'paste error.

This should be recently fixed:

https://git.kernel.org/cgit/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next=93b4a09f0f3068e3190548393f39262e5295960e

> Also in the same file
>
> [drivers/net/wireless/ath/ath6kl/wmi.c:1220]: (style) Variable 'rate'
> is assigned a value that is never used.
> [drivers/net/wireless/ath/ath6kl/wmi.c:1484]: (style) Variable
> 'new_threshold' is assigned a value that is never used.
> [drivers/net/wireless/ath/ath6kl/wmi.c:3521]: (style) Variable 'ret'
> is assigned a value that is never used.

I guess these are still valid.

-- 
Kalle Valo

Re: [PATCH] ath10k: fix erroneous return value

[Valo, Kalle]

Anton Protopopov  writes:

> The ath10k_pci_hif_exchange_bmi_msg() function may return the positive
> value EIO instead of -EIO in case of error.
>
> Signed-off-by: Anton Protopopov 

Applied, thanks.

-- 
Kalle Valo

Re: [PATCH] ath10k: remove impossible code

[Valo, Kalle]

Sudip Mukherjee  writes:

> From: Sudip Mukherjee 
>
> len has been initialized with a value of 0 and buf_len with 4096. There
> is no way that this condition (len > buf_len) can be true now.
>
> Signed-off-by: Sudip Mukherjee 

Applied, thanks.

-- 
Kalle Valo

Re: [PATCH] ath9k: reduce stack usage in ar9003_aic_cal_post_process

[Valo, Kalle]

Arnd Bergmann  writes:

> In some configurations, this function uses more than the warning limit
> of 1024 bytes:
>
> drivers/net/wireless/ath/ath9k/ar9003_aic.c: In function 
> 'ar9003_aic_cal_post_process':
> drivers/net/wireless/ath/ath9k/ar9003_aic.c:434:1: error: the frame size of 
> 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> It turns out that there are two large arrays on the stack here, but
> almost all the data in them is never used outside of the loop in
> which it gets written, so we can replace the array with a single
> instance.
>
> The .valid flag is used later, so I'm replacing the array of structures
> with an array of bools. An obvious follow-up optimization would be
> to replace it with a bitmask and set_bit()/find_first_bit()/
> find_last_bit()/... operations. However, I have not tested this patch,
> so I sticked to the simpler transformation that does the job of
> reducing the stack usage to a harmless level.
>
> Signed-off-by: Arnd Bergmann 

Applied to ath.git, thanks.

-- 
Kalle Valo