Skip to contentSkip to conversation

fix mceusb endpoint type identification/handling

PATCH Superseded Matt DeVillier 2 participants 2 messages Apr 22, 2014 – Apr 23, 2014 Media (V4L/DVB)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Matt DeVillier

author
#0 lore raw

PATCH 1/1 fix mceusb endpoint type identification/handling

From: Matt DeVillier <[email protected]>

Change the I/O endpoint handling of the mceusb driver to respect the endpoint type reported by device (bulk/interrupt), rather than treating all endpoints as type interrupt, which breaks devices using bulk endpoints when connected to a xhci controller. Accordingly, change the function calls to initialize an endpoint's transfer pipe and urb handlers to use the correct function based on the endpoint type.

Signed-off-by Matt DeVillier <[email protected]>

---
This is a continuation of the work started in patch #21648
Patch compiled and tested against linux-media git master. Backported and tested against 3.14.1 stable as well.
---

mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +51 -29
@@ -747,11 +747,17 @@ static void mce_request_packet(struct mc
         }
 
         /* outbound data */
-        pipe = usb_sndintpipe(ir->usbdev,
-                      ir->usb_ep_out->bEndpointAddress);
-        usb_fill_int_urb(async_urb, ir->usbdev, pipe,
-            async_buf, size, mce_async_callback,
-            ir, ir->usb_ep_out->bInterval);
+        if ((ir->usb_ep_out->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+            == USB_ENDPOINT_XFER_INT) {
+            pipe = usb_sndintpipe(ir->usbdev, ir->usb_ep_out->bEndpointAddress);
+            usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf,
+                     size, mce_async_callback, ir, ir->usb_ep_out->bInterval);
+        } else {
+            pipe = usb_sndbulkpipe(ir->usbdev,
+                     ir->usb_ep_out->bEndpointAddress);
+            usb_fill_bulk_urb(async_urb, ir->usbdev, pipe, async_buf,
+                     size, mce_async_callback, ir);
+        }
         memcpy(async_buf, data, size);
 
     } else if (urb_type == MCEUSB_RX) {

@@ -1271,38 +1277,48 @@ static int mceusb_dev_probe(struct usb_i
 
         if ((ep_in == NULL)
             && ((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
-                == USB_DIR_IN)
-            && (((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_BULK)
-            || ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_INT))) {
-
-            ep_in = ep;
-            ep_in->bmAttributes = USB_ENDPOINT_XFER_INT;
-            ep_in->bInterval = 1;
-            dev_dbg(&intf->dev, "acceptable inbound endpoint found");
+            == USB_DIR_IN)) {
+
+            if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_BULK) {
+
+                ep_in = ep;
+                mce_dbg(&intf->dev, "acceptable bulk inbound endpoint found\n");
+            } else if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_INT) {
+
+                ep_in = ep;
+                ep_in->bInterval = 1;
+                mce_dbg(&intf->dev, "acceptable interrupt inbound endpoint found\n");
+            }
         }
 
         if ((ep_out == NULL)
             && ((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
-                == USB_DIR_OUT)
-            && (((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_BULK)
-            || ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_INT))) {
-
-            ep_out = ep;
-            ep_out->bmAttributes = USB_ENDPOINT_XFER_INT;
-            ep_out->bInterval = 1;
-            dev_dbg(&intf->dev, "acceptable outbound endpoint found");
+            == USB_DIR_OUT)) {
+            if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_BULK) {
+                ep_out = ep;
+                mce_dbg(&intf->dev, "acceptable bulk outbound endpoint found\n");
+            } else if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_INT) {
+                ep_out = ep;
+                ep_out->bInterval = 1;
+                mce_dbg(&intf->dev, "acceptable interrupt outbound endpoint found\n");
+            }
         }
     }
-    if (ep_in == NULL) {
+    if (ep_in == NULL || ep_out == NULL) {
         dev_dbg(&intf->dev, "inbound and/or endpoint not found");
         return -ENODEV;
     }
 
-    pipe = usb_rcvintpipe(dev, ep_in->bEndpointAddress);
+    if ((ep_in->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+        == USB_ENDPOINT_XFER_INT) {
+        pipe = usb_rcvintpipe(dev, ep_in->bEndpointAddress);
+    } else {
+        pipe = usb_rcvbulkpipe(dev, ep_in->bEndpointAddress);
+    }
     maxp = usb_maxpacket(dev, pipe, usb_pipeout(pipe));
 
     ir = kzalloc(sizeof(struct mceusb_dev), GFP_KERNEL);

@@ -1343,8 +1359,14 @@ static int mceusb_dev_probe(struct usb_i
         goto rc_dev_fail;
 
     /* wire up inbound data handler */
-    usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
-                mceusb_dev_recv, ir, ep_in->bInterval);
+    if ((ep_in->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+        == USB_ENDPOINT_XFER_INT) {
+        usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+                 mceusb_dev_recv, ir, ep_in->bInterval);
+    } else {
+        usb_fill_bulk_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+                 mceusb_dev_recv, ir);
+    }
     ir->urb_in->transfer_dma = ir->dma_in;
     ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
Tested-by on v1 1/1
#1 lore raw
Quoted 21 lines · Matt DeVillier

From: Matt DeVillier <[email protected]>

Change the I/O endpoint handling of the mceusb driver to respect the endpoint type reported by device (bulk/interrupt), rather than treating all endpoints as type interrupt, which breaks devices using bulk endpoints when connected to a xhci controller. Accordingly, change the function calls to initialize an endpoint's transfer pipe and urb handlers to use the correct function based on the endpoint type.

Signed-off-by Matt DeVillier <[email protected]>

---
This is a continuation of the work started in patch #21648 Patch compiled and tested against linux-media git master. Backported and tested against 3.14.1 stable as well.
---

mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +0 -0
@@ -747,11 +747,17 @@ static void mce_request_packet(struct mc
         }

mceusb.c 2014-04-22 14:46:12.378347584 -0500 static void mce_request_packet(struct mc · line 753

Quoted 10 lines
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +3 -5
         /* outbound data */
-        pipe = usb_sndintpipe(ir->usbdev,
-                      ir->usb_ep_out->bEndpointAddress);
-        usb_fill_int_urb(async_urb, ir->usbdev, pipe,
-            async_buf, size, mce_async_callback,
-            ir, ir->usb_ep_out->bInterval);
+        if ((ir->usb_ep_out->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+            == USB_ENDPOINT_XFER_INT) {
+            pipe = usb_sndintpipe(ir->usbdev,
 ir->usb_ep_out->bEndpointAddress);

The lines are wrapped and tabs have been replaced with spaces; your mail client messed with your patch.

See
https://www.kernel.org/doc/Documentation/email-clients.txt

Quoted 63 lines
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +8 -0
+            usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf,
+                     size, mce_async_callback, ir,
 ir->usb_ep_out->bInterval);
+        } else {
+            pipe = usb_sndbulkpipe(ir->usbdev,
+                     ir->usb_ep_out->bEndpointAddress);
+            usb_fill_bulk_urb(async_urb, ir->usbdev, pipe, async_buf,
+                     size, mce_async_callback, ir);
+        }
         memcpy(async_buf, data, size);

     } else if (urb_type == MCEUSB_RX) {
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +20 -20
@@ -1271,38 +1277,48 @@ static int mceusb_dev_probe(struct usb_i
 
         if ((ep_in == NULL)
             && ((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
-                == USB_DIR_IN)
-            && (((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_BULK)
-            || ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_INT))) {
-
-            ep_in = ep;
-            ep_in->bmAttributes = USB_ENDPOINT_XFER_INT;
-            ep_in->bInterval = 1;
-            dev_dbg(&intf->dev, "acceptable inbound endpoint found");
+            == USB_DIR_IN)) {
+
+            if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_BULK) {
+
+                ep_in = ep;
+                mce_dbg(&intf->dev, "acceptable bulk inbound
 endpoint found\n");
+            } else if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_INT) {
+
+                ep_in = ep;
+                ep_in->bInterval = 1;
+                mce_dbg(&intf->dev, "acceptable interrupt inbound
 endpoint found\n");
+            }
         }
 
         if ((ep_out == NULL)
             && ((ep->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
-                == USB_DIR_OUT)
-            && (((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_BULK)
-            || ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
-                == USB_ENDPOINT_XFER_INT))) {
-
-            ep_out = ep;
-            ep_out->bmAttributes = USB_ENDPOINT_XFER_INT;
-            ep_out->bInterval = 1;
-            dev_dbg(&intf->dev, "acceptable outbound endpoint found");
+            == USB_DIR_OUT)) {
+            if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+                == USB_ENDPOINT_XFER_BULK) {
+                ep_out = ep;
+                mce_dbg(&intf->dev, "acceptable bulk outbound
 endpoint found\n");
+            } else if ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)

mceusb.c 2014-04-22 14:46:12.378347584 -0500 static int mceusb_dev_probe(struct usb_i · line 1315

Quoted 10 lines
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +6 -1
+                == USB_ENDPOINT_XFER_INT) {
+                ep_out = ep;
+                ep_out->bInterval = 1;
+                mce_dbg(&intf->dev, "acceptable interrupt outbound
 endpoint found\n");
+            }
         }
     }
-    if (ep_in == NULL) {
+    if (ep_in == NULL || ep_out == NULL) {

Although I think this is correct this is unrelated to the rest of the patch.

Quoted 20 lines
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +6 -3
         dev_dbg(&intf->dev, "inbound and/or endpoint not found");
         return -ENODEV;
     }
 
-    pipe = usb_rcvintpipe(dev, ep_in->bEndpointAddress);
+    if ((ep_in->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+        == USB_ENDPOINT_XFER_INT) {
+        pipe = usb_rcvintpipe(dev, ep_in->bEndpointAddress);
+    } else {
+        pipe = usb_rcvbulkpipe(dev, ep_in->bEndpointAddress);
+    }
     maxp = usb_maxpacket(dev, pipe, usb_pipeout(pipe));
 
     ir = kzalloc(sizeof(struct mceusb_dev), GFP_KERNEL);

@@ -1343,8 +1359,14 @@ static int mceusb_dev_probe(struct usb_i
         goto rc_dev_fail;
 
     /* wire up inbound data handler */
-    usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
-                mceusb_dev_recv, ir, ep_in->bInterval);

mceusb.c 2014-04-22 14:46:12.378347584 -0500 static int mceusb_dev_probe(struct usb_i · line 1371

Quoted 10 lines
mceusb.c 2014-04-22 14:46:12.378347584 -0500 renamed from mceusb.c.orig 2014-04-22 13:48:51.186259472 -0500 +8 -0
+    if ((ep_in->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+        == USB_ENDPOINT_XFER_INT) {
+        usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+                 mceusb_dev_recv, ir, ep_in->bInterval);
+    } else {
+        usb_fill_bulk_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+                 mceusb_dev_recv, ir);
+    }
     ir->urb_in->transfer_dma = ir->dma_in;
     ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

I've tested this patch and it does fix my mceusb device with bulk endpoints on xhci. So

Tested-by Sean Young <[email protected]>