Hi Sarah,
Here is the patch that we needed to make isoc transfers work reliably with the
Synopsys xHCI controller.
Some high-speed USB 2.0 webcams did not work, because the code in
xhci_endpoint_init() and xhci_get_max_esit_payload() was masking off bit 10 of
the wMaxPacketSize value.
Occasionally the system would freeze when connecting or disconnecting a
USB 2.0 webcam to the xHCI host, or when starting up or shutting down the
webcam app (Cheese). We traced this to one of two issues:
- Sometimes one of the BUGs in xhci_find_new_dequeue_state() would trigger.
To avoid locking up the system, we replaced the BUGs with WARN_ONs.
- Sometimes there would be an infinite loop in handle_tx_event() because
'event_seg' was NULL. There were also problems with the code possibly
using an uninitialized 'event_trb', and not resetting 'ret' to 0 on
subsequent passes thru the loop.
The patch is against Linus' tree. The patch also has a couple of other fixes
that were already submitted by other folks, but did not make it into 2.6.35.
I left those in since this is just an RFC.
---
drivers/usb/host/xhci-mem.c | 4 +-
drivers/usb/host/xhci-ring.c | 51 ++++++++++++++++++++++++++---------------
2 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4e51343..bdf9056 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1043,7 +1043,7 @@ static inline u32 xhci_get_max_esit_payload(struct xhci_hcd *xhci,
if (udev->speed == USB_SPEED_SUPER)
return ep->ss_ep_comp.wBytesPerInterval;
- max_packet = ep->desc.wMaxPacketSize & 0x3ff;
+ max_packet = ep->desc.wMaxPacketSize & 0x7ff;
max_burst = (ep->desc.wMaxPacketSize & 0x1800) >> 11;
/* A 0 in max burst means 1 transfer per ESIT */
return max_packet * (max_burst + 1);
@@ -1133,7 +1133,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
/* Fall through */
case USB_SPEED_FULL:
case USB_SPEED_LOW:
- max_packet = ep->desc.wMaxPacketSize & 0x3ff;
+ max_packet = ep->desc.wMaxPacketSize & 0x7ff;
ep_ctx->ep_info2 |= MAX_PACKET(max_packet);
break;
default:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bc3f4f4..d9d89df 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -131,7 +131,7 @@ static void next_trb(struct xhci_hcd *xhci,
*seg = (*seg)->next;
*trb = ((*seg)->trbs);
} else {
- *trb = (*trb)++;
+ (*trb)++;
}
}
@@ -474,8 +474,11 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
state->new_deq_seg = find_trb_seg(cur_td->start_seg,
dev->eps[ep_index].stopped_trb,
&state->new_cycle_state);
- if (!state->new_deq_seg)
- BUG();
+ if (!state->new_deq_seg) {
+ WARN_ON(1);
+ return;
+ }
+
/* Dig out the cycle state saved by the xHC during the stop ep cmd */
xhci_dbg(xhci, "Finding endpoint context\n");
ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
@@ -486,8 +489,10 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
state->new_deq_seg = find_trb_seg(state->new_deq_seg,
state->new_deq_ptr,
&state->new_cycle_state);
- if (!state->new_deq_seg)
- BUG();
+ if (!state->new_deq_seg) {
+ WARN_ON(1);
+ return;
+ }
trb = &state->new_deq_ptr->generic;
if ((trb->field[3] & TRB_TYPE_BITMASK) == TRB_TYPE(TRB_LINK) &&
@@ -1551,6 +1556,10 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
/* calc actual length */
if (ep->skip) {
td->urb->iso_frame_desc[idx].actual_length = 0;
+ /* Update ring dequeue pointer */
+ while (ep_ring->dequeue != td->last_trb)
+ inc_deq(xhci, ep_ring, false);
+ inc_deq(xhci, ep_ring, false);
return finish_td(xhci, td, event_trb, event, ep, status, true);
}
@@ -1824,6 +1833,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
}
do {
+ ret = 0;
+
/* This TRB should be in the TD at the head of this ring's
* TD list.
*/
@@ -1839,7 +1850,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
xhci_dbg(xhci, "td_list is empty while skip "
"flag set. Clear skip flag.\n");
}
- ret = 0;
goto cleanup;
}
@@ -1874,20 +1884,23 @@ static int handle_tx_event(struct xhci_hcd *xhci,
"Skip it\n");
goto cleanup;
}
- }
- /* Now update the urb's actual_length and give back to
- * the core
- */
- if (usb_endpoint_xfer_control(&td->urb->ep->desc))
- ret = process_ctrl_td(xhci, td, event_trb, event, ep,
- &status);
- else if (usb_endpoint_xfer_isoc(&td->urb->ep->desc))
- ret = process_isoc_td(xhci, td, event_trb, event, ep,
- &status);
- else
- ret = process_bulk_intr_td(xhci, td, event_trb, event,
- ep, &status);
+ /* Now update the urb's actual_length and give back to
+ * the core
+ */
+ if (usb_endpoint_xfer_control(&td->urb->ep->desc))
+ ret = process_ctrl_td(xhci, td, event_trb, event, ep,
+ &status);
+ else if (usb_endpoint_xfer_isoc(&td->urb->ep->desc))
+ ret = process_isoc_td(xhci, td, event_trb, event, ep,
+ &status);
+ else
+ ret = process_bulk_intr_td(xhci, td, event_trb, event,
+ ep, &status);
+ } else {
+ xhci_dbg(xhci, "event_seg is NULL\n");
+ return -ENODEV;
+ }
cleanup:
/*
--
Paul
--
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
