[PATCH 0/3] ath10k: sdio: essential sdio fixes

[PATCH 0/3] ath10k: sdio: essential sdio fixes

[Niklas Cassel]

These fixes were originally embedded in a patch
"ath10k_sdio: virtual scatter gather for receive"
posted by Alagu Sankar <alagusankar@silex-india.com>
https://patchwork.kernel.org/patch/9979579/

However, while that patch implements scatter gather
(without using the proper sg types), it also
includes several other random fixes, including
but not limited to checkpatch.pl fixes.

This patch series is an attempt to extract
the most essential fixes included in that commit.

Each fix is now an individual commit, as upstream
requires that each logical change is self contained.

All of these fixes are essential to get working
sdio support for ath10k.

Other patches are needed to get working sdio support,
including the high-latency patch series from Erik Stromdahl.


Niklas Cassel (3):
  ath10k: sdio: use same endpoint id for all packets in a bundle
  ath10k: sdio: allocate correct size for RECV_1MORE_BLOCK rx packets
  ath10k: sdio: set skb len for all rx packets

 drivers/net/wireless/ath/ath10k/htc.h  | 1 +
 drivers/net/wireless/ath/ath10k/sdio.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Niklas Cassel]

All packets in a bundle should use the same endpoint id as the
first lookahead.

This matches how things are done is ath6kl, however,
this patch can theoretically handle several bundles
in ath10k_sdio_mbox_rx_process_packets().

Without this patch we get lots of errors about invalid endpoint id:

ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index d612ce8c9cff..d46523b0472c 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -434,12 +434,14 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
 	enum ath10k_htc_ep_id id;
 	int ret, i, *n_lookahead_local;
 	u32 *lookaheads_local;
+	int lookahead_idx = 0;
 
 	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
 		lookaheads_local = lookaheads;
 		n_lookahead_local = n_lookahead;
 
-		id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
+		id = ((struct ath10k_htc_hdr *)
+		      &lookaheads[lookahead_idx++])->eid;
 
 		if (id >= ATH10K_HTC_EP_COUNT) {
 			ath10k_warn(ar, "invalid endpoint in look-ahead: %d\n",
@@ -462,6 +464,7 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
 			/* Only read lookahead's from RX trailers
 			 * for the last packet in a bundle.
 			 */
+			lookahead_idx--;
 			lookaheads_local = NULL;
 			n_lookahead_local = NULL;
 		}
-- 
2.17.1

Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Kalle Valo]

Niklas Cassel <niklas.cassel@linaro.org> writes:

> All packets in a bundle should use the same endpoint id as the
> first lookahead.
>
> This matches how things are done is ath6kl, however,
> this patch can theoretically handle several bundles
> in ath10k_sdio_mbox_rx_process_packets().
>
> Without this patch we get lots of errors about invalid endpoint id:
>
> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>

You have Alagu's s-o-b first so that implies Alagu is the author. So
should I add From header for Alagu and add you (Niklas) as
Co-Developed-by? Or vice versa?

-- 
Kalle Valo

Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

> Niklas Cassel <niklas.cassel@linaro.org> writes:
>
>> All packets in a bundle should use the same endpoint id as the
>> first lookahead.
>>
>> This matches how things are done is ath6kl, however,
>> this patch can theoretically handle several bundles
>> in ath10k_sdio_mbox_rx_process_packets().
>>
>> Without this patch we get lots of errors about invalid endpoint id:
>>
>> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
>> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
>> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
>>
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
>
> You have Alagu's s-o-b first so that implies Alagu is the author. So
> should I add From header for Alagu and add you (Niklas) as
> Co-Developed-by? Or vice versa?

Ah, the same issue is with all three patches. So whatever we decide,
I'll do the same changes on all three.

-- 
Kalle Valo

Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Niklas Cassel]

On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
> 
> > Niklas Cassel <niklas.cassel@linaro.org> writes:
> >
> >> All packets in a bundle should use the same endpoint id as the
> >> first lookahead.
> >>
> >> This matches how things are done is ath6kl, however,
> >> this patch can theoretically handle several bundles
> >> in ath10k_sdio_mbox_rx_process_packets().
> >>
> >> Without this patch we get lots of errors about invalid endpoint id:
> >>
> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> >>
> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> >
> > You have Alagu's s-o-b first so that implies Alagu is the author. So
> > should I add From header for Alagu and add you (Niklas) as
> > Co-Developed-by? Or vice versa?

Hello Kalle,

It is not always obvious how the combination of git-author
and Co-Developed-by should be:
http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html

Alagu deserves most credit, since I have simply taken parts
of a very big patch and split it into smaller pieces.
I tried to do the right thing by having his Signed-off-by first.

Let's go with your suggestion and add a From: header with
Alagu's email, and a Co-Developed-by tag with my email.
(Note that both Signed-off-bys are still needed according to
submitting-patches.rst)

Do you want me to resend the patches with these two lines
added, or can you fix them up manually?

Regards,
Niklas

> 
> Ah, the same issue is with all three patches. So whatever we decide,
> I'll do the same changes on all three.
> 
> -- 
> Kalle Valo

Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Kalle Valo]

Niklas Cassel <niklas.cassel@linaro.org> writes:

> On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>> 
>> > Niklas Cassel <niklas.cassel@linaro.org> writes:
>> >
>> >> All packets in a bundle should use the same endpoint id as the
>> >> first lookahead.
>> >>
>> >> This matches how things are done is ath6kl, however,
>> >> this patch can theoretically handle several bundles
>> >> in ath10k_sdio_mbox_rx_process_packets().
>> >>
>> >> Without this patch we get lots of errors about invalid endpoint id:
>> >>
>> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
>> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
>> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
>> >>
>> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
>> >
>> > You have Alagu's s-o-b first so that implies Alagu is the author. So
>> > should I add From header for Alagu and add you (Niklas) as
>> > Co-Developed-by? Or vice versa?
>
> Hello Kalle,
>
> It is not always obvious how the combination of git-author
> and Co-Developed-by should be:
> http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html

Yeah, that's true.

> Alagu deserves most credit, since I have simply taken parts
> of a very big patch and split it into smaller pieces.
> I tried to do the right thing by having his Signed-off-by first.
>
> Let's go with your suggestion and add a From: header with
> Alagu's email, and a Co-Developed-by tag with my email.
> (Note that both Signed-off-bys are still needed according to
> submitting-patches.rst)

Ok.

> Do you want me to resend the patches with these two lines
> added, or can you fix them up manually?

No need, I did that already in the pending branch. Please check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6b23bd067990df1cc1e9a4c28327393a7a3ba718

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bf83a5c572e0737e59e026c989b190b4e52d2ef4

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=218f924d80b76520a4e60eead5e89f3ebd2e86c1

-- 
Kalle Valo

Re: [PATCH 1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Niklas Cassel]

On Fri, Jun 29, 2018 at 04:51:51PM +0300, Kalle Valo wrote:
> Niklas Cassel <niklas.cassel@linaro.org> writes:
> 
> > On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> >> Kalle Valo <kvalo@codeaurora.org> writes:
> >> 
> >> > Niklas Cassel <niklas.cassel@linaro.org> writes:
> >> >
> >> >> All packets in a bundle should use the same endpoint id as the
> >> >> first lookahead.
> >> >>
> >> >> This matches how things are done is ath6kl, however,
> >> >> this patch can theoretically handle several bundles
> >> >> in ath10k_sdio_mbox_rx_process_packets().
> >> >>
> >> >> Without this patch we get lots of errors about invalid endpoint id:
> >> >>
> >> >> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> >> >> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> >> >> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> >> >>
> >> >> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> >> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> >> >
> >> > You have Alagu's s-o-b first so that implies Alagu is the author. So
> >> > should I add From header for Alagu and add you (Niklas) as
> >> > Co-Developed-by? Or vice versa?
> >
> > Hello Kalle,
> >
> > It is not always obvious how the combination of git-author
> > and Co-Developed-by should be:
> > http://lkml.iu.edu/hypermail/linux/kernel/1801.2/00988.html
> 
> Yeah, that's true.
> 
> > Alagu deserves most credit, since I have simply taken parts
> > of a very big patch and split it into smaller pieces.
> > I tried to do the right thing by having his Signed-off-by first.
> >
> > Let's go with your suggestion and add a From: header with
> > Alagu's email, and a Co-Developed-by tag with my email.
> > (Note that both Signed-off-bys are still needed according to
> > submitting-patches.rst)
> 
> Ok.
> 
> > Do you want me to resend the patches with these two lines
> > added, or can you fix them up manually?
> 
> No need, I did that already in the pending branch. Please check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6b23bd067990df1cc1e9a4c28327393a7a3ba718
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bf83a5c572e0737e59e026c989b190b4e52d2ef4
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=218f924d80b76520a4e60eead5e89f3ebd2e86c1

Hello Kalle

It looks good :)

Regards,
Niklas

> 
> -- 
> Kalle Valo

Re: [1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Kalle Valo]

Niklas Cassel <niklas.cassel@linaro.org> wrote:

> All packets in a bundle should use the same endpoint id as the
> first lookahead.
> 
> This matches how things are done is ath6kl, however,
> this patch can theoretically handle several bundles
> in ath10k_sdio_mbox_rx_process_packets().
> 
> Without this patch we get lots of errors about invalid endpoint id:
> 
> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> 
> Co-Developed-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

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

679e1f07c862 ath10k: sdio: use same endpoint id for all packets in a bundle
d1d061b1395a ath10k: sdio: allocate correct size for RECV_1MORE_BLOCK rx packets
8530b4e7b22b ath10k: sdio: set skb len for all rx packets

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

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

Re: [1/3] ath10k: sdio: use same endpoint id for all packets in a bundle

[Kalle Valo]

Niklas Cassel <niklas.cassel@linaro.org> wrote:

> All packets in a bundle should use the same endpoint id as the
> first lookahead.
> 
> This matches how things are done is ath6kl, however,
> this patch can theoretically handle several bundles
> in ath10k_sdio_mbox_rx_process_packets().
> 
> Without this patch we get lots of errors about invalid endpoint id:
> 
> ath10k_sdio mmc2:0001:1: invalid endpoint in look-ahead: 224
> ath10k_sdio mmc2:0001:1: failed to get pending recv messages: -12
> ath10k_sdio mmc2:0001:1: failed to process pending SDIO interrupts: -12
> 
> Co-Developed-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

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

679e1f07c862 ath10k: sdio: use same endpoint id for all packets in a bundle
d1d061b1395a ath10k: sdio: allocate correct size for RECV_1MORE_BLOCK rx packets
8530b4e7b22b ath10k: sdio: set skb len for all rx packets

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

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

[PATCH 2/3] ath10k: sdio: allocate correct size for RECV_1MORE_BLOCK rx packets

[Niklas Cassel]

Without this, when receiving a packet that has this flag set
from firmware, we will read invalid trailer data from the packet,
which will be shown as various errors, e.g. "sdio mbox lookahead
is zero" or "invalid rx packet" or "payload length x exceeds max
htc length".

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 drivers/net/wireless/ath/ath10k/htc.h  | 1 +
 drivers/net/wireless/ath/ath10k/sdio.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 34877597dd6a..cf1068dc3254 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -58,6 +58,7 @@ enum ath10k_htc_tx_flags {
 };
 
 enum ath10k_htc_rx_flags {
+	ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK = 0x01,
 	ATH10K_HTC_FLAG_TRAILER_PRESENT = 0x02,
 	ATH10K_HTC_FLAG_BUNDLE_MASK     = 0xF0
 };
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index d46523b0472c..0c57d6aaa437 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -603,6 +603,9 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
 		 * ATH10K_HTC_FLAG_BUNDLE_MASK flag set, all bundled
 		 * packet skb's have been allocated in the previous step.
 		 */
+		if (htc_hdr->flags & ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK)
+			full_len += ATH10K_HIF_MBOX_BLOCK_SIZE;
+
 		ret = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[i],
 						    act_len,
 						    full_len,
-- 
2.17.1

[PATCH 3/3] ath10k: sdio: set skb len for all rx packets

[Niklas Cassel]

Without this, packets larger than 1500 will silently be dropped.
Easily reproduced by sending a ping packet with a size larger
than 1500.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 0c57d6aaa437..8ac47b04a17e 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -396,6 +396,7 @@ static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
 	int ret;
 
 	payload_len = le16_to_cpu(htc_hdr->len);
+	skb->len = payload_len + sizeof(struct ath10k_htc_hdr);
 
 	if (trailer_present) {
 		trailer = skb->data + sizeof(*htc_hdr) +
-- 
2.17.1