Re: [PATCH 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 10/25/2018 6:58 PM, Star Zeng wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> Extract new XhciInsertAsyncIntTransfer function from
> XhcAsyncInterruptTransfer.
> 
> It is code preparation for following patch,
> no essential functional change.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>   MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c      | 18 +-------
>   MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 74 +++++++++++++++++++++++++++++---
>   MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 28 ++++++++++++
>   3 files changed, 98 insertions(+), 22 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index f1c60bef01c0..7f64f9c7c982 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -1346,7 +1346,6 @@ XhcAsyncInterruptTransfer (
>     EFI_STATUS              Status;
>     UINT8                   SlotId;
>     UINT8                   Index;
> -  UINT8                   *Data;
>     EFI_TPL                 OldTpl;
>   
>     //
> @@ -1413,36 +1412,21 @@ XhcAsyncInterruptTransfer (
>       goto ON_EXIT;
>     }
>   
> -  Data = AllocateZeroPool (DataLength);
> -
> -  if (Data == NULL) {
> -    DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to allocate buffer\n"));
> -    Status = EFI_OUT_OF_RESOURCES;
> -    goto ON_EXIT;
> -  }
> -
> -  Urb = XhcCreateUrb (
> +  Urb = XhciInsertAsyncIntTransfer (
>             Xhc,
>             DeviceAddress,
>             EndPointAddress,
>             DeviceSpeed,
>             MaximumPacketLength,
> -          XHC_INT_TRANSFER_ASYNC,
> -          NULL,
> -          Data,
>             DataLength,
>             CallBackFunction,
>             Context
>             );
> -
>     if (Urb == NULL) {
> -    DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to create URB\n"));
> -    FreePool (Data);
>       Status = EFI_OUT_OF_RESOURCES;
>       goto ON_EXIT;
>     }
>   
> -  InsertHeadList (&Xhc->AsyncIntTransfers, &Urb->UrbList);
>     //
>     // Ring the doorbell
>     //
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 166c44bf5e66..2d7c08dc5bfa 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -264,11 +264,11 @@ XhcCreateTransferTrb (
>     // No need to remap.
>     //
>     if ((Urb->Data != NULL) && (Urb->DataMap == NULL)) {
> -    if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
> -      MapOp = EfiPciIoOperationBusMasterWrite;
> -    } else {
> -      MapOp = EfiPciIoOperationBusMasterRead;
> -    }
> +      if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
> +        MapOp = EfiPciIoOperationBusMasterWrite;
> +      } else {
> +        MapOp = EfiPciIoOperationBusMasterRead;
> +      }

Unnecessary change, right?

>   
>       Len = Urb->DataLen;
>       Status  = Xhc->PciIo->Map (Xhc->PciIo, MapOp, Urb->Data, &Len, &PhyAddr, &Map);
> @@ -1411,6 +1411,70 @@ XhciDelAllAsyncIntTransfers (
>   }
>   
>   /**
> +  Insert a single asynchronous interrupt transfer for
> +  the device and endpoint.
> +
> +  @param Xhc            The XHCI Instance
> +  @param BusAddr        The logical device address assigned by UsbBus driver
> +  @param EpAddr         Endpoint addrress
> +  @param DevSpeed       The device speed
> +  @param MaxPacket      The max packet length of the endpoint
> +  @param DataLen        The length of data buffer
> +  @param Callback       The function to call when data is transferred
> +  @param Context        The context to the callback
> +
> +  @return Created URB or NULL
> +
> +**/
> +URB *
> +XhciInsertAsyncIntTransfer (
> +  IN USB_XHCI_INSTANCE                  *Xhc,
> +  IN UINT8                              BusAddr,
> +  IN UINT8                              EpAddr,
> +  IN UINT8                              DevSpeed,
> +  IN UINTN                              MaxPacket,
> +  IN UINTN                              DataLen,
> +  IN EFI_ASYNC_USB_TRANSFER_CALLBACK    Callback,
> +  IN VOID                               *Context
> +  )
> +{
> +  VOID      *Data;
> +  URB       *Urb;
> +
> +  Data = AllocateZeroPool (DataLen);
> +  if (Data == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));
> +    return NULL;
> +  }
> +
> +  Urb = XhcCreateUrb (
> +          Xhc,
> +          BusAddr,
> +          EpAddr,
> +          DevSpeed,
> +          MaxPacket,
> +          XHC_INT_TRANSFER_ASYNC,
> +          NULL,
> +          Data,
> +          DataLen,
> +          Callback,
> +          Context
> +          );
> +  if (Urb == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));

FreePool (Data) is needed.

> +    return NULL;
> +  }
> +
> +  //
> +  // New asynchronous transfer must inserted to the head.
> +  // Check the comments in XhcMoniteAsyncRequests
> +  //
> +  InsertHeadList (&Xhc->AsyncIntTransfers, &Urb->UrbList);
> +
> +  return Urb;
> +}
> +
> +/**
>     Update the queue head for next round of asynchronous transfer
>   
>     @param  Xhc     The XHCI Instance.
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> index 097408828a1f..cd1403f2842a 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> @@ -853,6 +853,34 @@ XhciDelAllAsyncIntTransfers (
>     );
>   
>   /**
> +  Insert a single asynchronous interrupt transfer for
> +  the device and endpoint.
> +
> +  @param Xhc            The XHCI Instance
> +  @param BusAddr        The logical device address assigned by UsbBus driver
> +  @param EpAddr         Endpoint addrress
> +  @param DevSpeed       The device speed
> +  @param MaxPacket      The max packet length of the endpoint
> +  @param DataLen        The length of data buffer
> +  @param Callback       The function to call when data is transferred
> +  @param Context        The context to the callback
> +
> +  @return Created URB or NULL
> +
> +**/
> +URB *
> +XhciInsertAsyncIntTransfer (
> +  IN USB_XHCI_INSTANCE                  *Xhc,
> +  IN UINT8                              DeviceAddress,
> +  IN UINT8                              EndPointAddress,
> +  IN UINT8                              DeviceSpeed,
> +  IN UINTN                              MaximumPacketLength,
> +  IN UINTN                              DataLength,
> +  IN EFI_ASYNC_USB_TRANSFER_CALLBACK    CallBackFunction,
> +  IN VOID                               *Context
> +  );
> +
> +/**
>     Set Bios Ownership
>   
>     @param  Xhc          The XHCI Instance.
> 


-- 
Thanks,
Ray