From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E938D2117B57A for ; Thu, 25 Oct 2018 22:01:59 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2018 22:01:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,426,1534834800"; d="scan'208";a="85714223" Received: from shzintpr01.sh.intel.com (HELO [10.7.209.51]) ([10.239.4.80]) by orsmga006.jf.intel.com with ESMTP; 25 Oct 2018 22:01:58 -0700 To: "Ni, Ruiyu" , edk2-devel@lists.01.org Cc: Hao Wu , Jiewen Yao , star.zeng@intel.com References: <1540465113-103964-1-git-send-email-star.zeng@intel.com> <1540465113-103964-2-git-send-email-star.zeng@intel.com> From: "Zeng, Star" Message-ID: <168c15a0-97b3-170f-e170-938637bb1b70@intel.com> Date: Fri, 26 Oct 2018 13:01:27 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Oct 2018 05:02:00 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit On 2018/10/26 12:57, Ni, Ruiyu wrote: > 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 >> Cc: Hao Wu >> Cc: Jian J Wang >> Cc: Jiewen Yao >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Star Zeng >> --- >>   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. Oh yes, although the final code after this patch series does not need it, but this patch needs it. I need add it, same for patch 2. Thanks for catching it. :) Thanks, Star > >> +    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. >> > >