On 10/27/23 08:03, Hector Martin wrote:
On 27/10/2023 09.36, Marek Vasut wrote:
On 10/27/23 01:26, Hector Martin wrote:
Gah, I should've paid more attention to that rebase. Here's a dumb
fixup for this patch. I'll squash it into a v2 if needed alongside
any other changes, or if it looks good feel free to apply/squash
it in directly.
---
drivers/usb/host/xhci-ring.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e2bd2e999a8e..7f2507be0cf0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -532,6 +532,7 @@ static void reset_ep(struct usb_device *udev, int ep_index)
event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
if (!event)
return;
+ field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
xhci_acknowledge_event(ctrl);
Please squash, and add
Reviewed-by: Marek Vasut <ma...@denx.de>
Also, +CC Minda,
there has been a similar fix to this one but with much more information
about the failure, see:
[PATCH v1] usb: xhci: Check return value of wait for TRB_TRANSFER event
I think it would be useful to somehow include that information, so it
wouldn't be lost.
The root cause for *that* failure is what I fix in patch #5. From that
thread:
scanning bus xhci_pci for devices... WARN halted endpoint, queueing
URB anyway.
Without #5 and without #1, that situation then leads to fireworks.
A bunch of things are broken, which is why this is a series and not a
single patch. I have more fixes to timeout handling, mass storage, etc.
coming up after this. I fixed most of this in one long day of trying
random USB devices, so it's not so much subtle super specific problems I
could document the crashes for as just wide-ranging problems in the
u-boot USB stack. None of this is particularly hard to repro if you just
try a bunch of normal consumer USB devices, including things like USB
HDDs which take time to spin-up leading to timeouts etc.
I think majority of users I can think of use USB mass storage devices,
like USB pen drives, not so much HDDs as far as I can tell.
The crash dumps
aren't particularly useful, it's the subtleties of the xHCI interactions
that are, but for that you need to add and enable a lot more debugging
(patch #8).
The crash dumps are more for posterity, when someone searches for a
problem they see. Search engines tend to pick those up and it might
point those people in the right direction.
Also, it is good to include information about what triggered the crash,
e.g. which USB device on which hardware, in case this is needed in the
future.
I think the main reason all this stuff is broken and we're finding out
now is that u-boot has traditionally been used in embedded devices with
fairly narrow use cases for USB
Yes
, and now we're running it on
workstation-grade laptops and desktops and people are plugging in all
kinds of normal USB devices and expecting their bootloader not to freak
out with them (even though most of the time it doesn't need to talk to
them). It's really easy to run into this stuff when that's your use case.
It is really appreciated to see people fix things like this stuff, thanks !
