Skip to contentSkip to conversation

xhci: fix isoc endpoint dequeue from advancing too far on transaction error

PATCH New Mathias Nyman 1 participant 3 messages Apr 30, 2015 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/3 xhci: fix isoc endpoint dequeue from advancing too far on transaction error

Isoc TDs usually consist of one TRB, sometimes two. When all goes well we receive only one success event for a TD, and move the dequeue pointer to the next TD.

This fails if the TD consists of two TRBs and we get a transfer error on the first TRB, we will then see two events for that TD.

Fix this by making sure the event we get is for the last TRB in that TD before moving the dequeue pointer to the next TD. This will resolve some of the uvc and dvb issues with the
"ERROR Transfer event TRB DMA ptr not part of current TD" error message

---
 drivers/usb/host/xhci-ring.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/host/xhci-ring.c +5 -0
index f5397a5..ae0894c 100644
@@ -2026,8 +2026,13 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td,
 		break;
 	case COMP_DEV_ERR:
 	case COMP_STALL:
+		frame->status = -EPROTO;
+		skip_td = true;
+		break;
 	case COMP_TX_ERR:
 		frame->status = -EPROTO;
+		if (event_trb != td->last_trb)
+			return 0;
 		skip_td = true;
 		break;
 	case COMP_STOP:
Signature · 1 line
1.8.3.2
#2 lore raw

PATCH 2/3 xhci: Solve full event ring by increasing TRBS_PER_SEGMENT to 256

In reply to a message not in the archive

Our event ring consists of only one segment, and we risk filling the event ring in case we get isoc transfers with short intervals such as webcams that fill a TD every microframe (125us)

With 64 TRB segment size one usb camera could fill the event ring in 8ms. A setup with several cameras and other devices can fill up the event ring as it is shared between all devices.
This has occurred when uvcvideo queues 5 * 32TD URBs which then get cancelled when the video mode changes. The cancelled URBs are returned in the xhci interrupt context and blocks the interrupt handler from handling the new events.

A full event ring will block xhci from scheduling traffic and affect all devices conneted to the xhci, will see errors such as Missed Service Intervals for isoc devices, and and Split transaction errors for LS/FS interrupt devices.

Increasing the TRB_PER_SEGMENT will also increase the default endpoint ring size, which is welcome as for most isoc transfer we had to dynamically expand the endpoint ring anyway to be able to queue the 5 * 32TDs uvcvideo queues.

The default size used to be 64 TRBs per segment

---
 drivers/usb/host/xhci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci.h +1 -1
index 8e421b8..ea75e8c 100644
@@ -1267,7 +1267,7 @@ union xhci_trb {
  * since the command ring is 64-byte aligned.
  * It must also be greater than 16.
  */
-#define TRBS_PER_SEGMENT	64
+#define TRBS_PER_SEGMENT	256
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS	(TRBS_PER_SEGMENT - 3)
 #define TRB_SEGMENT_SIZE	(TRBS_PER_SEGMENT*16)
Signature · 1 line
1.8.3.2
#3 lore raw

PATCH 3/3 xhci: gracefully handle xhci_irq dead device

In reply to a message not in the archive

From: Joe Lawrence <[email protected]>

If the xHCI host controller has died (ie, device removed) or suffered other serious fatal error (STS_FATAL), then xhci_irq should handle this condition with IRQ_HANDLED instead of -ESHUTDOWN.

---
 drivers/usb/host/xhci-ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/host/xhci-ring.c +1 -1
index ae0894c..7d34cbf 100644
@@ -2645,7 +2645,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
 		xhci_halt(xhci);
 hw_died:
 		spin_unlock(&xhci->lock);
-		return -ESHUTDOWN;
+		return IRQ_HANDLED;
 	}
 
 	/*
Signature · 1 line
1.8.3.2