On 4.9.2024 19.36, Marc SCHAEFER wrote:
Hello,
On Wed, Sep 04, 2024 at 05:26:28PM +0300, Mathias Nyman wrote:
I can start working on some debugging patches as well if you have the time to try
Yes, with pleasure. I often have time to recompile stuff, and I can leave the test
system running.
Thanks, testpatch below, and also as attachment.
8<---
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index efdf4c228b8c..b5f97f75a5ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1749,14 +1749,14 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
i = urb_priv->num_tds_done;
if (i < urb_priv->num_tds)
- xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
- "Cancel URB %p, dev %s, ep 0x%x, "
- "starting at offset 0x%llx",
- urb, urb->dev->devpath,
- urb->ep->desc.bEndpointAddress,
- (unsigned long long) xhci_trb_virt_to_dma(
- urb_priv->td[i].start_seg,
- urb_priv->td[i].first_trb));
+ xhci_warn(xhci,
+ "Cancel URB %p, dev %s, ep 0x%x, stream_id %u starting at offset 0x%llx",
+ urb, urb->dev->devpath,
+ urb->ep->desc.bEndpointAddress,
+ urb->stream_id,
+ (unsigned long long) xhci_trb_virt_to_dma(
+ urb_priv->td[i].start_seg,
+ urb_priv->td[i].first_trb));
for (; i < urb_priv->num_tds; i++) {
td = &urb_priv->td[i];
(END)
+ char str[XHCI_MSG_MAX];
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2]));
@@ -1351,6 +1352,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
if (!ep_ring) {
xhci_warn(xhci, "WARN Set TR deq ptr command for freed stream ID %u\n",
stream_id);
+ xhci_warn(xhci, "MN: %s\n" ,
+ xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
+ le32_to_cpu(trb->generic.field[1]),
+ le32_to_cpu(trb->generic.field[2]),
+ le32_to_cpu(trb->generic.field[3])));
/* XXX: Harmless??? */
goto cleanup;
}
@@ -1386,6 +1392,12 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
cmd_comp_code);
break;
}
+ xhci_warn(xhci, "MN: %s\n",
+ xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
+ le32_to_cpu(trb->generic.field[1]),
+ le32_to_cpu(trb->generic.field[2]),
+ le32_to_cpu(trb->generic.field[3])));
+
/* OK what do we do now? The endpoint state is hosed, and we
* should never get to this point if the synchronization between
* queueing, and endpoint state are correct. This might happen
@@ -2864,6 +2876,12 @@ static int handle_tx_event(struct xhci_hcd *xhci,
trb_comp_code);
trb_in_td(xhci, td, ep_trb_dma, true);
+ if (xhci_halted_host_endpoint(ep_ctx, trb_comp_code)) {
+ xhci_err(xhci, "MN: No TD found, fix halted ep");
+ xhci_handle_halted_endpoint(xhci, ep, NULL, EP_HARD_RESET);
+ } else {
+ xhci_err(xhci, "MN: No TD found, ep not halted");
+ }
return -ESHUTDOWN;
}
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index efdf4c228b8c..b5f97f75a5ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1749,14 +1749,14 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
i = urb_priv->num_tds_done;
if (i < urb_priv->num_tds)
- xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
- "Cancel URB %p, dev %s, ep 0x%x, "
- "starting at offset 0x%llx",
- urb, urb->dev->devpath,
- urb->ep->desc.bEndpointAddress,
- (unsigned long long) xhci_trb_virt_to_dma(
- urb_priv->td[i].start_seg,
- urb_priv->td[i].first_trb));
+ xhci_warn(xhci,
+ "Cancel URB %p, dev %s, ep 0x%x, stream_id %u starting at offset 0x%llx",
+ urb, urb->dev->devpath,
+ urb->ep->desc.bEndpointAddress,
+ urb->stream_id,
+ (unsigned long long) xhci_trb_virt_to_dma(
+ urb_priv->td[i].start_seg,
+ urb_priv->td[i].first_trb));
for (; i < urb_priv->num_tds; i++) {
td = &urb_priv->td[i];
From bf7bbf8dbf92dc06e108a103f5f01b3f416339da Mon Sep 17 00:00:00 2001
From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Date: Thu, 5 Sep 2024 16:20:22 +0300
Subject: [PATCH] xhci: Debug patch: handle halted ep if TD is not found.
A host side halted endpoint may not be recovered if the Babble error
event does not point to any pending tranfer descriptor (TD).
Handle halted endpoint event if transfer event does not point to
any pending TD. The Babble error may have been for a TD that has already
been given back.
Aklso add debuiggoing for TRB_ERROR issues seen when queuing a
Set TR Deq pointer command
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci-ring.c | 20 +++++++++++++++++++-
drivers/usb/host/xhci.c | 16 ++++++++--------
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4ea2c3e072a9..4ee8b2b42ac5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1008,7 +1008,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
if (cached_td) {
if (cached_td->urb->stream_id != td->urb->stream_id) {
/* Multiple streams case, defer move dq */
- xhci_dbg(xhci,
+ xhci_warn(xhci,
"Move dq deferred: stream %u URB %p\n",
td->urb->stream_id, td->urb);
td->cancel_status = TD_CLEARING_CACHE_DEFERRED;
@@ -1340,6 +1340,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_slot_ctx *slot_ctx;
struct xhci_td *td, *tmp_td;
bool deferred = false;
+ char str[XHCI_MSG_MAX];
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2]));
@@ -1351,6 +1352,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
if (!ep_ring) {
xhci_warn(xhci, "WARN Set TR deq ptr command for freed stream ID %u\n",
stream_id);
+ xhci_warn(xhci, "MN: %s\n" ,
+ xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
+ le32_to_cpu(trb->generic.field[1]),
+ le32_to_cpu(trb->generic.field[2]),
+ le32_to_cpu(trb->generic.field[3])));
/* XXX: Harmless??? */
goto cleanup;
}
@@ -1386,6 +1392,12 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
cmd_comp_code);
break;
}
+ xhci_warn(xhci, "MN: %s\n",
+ xhci_decode_trb(str, XHCI_MSG_MAX, le32_to_cpu(trb->generic.field[0]),
+ le32_to_cpu(trb->generic.field[1]),
+ le32_to_cpu(trb->generic.field[2]),
+ le32_to_cpu(trb->generic.field[3])));
+
/* OK what do we do now? The endpoint state is hosed, and we
* should never get to this point if the synchronization between
* queueing, and endpoint state are correct. This might happen
@@ -2864,6 +2876,12 @@ static int handle_tx_event(struct xhci_hcd *xhci,
trb_comp_code);
trb_in_td(xhci, td, ep_trb_dma, true);
+ if (xhci_halted_host_endpoint(ep_ctx, trb_comp_code)) {
+ xhci_err(xhci, "MN: No TD found, fix halted ep");
+ xhci_handle_halted_endpoint(xhci, ep, NULL, EP_HARD_RESET);
+ } else {
+ xhci_err(xhci, "MN: No TD found, ep not halted");
+ }
return -ESHUTDOWN;
}
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index efdf4c228b8c..b5f97f75a5ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1749,14 +1749,14 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
i = urb_priv->num_tds_done;
if (i < urb_priv->num_tds)
- xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
- "Cancel URB %p, dev %s, ep 0x%x, "
- "starting at offset 0x%llx",
- urb, urb->dev->devpath,
- urb->ep->desc.bEndpointAddress,
- (unsigned long long) xhci_trb_virt_to_dma(
- urb_priv->td[i].start_seg,
- urb_priv->td[i].first_trb));
+ xhci_warn(xhci,
+ "Cancel URB %p, dev %s, ep 0x%x, stream_id %u starting at offset 0x%llx",
+ urb, urb->dev->devpath,
+ urb->ep->desc.bEndpointAddress,
+ urb->stream_id,
+ (unsigned long long) xhci_trb_virt_to_dma(
+ urb_priv->td[i].start_seg,
+ urb_priv->td[i].first_trb));
for (; i < urb_priv->num_tds; i++) {
td = &urb_priv->td[i];
--
2.25.1
