[RFC] xhci: Always set urb->status to zero for isoc endpoints.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When the xHCI driver encounters a Missed Service Interval event for an
isochronous endpoint ring, it means the host controller skipped over
one or more isochronous TDs.  For TD that is skipped, skip_isoc_td() is
called.  This sets the frame descriptor status to -EXDEV, and also sets
the value stored in the int pointed to by status to -EXDEV.

If the isochronous TD happens to be the last TD in an URB,
handle_tx_event() will use the status variable to give back the URB to
the USB core.  That means drivers will see urb->status as -EXDEV.

It turns out that both EHCI and UHCI always set urb->status to zero for an
isochronous urb, regardless of what the frame status is.  See itd_complete() in
ehci-sched.c:

                } else {
                        /* URB was too late */
                        desc->status = -EXDEV;
                }
        }

        /* handle completion now? */
        if (likely ((urb_index + 1) != urb->number_of_packets))
                goto done;

        /* ASSERT: it's really the last itd for this urb
        list_for_each_entry (itd, &stream->td_list, itd_list)
                BUG_ON (itd->urb == urb);
         */

        /* give urb back to the driver; completion often (re)submits */
        dev = urb->dev;
        ehci_urb_done(ehci, urb, 0);

ehci_urb_done() completes the URB with the status of the third argument, which
is always zero in this case.

It turns out that many USB webcam drivers, such as uvcvideo, cannot
handle urb->status set to a non-zero value.  They will not resubmit
their isochronous URBs in that case, and userspace will see a frozen
video.

Change the xHCI driver to be consistent with the EHCI and UHCI driver,
and always set urb->status to 0 for isochronous URBs.

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/xhci-ring.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 99668ba..17695a6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1798,8 +1798,13 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	idx = urb_priv->td_cnt;
 	frame = &td->urb->iso_frame_desc[idx];
 
-	/* The transfer is partly done */
-	*status = -EXDEV;
+	/* The transfer is partly done.
+	 *
+	 * Do not set status to -EXDEV.  If we are on the last isoc TD of an
+	 * URB, urb->status will be set to -EXDEV.  Both UHCI and EHCI return
+	 * zero for urb->status, even if they set -EXDEV in the status of some
+	 * iso_frame_desc.
+	 */
 	frame->status = -EXDEV;
 
 	/* calc actual length */
@@ -2191,7 +2196,13 @@ cleanup:
 						urb->transfer_buffer_length,
 						status);
 			spin_unlock(&xhci->lock);
-			usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, status);
+			/* EHCI and UHCI always unconditionally set the
+			 * urb->status of an isochronous endpoint to 0.
+			 */
+			if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS)
+				usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, 0);
+			else
+				usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, status);
 			spin_lock(&xhci->lock);
 		}
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux