These fixes were originally embedded in a patch
"ath10k_sdio: virtual scatter gather for receive"
posted by Alagu Sankar <[email protected]>
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
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 <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
---
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
Kalle Valo <[email protected]> writes:
> Niklas Cassel <[email protected]> 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 <[email protected]>
>> Signed-off-by: Niklas Cassel <[email protected]>
>
> 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
Niklas Cassel <[email protected]> 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 <[email protected]>
> Signed-off-by: Niklas Cassel <[email protected]>
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
On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> Kalle Valo <[email protected]> writes:
>
> > Niklas Cassel <[email protected]> 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 <[email protected]>
> >> Signed-off-by: Niklas Cassel <[email protected]>
> >
> > 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
Niklas Cassel <[email protected]> writes:
> On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
>> Kalle Valo <[email protected]> writes:
>>
>> > Niklas Cassel <[email protected]> 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 <[email protected]>
>> >> Signed-off-by: Niklas Cassel <[email protected]>
>> >
>> > 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
On Fri, Jun 29, 2018 at 04:51:51PM +0300, Kalle Valo wrote:
> Niklas Cassel <[email protected]> writes:
>
> > On Fri, Jun 29, 2018 at 02:53:44PM +0300, Kalle Valo wrote:
> >> Kalle Valo <[email protected]> writes:
> >>
> >> > Niklas Cassel <[email protected]> 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 <[email protected]>
> >> >> Signed-off-by: Niklas Cassel <[email protected]>
> >> >
> >> > 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
Niklas Cassel <[email protected]> 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 <[email protected]>
> Signed-off-by: Alagu Sankar <[email protected]>
> Signed-off-by: Niklas Cassel <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
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