[PATCH] usb: ehci: Fix "EHCI timed out on TD - token=XXXX" error on ehci-hcd

[Marek Vasut]

On 3/1/20 6:19 PM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

>> On 2/26/20 12:29 PM, Lukasz Majewski wrote:
>>> This patch aims to improve robustness of 'usb' command operation on
>>> the ehci-hcd IP block as it ports to contemporary U-Boot the patch
>>> described and provided in [1] (originally applicable to U-Boot
>>> 2016.05).
>>>
>>> According to the fix author - "rayvt" (from [1]):  
>>
>> [...]
>>
>>> diff --git a/common/usb_storage.c b/common/usb_storage.c
>>> index 097b6729c1..48c8c2ae64 100644
>>> --- a/common/usb_storage.c
>>> +++ b/common/usb_storage.c
>>> @@ -111,6 +111,18 @@ int usb_stor_get_info(struct usb_device *dev,
>>> struct us_data *us, struct blk_desc *dev_desc);
>>>  int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
>>>  		      struct us_data *ss);
>>> +
>>> +#ifdef CONFIG_USB_EHCI_HCD
>>> +	/*
>>> +	 * The U-Boot EHCI driver can handle any transfer length
>>> as long as
>>> +	 * there is enough free heap space left, but the SCSI
>>> READ(10) and
>>> +	 * WRITE(10) commands are limited to 65535 blocks.
>>> +	 */
>>> +int usb_max_xfer_blk = 4096;
>>> +#else
>>> +int usb_max_xfer_blk = 20;
>>> +#endif  
>>
>> This all looks horribly wrong and exactly what
>> 7d6fd7f0ba71cd93d94079132f958d9630f27a89 and
>> 02b0e1a36c5bc20174299312556ec4e266872bd680 fixed properly.
>>
>> All those "dynamic reduction of transfer size" attempts are
>> nonsensical, the real solution (sadly) is to reduce the transfer size
>> to cater for the most limited devices and profile/fix the remaining
>> delays in the USB stack (which should have already been done, see the
>> commits above). This is also what the Linux USB stack does.
>>
>> What is the problem you are trying to solve here ?
> 
> As it was stated in the commit message:
> 
> On TPC70 (i.MX6Q) once per ~10 times (without this patch):
> 
> Bus usb at 2184200: USB EHCI 1.00
> scanning bus usb at 2184200 for devices... 2 USB Device(s) found
>        scanning usb for storage devices... 1 Storage Device(s) found
> EHCI timed out on TD - token=0x1f8c80
> 
> 
> The problem is that even with the mentioned fix in:
> 7d6fd7f0ba71cd93d94079132f958d9630f27a89 from time to time I do see the:
> 
> EHCI timed out on TD - token=0x1f8c80
> 
> error. The same issue was already reported by Tom.

OK. Why do you see the error ? Can you dig in further ?

> The description of the fix written by the original author:
> https://forum.doozan.com/read.php?3,35295,35295#msg-35295
> 
> states that the reduction of the transfer is done just for "spin
> up"/"detection" of the pen drive. The issue is that not all pen drive
> disks are able to be properly detected in the first place.

But the commit message claims this patch reduces the transfer rate by
20% always. So that's a NAK right away, sorry, you need to find a better
solution which doesn't have such an enormous impact.

I'm fine with the spin-up handling, but this auto-detection of transfer
size should be pulled into separate patch and is likely wrong anyway.

So try to separate this into two patches, one for handling the spin up,
another one for this auto-detection of the transfer size. And I think
you will end up needed only the first one.

> Last but not least - the mainline is still broken. The "token=0x1f8c80"
> errors pops up from time to time. However, after applying the approach
> from this fix - the error is gone (and pass some acceptance tests).

It works for me. Do you have a specific device which fails ? Which
device and on which hardware ?

Also, what is "some acceptance test" ?

-- 
Best regards,
Marek Vasut