[PATCH v3 0/1] xhci: Some improvement for Etron xHCI host

[PATCH v3 0/1] xhci: Some improvement for Etron xHCI host

[Kuangyi Chiang]

To prevent the xHCI driver from printing a "Transfer event TRB DMA
ptr not part of current TD" error message when an error is detected
while processing the last TRB of an isoc TD:
  xhci: Correctly handle last TRB of isoc TD on Etron xHCI host

---
Changes in v3:
- Update subject and commit message
- Use error_mid_td instead of last_td_was_short to solve the problem, as suggested by Michal
- Link to v2: https://lore.kernel.org/all/20241028025337.6372-1-ki.chiang65@gmail.com

Changes in v2:
- Modify commit message
- Remove XHCI_NO_RESET_DEVICE/XHCI_NO_BREAK_CTRL_TD quirk flag
- Add XHCI_ETRON_HOST quirk flag, thanks for the suggestion by Michal
- Check device speed before invoking the workaround
- Add (xhci: Combine two if statements for Etron xHCI host)
- Add (xhci: Correct handling of one-TRB isoc TD on Etron xHCI host)
- Link to v1: https://lore.kernel.org/all/20240911051716.6572-4-ki.chiang65@gmail.com

Kuangyi Chiang (1):
  xhci: Correctly handle last TRB of isoc TD on Etron xHCI host

 drivers/usb/host/xhci-ring.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH v3 1/1] xhci: Correctly handle last TRB of isoc TD on Etron xHCI host

[Kuangyi Chiang]

Unplugging a USB3.0 webcam while streaming results in errors like this:

[ 132.646387] xhci_hcd 0000:03:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 18 comp_code 13
[ 132.646446] xhci_hcd 0000:03:00.0: Looking for event-dma 000000002fdf8630 trb-start 000000002fdf8640 trb-end 000000002fdf8650 seg-start 000000002fdf8000 seg-end 000000002fdf8ff0
[ 132.646560] xhci_hcd 0000:03:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 18 comp_code 13
[ 132.646568] xhci_hcd 0000:03:00.0: Looking for event-dma 000000002fdf8660 trb-start 000000002fdf8670 trb-end 000000002fdf8670 seg-start 000000002fdf8000 seg-end 000000002fdf8ff0

If an error is detected while processing the last TRB of an isoc TD,
the Etron xHC generates two transfer events for the TRB where the
error was detected. The first event can be any sort of error (like
USB Transaction or Babble Detected, etc), and the final event is
Success.

The xHCI driver will handle the TD after the first event and remove it
from its internal list, and then print an "Transfer event TRB DMA ptr
not part of current TD" error message after the final event.

Commit 5372c65e1311 ("xhci: process isoc TD properly when there was a
transaction error mid TD.") is designed to address isoc transaction
errors, but unfortunately it doesn't account for this scenario.

To work around this by reusing the logic that handles isoc transaction
errors, but continuing to wait for the final event when this condition
occurs. Sometimes we see the Stopped event after an error mid TD, this
is a normal event for a pending TD and we can think of it as the final
event we are waiting for.

Check if the XHCI_ETRON_HOST quirk flag is set before invoking the
workaround in process_isoc_td().

Fixes: 5372c65e1311 ("xhci: process isoc TD properly when there was a transaction error mid TD.")
Cc: <stable@vger.kernel.org>
Signed-off-by: Kuangyi Chiang <ki.chiang65@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4cf5363875c7..a51eb3526ae3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2450,8 +2450,10 @@ static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 	switch (trb_comp_code) {
 	case COMP_SUCCESS:
 		/* Don't overwrite status if TD had an error, see xHCI 4.9.1 */
-		if (td->error_mid_td)
+		if (td->error_mid_td) {
+			td->error_mid_td = false;
 			break;
+		}
 		if (remaining) {
 			frame->status = short_framestatus;
 			sum_trbs_for_length = true;
@@ -2466,25 +2468,36 @@ static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 	case COMP_BANDWIDTH_OVERRUN_ERROR:
 		frame->status = -ECOMM;
 		break;
+	case COMP_USB_TRANSACTION_ERROR:
 	case COMP_BABBLE_DETECTED_ERROR:
 		sum_trbs_for_length = true;
 		fallthrough;
 	case COMP_ISOCH_BUFFER_OVERRUN:
 		frame->status = -EOVERFLOW;
+		if (trb_comp_code == COMP_USB_TRANSACTION_ERROR)
+			frame->status = -EPROTO;
 		if (ep_trb != td->end_trb)
 			td->error_mid_td = true;
+		else
+			td->error_mid_td = false;
+
+		/*
+		 * If an error is detected on the last TRB of the TD,
+		 * wait for the final event.
+		 */
+		if ((xhci->quirks & XHCI_ETRON_HOST) &&
+		    td->urb->dev->speed >= USB_SPEED_SUPER &&
+		    ep_trb == td->end_trb)
+			td->error_mid_td = true;
 		break;
 	case COMP_INCOMPATIBLE_DEVICE_ERROR:
 	case COMP_STALL_ERROR:
 		frame->status = -EPROTO;
 		break;
-	case COMP_USB_TRANSACTION_ERROR:
-		frame->status = -EPROTO;
-		sum_trbs_for_length = true;
-		if (ep_trb != td->end_trb)
-			td->error_mid_td = true;
-		break;
 	case COMP_STOPPED:
+		/* Think of it as the final event if TD had an error */
+		if (td->error_mid_td)
+			td->error_mid_td = false;
 		sum_trbs_for_length = true;
 		break;
 	case COMP_STOPPED_SHORT_PACKET:
@@ -2517,7 +2530,7 @@ static void process_isoc_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
 
 finish_td:
 	/* Don't give back TD yet if we encountered an error mid TD */
-	if (td->error_mid_td && ep_trb != td->end_trb) {
+	if (td->error_mid_td) {
 		xhci_dbg(xhci, "Error mid isoc TD, wait for final completion event\n");
 		td->urb_length_set = true;
 		return;
-- 
2.25.1