On Sun, Oct 29, 2023 at 2:33 PM Peter Robinson <pbrobin...@gmail.com> wrote: > > On Sun, Oct 29, 2023 at 9:25 PM Marek Vasut <ma...@denx.de> wrote: > > > > 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. > > We see a bunch of spinning rust users in Fedora, these sorts of > devices are used by a bunch of people that want to run up cheap home > NAS devices so from experience I'd say while not usual to be USB > sticks and some form of solid state storage, spinning disk isn't > unusual.
What Peter said. A very common use case, even more so than USB flash drives, is for the consumers to plug in a USB HDD to their routers or home NAS, as a cheap and quick solution. I've seen this type of timeout failure happen quite often with large >= 3TB HDDs in USB enclosure. All the best, Tony > > > > 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 !