[EXT] Re: [PATCH] usb: ehci-hcd: Add IAA handshake for removing async QH

[Marek Vasut]

On 3/9/21 10:32 AM, Ye Li wrote:
> Hi Marek,

Hi,

[...]

>>>>> +                     10 * 1000); /* 10ms timeout */
>>>>> +     if (ret < 0)
>>>>> +             printf("EHCI fail timeout STS_IAA set\n");
>>>> Shouldn't there be some abort ^ if ret < 0 ?
>>>> Also, add the return value into the printf, it is useful for
>>>> debugging.
>>> I don't think need to abort anything when ret < 0. The ret value is
>>> very simple in handshake, when ret < 0, it must be -1.
>>> Actually the codes refer the ehci_enable_async for the usage of
>>> handshake.
>> If the handshake timed out, that indicates a problem and that problem
>> shouldn't be ignored, but rather propagated, no ? Maybe the entire
>> ehci
>> transfer should be aborted ?
>>
> I supposed the problem means QH does not advance, so next submit of
> async transfer will have QTD timeout. But it is ok to abort transfer as
> the error handling.

If this is an error which would always cause subsequent error, abort 
here already.

>>>>> +             ehci_writel(&ctrl->hcor->or_usbsts, STS_IAA);
>>>>> +
>>>>> +     return ret;
>>>>> +}
>>>> Is this a bugfix for this release or is this for next release ?
>>>> We're in rc3 already, so I would suggest to be careful.
>>> It is ok to add the fix for next release. From our tests, the issue
>>> is
>>> not very easy to reproduce by just using usb read/write.  In
>>> SCT, generally we need to run about 1 hour to see the issue.
>> Can you add details on how to reproduce the issue into the commit
>> message ? That would be highly helpful.
> ok. I will add more.

Thank you