Hi Ran, On Tue, Nov 10, 2020 at 4:31 PM Ran Wang <ran.wan...@nxp.com> wrote: > > Hi Bin, > > On Tuesday, November 10, 2020 4:06 PM Ran Wang wrote: > > 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. > > Confirm completion code is 13 when issue happen (please search ' > complete_code:13'):
Thank you very much for your testing. > > dev=00000000fbb4f3c0, pipe=c0010283, buffer=00000000fbb4fd80, > length=2048----------------------------buffer would cross 64KB boundary, so > we will send request in more than 1 TRB, this is the key issue trigger > condition > xhci_bulk_tx()#0.1.running_total:0x280 > xhci_bulk_tx()#0.2.trb_buff_len:0x280 > xhci_bulk_tx()#0.3.running_total:0x280 > xhci_bulk_tx()#0.4.num_trbs:0x2--------------------------2 Transfer TRB > xhci_bulk_tx()#0.5.running_total:0x10280 > xhci_bulk_tx()#0.start_trb:0x00000000fbb47140 > ----------xhci_bulk_tx()#0.&ring->enqueue->generic:0x00000000fbb47140----------Assemble > 1st Transfer TRB > xhci_bulk_tx()#0.addr:0xfbb4fd80 > xhci_bulk_tx()#0.trb_buff_len:0x280 > xhci_bulk_tx()#0.running_total:0x280 > xhci_bulk_tx()#0.length:0x800 > xhci_bulk_tx()#0.TRB_MAX_BUFF_SIZE:0x10000 > ----------xhci_bulk_tx()#0.&ring->enqueue->generic:0x00000000fbb47150----------Assemble > 2nd Transfer TRB > xhci_bulk_tx()#0.addr:0xfbb50000 > xhci_bulk_tx()#0.trb_buff_len:0x580 > xhci_bulk_tx()#0.running_total:0x800 > xhci_bulk_tx()#0.length:0x800 > xhci_bulk_tx()#0.TRB_MAX_BUFF_SIZE:0x10000 > xhci_bulk_tx()#0.event->trans_event.buffer:0x00000000fbb47140----------------handle > event TRB for 1st Transfer TRB > xhci_bulk_tx()#0.event->trans_event.transfer_len:0xd000180 > xhci_bulk_tx()#0.event->trans_event.flags:0x1058001 > xhci_bulk_tx()#0.event->len:384 > xhci_bulk_tx()#0.event->complete_code:13 > xhci_bulk_tx()#1.field:0x1058001 > xhci_bulk_tx()#2.TRB_TO_EP_ENDEX(field):0x4 > xhci_bulk_tx()#3.ep_index:0x4----------------------------------------------------------xhci_bulk_tx() > return > > dev=00000000fbb4f3c0, pipe=c0018203, buffer=00000000fbb2f9c0, > length=110----------an other call comming, and for different EP (ep_index = > 05) > xhci_bulk_tx()#0.1.running_total:0x640 > xhci_bulk_tx()#0.2.trb_buff_len:0x640 > xhci_bulk_tx()#0.3.running_total:0x640 > xhci_bulk_tx()#0.4.num_trbs:0x1 > xhci_bulk_tx()#0.5.running_total:0x640 > xhci_bulk_tx()#0.start_trb:0x00000000fbb47610 > ----------xhci_bulk_tx()#0.&ring->enqueue->generic:0x00000000fbb47610---------queue > 1st transfer TRB > xhci_bulk_tx()#0.addr:0xfbb2f9c0 > xhci_bulk_tx()#0.trb_buff_len:0x6e > xhci_bulk_tx()#0.running_total:0x6e > xhci_bulk_tx()#0.length:0x6e > xhci_bulk_tx()#0.TRB_MAX_BUFF_SIZE:0x10000 > xhci_bulk_tx()#0.event->trans_event.buffer:0x00000000fbb47150------------hand > event TRB, note that this buffer is for last Transfer TRB in last call (see > above '2nd Transfer TRB') > xhci_bulk_tx()#0.event->trans_event.transfer_len:0x1000000 > xhci_bulk_tx()#0.event->trans_event.flags:0x1058000 > xhci_bulk_tx()#0.event->len:0 > xhci_bulk_tx()#0.event->complete_code:1 > xhci_bulk_tx()#1.field:0x1058000 > xhci_bulk_tx()#2.TRB_TO_EP_ENDEX(field):0x4----------------ep_index is 4 > rather than 5, then cause BUG() > xhci_bulk_tx()#3.ep_index:0x5------------------------------------- > ep_index mis-match wait for event > again!!!!!!!!!---------------------------------- > BUG at drivers/usb/host/xhci-ring.c:759/xhci_bulk_tx()! > BUG! > ### ERROR ### Please RESET the board ### > Based on your log, it matches what I suspected and we are getting close to the root cause. Could you please try the following simple test case to see if it can trigger the issue with a USB disk? After U-Boot boot on LS1028, with a USB flash disk attached, then type "usb start" to see if the USB disk can be recognized by U-Boot? commit 60e68ed667362870c20b36ae26dacc1af903e81e Author: Bin Meng <bmeng...@gmail.com> Date: Tue Nov 10 16:19:06 2020 +0800 WIP: usb: A simple test case to trigger the TRB 64K boundary issue with mass storage device This should not be applied as it aims to provide a simple test case to trigger the TRB 64K boundary issue as reported by Ran Wang @ NXP. Signed-off-by: Bin Meng <bmeng...@gmail.com> diff --git a/common/usb_storage.c b/common/usb_storage.c index ff25441..c8aec2e 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -710,7 +710,15 @@ static int usb_stor_BBB_transport(struct scsi_cmd *srb, struct us_data *us) int dir_in; int actlen, data_actlen; unsigned int pipe, pipein, pipeout; +#if 0 ALLOC_CACHE_ALIGN_BUFFER(struct umass_bbb_csw, csw, 1); +#else + struct umass_bbb_csw *csw_org, *csw; + csw_org = malloc(0x10000 + UMASS_BBB_CSW_SIZE); + csw = (struct umass_bbb_csw *)roundup((ulong)csw_org, 0x10000); + csw = (struct umass_bbb_csw *)((ulong)csw - 2); + printf("csw org %p, adjusted to %p\n", csw_org, csw); +#endif #ifdef BBB_XPORT_TRACE unsigned char *ptr; int index; @@ -824,6 +832,7 @@ again: return USB_STOR_TRANSPORT_FAILED; } + free(csw_org); return result; } Regards, Bin