When a control transfer has a short data stage, the xHCI controller generates
two transfer events: a COMP_SHORT_TX event that specifies the untransferred
amount, and a COMP_SUCCESS event. But when the data stage is not short, only
the COMP_SUCCESS event occurs. Therefore, xhci-hcd sets urb->actual_length
to urb->transfer_buffer_length while processing the COMP_SUCCESS event,
unless urb->actual_length was set already by a previous COMP_SHORT_TX event.
The driver checks this by seeing whether urb->actual_length == 0, but this
alone is the wrong test, as it is entirely possible for a short transfer to
have an urb->actual_length = 0.
This patch changes the xhci driver to set the urb->actual_length in advance
to the expected value of a successful control transfer.
The urb->actual_length is then only adjusted in case of short transfers or
other special events, but not on COMP_SUCCESS events.
This fixes a bug which affected the HSO plugin, which relies on URBs with
urb->actual_length == 0 to halt re-submitting the RX URB in the control
endpoint.
Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 73 ++++++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 36 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b46b5b9..0e02e79 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -732,7 +732,11 @@ remove_finished_td:
/* Clean up the cancelled URB */
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
+ * Control urbs have the urb->actual_length pre-set, clear it
+ * as well
*/
+ if (usb_endpoint_xfer_control(&cur_td->urb->ep->desc))
+ cur_td->urb->actual_length = 0;
xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
@@ -755,6 +759,7 @@ static void xhci_kill_ring_urbs(struct xhci_hcd *xhci,
struct xhci_ring *ring)
list_del_init(&cur_td->td_list);
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
+ cur_td->urb->actual_length = 0;
xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
}
}
@@ -792,6 +797,7 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
cur_td = list_first_entry(&ep->cancelled_td_list,
struct xhci_td, cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
+ cur_td->urb->actual_length = 0;
xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
}
}
@@ -1888,6 +1894,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct
xhci_td *td,
int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 trb_comp_code;
+ bool force_finish_td = false;
slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
xdev = xhci->devs[slot_id];
@@ -1906,7 +1913,8 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct
xhci_td *td,
xhci_warn(xhci, "WARN: Success on ctrl data TRB "
"without IOC set??\n");
*status = -ESHUTDOWN;
- } else {
+ } else if (*status == -EINPROGRESS) {
+ /* only set to 0 if no previous event set it earlier */
*status = 0;
}
break;
@@ -1918,6 +1926,8 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct
xhci_td *td,
break;
case COMP_STOP_INVAL:
case COMP_STOP:
+ /* we don't continue stopped TDs, so length can be set to 0 */
+ td->urb->actual_length = 0;
return finish_td(xhci, td, event_trb, event, ep, status, false);
default:
if (!xhci_requires_manual_halt_cleanup(xhci,
@@ -1928,44 +1938,26 @@ static int process_ctrl_td(struct xhci_hcd *xhci,
struct xhci_td *td,
trb_comp_code, ep_index);
/* else fall through */
case COMP_STALL:
- /* Did we transfer part of the data (middle) phase? */
- if (event_trb != ep_ring->dequeue &&
- event_trb != td->last_trb)
- td->urb->actual_length =
- td->urb->transfer_buffer_length -
- EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
- else
- td->urb->actual_length = 0;
-
- return finish_td(xhci, td, event_trb, event, ep, status, false);
+ /* length will be set later below if we stall on data stage */
+ td->urb->actual_length = 0;
+ force_finish_td = true;
+ break;
}
+
+ /* If in setup stage, nothing transferred */
+ if (event_trb == ep_ring->dequeue)
+ td->urb->actual_length = 0;
+
/*
- * Did we transfer any data, despite the errors that might have
- * happened? I.e. did we get past the setup stage?
+ * In data stage, check if we transferred any data despite the possible
+ * errors that might have happened. Give back the urb if stalled,
+ * otherwise wait for the status stage event.
*/
- if (event_trb != ep_ring->dequeue) {
- /* The event was for the status stage */
- if (event_trb == td->last_trb) {
- if (td->urb->actual_length != 0) {
- /* Don't overwrite a previously set error code
- */
- if ((*status == -EINPROGRESS || *status == 0) &&
- (td->urb->transfer_flags
- & URB_SHORT_NOT_OK))
- /* Did we already see a short data
- * stage? */
- *status = -EREMOTEIO;
- } else {
- td->urb->actual_length =
- td->urb->transfer_buffer_length;
- }
- } else {
- /* Maybe the event was for the data stage? */
- td->urb->actual_length =
- td->urb->transfer_buffer_length -
- EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
- xhci_dbg(xhci, "Waiting for status "
- "stage event\n");
+ if (event_trb != td->last_trb) {
+ td->urb->actual_length = td->urb->transfer_buffer_length -
+ EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
+ if (!force_finish_td) {
+ xhci_dbg(xhci, "Waiting for status stage event\n");
return 0;
}
}
@@ -3346,6 +3338,15 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
*/
if (urb->transfer_buffer_length > 0)
num_trbs++;
+
+ /*
+ * We want to set the urb->actual_length in advance and change it in
+ * case of error or short transfer. A SHORT_TX event may be followed
+ * by a SUCCESS event for that same TD, messing up the transfer length.
+ * So we can't touch urb->actual_length later on successful transfers
+ */
+ urb->actual_length = urb->transfer_buffer_length;
+
ret = prepare_transfer(xhci, xhci->devs[slot_id],
ep_index, urb->stream_id,
num_trbs, urb, 0, mem_flags);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
