Re: [PATCH edk2-platforms 36/39] DWC3 : Add DWC3 USB controller initialization driver.

[Leif Lindholm <leif.lindholm@linaro.org>]

Minor comments below.

On Fri, Feb 16, 2018 at 02:20:32PM +0530, Meenakshi wrote:
> From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> 
> Add support of DWC3 controller driver which
> Performs DWC3 controller initialization and
> Register itself as NonDiscoverableMmioDevice
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c   | 219 +++++++++++++++++++++++++++
>  Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h   | 142 +++++++++++++++++
>  Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf |  48 ++++++
>  Silicon/NXP/NxpQoriqLs.dec                   |   5 +
>  4 files changed, 414 insertions(+)
>  create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
>  create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
>  create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
> 
> diff --git a/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
> new file mode 100644
> index 0000000..b08e19c
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
> @@ -0,0 +1,219 @@
> +/** @file
> +
> +  Copyright 2017 NXP
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Bitops.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +
> +#include "UsbHcd.h"
> +
> +STATIC
> +VOID
> +XhciSetBeatBurstLength (
> +  IN  UINTN  UsbReg
> +  )
> +{
> +  Dwc3       *Dwc3Reg;
> +
> +  Dwc3Reg = (VOID *)(UsbReg + DWC3_REG_OFFSET);
> +
> +  MmioAndThenOr32 ((UINTN)&Dwc3Reg->GSBusCfg0, ~USB3_ENABLE_BEAT_BURST_MASK,
> +                                              USB3_ENABLE_BEAT_BURST);
> +  MmioOr32 ((UINTN)&Dwc3Reg->GSBusCfg1, USB3_SET_BEAT_BURST_LIMIT);
> +
> +  return;
> +}
> +
> +STATIC
> +VOID
> +Dwc3SetFladj (
> +  IN  Dwc3   *Dwc3Reg,
> +  IN  UINT32 Val
> +  )
> +{
> +  MmioOr32 ((UINTN)&Dwc3Reg->GFLAdj, GFLADJ_30MHZ_REG_SEL |
> +                        GFLADJ_30MHZ(Val));
> +}
> +
> +VOID
> +Dwc3SetMode (
> +  IN  Dwc3   *Dwc3Reg,
> +  IN  UINT32 Mode
> +  )
> +{
> +  MmioAndThenOr32 ((UINTN)&Dwc3Reg->GCtl,
> +               ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG)),
> +               DWC3_GCTL_PRTCAPDIR(Mode));
> +}
> +
> +STATIC
> +VOID
> +Dwc3CoreSoftReset (
> +  IN  Dwc3   *Dwc3Reg
> +  )
> +{
> +  MmioOr32 ((UINTN)&Dwc3Reg->GCtl, DWC3_GCTL_CORESOFTRESET);
> +  MmioOr32 ((UINTN)&Dwc3Reg->GUsb3PipeCtl[0], DWC3_GUSB3PIPECTL_PHYSOFTRST);
> +  MmioOr32 ((UINTN)&Dwc3Reg->GUsb2PhyCfg, DWC3_GUSB2PHYCFG_PHYSOFTRST);
> +  MmioAnd32 ((UINTN)&Dwc3Reg->GUsb3PipeCtl[0], ~DWC3_GUSB3PIPECTL_PHYSOFTRST);
> +  MmioAnd32 ((UINTN)&Dwc3Reg->GUsb2PhyCfg, ~DWC3_GUSB2PHYCFG_PHYSOFTRST);
> +  MmioAnd32 ((UINTN)&Dwc3Reg->GCtl, ~DWC3_GCTL_CORESOFTRESET);
> +
> +  return;
> +}
> +
> +STATIC
> +EFI_STATUS
> +Dwc3CoreInit (
> +  IN  Dwc3   *Dwc3Reg
> +  )
> +{
> +  UINT32     Revision;
> +  UINT32     Reg;
> +  UINTN      Dwc3Hwparams1;
> +
> +  Revision = MmioRead32 ((UINTN)&Dwc3Reg->GSnpsId);
> +  //
> +  // This should read as 0x5533, ascii of U3(DWC_usb3) followed by revision number
> +  //
> +  if ((Revision & DWC3_GSNPSID_MASK) != DWC3_SYNOPSIS_ID) {

Should this be _SYNOPSYS_ rather than _SYNOPSIS_?

> +    DEBUG ((DEBUG_ERROR,"This is not a DesignWare USB3 DRD Core.\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Dwc3CoreSoftReset (Dwc3Reg);
> +
> +  Reg = MmioRead32 ((UINTN)&Dwc3Reg->GCtl);
> +  Reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> +  Reg &= ~DWC3_GCTL_DISSCRAMBLE;
> +
> +  Dwc3Hwparams1 = MmioRead32 ((UINTN)&Dwc3Reg->GHwParams1);
> +
> +  if (DWC3_GHWPARAMS1_EN_PWROPT(Dwc3Hwparams1) == DWC3_GHWPARAMS1_EN_PWROPT_CLK) {
> +    Reg &= ~DWC3_GCTL_DSBLCLKGTNG;
> +  } else {
> +    DEBUG ((DEBUG_ERROR,"No power optimization available.\n"));
> +  }
> +
> +  if ((Revision & DWC3_RELEASE_MASK) < DWC3_RELEASE_190a) {
> +    Reg |= DWC3_GCTL_U2RSTECN;
> +  }
> +
> +  MmioWrite32 ((UINTN)&Dwc3Reg->GCtl, Reg);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +XhciCoreInit (
> +  IN  UINTN  UsbReg
> +  )
> +{
> +  EFI_STATUS Status;
> +  Dwc3       *Dwc3Reg;
> +
> +  Dwc3Reg = (VOID *)(UsbReg + DWC3_REG_OFFSET);
> +
> +  Status = Dwc3CoreInit (Dwc3Reg);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Dwc3CoreInit Failed for controller 0x%x (0x%x) \n",
> +                  UsbReg, Status));
> +    return Status;
> +  }
> +
> +  Dwc3SetMode (Dwc3Reg, DWC3_GCTL_PRTCAP_HOST);
> +
> +  Dwc3SetFladj (Dwc3Reg, GFLADJ_30MHZ_DEFAULT);
> +
> +  return Status;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InitializeUsbController (
> +  IN  UINTN  UsbReg
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = XhciCoreInit (UsbReg);
> +
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Change beat burst and outstanding pipelined transfers requests
> +  //
> +  XhciSetBeatBurstLength (UsbReg);
> +
> +  return Status;
> +}
> +
> +/**
> +  The Entry Point of module. It follows the standard UEFI driver model.
> +
> +  @param[in] ImageHandle   The firmware allocated handle for the EFI image.
> +  @param[in] SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS      The entry point is executed successfully.
> +  @retval other            Some error occurs when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InitializeUsbHcd (
> +  IN EFI_HANDLE            ImageHandle,
> +  IN EFI_SYSTEM_TABLE      *SystemTable
> +  )
> +{
> +  EFI_STATUS               Status;
> +  UINT32                   NumUsbController;
> +  UINT32                   ControllerAddr;
> +
> +  Status = EFI_SUCCESS;
> +  NumUsbController = PcdGet32 (PcdNumUsbController);
> +
> +  while (NumUsbController) {
> +    NumUsbController--;
> +    ControllerAddr = PcdGet32 (PcdUsbBaseAddr) +
> +                     (NumUsbController * PcdGet32 (PcdUsbSize));
> +
> +    Status = InitializeUsbController (ControllerAddr);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "USB Controller initialization Failed for %d (0x%x)\n",
> +                            ControllerAddr, Status));
> +      continue;
> +    }
> +
> +    Status = RegisterNonDiscoverableMmioDevice (
> +               NonDiscoverableDeviceTypeXhci,
> +               NonDiscoverableDeviceDmaTypeNonCoherent,
> +               NULL,
> +               NULL,
> +               1,
> +               ControllerAddr, PcdGet32 (PcdUsbSize)
> +             );
> +
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "Failed to register USB device (0x%x) with error 0x%x \n",
> +                           ControllerAddr, Status));
> +    }
> +  }
> +
> +  return Status;
> +}
> diff --git a/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
> new file mode 100644
> index 0000000..3237f5d
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
> @@ -0,0 +1,142 @@
> +/** @file
> +
> +  Copyright 2017 NXP
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __USB_HCD__
> +#define __USB_HCD__
> +
> +/* Global constants */
> +#define DWC3_GSNPSID_MASK                      0xffff0000
> +#define DWC3_SYNOPSIS_ID                       0x55330000
> +#define DWC3_RELEASE_MASK                      0xffff
> +#define DWC3_REG_OFFSET                        0xC100
> +#define DWC3_RELEASE_190a                      0x190a
> +
> +/* Global Configuration Register */
> +#define DWC3_GCTL_U2RSTECN                     BIT(16)
> +#define DWC3_GCTL_PRTCAPDIR(n)                 ((n) << 12)
> +#define DWC3_GCTL_PRTCAP_HOST                  1
> +#define DWC3_GCTL_PRTCAP_OTG                   3
> +#define DWC3_GCTL_CORESOFTRESET                BIT(11)
> +#define DWC3_GCTL_SCALEDOWN(n)                 ((n) << 4)
> +#define DWC3_GCTL_SCALEDOWN_MASK               DWC3_GCTL_SCALEDOWN(3)
> +#define DWC3_GCTL_DISSCRAMBLE                  BIT(3)
> +#define DWC3_GCTL_DSBLCLKGTNG                  BIT(0)
> +
> +/* Global HWPARAMS1 Register */
> +#define DWC3_GHWPARAMS1_EN_PWROPT(n)           (((n) & (3 << 24)) >> 24)
> +#define DWC3_GHWPARAMS1_EN_PWROPT_CLK          1
> +
> +/* Global USB2 PHY Configuration Register */
> +#define DWC3_GUSB2PHYCFG_PHYSOFTRST            BIT(31)
> +
> +/* Global USB3 PIPE Control Register */
> +#define DWC3_GUSB3PIPECTL_PHYSOFTRST           BIT(31)
> +
> +/* Global Frame Length Adjustment Register */
> +#define GFLADJ_30MHZ_REG_SEL                   BIT(7)
> +#define GFLADJ_30MHZ(n)                        ((n) & 0x3f)
> +#define GFLADJ_30MHZ_DEFAULT                   0x20
> +
> +/* Default to the FSL XHCI defines */
> +#define USB3_ENABLE_BEAT_BURST                 0xF
> +#define USB3_ENABLE_BEAT_BURST_MASK            0xFF
> +#define USB3_SET_BEAT_BURST_LIMIT              0xF00
> +
> +typedef struct {
> +  UINT32 GEvntAdrLo;
> +  UINT32 GEvntAdrHi;
> +  UINT32 GEvntSiz;
> +  UINT32 GEvntCount;
> +} GEventBuffer;

Generally, typedeffed structs are given
UPPERCASE_NAMES_WITH_UNDERSCORES in edk2.
(Applies throughout.)

> +
> +typedef struct {
> +  UINT32 DDepCmdPar2;
> +  UINT32 DDepCmdPar1;
> +  UINT32 DDepCmdPar0;
> +  UINT32 DDepCmd;
> +} DPhysicalEndpoint;
> +
> +typedef struct {
> +  UINT32 GSBusCfg0;
> +  UINT32 GSBusCfg1;
> +  UINT32 GTxThrCfg;
> +  UINT32 GRxThrCfg;
> +  UINT32 GCtl;
> +  UINT32 Res1;
> +  UINT32 GSts;
> +  UINT32 Res2;
> +  UINT32 GSnpsId;
> +  UINT32 GGpio;
> +  UINT32 GUid;
> +  UINT32 GUctl;
> +  UINT64 GBusErrAddr;
> +  UINT64 GPrtbImap;
> +  UINT32 GHwParams0;
> +  UINT32 GHwParams1;
> +  UINT32 GHwParams2;
> +  UINT32 GHwParams3;
> +  UINT32 GHwParams4;
> +  UINT32 GHwParams5;
> +  UINT32 GHwParams6;
> +  UINT32 GHwParams7;
> +  UINT32 GDbgFifoSpace;
> +  UINT32 GDbgLtssm;
> +  UINT32 GDbgLnmcc;
> +  UINT32 GDbgBmu;
> +  UINT32 GDbgLspMux;
> +  UINT32 GDbgLsp;
> +  UINT32 GDbgEpInfo0;
> +  UINT32 GDbgEpInfo1;
> +  UINT64 GPrtbImapHs;
> +  UINT64 GPrtbImapFs;
> +  UINT32 Res3[28];
> +  UINT32 GUsb2PhyCfg[16];
> +  UINT32 GUsb2I2cCtl[16];
> +  UINT32 GUsb2PhyAcc[16];
> +  UINT32 GUsb3PipeCtl[16];
> +  UINT32 GTxFifoSiz[32];
> +  UINT32 GRxFifoSiz[32];
> +  GEventBuffer GEvntBuf[32];
> +  UINT32 GHwParams8;
> +  UINT32 Res4[11];
> +  UINT32 GFLAdj;
> +  UINT32 Res5[51];
> +  UINT32 DCfg;
> +  UINT32 DCtl;
> +  UINT32 DEvten;
> +  UINT32 DSts;
> +  UINT32 DGCmdPar;
> +  UINT32 DGCmd;
> +  UINT32 Res6[2];
> +  UINT32 DAlepena;
> +  UINT32 Res7[55];
> +  DPhysicalEndpoint DPhyEpCmd[32];
> +  UINT32 Res8[128];
> +  UINT32 OCfg;
> +  UINT32 OCtl;
> +  UINT32 OEvt;
> +  UINT32 OEvtEn;
> +  UINT32 OSts;
> +  UINT32 Res9[3];
> +  UINT32 AdpCfg;
> +  UINT32 AdpCtl;
> +  UINT32 AdpEvt;
> +  UINT32 AdpEvten;
> +  UINT32 BcCfg;
> +  UINT32 Res10;
> +  UINT32 BcEvt;
> +  UINT32 BcEvten;
> +} Dwc3;
> +
> +#endif
> diff --git a/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
> new file mode 100644
> index 0000000..cefb8bd
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
> @@ -0,0 +1,48 @@
> +#  UsbHcd.inf
> +#
> +#  Copyright 2017 NXP
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution. The full text of the license may be found
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +
> +[Defines]
> +  INF_VERSION                     = 0x0001000A

0x0001001a

/
    Leif

> +  BASE_NAME                       = UsbHcdDxe
> +  FILE_GUID                       = 196e7c2a-37b2-4b85-8683-718588952449
> +  MODULE_TYPE                     = DXE_DRIVER
> +  VERSION_STRING                  = 1.0
> +  ENTRY_POINT                     = InitializeUsbHcd
> +
> +[Sources.common]
> +  UsbHcd.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  IoLib
> +  MemoryAllocationLib
> +  NonDiscoverableDeviceRegistrationLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[FixedPcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController
> +  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize
> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index a3508b5..90e9957 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -83,6 +83,11 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdRomSize|0x0|UINT64|0x0000012D
>  
>    #
> +  # USB PCDs
> +  #
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController|0|UINT32|0x00000170
> +
> +  #
>    # PCI PCDs
>    #
>    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x000001D0
> -- 
> 1.9.1
>