[PATCH] fixup! usb: xhci: Guard all calls to xhci_wait_for_event

[Hector Martin]

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 <marex at 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. 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).

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, 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.

- Hector