Re: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg/Xhci: Fix TRT when data length is 0

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> wenyi,xie via groups.io
> Sent: Monday, May 31, 2021 11:18 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io; Wang, Jian J
> <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: songdongkuang@huawei.com
> Subject: Re: [edk2-devel] [PATCH EDK2 v1 1/1] MdeModulePkg/Xhci: Fix TRT
> when data length is 0
> 
> 
> 
> On 2021/5/31 9:44, Wu, Hao A wrote:
> >> -----Original Message-----
> >> From: Wenyi Xie <xiewenyi2@huawei.com>
> >> Sent: Thursday, May 27, 2021 8:04 PM
> >> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu,
> >> Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> >> Cc: songdongkuang@huawei.com; xiewenyi2@huawei.com
> >> Subject: [PATCH EDK2 v1 1/1] MdeModulePkg/Xhci: Fix TRT when data
> >> length is 0
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3418
> >>
> >> According to xhci spec, at USB packet level, a Control Transfer
> >> consists of multiple transactions partitioned into stages: a setup
> >> stage, an optional data stage, and a terminating status stage. If
> >> Data Stage does not exist, the Transfer Type flag(TRT) should be No Data
> Stage.
> >> So if data length equals to 0, TRT is set to 0.
> >
> >
> > Thanks for the patch, the changes are good to me.
> > Could you help to provide the information on what kind of test has been
> performed for this patch?
> >
> > Best Regards,
> > Hao Wu
> >
> Hi, Wu Hao
> 
> We use an AArch64 platform to test this patch. After merging this patch,
> during USB enumeration, Control Transfer with or without Data Stage works
> well. Our platform dsc only include XhciDxe.


Thanks a lot.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

I will wait for a couple of days to see if others have additional comments.

Best Regards,
Hao Wu


> 
> Thanks
> Wenyi
> >
> >>
> >> Cc: Jian J Wang <jian.j.wang@intel.com>
> >> Cc: Hao A Wu <hao.a.wu@intel.com>
> >> Cc: Ray Ni <ray.ni@intel.com>
> >> Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>
> >> ---
> >>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 13 +++++++++----
> >> MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 13 +++++++++----
> >>  2 files changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> >> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> >> index dc36945962a0..7cbc9a8502ea 100644
> >> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> >> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> >> @@ -298,10 +298,15 @@ XhcCreateTransferTrb (
> >>        TrbStart->TrbCtrSetup.IOC           = 1;
> >>        TrbStart->TrbCtrSetup.IDT           = 1;
> >>        TrbStart->TrbCtrSetup.Type          = TRB_TYPE_SETUP_STAGE;
> >> -      if (Urb->Ep.Direction == EfiUsbDataIn) {
> >> -        TrbStart->TrbCtrSetup.TRT = 3;
> >> -      } else if (Urb->Ep.Direction == EfiUsbDataOut) {
> >> -        TrbStart->TrbCtrSetup.TRT = 2;
> >> +      if (Urb->DataLen > 0) {
> >> +        if (Urb->Ep.Direction == EfiUsbDataIn) {
> >> +          TrbStart->TrbCtrSetup.TRT = 3;
> >> +        } else if (Urb->Ep.Direction == EfiUsbDataOut) {
> >> +          TrbStart->TrbCtrSetup.TRT = 2;
> >> +        } else {
> >> +          DEBUG ((DEBUG_ERROR, "XhcCreateTransferTrb: Direction
> >> + sholud be
> >> IN or OUT when Data exists!\n"));
> >> +          ASSERT (FALSE);
> >> +        }
> >>        } else {
> >>          TrbStart->TrbCtrSetup.TRT = 0;
> >>        }
> >> diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c
> >> b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c
> >> index 32d72ef03c2d..5b9892a1cbbb 100644
> >> --- a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c
> >> +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c
> >> @@ -291,10 +291,15 @@ XhcPeiCreateTransferTrb (
> >>        TrbStart->TrbCtrSetup.IOC           = 1;
> >>        TrbStart->TrbCtrSetup.IDT           = 1;
> >>        TrbStart->TrbCtrSetup.Type          = TRB_TYPE_SETUP_STAGE;
> >> -      if (Urb->Ep.Direction == EfiUsbDataIn) {
> >> -        TrbStart->TrbCtrSetup.TRT = 3;
> >> -      } else if (Urb->Ep.Direction == EfiUsbDataOut) {
> >> -        TrbStart->TrbCtrSetup.TRT = 2;
> >> +      if (Urb->DataLen > 0) {
> >> +        if (Urb->Ep.Direction == EfiUsbDataIn) {
> >> +          TrbStart->TrbCtrSetup.TRT = 3;
> >> +        } else if (Urb->Ep.Direction == EfiUsbDataOut) {
> >> +          TrbStart->TrbCtrSetup.TRT = 2;
> >> +        } else {
> >> +          DEBUG ((DEBUG_ERROR, "XhcPeiCreateTransferTrb: Direction
> >> + sholud
> >> be IN or OUT when Data exists!\n"));
> >> +          ASSERT (FALSE);
> >> +        }
> >>        } else {
> >>          TrbStart->TrbCtrSetup.TRT = 0;
> >>        }
> >> --
> >> 2.20.1.windows.1
> >
> > .
> >
> 
> 
> 
>