Skip to contentSkip to conversation

ath9k-htc: convert EP3 and EP4 back from bulk to int

RFC PATCH New Oleksij Rempel 2 participants 3 messages Aug 13, 2013 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 ath9k-htc: convert EP3 and EP4 back from bulk to int

If usb auto suspend is enabled or system run in to suspend/resume cycle ath9k-htc adapter will stop to response. It is reproducible on xhci HCs.

Host part of problem:
XHCI do timing calculation based on Transfer Type and bInterval, immediately after device was detected. Ath9k-htc try to overwrite this parameters on module probe and some changes in FW,
since we do not initiate usb reset from the driver this changes are not took to account. So, before any kind of suspend or reset, host controller will operate with old parameters. Only after suspend/resume and if interface id stay unchanged, new parameters will by applied. Host will send bulk data with no intervals (?), which will cause overflow on FIFO of EP4.

Firmware part of problem:
By default, ath9k-htc adapters configured with EP3 and EP4 as interrupt endpoints. Current firmware will try to overwrite ConfigDescriptor to make EP3 and EP4 bulk. FIFO for this endpoints stay not reconfigured, so under the hood it is still Int EP.

---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 36 ++++++++++----------------------
 1 file changed, 11 insertions(+), 25 deletions(-)
drivers/net/wireless/ath/ath9k/hif_usb.c +11 -25
index 5205a36..d99ea45 100644
@@ -115,10 +115,10 @@ static int hif_usb_send_regout(struct hif_device_usb *hif_dev,
 	cmd->skb = skb;
 	cmd->hif_dev = hif_dev;
 
-	usb_fill_bulk_urb(urb, hif_dev->udev,
-			 usb_sndbulkpipe(hif_dev->udev, USB_REG_OUT_PIPE),
+	usb_fill_int_urb(urb, hif_dev->udev,
+			 usb_sndintpipe(hif_dev->udev, USB_REG_OUT_PIPE),
 			 skb->data, skb->len,
-			 hif_usb_regout_cb, cmd);
+			 hif_usb_regout_cb, cmd, 1);
 
 	usb_anchor_urb(urb, &hif_dev->regout_submitted);
 	ret = usb_submit_urb(urb, GFP_KERNEL);
@@ -723,11 +723,11 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 			return;
 		}
 
-		usb_fill_bulk_urb(urb, hif_dev->udev,
-				 usb_rcvbulkpipe(hif_dev->udev,
+		usb_fill_int_urb(urb, hif_dev->udev,
+				 usb_rcvintpipe(hif_dev->udev,
 						 USB_REG_IN_PIPE),
 				 nskb->data, MAX_REG_IN_BUF_SIZE,
-				 ath9k_hif_usb_reg_in_cb, nskb);
+				 ath9k_hif_usb_reg_in_cb, nskb, 1);
 	}
 
 resubmit:
@@ -909,11 +909,11 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 			goto err_skb;
 		}
 
-		usb_fill_bulk_urb(urb, hif_dev->udev,
-				  usb_rcvbulkpipe(hif_dev->udev,
+		usb_fill_int_urb(urb, hif_dev->udev,
+				  usb_rcvintpipe(hif_dev->udev,
 						  USB_REG_IN_PIPE),
 				  skb->data, MAX_REG_IN_BUF_SIZE,
-				  ath9k_hif_usb_reg_in_cb, skb);
+				  ath9k_hif_usb_reg_in_cb, skb, 1);
 
 		/* Anchor URB */
 		usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
@@ -1033,7 +1033,7 @@ static int ath9k_hif_usb_dev_init(struct hif_device_usb *hif_dev)
 {
 	struct usb_host_interface *alt = &hif_dev->interface->altsetting[0];
 	struct usb_endpoint_descriptor *endp;
-	int ret, idx;
+	int ret;
 
 	ret = ath9k_hif_usb_download_fw(hif_dev);
 	if (ret) {
@@ -1043,20 +1043,6 @@ static int ath9k_hif_usb_dev_init(struct hif_device_usb *hif_dev)
 		return ret;
 	}
 
-	/* On downloading the firmware to the target, the USB descriptor of EP4
-	 * is 'patched' to change the type of the endpoint to Bulk. This will
-	 * bring down CPU usage during the scan period.
-	 */
-	for (idx = 0; idx < alt->desc.bNumEndpoints; idx++) {
-		endp = &alt->endpoint[idx].desc;
-		if ((endp->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-				== USB_ENDPOINT_XFER_INT) {
-			endp->bmAttributes &= ~USB_ENDPOINT_XFERTYPE_MASK;
-			endp->bmAttributes |= USB_ENDPOINT_XFER_BULK;
-			endp->bInterval = 0;
-		}
-	}
-
 	/* Alloc URBs */
 	ret = ath9k_hif_usb_alloc_urbs(hif_dev);
 	if (ret) {
@@ -1268,7 +1254,7 @@ static void ath9k_hif_usb_reboot(struct usb_device *udev)
 	if (!buf)
 		return;
 
-	ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, USB_REG_OUT_PIPE),
+	ret = usb_interrupt_msg(udev, usb_sndintpipe(udev, USB_REG_OUT_PIPE),
 			   buf, 4, NULL, HZ);
 	if (ret)
 		dev_err(&udev->dev, "ath9k_htc: USB reboot failed\n");
Signature · 1 line
1.8.1.2
#1 lore raw

RFC PATCH 1/1 ath9k-htc: convert EP3 and EP4 back from bulk to int

Am 13.08.2013 10:09, schrieb Oleksij Rempel:

Quoted 14 lines

If usb auto suspend is enabled or system run in to suspend/resume cycle ath9k-htc adapter will stop to response. It is reproducible on xhci HCs.

Host part of problem:
XHCI do timing calculation based on Transfer Type and bInterval, immediately after device was detected. Ath9k-htc try to overwrite this parameters on module probe and some changes in FW,
since we do not initiate usb reset from the driver this changes are not took to account. So, before any kind of suspend or reset, host controller will operate with old parameters. Only after suspend/resume and if interface id stay unchanged, new parameters will by applied. Host will send bulk data with no intervals (?), which will cause overflow on FIFO of EP4.

Quoted 6 lines

Firmware part of problem:
By default, ath9k-htc adapters configured with EP3 and EP4 as interrupt endpoints. Current firmware will try to overwrite ConfigDescriptor to make EP3 and EP4 bulk. FIFO for this endpoints stay not reconfigured, so under the hood it is still Int EP.

After some digging in the code, i see that converting EP3 and EP4 to bulk was a bug. This endpoints can't handle more then 64bytes, and normally if we would provide it as usb descriptor, we would got warned. See this code:
drivers/usb/core/config.c

         /*
          * Some buggy high speed devices have bulk endpoints using
          * maxpacket sizes other than 512.  High speed HCDs may not
          * be able to handle that particular bug, so let's warn...
          */
         if (to_usb_device(ddev)->speed == USB_SPEED_HIGH
                         && usb_endpoint_xfer_bulk(d)) {
                 unsigned maxp;

                 maxp = usb_endpoint_maxp(&endpoint->desc) & 0x07ff;
                 if (maxp != 512)
                         dev_warn(ddev, "config %d interface %d 
altsetting %d "
                                 "bulk endpoint 0x%X has invalid 
maxpacket %d\n",
                                 cfgno, inum, asnum, d->bEndpointAddress,
                                 maxp);
         }
Signature · 2 lines
Regards,
Oleksij
#2 lore raw

RFC PATCH 1/1 ath9k-htc: convert EP3 and EP4 back from bulk to int

In reply to Oleksij Rempel
Quoted 14 lines · Oleksij Rempel

If usb auto suspend is enabled or system run in to suspend/resume cycle ath9k-htc adapter will stop to response. It is reproducible on xhci HCs.

Host part of problem:
XHCI do timing calculation based on Transfer Type and bInterval, immediately after device was detected. Ath9k-htc try to overwrite this parameters on module probe and some changes in FW,
since we do not initiate usb reset from the driver this changes are not took to account. So, before any kind of suspend or reset, host controller will operate with old parameters. Only after suspend/resume and if interface id stay unchanged, new parameters will by applied. Host will send bulk data with no intervals (?), which will cause overflow on FIFO of EP4.

Quoted 5 lines

Firmware part of problem:
By default, ath9k-htc adapters configured with EP3 and EP4 as interrupt endpoints. Current firmware will try to overwrite ConfigDescriptor to make EP3 and EP4 bulk. FIFO for this endpoints stay not reconfigured, so under the hood it is still Int EP.

One little comment on the patch title. You aren't converting the endpoints back from bulk to interrupt; rather you are preventing the driver and firmware from trying to convert them from interrupt to bulk.

Alan Stern