On Tue, Jul 20, 2010 at 04:49:08PM +0800, Andiry Xu wrote:
> Hi Sarah,
>
> This is xHCI isoc patchset v8.
>
> Changelog from v7:
>
> 1. Leave urb->status intact during host controller process.
> 2. Add the allocate bigger ring for isoc endpoint patch.
> 3. Set isoc urb status to -EXDEV when missed service event occurs.
> 4. Do not set isoc urb packet status when COMP_STOP or COMP_STOP_INVAL
> happens. Group COMP_BUFF_OVER and COMP_BABBLE case to remove redundancy.
Your change log says that the urb packet status isn't set when COMP_STOP
or COMP_STOP_INVAL is encounted, but your code doesn't match that
statement:
On Tue, Jul 20, 2010 at 04:49:30PM +0800, Andiry Xu wrote:
> +static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
> + union xhci_trb *event_trb, struct xhci_transfer_event *event,
> + struct xhci_virt_ep *ep, int *status)
> +{
...
> + /* handle completion code */
> + switch (trb_comp_code) {
> + case COMP_SUCCESS:
> + td->urb->iso_frame_desc[idx].status = 0;
> + xhci_dbg(xhci, "Successful isoc transfer!\n");
> + break;
> + case COMP_SHORT_TX:
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + td->urb->iso_frame_desc[idx].status =
> + -EREMOTEIO;
> + else
> + td->urb->iso_frame_desc[idx].status = 0;
> + break;
> + case COMP_BW_OVER:
> + td->urb->iso_frame_desc[idx].status = -ECOMM;
> + skip_td = 1;
> + break;
> + case COMP_BUFF_OVER:
> + case COMP_BABBLE:
> + td->urb->iso_frame_desc[idx].status = -EOVERFLOW;
> + skip_td = 1;
> + break;
> + case COMP_STALL:
> + td->urb->iso_frame_desc[idx].status = -EPROTO;
> + skip_td = 1;
> + break;
> + default:
> + td->urb->iso_frame_desc[idx].status = -1;
> + break;
> + }
> + }
If the TRB completion code is set to COMP_STOP or COMP_STOP_INVAL, the
packet's status will get set to -1, because of the default case in the
switch statement. You just need to add cases for those two with a break
statement:
case COMP_STALL:
td->urb->iso_frame_desc[idx].status = -EPROTO;
skip_td = 1;
break;
case COMP_STOP:
case COMP_STOP_INVAL:
break;
default:
td->urb->iso_frame_desc[idx].status = -1;
break;
}
Other than that, the whole patchset looks fine. Perhaps you want to
resend just this patch?
Sarah Sharp
--
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
