[PATCH 3/3] ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

[PATCH 3/3] ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

This patch copies skb->data of register out command to kmalloced buffer,
which is cacheline aligned(dma safe) and passed into usb hcd. Since the
data in register out command is not very much, we can ignore the
performance loss caused by the copy.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c |   35 +++++++++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/hif_usb.h |    1 +
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index be4dfbd..b9d42ff 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -29,6 +29,30 @@ MODULE_DEVICE_TABLE(usb, ath9k_hif_usb_ids);
 
 static int __hif_usb_tx(struct hif_device_usb *hif_dev);
 
+static inline struct cmd_buf *alloc_cmd(int len)
+{
+	struct cmd_buf *cmd;
+
+	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+	if (!cmd)
+		return NULL;
+
+	cmd->usb_buf = kmalloc(len, GFP_KERNEL);
+	if (!cmd->usb_buf) {
+		kfree(cmd);
+		return NULL;
+	}
+
+	return cmd;
+}
+
+static inline void free_cmd(struct cmd_buf *cmd)
+{
+	kfree(cmd->usb_buf);
+	kfree(cmd);
+}
+
+
 static void hif_usb_regout_cb(struct urb *urb)
 {
 	struct cmd_buf *cmd = (struct cmd_buf *)urb->context;
@@ -48,13 +72,13 @@ static void hif_usb_regout_cb(struct urb *urb)
 	if (cmd) {
 		ath9k_htc_txcompletion_cb(cmd->hif_dev->htc_handle,
 					  cmd->skb, 1);
-		kfree(cmd);
+		free_cmd(cmd);
 	}
 
 	return;
 free:
 	dev_kfree_skb_any(cmd->skb);
-	kfree(cmd);
+	free_cmd(cmd);
 }
 
 static int hif_usb_send_regout(struct hif_device_usb *hif_dev,
@@ -68,7 +92,7 @@ static int hif_usb_send_regout(struct hif_device_usb *hif_dev,
 	if (urb == NULL)
 		return -ENOMEM;
 
-	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+	cmd = alloc_cmd(skb->len);
 	if (cmd == NULL) {
 		usb_free_urb(urb);
 		return -ENOMEM;
@@ -76,17 +100,18 @@ static int hif_usb_send_regout(struct hif_device_usb *hif_dev,
 
 	cmd->skb = skb;
 	cmd->hif_dev = hif_dev;
+	memcpy(cmd->usb_buf, skb->data, skb->len);
 
 	usb_fill_int_urb(urb, hif_dev->udev,
 			 usb_sndintpipe(hif_dev->udev, USB_REG_OUT_PIPE),
-			 skb->data, skb->len,
+			 cmd->usb_buf, skb->len,
 			 hif_usb_regout_cb, cmd, 1);
 
 	usb_anchor_urb(urb, &hif_dev->regout_submitted);
 	ret = usb_submit_urb(urb, GFP_KERNEL);
 	if (ret) {
 		usb_unanchor_urb(urb);
-		kfree(cmd);
+		free_cmd(cmd);
 	}
 	usb_free_urb(urb);
 
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 7d49a8a..ebc8c41 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -76,6 +76,7 @@ struct hif_usb_tx {
 struct cmd_buf {
 	struct sk_buff *skb;
 	struct hif_device_usb *hif_dev;
+	unsigned char *usb_buf;
 };
 
 #define HIF_USB_START BIT(0)
-- 
1.6.2.5

[PATCH 3/3] ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

[Sujith]

tom.leiming@gmail.com wrote:
> This patch copies skb->data of register out command to kmalloced buffer,
> which is cacheline aligned(dma safe) and passed into usb hcd. Since the
> data in register out command is not very much, we can ignore the
> performance loss caused by the copy.

The skb for every frame sent out through the REG_OUT pipe is allocated
using dev_alloc_skb(). So again, am not sure about your cacheline argument.

And the extra memcpy would certainly cause a performance loss, since we
already do a memcpy once to copy the data in ath9k_wmi_cmd().

Are you having DMA issues with the driver ?
If so, on which architecture ?

Sujith

[PATCH 3/3] ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

[Sujith]

tom.leiming@gmail.com wrote:
> This patch copies skb->data of register out command to kmalloced buffer,
> which is cacheline aligned(dma safe) and passed into usb hcd. Since the
> data in register out command is not very much, we can ignore the
> performance loss caused by the copy.

Actually, the current code is correct, though a bit sub-optimal.
The inital dev_alloc_skb() in ath9k_wmi_cmd() allocates a headroom
for WMI header + HTC header. The headers are then pushed as the packet
moves down through HTC. Finally, skb->data is passed to submit_urb().

To take care of cacheline alignment, you can just replace the dev_alloc_skb()
with alloc_skb(). A patch is welcome. :-)

Sujith

Re: [PATCH 3/3] ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

[Ming Lei]

2010/4/12 Sujith <Sujith.Manoharan@atheros.com>:
> tom.leiming@gmail.com wrote:
>> This patch copies skb->data of register out command to kmalloced buffer,
>> which is cacheline aligned(dma safe) and passed into usb hcd. Since the
>> data in register out command is not very much, we can ignore the
>> performance loss caused by the copy.
>
> Actually, the current code is correct, though a bit sub-optimal.
> The inital dev_alloc_skb() in ath9k_wmi_cmd() allocates a headroom
> for WMI header + HTC header. The headers are then pushed as the packet
> moves down through HTC. Finally, skb->data is passed to submit_urb().
>
> To take care of cacheline alignment, you can just replace the dev_alloc_skb()
> with alloc_skb(). A patch is welcome. :-)

OK, clever idea, I'll submit a new version.

-- 
Lei Ming