On Tue, Aug 27, 2019 at 4:03 PM Wen Gong [off-list ref] wrote:

quoted

-----Original Message-----
From: ath10k <redacted> On Behalf Of Nicolas
Boichat
Sent: Tuesday, August 27, 2019 3:41 PM
To: Wen Gong <redacted>
Cc: open list:NETWORKING DRIVERS (WIRELESS) <linux-
wireless@vger.kernel.org>; ath10k@lists.infradead.org
Subject: [EXT] Re: [PATCH 1/7] ath10k: enable RX bundle receive for sdio

quoted

-static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
+static int ath10k_sdio_mbox_rx_fetch_bundle(struct ath10k *ar)
 {
        struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+       struct ath10k_sdio_rx_data *pkt;
        int ret, i;
+       u32 pkt_offset, virt_pkt_len;

+       virt_pkt_len = 0;
        for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
-               ret = ath10k_sdio_mbox_rx_packet(ar,
-                                                &ar_sdio->rx_pkts[i]);
-               if (ret)
+               virt_pkt_len += ar_sdio->rx_pkts[i].alloc_len;
+       }
+
+       if (virt_pkt_len < ATH10K_SDIO_DMA_BUF_SIZE) {
+               ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
+                                        ar_sdio->vsg_buffer, virt_pkt_len);
+               if (ret) {
+                       i = 0;
                        goto err;
+               }
+       } else {
+               ath10k_err(ar, "size exceeding limit %d\n", virt_pkt_len);
+       }
+
+       pkt_offset = 0;
+       for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
+               struct sk_buff *skb = ar_sdio->rx_pkts[i].skb;
+
+               pkt = &ar_sdio->rx_pkts[i];
+               memcpy(skb->data, ar_sdio->vsg_buffer + pkt_offset,
+                      pkt->alloc_len);

Why do you copy alloc_len if you only plan to use act_len?

alloc_len is aligned to block size(256),  and act_len maybe not same for each packet in the bundle.
Eg a bundle:
Packet 1 len 240,   alloc_len: 256, 1st time: act_len 240, left padding size is 16,
Packet 1 len 250, alloc_len: 256, 1st time: act_len 240, left padding size is 6,
Packet 1 len 230,   alloc_len: 256, 1st time: act_len 240, left padding size is 26,

The bundled buffer len is 256 * 3 = 768, it has 256 bytes for each packet, the left size is padding which
Is not needed, but the left padding is not same for each packet, before read all the buffer from sdio bus,
It does not know each packet's act len, it only know the 1st packet's act len.
So it need to copy all the alloc_len's buffer to ensure it will not lose data.

Gotcha, thanks.

quoted

Actually, just use skb_put_data.

Also, do you have the same issue as
https://patchwork.kernel.org/patch/11116215/ w.r.t. act_len being
incorrect?

quoted

+               pkt->status = 0;
+               skb_put(skb, pkt->act_len);

So I guess this is incorrect, see patchwork linked above.

quoted

quoted

+               pkt_offset += pkt->alloc_len;
        }

        return 0;

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k