From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Marc SCHAEFER <schaefer@alphanet.ch>
Cc: Micha?? Pecio <michal.pecio@gmail.com>, linux-usb@vger.kernel.org
Subject: Re: Strange issues with UAS URB cancellation
Date: Thu, 5 Sep 2024 16:52:41 +0300 [thread overview]
Message-ID: <e16c21cd-41f3-4191-9957-6e61ddfefd24@linux.intel.com> (raw)
In-Reply-To: <ZtiMp39CWrI0e+GB@alphanet.ch>
[-- Attachment #1: Type: text/plain, Size: 5704 bytes --]
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];
[-- Attachment #2: 0001-xhci-Debug-patch-handle-halted-ep-if-TD-is-not-found.patch --]
[-- Type: text/x-patch, Size: 4408 bytes --]
From bf7bbf8dbf92dc06e108a103f5f01b3f416339da Mon Sep 17 00:00:00 2001
From: Mathias Nyman <mathias.nyman@linux.intel.com>
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@linux.intel.com>
---
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
next prev parent reply other threads:[~2024-09-05 13:50 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-23 19:18 Strange issues with USB device Marc SCHAEFER
2024-08-24 6:44 ` Michał Pecio
2024-08-24 6:51 ` Marc SCHAEFER
2024-08-24 8:44 ` Michał Pecio
2024-08-26 5:17 ` Marc SCHAEFER
2024-09-03 7:48 ` Strange issues with UAS URB cancellation Michał Pecio
2024-09-03 12:55 ` Marc SCHAEFER
2024-09-03 13:22 ` Michał Pecio
2024-09-03 13:50 ` Marc SCHAEFER
2024-09-03 13:52 ` Marc SCHAEFER
2024-09-03 13:55 ` Marc SCHAEFER
2024-09-03 15:45 ` Michał Pecio
2024-09-03 19:40 ` Marc SCHAEFER
2024-09-04 14:26 ` Mathias Nyman
2024-09-04 16:36 ` Marc SCHAEFER
2024-09-05 13:52 ` Mathias Nyman [this message]
2024-09-05 15:01 ` Marc SCHAEFER
2024-09-05 15:06 ` Marc SCHAEFER
2024-09-05 17:24 ` Marc SCHAEFER
2024-09-05 18:20 ` Marc SCHAEFER
2024-09-09 15:24 ` Mathias Nyman
2024-09-09 16:21 ` Marc SCHAEFER
2024-09-11 14:25 ` Mathias Nyman
2024-09-12 15:22 ` Mathias Nyman
2024-08-25 16:32 ` Strange issues with USB device Marc SCHAEFER
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e16c21cd-41f3-4191-9957-6e61ddfefd24@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=michal.pecio@gmail.com \
--cc=schaefer@alphanet.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.