[PATCH 1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT

[PATCH 1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT

[Srinivas Kandagatla]

DIAG_TRANSFER_LIMIT is redefined with same value and comments
just below this entry, remove this duplicate entry.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/net/wireless/ath/ath10k/pci.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 9854ad5..c76789d 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -25,11 +25,6 @@
 #include "ahb.h"
 
 /*
- * maximum number of bytes that can be handled atomically by DiagRead/DiagWrite
- */
-#define DIAG_TRANSFER_LIMIT 2048
-
-/*
  * maximum number of bytes that can be
  * handled atomically by DiagRead/DiagWrite
  */
-- 
2.10.1

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

[Srinivas Kandagatla]

use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/net/wireless/ath/ath10k/ce.c  | 9 +--------
 drivers/net/wireless/ath/ath10k/pci.c | 3 +--
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 0b4d796..c2b388f 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -958,7 +958,7 @@ ath10k_ce_alloc_dest_ring(struct ath10k *ar, unsigned int ce_id,
 	 * coherent DMA are unsupported
 	 */
 	dest_ring->base_addr_owner_space_unaligned =
-		dma_alloc_coherent(ar->dev,
+		dma_zalloc_coherent(ar->dev,
 				   (nentries * sizeof(struct ce_desc) +
 				    CE_DESC_RING_ALIGN),
 				   &base_addr, GFP_KERNEL);
@@ -969,13 +969,6 @@ ath10k_ce_alloc_dest_ring(struct ath10k *ar, unsigned int ce_id,
 
 	dest_ring->base_addr_ce_space_unaligned = base_addr;
 
-	/*
-	 * Correctly initialize memory to 0 to prevent garbage
-	 * data crashing system when download firmware
-	 */
-	memset(dest_ring->base_addr_owner_space_unaligned, 0,
-	       nentries * sizeof(struct ce_desc) + CE_DESC_RING_ALIGN);
-
 	dest_ring->base_addr_owner_space = PTR_ALIGN(
 			dest_ring->base_addr_owner_space_unaligned,
 			CE_DESC_RING_ALIGN);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index b541a1c..855e3de 100644
--- 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);
@@ -905,7 +905,6 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
 		ret = -ENOMEM;
 		goto done;
 	}
-	memset(data_buf, 0, alloc_nbytes);
 
 	remaining_bytes = nbytes;
 	ce_data = ce_data_base;
-- 
2.10.1

[PATCH 3/3] ath10k: fix typo in addr calculation

[Srinivas Kandagatla]

CORE_CTRL_ADDRESS is offset in register address space, it does not
make sense to OR it to derive the final address. It looks like its
a typo, so fix it.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/net/wireless/ath/ath10k/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 855e3de..023ab10 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1936,7 +1936,7 @@ static int ath10k_pci_wake_target_cpu(struct ath10k *ar)
 {
 	u32 addr, val;
 
-	addr = SOC_CORE_BASE_ADDRESS | CORE_CTRL_ADDRESS;
+	addr = SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS;
 	val = ath10k_pci_read32(ar, addr);
 	val |= CORE_CTRL_CPU_INTR_MASK;
 	ath10k_pci_write32(ar, addr, val);
-- 
2.10.1

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

[Joe Perches]

On Mon, 2017-01-23 at 15:04 +0000, 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);

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

[Valo, Kalle]

Joe Perches <joe@perches.com> writes:

> On Mon, 2017-01-23 at 15:04 +0000, 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

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

[Joe Perches]

On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
> Joe Perches <joe@perches.com> writes:
> 
> > On Mon, 2017-01-23 at 15:04 +0000, 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.

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

[Valo, Kalle]

Joe Perches <joe@perches.com> writes:

> On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
>> Joe Perches <joe@perches.com> writes:
>> 
>> > On Mon, 2017-01-23 at 15:04 +0000, 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

Re: [1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT

[Kalle Valo]

Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> DIAG_TRANSFER_LIMIT is redefined with same value and comments
> just below this entry, remove this duplicate entry.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

3 patches applied to ath-next branch of ath.git, thanks.

dd51fa3d8a1d ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT
0de4df5ba2ad ath10k: use dma_zalloc_coherent()
1ad38fd719da ath10k: fix typo in addr calculation

-- 
https://patchwork.kernel.org/patch/9532691/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches