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>]

Patch pushed via:
PR - https://github.com/tianocore/edk2/pull/1683
Commit - https://github.com/tianocore/edk2/commit/b5379899b38ed84561db6dc07dc4641a049ae238

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Monday, May 31, 2021 11:20 AM
> To: devel@edk2.groups.io; xiewenyi2@huawei.com; 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
> 
> > -----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
> > >
> > > .
> > >
> >
> >
> >
> >
> 
> 
> 
> 
>