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

[Ye Li]

Hi Marek,

On Tue, 2021-03-09 at 09:08 +0100, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/9/21 4:18 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.


> [...]
> 
> > 
> > > 
> > > > 
> > > > 
> > > > +             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.

Best regards,
Ye Li