From 3abeca998a44205cfd837fa0bf1f7c24f8294acb Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 5 May 2011 19:08:09 -0700 Subject: xhci: Fix bug in control transfer cancellation. When the xHCI driver attempts to cancel a transfer, it issues a Stop Endpoint command and waits for the host controller to indicate which TRB it was in the middle of processing. The host will put an event TRB with completion code COMP_STOP on the event ring if it stops on a control transfer TRB (or other types of transfer TRBs). The ring handling code is supposed to set ep->stopped_trb to the TRB that the host stopped on when this happens. Unfortunately, there is a long-standing bug in the control transfer completion code. It doesn't actually check to see if COMP_STOP is set before attempting to process the transfer based on which part of the control TD completed. So when we get an event on the data phase of the control TRB with COMP_STOP set, it thinks it's a normal completion of the transfer and doesn't set ep->stopped_td or ep->stopped_trb. When the ring handling code goes on to process the completion of the Stop Endpoint command, it sees that ep->stopped_trb is not a part of the TD it's trying to cancel. It thinks the hardware has its enqueue pointer somewhere further up in the ring, and thinks it's safe to turn the control TRBs into no-op TRBs. Since the hardware was in the middle of the control TRBs to be cancelled, the proper software behavior is to issue a Set TR dequeue pointer command. It turns out that the NEC host controllers can handle active TRBs being set to no-op TRBs after a stop endpoint command, but other host controllers have issues with this out-of-spec software behavior. Fix this behavior. This patch should be backported to kernels as far back as 2.6.31, but it may be a bit challenging, since process_ctrl_td() was introduced in some refactoring done in 2.6.36, and some endian-safe patches added in 2.6.40 that touch the same lines. Signed-off-by: Sarah Sharp Cc: Dmitry Torokhov Cc: stable@kernel.org --- drivers/usb/host/xhci-ring.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/usb/host/xhci-ring.c') diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index c35058b..237a765 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1641,6 +1641,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, else *status = 0; break; + case COMP_STOP_INVAL: + case COMP_STOP: + return finish_td(xhci, td, event_trb, event, ep, status, false); default: if (!xhci_requires_manual_halt_cleanup(xhci, ep_ctx, trb_comp_code)) @@ -1685,15 +1688,12 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, } } else { /* Maybe the event was for the data stage? */ - if (trb_comp_code != COMP_STOP_INVAL) { - /* We didn't stop on a link TRB in the middle */ - td->urb->actual_length = - td->urb->transfer_buffer_length - - TRB_LEN(le32_to_cpu(event->transfer_len)); - xhci_dbg(xhci, "Waiting for status " - "stage event\n"); - return 0; - } + td->urb->actual_length = + td->urb->transfer_buffer_length - + TRB_LEN(le32_to_cpu(event->transfer_len)); + xhci_dbg(xhci, "Waiting for status " + "stage event\n"); + return 0; } } -- cgit v1.1