[PATCH] usb: xhci: fix event trb handling missed

[Ran Wang]

Hi Bin,

On Tuesday, November 10, 2020 3:47 PM Bin Meng wrote:
> 
> Hi Ran,
<snip>
> > > > > send request in more than 1 Transfer TRB by chaining them, but
> > > > > then handle only 1 event TRB to mark request completed.
> > > > >
> > > > > However, on Layerscape platforms (LS1028A, LS1088A, etc), we
> > > > > observe xhci controller will generated more than 1 event TRB
> > > > > sometimes, this cause that
> > > >
> > > > I am not sure if it's fair to say this is Layerscape specific
> > > > behavior. Based on the xHCI spec, the spec indicates 1 event trb
> > > > should be generated when IOC/ISP flag is set to 1 or an error occurs.
> > >
> > > Ah, ISP flag is set if the pipe is from an IN endpoint. Currently we have:
> > >
> > >         /* Only set interrupt on short packet for IN endpoints */
> > >         if (usb_pipein(pipe))
> > >             field |= TRB_ISP;
> > >
> > > Can you verify that if removing the above codes, and without your
> > > changes in this patch, the original issue can be resolved on LS1028?
> >
> > Bingo, removing above codes can resolve my issue, too
> 
> Thank you for your testing.
> 
> >
> > > > I will see if I can reproduce your issue on an x86 board.
> > > >
> > >
> > > Note this patch does not apply on top of u-boot/master. Please rebase.
> >
> > Sure, I can rebase my patch, but I am nor sure my solution is still worth:
> > xHCI says "The detection of a USB Short Packet (i.e. the actual number
> > of bytes received was less than the expected number of bytes defined
> > by the Transfer TRB) during a transfer does not necessarily generate an Event.
> "
> >
> 
> Yes, that's what the spec says. So in your case, can you add some logs to verify
> that there is a transfer event trb generated by the xHC and the completion
> code is 13 (short packet)? You can add some debug output in
> record_transfer_result().

Sure, let me try this can get back to you later.

> > So does it mean above workaround you suggest would not violate spec,
> > either (although current Linux kernel driver still set ISP for this
> > case, but may have a more robust mechanism for event TRB handling)?
> 
> I need to do more testing to see if this is a LS1028 behavior or generic xHCI
> behavior, ie: on an x86 board. Do you happen to know if there is any errata
> document for LS1028 that is related to this?

I don’t see any errata for LS1028A for this case.

Actually this issue only randomly happens in U-Boot + USB dongle
(I have tried 3 different brands, such as TP-LINK UE300, etc).
And U-Boot + USB mass storage devicecannot reproduce this issue.

Under Linux, no such issue observed neither (with those dongles), all of them works fine.
That's why I decide to improve event TRB handling logic here.

Below is the U-Boot test commands I used (repeatedly do tftp file transfer), issue would happen in less 5 minutes.
for your reference:

setenv tt 'usb reset&&setenv ethact asix_eth&&tftp a0000000 tmp/1MB_test_file'
setenv ttt 'run tt&&run tt&&run tt&&run tt&&run tt&&run tt&&run tt&&run tt&&run tt&&run tt'
setenv tttt 'run ttt&&run ttt&&run ttt&&run ttt&&run ttt&&run ttt&&run ttt&&run ttt&&run ttt'
setenv u 'run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt&&run tttt'
run u

Regards,
Ran