On Thu, 16 Jun 2011, Sarah Sharp wrote: > Change the xHCI driver to be consistent with the EHCI and UHCI driver, > and always set urb->status to 0 for isochronous URBs. > @@ -1798,8 +1795,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. > + */ This comment says more than is necessary, and it makes the driver sound as though it has an inferiority complex. :-) Just say something simple, like: Isochronous URBs report status in the individual iso_frame_desc elements, not in urb->status. > frame->status = -EXDEV; > > /* calc actual length */ > @@ -2191,7 +2193,13 @@ cleanup: > urb->transfer_buffer_length, > status); > spin_unlock(&xhci->lock); > - usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, status); > + /* EHCI, UHCI, and OHCI 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); Is this part needed at all? If it is, you can do it like this: + if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) + status = 0; usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, status); But it would be even better to set status to 0 somewhere in the code that's specifically responsible for handling isochronous URBs, instead of adding an extra test here. Alan Stern -- 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
