Skip to contentSkip to conversation

ath9k-htc: pass cacheline aligned buffer to usb hcd in register out path

PATCH New [email protected] 2 participants 4 messages Apr 10, 2010 – Apr 12, 2010 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore

[email protected]

author
#0 lore raw

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

From: Ming Lei <[email protected]>

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 <[email protected]>
---
 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(-)
drivers/net/wireless/ath/ath9k/hif_usb.c +30 -5
index be4dfbd..b9d42ff 100644
@@ -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);
drivers/net/wireless/ath/ath9k/hif_usb.h +1 -0
index 7d49a8a..ebc8c41 100644
@@ -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)
Signature · 1 line
1.6.2.5
#1 lore raw

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

Quoted 4 lines · [email protected] 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

#2 lore raw

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

In reply to [email protected]
Quoted 4 lines · [email protected] 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

author
#3 lore raw

2010/4/12 Sujith <[email protected]>:

Quoted 6 lines
Quoted 4 lines · [email protected] 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.

Quoted 7 lines

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.

Signature · 1 line
Lei Ming