TRBPtr->Length can be used in case of short packet as well? It's same case statement for TRB_COMPLETION_SHORT_PACKET and TRB_COMPLETION_SUCCESS.
-----Original Message----- From: Tian, Feng [mailto:feng.t...@intel.com] Sent: Tuesday, April 21, 2015 2:29 AM To: Laszlo Ersek Cc: edk2-devel@lists.sourceforge.net; Anbazhagan, Baraneedharan; Tian, Feng Subject: RE: [edk2] XHCI data transfer size Laszlo, Please note the code is checking the field of Event TRBs, so you need to refer to Xhci 1.0 spec Table 85 for the meaning of TRB Transfer Length field. For your concern on Isoch Endpoint Error Handling, the Xhci spec states it's for Error case rather than success case. Thanks Feng -----Original Message----- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Tuesday, April 21, 2015 15:09 To: Tian, Feng Cc: edk2-devel@lists.sourceforge.net; Anbazhagan, Baraneedharan Subject: Re: [edk2] XHCI data transfer size On 04/21/15 05:04, Tian, Feng wrote: > Baranee & Laszlo, > > I agree this is a bug and I made a unit test on this logic and found why this > issue didn't get raised before is because Fat&DiskIo driver will split big > data transfer request (>64K) to multiple of 64K. This way will cause a > request, that is a URB, only needs a transfer TRB, so it will not return > wrong completed data length as the Urb->DataLen is only accumulated once for > every URB request. > > But if a URB request needs multiple TRBs, then the Urb->Completed is > calculated wrongly. > > Here is my proposed fix, could you help review it? > > Index: XhciSched.c > =================================================================== > --- XhciSched.c (revision 17189) > +++ XhciSched.c (working copy) > @@ -1137,7 +1137,7 @@ > if ((TRBType == TRB_TYPE_DATA_STAGE) || > (TRBType == TRB_TYPE_NORMAL) || > (TRBType == TRB_TYPE_ISOCH)) { > - CheckedUrb->Completed += (CheckedUrb->DataLen - EvtTrb->Lenth); > + CheckedUrb->Completed += > + (((TRANSFER_TRB_NORMAL*)TRBPtr)->Length - EvtTrb->Lenth); > } > > The TRBPtr->Length field means expected transfer length, and the > EvtTrb->Lenth means the residual number of bytes not transferred. Can you please explain why "EvtTrb->Lenth" is the residue (ie. the number of bytes *not* transferred) instead of the number of bytes that have been transferred? My understanding would be helped most if you could provide: - an explanation of the code that implies this (it's quite complicated code...) - and a reference from the XHCI spec that the code implements. I'm asking because (as I wrote earlier) the XHCI spec defines EvtTrb->Lenth with different meanings in different contexts. In some contexts it is indeed a residue, but in other contexts EvtTrb->Lenth is the number of bytes actually transferred. I'm unsure which case we have at hand. Thanks! Laszlo ------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
