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

From: Mohammed Shafi Shajakhan <[email protected]>

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 <[email protected]>
---
[v2 rebased over top of tree]

drivers/net/wireless/ath/ath10k/htt_tx.c | 76 ++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index ae5b33f..786fbd7 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -229,6 +229,33 @@ void ath10k_htt_tx_free_msdu_id(struct ath10k_htt *htt, u16 msdu_id)
idr_remove(&htt->pending_tx, msdu_id);
}

+static void ath10k_htt_tx_free_cont_txbuf(struct ath10k_htt *htt)
+{
+ size_t size;
+
+ if (!htt->txbuf.vaddr)
+ return;
+
+ size = htt->max_num_pending_tx * sizeof(struct ath10k_htt_txbuf);
+ dma_free_coherent(htt->ar->dev, size, htt->txbuf.vaddr,
+ htt->txbuf.paddr);
+}
+
+static int ath10k_htt_tx_alloc_cont_txbuf(struct ath10k_htt *htt)
+{
+ struct ath10k *ar = htt->ar;
+ size_t size;
+
+ size = htt->max_num_pending_tx * sizeof(struct ath10k_htt_txbuf);
+ htt->txbuf.vaddr = dma_alloc_coherent(ar->dev, size,
+ &htt->txbuf.paddr,
+ GFP_KERNEL);
+ if (!htt->txbuf.vaddr)
+ return -ENOMEM;
+
+ return 0;
+}
+
static void ath10k_htt_tx_free_cont_frag_desc(struct ath10k_htt *htt)
{
size_t size;
@@ -310,10 +337,26 @@ static int ath10k_htt_tx_alloc_txq(struct ath10k_htt *htt)
return 0;
}

+static void ath10k_htt_tx_free_txdone_fifo(struct ath10k_htt *htt)
+{
+ WARN_ON(!kfifo_is_empty(&htt->txdone_fifo));
+ kfifo_free(&htt->txdone_fifo);
+}
+
+static int ath10k_htt_tx_alloc_txdone_fifo(struct ath10k_htt *htt)
+{
+ int ret;
+ size_t size;
+
+ size = roundup_pow_of_two(htt->max_num_pending_tx);
+ ret = kfifo_alloc(&htt->txdone_fifo, size, GFP_KERNEL);
+ return ret;
+}
+
int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
{
struct ath10k *ar = htt->ar;
- int ret, size;
+ int ret;

ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt tx max num pending tx %d\n",
htt->max_num_pending_tx);
@@ -321,13 +364,9 @@ int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
spin_lock_init(&htt->tx_lock);
idr_init(&htt->pending_tx);

- size = htt->max_num_pending_tx * sizeof(struct ath10k_htt_txbuf);
- htt->txbuf.vaddr = dma_alloc_coherent(ar->dev, size,
- &htt->txbuf.paddr,
- GFP_KERNEL);
- if (!htt->txbuf.vaddr) {
- ath10k_err(ar, "failed to alloc tx buffer\n");
- ret = -ENOMEM;
+ ret = ath10k_htt_tx_alloc_cont_txbuf(htt);
+ if (ret) {
+ ath10k_err(ar, "failed to alloc cont tx buffer: %d\n", ret);
goto free_idr_pending_tx;
}

@@ -343,8 +382,7 @@ int ath10k_htt_tx_alloc(struct ath10k_htt *htt)
goto free_frag_desc;
}

- size = roundup_pow_of_two(htt->max_num_pending_tx);
- ret = kfifo_alloc(&htt->txdone_fifo, size, GFP_KERNEL);
+ ret = ath10k_htt_tx_alloc_txdone_fifo(htt);
if (ret) {
ath10k_err(ar, "failed to alloc txdone fifo: %d\n", ret);
goto free_txq;
@@ -359,10 +397,7 @@ free_frag_desc:
ath10k_htt_tx_free_cont_frag_desc(htt);

free_txbuf:
- size = htt->max_num_pending_tx *
- sizeof(struct ath10k_htt_txbuf);
- dma_free_coherent(htt->ar->dev, size, htt->txbuf.vaddr,
- htt->txbuf.paddr);
+ ath10k_htt_tx_free_cont_txbuf(htt);

free_idr_pending_tx:
idr_destroy(&htt->pending_tx);
@@ -388,22 +423,15 @@ static int ath10k_htt_tx_clean_up_pending(int msdu_id, void *skb, void *ctx)

void ath10k_htt_tx_free(struct ath10k_htt *htt)
{
- int size;
+ tasklet_kill(&htt->txrx_compl_task);

idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
idr_destroy(&htt->pending_tx);

- if (htt->txbuf.vaddr) {
- size = htt->max_num_pending_tx *
- sizeof(struct ath10k_htt_txbuf);
- dma_free_coherent(htt->ar->dev, size, htt->txbuf.vaddr,
- htt->txbuf.paddr);
- }
-
+ ath10k_htt_tx_free_cont_txbuf(htt);
ath10k_htt_tx_free_txq(htt);
ath10k_htt_tx_free_cont_frag_desc(htt);
- WARN_ON(!kfifo_is_empty(&htt->txdone_fifo));
- kfifo_free(&htt->txdone_fifo);
+ ath10k_htt_tx_free_txdone_fifo(htt);
}

void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
--
1.9.1


2016-10-12 07:09:28

by Mohammed Shafi Shajakhan

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

Hi Kalle,

On Tue, Oct 11, 2016 at 01:36:22PM +0200, Kalle Valo wrote:
> Mohammed Shafi Shajakhan <[email protected]> wrote:
> > From: Mohammed Shafi Shajakhan <[email protected]>
> >
> > 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 <[email protected]>
>
> 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.

>
> drivers/net/wireless/ath/ath10k/htt_tx.c: In function ‘ath10k_htt_tx_free’:
> drivers/net/wireless/ath/ath10k/htt_tx.c:424:19: error: ‘struct ath10k_htt’ has no member named ‘txrx_compl_task’
> make[5]: *** [drivers/net/wireless/ath/ath10k/htt_tx.o] Error 1
> make[4]: *** [drivers/net/wireless/ath/ath10k] Error 2
> make[3]: *** [drivers/net/wireless/ath] Error 2
> make[2]: *** [drivers/net/wireless] Error 2
> make[1]: *** [drivers/net] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>
> 2 patches set to Changes Requested.
>
> 9363863 [v2,1/2] ath10k: clean up HTT tx buffer allocation and free
> 9363861 [v2,2/2] ath10k: Remove extraneous error message in tx alloc
>
> --
> https://patchwork.kernel.org/patch/9363863/
>
> Documentation about submitting wireless patches and checking status
> from patchwork:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>

2016-10-11 11:48:44

by Kalle Valo

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

Mohammed Shafi Shajakhan <[email protected]> wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> 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 <[email protected]>

Applies but fails to build:

drivers/net/wireless/ath/ath10k/htt_tx.c: In function ‘ath10k_htt_tx_free’:
drivers/net/wireless/ath/ath10k/htt_tx.c:424:19: error: ‘struct ath10k_htt’ has no member named ‘txrx_compl_task’
make[5]: *** [drivers/net/wireless/ath/ath10k/htt_tx.o] Error 1
make[4]: *** [drivers/net/wireless/ath/ath10k] Error 2
make[3]: *** [drivers/net/wireless/ath] Error 2
make[2]: *** [drivers/net/wireless] Error 2
make[1]: *** [drivers/net] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2

2 patches set to Changes Requested.

9363863 [v2,1/2] ath10k: clean up HTT tx buffer allocation and free
9363861 [v2,2/2] ath10k: Remove extraneous error message in tx alloc

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

Documentation about submitting wireless patches and checking status
from patchwork:

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

2016-10-12 07:38:59

by Kalle Valo

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

Mohammed Shafi Shajakhan <[email protected]> writes:

> Hi Kalle,
>
> On Tue, Oct 11, 2016 at 01:36:22PM +0200, Kalle Valo wrote:
>> Mohammed Shafi Shajakhan <[email protected]> wrote:
>> > From: Mohammed Shafi Shajakhan <[email protected]>
>> >=20
>> > 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
>> >=20
>> > Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
>>=20
>> Applies but fails to build:
>
> [shafi] sorry for the trouble again, I just figured out I had a private p=
atch by
> mistake and it worked for me, I will make sure that v3 is properly rebas=
ed
> 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 :)

--=20
Kalle Valo=

Subject: [PATCH v2 2/2] ath10k: Remove extraneous error message in tx alloc

From: Mohammed Shafi Shajakhan <[email protected]>

Remove extraneous error message in 'ath10k_htt_tx_alloc_cont_frag_desc'
as the caller 'ath10k_htt_tx_alloc' already dumps a proper error
message

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
[v2 rebased over top of tree]

drivers/net/wireless/ath/ath10k/htt_tx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 786fbd7..4255c1a 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -283,10 +283,8 @@ static int ath10k_htt_tx_alloc_cont_frag_desc(struct ath10k_htt *htt)
htt->frag_desc.vaddr = dma_alloc_coherent(ar->dev, size,
&htt->frag_desc.paddr,
GFP_KERNEL);
- if (!htt->frag_desc.vaddr) {
- ath10k_err(ar, "failed to alloc fragment desc memory\n");
+ if (!htt->frag_desc.vaddr)
return -ENOMEM;
- }

return 0;
}
--
1.9.1