[Linaro-uefi,3/9] Drivers/Usb/DwUsb: support designware usb

Support Designware USB controller.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Drivers/Usb/DwUsbDxe/DwUsbDxe.c   | 796 ++++++++++++++++++++++++++++++++++++++
 Drivers/Usb/DwUsbDxe/DwUsbDxe.dec |  46 +++
 Drivers/Usb/DwUsbDxe/DwUsbDxe.h   | 627 ++++++++++++++++++++++++++++++
 Drivers/Usb/DwUsbDxe/DwUsbDxe.inf |  52 +++
 Include/Protocol/DwUsb.h          |  81 ++++
 5 files changed, 1602 insertions(+)
 create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.c
 create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
 create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.h
 create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
 create mode 100644 Include/Protocol/DwUsb.h

On Sat, Feb 04, 2017 at 01:13:48PM +0800, Haojian Zhuang wrote:
> Support Designware USB controller.

A bit more detailed commit message please - which USB controller,
which USB standards does it support?
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.c   | 796 ++++++++++++++++++++++++++++++++++++++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.dec |  46 +++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.h   | 627 ++++++++++++++++++++++++++++++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.inf |  52 +++
>  Include/Protocol/DwUsb.h          |  81 ++++
>  5 files changed, 1602 insertions(+)
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.c
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.h
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
>  create mode 100644 Include/Protocol/DwUsb.h
> 
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.c b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
> new file mode 100644
> index 0000000..bdd8405
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
> @@ -0,0 +1,796 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 <IndustryStandard/Usb.h>
> +#include <Library/ArmLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UncachedMemoryAllocationLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/BaseLib.h>
> +#include <Protocol/DwUsb.h>
> +#include <Protocol/UsbDevice.h>

Please sort include files alphabetically.

> +
> +#include "DwUsbDxe.h"
> +
> +#define USB_TYPE_LENGTH               16
> +#define USB_BLOCK_HIGH_SPEED_SIZE     512
> +#define DATA_SIZE                     32768
> +#define CMD_SIZE                      512
> +#define MATCH_CMD_LITERAL(Cmd, Buf)   !AsciiStrnCmp (Cmd, Buf, sizeof (Cmd) - 1)

Parentheses around all Cmd and Buf instances in macro please?

> +
> +// The time between interrupt polls, in units of 100 nanoseconds
> +// 10 Microseconds
> +#define DW_INTERRUPT_POLL_PERIOD      10000
> +
> +EFI_GUID  gDwUsbProtocolGuid = DW_USB_PROTOCOL_GUID;
> +
> +STATIC dwc_otg_dev_dma_desc_t *gDmaDesc,*gDmaDescEp0,*gDmaDescIn;
> +STATIC USB_DEVICE_REQUEST *gCtrlReq;
> +STATIC VOID *RxBuf;
> +STATIC UINTN RxDescBytes = 0;

Static varibles are automatically initialised to 0.

> +STATIC UINTN mNumDataBytes;
> +
> +STATIC DW_USB_PROTOCOL          *DwUsb;
> +
> +STATIC USB_DEVICE_DESCRIPTOR    *mDeviceDescriptor;
> +
> +// The config descriptor, interface descriptor, and endpoint descriptors in a
> +// buffer (in that order)
> +STATIC VOID                     *mDescriptors;
> +// Convenience pointers to those descriptors inside the buffer:
> +STATIC USB_INTERFACE_DESCRIPTOR *mInterfaceDescriptor;
> +STATIC USB_CONFIG_DESCRIPTOR    *mConfigDescriptor;
> +STATIC USB_ENDPOINT_DESCRIPTOR  *mEndpointDescriptors;
> +
> +STATIC USB_DEVICE_RX_CALLBACK   mDataReceivedCallback;
> +STATIC USB_DEVICE_TX_CALLBACK   mDataSentCallback;

This is a huge amount of global static variables - could they be
wrapped in a struct please? Also, this makes the driver hard-wired to
only be able to support a single controller.

> +
> +
> +/* To detect which mode was run, high speed or full speed */
> +STATIC
> +UINTN
> +UsbDrvPortSpeed (
> +  VOID
> +  )
> +{
> +  /*
> +  * 2'b00: High speed (PHY clock is running at 30 or 60 MHz)
> +  */
> +  UINT32          Val = READ_REG32 (DSTS) & 2;

No magic numbers in code plese.

> +  return (!Val);

No need for the temporary variable.

> +}
> +
> +STATIC
> +VOID
> +ResetEndpoints (
> +  VOID
> +  )
> +{
> +  /* EP0 IN ACTIVE NEXT=1 */
> +  WRITE_REG32 (DIEPCTL0, DXEPCTL_USBACTEP | BIT11);

Can you create a more descriptive #define for that BIT11?

> +
> +  /* EP0 OUT ACTIVE */
> +  WRITE_REG32 (DOEPCTL0, DXEPCTL_USBACTEP);
> +
> +  /* Clear any pending OTG Interrupts */
> +  WRITE_REG32 (GOTGINT, ~0);
> +
> +  /* Clear any pending interrupts */
> +  WRITE_REG32 (GINTSTS, ~0);
> +  WRITE_REG32 (DIEPINT0, ~0);
> +  WRITE_REG32 (DOEPINT0, ~0);
> +  WRITE_REG32 (DIEPINT1, ~0);
> +  WRITE_REG32 (DOEPINT1, ~0);
> +
> +  /* IN EP interrupt mask */
> +  WRITE_REG32 (DIEPMSK, DXEPMSK_TIMEOUTMSK | DXEPMSK_AHBERMSK | DXEPMSK_XFERCOMPLMSK);
> +  /* OUT EP interrupt mask */
> +  WRITE_REG32 (DOEPMSK, DXEPMSK_TIMEOUTMSK | DXEPMSK_AHBERMSK | DXEPMSK_XFERCOMPLMSK);
> +  /* Enable interrupts on Ep0 */
> +  WRITE_REG32 (DAINTMSK, (1 << DAINTMSK_OUTEPMSK_SHIFT) | (1 << DAINTMSK_INEPMSK_SHIFT));
> +
> +  /* EP0 OUT Transfer Size:64 Bytes, 1 Packet, 3 Setup Packet, Read to receive setup packet*/
> +  WRITE_REG32 (DOEPTSIZ0, DXEPTSIZ_SUPCNT(3) | DXEPTSIZ_PKTCNT(1) | DXEPTSIZ_XFERSIZE(64));
> +
> +  //notes that:the compulsive conversion is expectable.
> +  gDmaDescEp0->status.b.bs = 0x3;
> +  gDmaDescEp0->status.b.mtrf = 0;
> +  gDmaDescEp0->status.b.sr = 0;
> +  gDmaDescEp0->status.b.l = 1;
> +  gDmaDescEp0->status.b.ioc = 1;
> +  gDmaDescEp0->status.b.sp = 0;
> +  gDmaDescEp0->status.b.bytes = 64;
> +  gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);

Please drop the brackets around gCtrlReg.
(Also future instances, including gDmaDescEp0 below.)

> +  gDmaDescEp0->status.b.sts = 0;
> +  gDmaDescEp0->status.b.bs = 0x0;

A lot of magic numbers above - please replace with #defines.

> +  WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
> +  /* EP0 OUT ENABLE CLEARNAK */
> +  WRITE_REG32 (DOEPCTL0, (READ_REG32 (DOEPCTL0) | DXEPCTL_EPENA | DXEPCTL_CNAK));
> +}
> +
> +STATIC
> +VOID
> +EpTx (
> +  IN UINT8          Ep,

Is Ep shohrt for EndPoint? If so, please expand to the longer version.
Preferably also in the function name. And for other functions in this
driver.

> +  IN CONST VOID    *Ptr,
> +  IN UINTN          Len
> +  )
> +{
> +    UINT32          BlockSize;
> +    UINT32          Packets;
> +
> +    /* EPx OUT ACTIVE */
> +    WRITE_REG32 (DIEPCTL (Ep), (READ_REG32 (DIEPCTL (Ep))) | DXEPCTL_USBACTEP);
> +    if (!Ep) {
> +        BlockSize = 64;
> +    } else {
> +        BlockSize = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;
> +    }
> +    Packets = (Len + BlockSize - 1) / BlockSize;
> +
> +    if (!Len) {
> +      /* send one empty packet */
> +      gDmaDescIn->status.b.bs = 0x3;
> +      gDmaDescIn->status.b.l = 1;
> +      gDmaDescIn->status.b.ioc = 1;
> +      gDmaDescIn->status.b.sp = 1;
> +      gDmaDescIn->status.b.bytes = 0;
> +      gDmaDescIn->buf = 0;
> +      gDmaDescIn->status.b.sts = 0;
> +      gDmaDescIn->status.b.bs = 0x0;

More magic numbers here, please replace with #defines.

> +
> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));             // DMA Address (DMAAddr) is zero
> +    } else {
> +      WRITE_REG32 (DIEPTSIZ (Ep), Len | (Packets << 19));

"Len | (Packets << 19)" would be better to replace with a macro with a
desriptive name.

> +
> +      //flush cache
> +      WriteBackDataCacheRange ((VOID *)Ptr, Len);
> +
> +      gDmaDescIn->status.b.bs = 0x3;
> +      gDmaDescIn->status.b.l = 1;
> +      gDmaDescIn->status.b.ioc = 1;
> +      gDmaDescIn->status.b.sp = 1;
> +      gDmaDescIn->status.b.bytes = Len;
> +      gDmaDescIn->buf = (UINT32)((UINTN)Ptr);
> +      gDmaDescIn->status.b.sts = 0;
> +      gDmaDescIn->status.b.bs = 0x0;

Magic numbers.

> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));         // Ptr is DMA address
> +    }
> +    ArmDataSynchronizationBarrier ();
> +    /* epena & cnak */
> +    WRITE_REG32 (DIEPCTL (Ep), READ_REG32 (DIEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK | BIT11);
> +}
> +
> +STATIC
> +VOID
> +EpRx (
> +  IN UINTN            Ep,
> +  IN UINTN            Len
> +  )
> +{
> +  /* EPx UNSTALL */
> +  WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & (~DXEPCTL_STALL)));
> +  /* EPx OUT ACTIVE */
> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_USBACTEP));

Some macros would help make the above more readable.

> +
> +  if (Len >= DATA_SIZE) {
> +    RxDescBytes = DATA_SIZE;
> +  } else {
> +    RxDescBytes = Len;
> +  }
> +
> +  RxBuf = AllocatePool (DATA_SIZE);
> +  ASSERT (RxBuf != NULL);

Surely this should return rather than progress if running a RELEASE
build?

> +
> +  InvalidateDataCacheRange (RxBuf, Len);
> +
> +  gDmaDesc->status.b.bs = 0x3;
> +  gDmaDesc->status.b.mtrf = 0;
> +  gDmaDesc->status.b.sr = 0;
> +  gDmaDesc->status.b.l = 1;
> +  gDmaDesc->status.b.ioc = 1;
> +  gDmaDesc->status.b.sp = 0;
> +  gDmaDesc->status.b.bytes = (UINT32)RxDescBytes;
> +  gDmaDesc->buf = (UINT32)((UINTN)RxBuf);
> +  gDmaDesc->status.b.sts = 0;
> +  gDmaDesc->status.b.bs = 0x0;

Lot of magic values.

> +
> +  ArmDataSynchronizationBarrier ();
> +  WRITE_REG32 (DOEPDMA (Ep), (UINT32)((UINTN)gDmaDesc));
> +  /* EPx OUT ENABLE CLEARNAK */
> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK));

Macros would help again, here.

> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleGetDescriptor (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  UINT8       DescriptorType;
> +  UINTN       ResponseSize;
> +  VOID       *ResponseData;
> +  EFI_USB_STRING_DESCRIPTOR        *Descriptor = NULL;
> +  UINTN                             DescriptorSize;
> +
> +  ResponseSize = 0;
> +  ResponseData = NULL;
> +
> +  // Pretty confused if bmRequestType is anything but this:
> +  ASSERT (Request->RequestType == USB_DEV_GET_DESCRIPTOR_REQ_TYPE);

Return error instead?

> +
> +  // Choose the response
> +  DescriptorType = Request->Value >> 8;
> +  switch (DescriptorType) {
> +  case USB_DESC_TYPE_DEVICE:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for device descriptor\n"));

"USB" is too generic an identifier for an error message.
"DwUsbDxe" would be more appropriate for this driver, but preferably
as a #define, and maybe hidden behind a local macro.
(Comment applies globally to debug printouts in this driver.)

> +    ResponseSize = sizeof (USB_DEVICE_DESCRIPTOR);
> +    ResponseData = mDeviceDescriptor;
> +    break;
> +  case USB_DESC_TYPE_CONFIG:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for config descriptor\n"));
> +    ResponseSize = mConfigDescriptor->TotalLength;
> +    ResponseData = mDescriptors;
> +    break;
> +  case USB_DESC_TYPE_STRING:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for String descriptor %d\n", Request->Value & 0xFF));
> +    switch (Request->Value & 0xff) {

((x)->Value & 0xff) as a macro?

> +    case 0:

Please use defines rather than magic numbers.

> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       LANG_LENGTH * sizeof (CHAR16) + 1;

Why "+ 1"? If CHAR16, should it not be "+ 2", or even better
"+ sizeof (CHAR16)"? Same pattern repeats in many places in the file.

> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);

return error

> +      Descriptor->Length = LANG_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetLang (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 1:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);

return error
I will stop repeating myself here. But please address: all ASSERTs
that check whether a pointer is NULL before dereferencing it must
return an error instead of triggering a synchronous exception at
runtime in RELEASE builds.

> +      Descriptor->Length = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetManuFacturer (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 2:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       PRODUCT_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = PRODUCT_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetProduct (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 3:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       SERIAL_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = SERIAL_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetSerialNo (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    }
> +    break;
> +  default:
> +    DEBUG ((DEBUG_INFO, "USB: Didn't understand request for descriptor 0x%04x\n", Request->Value));
> +    break;
> +  }
> +
> +  // Send the response
> +  if (ResponseData) {
> +    ASSERT (ResponseSize != 0);

ResponseSize is set explicitly by the switch statement above.
Rather than ASSERTing that it went wrong, why not return an error from
the default target?
You could then also drop the "if (ResponseData)" test, and the "if
(Descriptor)" test below.

> +
> +    if (Request->Length < ResponseSize) {
> +      // Truncate response
> +      ResponseSize = Request->Length;
> +    } else if (Request->Length > ResponseSize) {
> +      DEBUG ((DEBUG_INFO, "USB: Info: ResponseSize < wLength\n"));
> +    }
> +
> +    EpTx (0, ResponseData, ResponseSize);
> +  }
> +  if (Descriptor) {

Here.

> +    FreePool (Descriptor);
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleSetAddress (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  // Pretty confused if bmRequestType is anything but this:

So is there any reason not to return an error?

> +  ASSERT (Request->RequestType == USB_DEV_SET_ADDRESS_REQ_TYPE);
> +  DEBUG ((DEBUG_INFO, "USB: Setting address to %d\n", Request->Value));
> +  ResetEndpoints ();
> +
> +  WRITE_REG32 (DCFG, (READ_REG32 (DCFG) & ~DCFG_DEVADDR_MASK) | DCFG_DEVADDR(Request->Value));
> +  EpTx (0, 0, 0);

What is the significance of transmitting 0 bytes of data?
Probably this should be replaced by a macro. And that macro should use
NULL, not 0, for the invalid pointer.

> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +UINTN
> +UsbDrvRequestEndpoint (
> +  IN UINTN           Type,
> +  IN UINTN           Dir
> +  )
> +{
> +  UINTN              Ep = 1;
> +  UINTN              Ret, NewBits;
> +
> +  Ret = Ep | Dir;
> +  NewBits = (Type << 18) | 0x10000000;

No magic values, please. Replace with macro(s).

> +
> +  /*
> +   * (Type << 18):Endpoint Type (EPType)
> +   * 0x10000000:Endpoint Enable (EPEna)
> +   * 0x000C000:Endpoint Type (EPType);Hardcoded to 00 for control.
> +   * (ep<<22):TxFIFO Number (TxFNum)
> +   * 0x20000:NAK Status (NAKSts);The core is transmitting NAK handshakes on this endpoint.
> +   */
> +  if (Dir) {  // IN: to host
> +    WRITE_REG32 (DIEPCTL (Ep), ((READ_REG32 (DIEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits | (Ep << 22) | DXEPCTL_NAKSTS);
> +  } else {    // OUT: to device
> +    WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits);

More macros, please. There WRITE_REG32 lines are unreadable.

> +  }
> +
> +  return Ret;
> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleSetConfiguration (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  ASSERT (Request->RequestType == USB_DEV_SET_CONFIGURATION_REQ_TYPE);
> +
> +  // Cancel all transfers
> +  ResetEndpoints ();
> +
> +  UsbDrvRequestEndpoint (2, 0);
> +  UsbDrvRequestEndpoint (2, 0x80);

No magic values, please.

> +
> +  WRITE_REG32 (DIEPCTL1, (READ_REG32 (DIEPCTL1)) | BIT28 | BIT19 | DXEPCTL_USBACTEP | BIT11);

What is this operation doing?
Please simplify with a temporary variable and some more descriptive
#defines.

> +
> +  /* Enable interrupts on all endpoints */
> +  WRITE_REG32 (DAINTMSK, ~0);
> +
> +  EpRx (1, CMD_SIZE);

The global variables also makes this code less readable.

> +  EpTx (0, 0, 0);
> +  return EFI_SUCCESS;
> +}
> +
> +
> +STATIC
> +EFI_STATUS
> +HandleDeviceRequest (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  switch (Request->Request) {
> +  case USB_DEV_GET_DESCRIPTOR:
> +    Status = HandleGetDescriptor (Request);
> +    break;
> +  case USB_DEV_SET_ADDRESS:
> +    Status = HandleSetAddress (Request);
> +    break;
> +  case USB_DEV_SET_CONFIGURATION:
> +    Status = HandleSetConfiguration (Request);
> +    break;
> +  default:
> +    DEBUG ((DEBUG_ERROR,
> +      "Didn't understand RequestType 0x%x Request 0x%x\n",
> +      Request->RequestType, Request->Request));
> +      Status = EFI_INVALID_PARAMETER;
> +    break;
> +  }
> +
> +  return Status;
> +}
> +
> +
> +// Instead of actually registering interrupt handlers, we poll the controller's
> +//  interrupt source register in this function.
> +STATIC
> +VOID
> +CheckInterrupts (
> +  IN EFI_EVENT        Event,
> +  IN VOID            *Context
> +  )
> +{
> +  UINT32              Ints, EpInts;
> +
> +
> +  // interrupt register
> +  Ints = READ_REG32 (GINTSTS);
> +
> +  /*
> +   * bus reset
> +   * The core sets this bit to indicate that a reset is detected on the USB.
> +   */
> +  if (Ints & GINTSTS_USBRST) {
> +    WRITE_REG32 (DCFG, DCFG_DESCDMA | DCFG_NZ_STS_OUT_HSHK);
> +    ResetEndpoints ();
> +  }
> +
> +  /*
> +   * enumeration done, we now know the speed
> +   * The core sets this bit to indicate that speed enumeration is complete. The
> +   * application must read the Device Status (DSTS) register to obtain the
> +   * enumerated speed.
> +   */
> +  if (Ints & GINTSTS_ENUMDONE) {
> +    /* Set up the maximum packet sizes accordingly */
> +    UINTN MaxPacket = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;

A #define for that 64 please.

> +    //Set Maximum In Packet Size (MPS)
> +    WRITE_REG32 (DIEPCTL1, ((READ_REG32 (DIEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);
> +    //Set Maximum Out Packet Size (MPS)
> +    WRITE_REG32 (DOEPCTL1, ((READ_REG32 (DOEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);

This seems to be a recurring pattern, so should there be a macro
UPDATE_REG32 (Regname, Mask, Value) ?

Alternatively, please use a temporary variable to do the read separately.

> +  }
> +
> +  /*
> +   * IN EP event
> +   * The core sets this bit to indicate that an interrupt is pending on one of the IN
> +   * endpoInts of the core (in Device mode). The application must read the
> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
> +   * number of the IN endpoint on which the interrupt occurred, and then read
> +   * the corresponding Device IN Endpoint-n Interrupt (DIEPINTn) register to
> +   * determine the exact cause of the interrupt. The application must clear the
> +   * appropriate status bit in the corresponding DIEPINTn register to clear this bit.
> +   */
> +  if (Ints & GINTSTS_IEPINT) {
> +    EpInts = READ_REG32 (DIEPINT0);
> +    WRITE_REG32 (DIEPINT0, EpInts);
> +    if (EpInts & DXEPINT_XFERCOMPL) {
> +      DEBUG ((DEBUG_INFO, "INT: IN TX completed.DIEPTSIZ (0) = 0x%x.\n", READ_REG32 (DIEPTSIZ0)));
> +    }
> +
> +    EpInts = READ_REG32 (DIEPINT1);
> +    WRITE_REG32 (DIEPINT1, EpInts);
> +    if (EpInts & DXEPINT_XFERCOMPL) {
> +      DEBUG ((DEBUG_INFO, "ep1: IN TX completed\n"));
> +    }
> +  }

Turn into helper function, please.

> +
> +  /*
> +   * OUT EP event
> +   * The core sets this bit to indicate that an interrupt is pending on one of the
> +   * OUT endpoints of the core (in Device mode). The application must read the
> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
> +   * number of the OUT endpoint on which the interrupt occurred, and then read
> +   * the corresponding Device OUT Endpoint-n Interrupt (DOEPINTn) register
> +   * to determine the exact cause of the interrupt. The application must clear the
> +   * appropriate status bit in the corresponding DOEPINTn register to clear this bit.
> +   */
> +  if (Ints & GINTSTS_OEPINT) {
> +    /* indicates the status of an endpoint
> +     * with respect to USB- and AHB-related events. */
> +    EpInts = READ_REG32 (DOEPINT0);
> +    if (EpInts) {
> +      WRITE_REG32 (DOEPINT0, EpInts);
> +      if (EpInts & DXEPINT_XFERCOMPL) {
> +        DEBUG ((DEBUG_INFO, "INT: EP0 RX completed. DOEPTSIZ(0) = 0x%x.\n", READ_REG32 (DOEPTSIZ0)));
> +      }
> +      /*
> +       *
> +       IN Token Received When TxFIFO is Empty (INTknTXFEmp)
> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
> +       * was received.
> +       */
> +      if (EpInts & BIT3) { /* SETUP phase done */
> +        WRITE_REG32 (DIEPCTL0, READ_REG32 (DIEPCTL0) | DXEPCTL_SNAK);
> +        WRITE_REG32 (DOEPCTL0, READ_REG32 (DOEPCTL0) | DXEPCTL_SNAK);
> +        /*clear IN EP intr*/
> +        WRITE_REG32 (DIEPINT0, ~0);
> +        HandleDeviceRequest((USB_DEVICE_REQUEST *)gCtrlReq);
> +      }
> +
> +      /* Make sure EP0 OUT is set up to accept the next request */
> +      WRITE_REG32 (DOEPTSIZ0, DXEPTSIZ_SUPCNT(3) | DXEPTSIZ_PKTCNT(1) | DXEPTSIZ_XFERSIZE(64));
> +      /*
> +       * IN Token Received When TxFIFO is Empty (INTknTXFEmp)
> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
> +       * was received.
> +       */
> +      gDmaDescEp0->status.b.bs = 0x3;
> +      gDmaDescEp0->status.b.mtrf = 0;
> +      gDmaDescEp0->status.b.sr = 0;
> +      gDmaDescEp0->status.b.l = 1;
> +      gDmaDescEp0->status.b.ioc = 1;
> +      gDmaDescEp0->status.b.sp = 0;
> +      gDmaDescEp0->status.b.bytes = 64;
> +      gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);
> +      gDmaDescEp0->status.b.sts = 0;
> +      gDmaDescEp0->status.b.bs = 0x0;
> +      WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
> +      // endpoint enable; clear NAK
> +      WRITE_REG32 (DOEPCTL0, DXEPCTL_EPENA | DXEPCTL_CNAK);
> +    }
> +
> +    EpInts = (READ_REG32 (DOEPINT1));
> +    if (EpInts) {
> +      WRITE_REG32 (DOEPINT1, EpInts);
> +      /* Transfer Completed Interrupt (XferCompl);Transfer completed */
> +      if (EpInts & DXEPINT_XFERCOMPL) {
> +
> +        UINTN Bytes = RxDescBytes - gDmaDesc->status.b.bytes;
> +        UINTN Len = 0;
> +
> +        ArmDataSynchronizationBarrier ();
> +        if (MATCH_CMD_LITERAL ("download", RxBuf)) {
> +          mNumDataBytes = AsciiStrHexToUint64 (RxBuf + sizeof ("download"));
> +        } else {
> +          if (mNumDataBytes != 0) {
> +            mNumDataBytes -= Bytes;
> +          }
> +        }
> +
> +        mDataReceivedCallback (Bytes, RxBuf);
> +
> +        if (mNumDataBytes == 0) {
> +          Len = CMD_SIZE;
> +        } else if (mNumDataBytes > DATA_SIZE) {
> +          Len = DATA_SIZE;
> +        } else {
> +          Len = mNumDataBytes;
> +        }
> +
> +        EpRx (1, Len);
> +      }
> +    }
> +  }

Turn into helper function, please.

> +
> +  //WRITE_REG32 clear ints
> +  WRITE_REG32 (GINTSTS, Ints);
> +}
> +
> +EFI_STATUS
> +DwUsbSend (
> +  IN        UINT8  EndpointIndex,
> +  IN        UINTN  Size,
> +  IN  CONST VOID  *Buffer
> +  )
> +{
> +    EpTx (EndpointIndex, Buffer, Size);
> +    return EFI_SUCCESS;
> +}

What is the purpose of this function?

> +
> +STATIC
> +VOID
> +DwUsbInit (
> +  VOID
> +  )
> +{
> +  VOID     *Buf;
> +  UINT32   Data;
> +
> +  Buf = UncachedAllocatePages (16);

No magic values, please.

> +  gDmaDesc = Buf;
> +  gDmaDescEp0 = gDmaDesc + sizeof (dwc_otg_dev_dma_desc_t);
> +  gDmaDescIn = gDmaDescEp0 + sizeof (dwc_otg_dev_dma_desc_t);
> +  gCtrlReq = (USB_DEVICE_REQUEST *)gDmaDescIn + sizeof (dwc_otg_dev_dma_desc_t);
> +
> +  ZeroMem (gDmaDesc, sizeof (dwc_otg_dev_dma_desc_t));
> +  ZeroMem (gDmaDescEp0, sizeof (dwc_otg_dev_dma_desc_t));
> +  ZeroMem (gDmaDescIn, sizeof (dwc_otg_dev_dma_desc_t));

Why not zero all of Buf once?

> +
> +  /*Reset usb controller.*/
> +  /* Wait for OTG AHB master idle */
> +  do {
> +    Data = READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE;
> +  } while (Data == 0);
> +
> +  /* OTG: Assert Software Reset */
> +  WRITE_REG32 (GRSTCTL, GRSTCTL_CSFTRST);
> +
> +  /* Wait for OTG to ack reset */
> +  while (READ_REG32 (GRSTCTL) & GRSTCTL_CSFTRST);
> +
> +  /* Wait for OTG AHB master idle */
> +  while ((READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE) == 0);

Second use of an explicit loop on this in this function - and coded
differently to the first one. Turn into a macro instead?

> +
> +  WRITE_REG32 (GDFIFOCFG, DATA_FIFO_CONFIG);
> +  WRITE_REG32 (GRXFSIZ, RX_SIZE);
> +  WRITE_REG32 (GNPTXFSIZ, ENDPOINT_TX_SIZE);
> +  WRITE_REG32 (DIEPTXF1, DATA_IN_ENDPOINT_TX_FIFO1);
> +  WRITE_REG32 (DIEPTXF2, DATA_IN_ENDPOINT_TX_FIFO2);
> +  WRITE_REG32 (DIEPTXF3, DATA_IN_ENDPOINT_TX_FIFO3);
> +  WRITE_REG32 (DIEPTXF4, DATA_IN_ENDPOINT_TX_FIFO4);
> +  WRITE_REG32 (DIEPTXF5, DATA_IN_ENDPOINT_TX_FIFO5);
> +  WRITE_REG32 (DIEPTXF6, DATA_IN_ENDPOINT_TX_FIFO6);
> +  WRITE_REG32 (DIEPTXF7, DATA_IN_ENDPOINT_TX_FIFO7);
> +  WRITE_REG32 (DIEPTXF8, DATA_IN_ENDPOINT_TX_FIFO8);
> +  WRITE_REG32 (DIEPTXF9, DATA_IN_ENDPOINT_TX_FIFO9);
> +  WRITE_REG32 (DIEPTXF10, DATA_IN_ENDPOINT_TX_FIFO10);
> +  WRITE_REG32 (DIEPTXF11, DATA_IN_ENDPOINT_TX_FIFO11);
> +  WRITE_REG32 (DIEPTXF12, DATA_IN_ENDPOINT_TX_FIFO12);
> +  WRITE_REG32 (DIEPTXF13, DATA_IN_ENDPOINT_TX_FIFO13);
> +  WRITE_REG32 (DIEPTXF14, DATA_IN_ENDPOINT_TX_FIFO14);
> +  WRITE_REG32 (DIEPTXF15, DATA_IN_ENDPOINT_TX_FIFO15);
> +
> +  /*
> +   * set Periodic TxFIFO Empty Level,
> +   * Non-Periodic TxFIFO Empty Level,
> +   * Enable DMA, Unmask Global Intr
> +   */
> +  WRITE_REG32 (GAHBCFG, GAHBCFG_CTRL_MASK);
> +
> +  /*select 8bit UTMI+, ULPI Inerface*/
> +  WRITE_REG32 (GUSBCFG, 0x2400);

No magic numbers, please.

> +
> +  /* Detect usb work mode,host or device? */
> +  do {
> +    Data = READ_REG32 (GINTSTS);
> +  } while (Data & GINTSTS_CURMODE_HOST);
> +  MicroSecondDelay (3);

Why a delay? Why 3us? Is a barrier needed?

> +
> +  /*Init global and device mode csr register.*/
> +  /*set Non-Zero-Length status out handshake */
> +  Data = (0x20 << DCFG_EPMISCNT_SHIFT) | DCFG_NZ_STS_OUT_HSHK;

No magic values, please.

> +  WRITE_REG32 (DCFG, Data);
> +
> +  /* Interrupt unmask: IN event, OUT event, bus reset */

(And enumeration complete?)

> +  Data = GINTSTS_OEPINT | GINTSTS_IEPINT | GINTSTS_ENUMDONE | GINTSTS_USBRST;
> +  WRITE_REG32 (GINTMSK, Data);
> +
> +  do {
> +    Data = READ_REG32 (GINTSTS) & GINTSTS_ENUMDONE;
> +  } while (Data);
> +
> +  /* Clear any pending OTG Interrupts */
> +  WRITE_REG32 (GOTGINT, ~0);
> +  /* Clear any pending interrupts */
> +  WRITE_REG32 (GINTSTS, ~0);
> +  WRITE_REG32 (GINTMSK, ~0);
> +  Data = READ_REG32 (GOTGINT);
> +  Data &= ~0x3000;

No magic values, please.

> +  WRITE_REG32 (GOTGINT, Data);
> +
> +  /* endpoint settings cfg */
> +  ResetEndpoints ();
> +  MicroSecondDelay (1);

Why a delay? Why 1us? Is a barrier needed?

> +
> +  /* init finish. and ready to transfer data */
> +
> +  /* Soft Disconnect */
> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE | DCTL_SFTDISCON);
> +  MicroSecondDelay (10000);

Why a delay? Why 10ms? Is a barrier needed?

> +
> +  /* Soft Reconnect */
> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +DwUsbStart (
> +  IN USB_DEVICE_DESCRIPTOR   *DeviceDescriptor,
> +  IN VOID                   **Descriptors,
> +  IN USB_DEVICE_RX_CALLBACK   RxCallback,
> +  IN USB_DEVICE_TX_CALLBACK   TxCallback
> +  )
> +{
> +  UINT8                    *Ptr;
> +  EFI_STATUS                Status;
> +  EFI_EVENT                 TimerEvent;
> +
> +  ASSERT (DeviceDescriptor != NULL);
> +  ASSERT (Descriptors[0] != NULL);
> +  ASSERT (RxCallback != NULL);
> +  ASSERT (TxCallback != NULL);

Can we do anything sane if any of these are NULL?
If not, return error.

> +
> +  DwUsbInit();
> +
> +  mDeviceDescriptor = DeviceDescriptor;
> +  mDescriptors = Descriptors[0];
> +
> +  // Right now we just support one configuration
> +  ASSERT (mDeviceDescriptor->NumConfigurations == 1);
> +  mDeviceDescriptor->StrManufacturer = 1;
> +  mDeviceDescriptor->StrProduct = 2;
> +  mDeviceDescriptor->StrSerialNumber = 3;

No magic values, please.
Presumably these are just indexes into a string table, but use
#defines please.

> +  // ... and one interface
> +  mConfigDescriptor = (USB_CONFIG_DESCRIPTOR *)mDescriptors;
> +  ASSERT (mConfigDescriptor->NumInterfaces == 1);
> +
> +  Ptr = ((UINT8 *) mDescriptors) + sizeof (USB_CONFIG_DESCRIPTOR);
> +  mInterfaceDescriptor = (USB_INTERFACE_DESCRIPTOR *) Ptr;
> +  Ptr += sizeof (USB_INTERFACE_DESCRIPTOR);
> +
> +  mEndpointDescriptors = (USB_ENDPOINT_DESCRIPTOR *) Ptr;
> +
> +  mDataReceivedCallback = RxCallback;
> +  mDataSentCallback = TxCallback;
> +
> +  // Register a timer event so CheckInterupts gets called periodically
> +  Status = gBS->CreateEvent (
> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  CheckInterrupts,
> +                  NULL,
> +                  &TimerEvent
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = gBS->SetTimer (
> +                  TimerEvent,
> +                  TimerPeriodic,
> +                  DW_INTERRUPT_POLL_PERIOD
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
> +USB_DEVICE_PROTOCOL mUsbDevice = {
> +  DwUsbStart,
> +  DwUsbSend
> +};
> +
> +
> +EFI_STATUS
> +EFIAPI
> +DwUsbEntryPoint (
> +  IN EFI_HANDLE                            ImageHandle,
> +  IN EFI_SYSTEM_TABLE                      *SystemTable
> +  )
> +{
> +  EFI_STATUS      Status;
> +
> +  Status = gBS->LocateProtocol (&gDwUsbProtocolGuid, NULL, (VOID **) &DwUsb);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = DwUsb->PhyInit(USB_DEVICE_MODE);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return gBS->InstallProtocolInterface (
> +                &ImageHandle,
> +                &gUsbDeviceProtocolGuid,
> +                EFI_NATIVE_INTERFACE,
> +                &mUsbDevice
> +                );
> +}
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> new file mode 100644
> index 0000000..f991492
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> @@ -0,0 +1,46 @@
> +#/** @file
> +# Framework Module Development Environment Industry Standards
> +#
> +# This Package provides headers and libraries that conform to EFI/PI Industry standards.
> +# Copyright (c) 2007, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2012-2014, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2015-2017, Linaro. All rights reserved.<BR>
> +#
> +#    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.
> +#
> +#**/
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 0x00010019
> +  PACKAGE_NAME                   = OpenPlatformDriversUsbDwUsbDxePkg

I think DwUsbDxePkg would be sufficient.

> +  PACKAGE_GUID                   = 114a3be9-10f7-4bf1-81ca-09ac52d4c3d5
> +  PACKAGE_VERSION                = 0.1
> +
> +
> +################################################################################
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#                   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +################################################################################
> +
> +[Guids.common]
> +  gAndroidFastbootUsbGuid       = { 0xf6bec3fe, 0x88fb, 0x11e3, { 0xae, 0x84, 0xe7, 0x3b, 0x77, 0x56, 0x1c, 0x35 }}

Is this included only for the DEPEX?
This does not feel like the right way of doing this.

> +  gDwUsbDxeTokenSpaceGuid       = { 0x131c4d02, 0x9449, 0x4ee9, { 0xba, 0x3d, 0x69, 0x50, 0x21, 0x89, 0x26, 0x0b }}
> +
> +[Protocols.common]
> +  gDwUsbProtocolGuid            = { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> +
> +[PcdsFixedAtBuild.common]
> +  # DwUsb Driver PCDs
> +  gDwUsbDxeTokenSpaceGuid.PcdDwUsbDxeBaseAddress|0x0|UINT32|0x00000001
> +  gDwUsbDxeTokenSpaceGuid.PcdSysCtrlBaseAddress|0x0|UINT32|0x00000002

I am really not happy about adding additional drivers with hard-coded
base addresses. Especially not ones that claim to be DXE/UEFI drivers.

> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.h b/Drivers/Usb/DwUsbDxe/DwUsbDxe.h
> new file mode 100644
> index 0000000..7f909c3
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.h
> @@ -0,0 +1,627 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 __DW_USB_DXE_H__
> +#define __DW_USB_DXE_H__
> +
> +#define DW_USB_BASE                     FixedPcdGet32 (PcdDwUsbDxeBaseAddress)
> +
> +#define READ_REG32(Offset)              MmioRead32 (DW_USB_BASE + Offset)
> +#define READ_REG16(Offset)              (UINT16)READ_REG32 (Offset)
> +#define WRITE_REG32(Offset, Val)        MmioWrite32 (DW_USB_BASE + Offset, Val)
> +#define WRITE_REG16(Offset, Val)        MmioWrite32 (DW_USB_BASE + Offset, (UINT32) Val)
> +#define WRITE_REG8(Offset, Val)         MmioWrite32 (DW_USB_BASE + Offset, (UINT32) Val)
> +
> +// Max packet size in bytes (For Full Speed USB 64 is the only valid value)
> +#define MAX_PACKET_SIZE_CONTROL         64
> +
> +#define MAX_PACKET_SIZE_BULK            512
> +
> +// 8 Endpoints, in and out. Don't count the Endpoint 0 setup buffer
> +#define DW_NUM_ENDPOINTS                16
> +
> +// Endpoint Indexes
> +#define DW_EP0SETUP                     0x20
> +#define DW_EP0RX                        0x00
> +#define DW_EP0TX                        0x01
> +#define DW_EP1RX                        0x02
> +#define DW_EP1TX                        0x03
> +
> +// DcInterrupt bits
> +#define DW_DC_INTERRUPT_EP1TX           BIT13
> +#define DW_DC_INTERRUPT_EP1RX           BIT12
> +#define DW_DC_INTERRUPT_EP0TX           BIT11
> +#define DW_DC_INTERRUPT_EP0RX           BIT10
> +#define DW_DC_INTERRUPT_EP0SETUP        BIT8
> +#define DW_DC_INTERRUPT_VBUS            BIT7
> +#define DW_DC_INTERRUPT_DMA             BIT6
> +#define DW_DC_INTERRUPT_HS_STAT         BIT5
> +#define DW_DC_INTERRUPT_RESUME          BIT4
> +#define DW_DC_INTERRUPT_SUSP            BIT3
> +#define DW_DC_INTERRUPT_PSOF            BIT2
> +#define DW_DC_INTERRUPT_SOF             BIT1
> +#define DW_DC_INTERRUPT_BRESET          BIT0
> +// All valid peripheral controller int  rrupts
> +#define DW_DC_INTERRUPT_MASK            0x003FFFDFF
> +
> +#define DW_ADDRESS                      0x200
> +#define DW_ADDRESS_DEVEN                BIT7
> +
> +#define DW_MODE                         0x20C
> +#define DW_MODE_DATA_BUS_WIDTH          BIT8
> +#define DW_MODE_CLKAON                  BIT7
> +#define DW_MODE_SFRESET                 BIT4
> +#define DW_MODE_WKUPCS                  BIT2
> +
> +#define DW_ENDPOINT_MAX_PACKET_SIZE     0x204
> +
> +#define DW_ENDPOINT_TYPE                0x208
> +#define DW_ENDPOINT_TYPE_NOEMPKT        BIT4
> +#define DW_ENDPOINT_TYPE_ENABLE         BIT3
> +
> +#define DW_INTERRUPT_CONFIG             0x210
> +// Interrupt config value to only interrupt on ACK of IN and OUT tokens
> +#define DW_INTERRUPT_CONFIG_ACK_ONLY    (BIT2 | BIT5 | BIT6)
> +
> +#define DW_DC_INTERRUPT                 0x218
> +#define DW_DC_INTERRUPT_ENABLE          0x214
> +
> +#define DW_CTRL_FUNCTION                0x228
> +#define DW_CTRL_FUNCTION_VENDP          BIT3
> +#define DW_CTRL_FUNCTION_DSEN           BIT2
> +#define DW_CTRL_FUNCTION_STATUS         BIT1
> +
> +#define DW_DEVICE_UNLOCK                0x27C
> +#define DW_DEVICE_UNLOCK_MAGIC          0xAA37
> +
> +#define DW_SW_RESET_REG                 0x30C
> +#define DW_SW_RESET_ALL                 BIT0
> +
> +#define DW_DEVICE_ID                    0x370
> +
> +#define DW_OTG_CTRL_SET                 0x374
> +#define DW_OTG_CTRL_CLR                 (OTG_CTRL_SET + 2)
> +#define DW_OTG_CTRL_OTG_DISABLE         BIT10
> +#define DW_OTG_CTRL_VBUS_CHRG           BIT6
> +#define DW_OTG_CTRL_VBUS_DISCHRG        BIT5
> +#define DW_OTG_CTRL_DM_PULLDOWN         BIT2
> +#define DW_OTG_CTRL_DP_PULLDOWN         BIT1
> +#define DW_OTG_CTRL_DP_PULLUP           BIT0
> +
> +#define DW_OTG_STATUS                   0x378
> +#define DW_OTG_STATUS_B_SESS_END        BIT7
> +#define DW_OTG_STATUS_A_B_SESS_VLD      BIT1
> +
> +#define DW_OTG_INTERRUPT_LATCH_SET      0x37C
> +#define DW_OTG_INTERRUPT_LATCH_CLR      0x37E
> +#define DW_OTG_INTERRUPT_ENABLE_RISE    0x384
> +
> +#define DW_DMA_ENDPOINT_INDEX           0x258
> +
> +#define DW_ENDPOINT_INDEX               0x22c
> +#define DW_DATA_PORT                    0x220
> +#define DW_BUFFER_LENGTH                0x21c
> +
> +// Device ID Values
> +#define PHILLIPS_VENDOR_ID_VAL          0x04cc
> +#define DW_PRODUCT_ID_VAL               0x1761
> +#define DW_DEVICE_ID_VAL                ((ISP1761_PRODUCT_ID_VAL << 16) | \
> +                                         PHILLIPS_VENDOR_ID_VAL)
> +
> +#define DWC_OTG_BASE                    DW_USB_BASE
> +
> +#define USB_NUM_ENDPOINTS               2
> +#define MAX_EPS_CHANNELS                16
> +
> +#define BULK_OUT_EP                     1
> +#define BULK_IN_EP                      1
> +
> +#define RX_REQ_LEN                      512
> +#define MAX_PACKET_LEN                  512
> +
> +#define DATA_FIFO_CONFIG                0x0F801000
> +/* RX FIFO: 2048 bytes */
> +#define RX_SIZE                         0x00000200
> +/* Non-periodic TX FIFO: 128 bytes. start address: 0x200 * 4. */
> +#define ENDPOINT_TX_SIZE                0x01000200
> +
> +/* EP1  TX FIFO: 1024 bytes. start address: 0x220 * 4. */
> +/* EP2  TX FIFO: 1024 bytes. start address: 0x320 * 4. */
> +/* EP3  TX FIFO: 1024 bytes. start address: 0x420 * 4. */
> +/* EP4  TX FIFO: 1024 bytes. start address: 0x520 * 4. */
> +/* EP5  TX FIFO: 128 bytes. start address: 0x620 * 4. */
> +/* EP6  TX FIFO: 128 bytes. start address: 0x640 * 4. */
> +/* EP7  TX FIFO: 128 bytes. start address: 0x660 * 4. */
> +/* EP8  TX FIFO: 128 bytes. start address: 0x680 * 4. */
> +/* EP9  TX FIFO: 128 bytes. start address: 0x6A0 * 4. */
> +/* EP10 TX FIFO: 128 bytes. start address: 0x6C0 * 4. */
> +/* EP11 TX FIFO: 128  bytes. start address: 0x6E0 * 4. */
> +/* EP12 TX FIFO: 128  bytes. start address: 0x700 * 4. */
> +/* EP13 TX FIFO: 128  bytes. start address: 0x720 * 4. */
> +/* EP14 TX FIFO: 128  bytes. start address: 0x740 * 4. */
> +/* EP15 TX FIFO: 128  bytes. start address: 0x760 * 4. */
> +
> +#define DATA_IN_ENDPOINT_TX_FIFO1       0x01000220
> +#define DATA_IN_ENDPOINT_TX_FIFO2       0x01000320
> +#define DATA_IN_ENDPOINT_TX_FIFO3       0x01000420
> +#define DATA_IN_ENDPOINT_TX_FIFO4       0x01000520
> +#define DATA_IN_ENDPOINT_TX_FIFO5       0x00200620
> +#define DATA_IN_ENDPOINT_TX_FIFO6       0x00200640
> +#define DATA_IN_ENDPOINT_TX_FIFO7       0x00200680
> +#define DATA_IN_ENDPOINT_TX_FIFO8       0x002006A0
> +#define DATA_IN_ENDPOINT_TX_FIFO9       0x002006C0
> +#define DATA_IN_ENDPOINT_TX_FIFO10      0x002006E0
> +#define DATA_IN_ENDPOINT_TX_FIFO11      0x00200700
> +#define DATA_IN_ENDPOINT_TX_FIFO12      0x00200720
> +#define DATA_IN_ENDPOINT_TX_FIFO13      0x00200740
> +#define DATA_IN_ENDPOINT_TX_FIFO14      0x00200760
> +#define DATA_IN_ENDPOINT_TX_FIFO15      0x00200F00
> +
> +/*DWC_OTG regsiter descriptor*/
> +/*Device mode CSR MAP*/
> +#define DEVICE_CSR_BASE                 0x800
> +/*Device mode CSR MAP*/
> +#define DEVICE_INEP_BASE                0x900
> +/*Device mode CSR MAP*/
> +#define DEVICE_OUTEP_BASE               0xB00
> +
> +/*** OTG LINK CORE REGISTERS ***/
> +/* Core Global Registers */
> +#define GOTGCTL                         0x000
> +#define GOTGINT                         0x004
> +#define GAHBCFG                         0x008
> +#define GAHBCFG_P_TXF_EMP_LVL           (1 << 8)
> +#define GAHBCFG_NP_TXF_EMP_LVL          (1 << 7)
> +#define GAHBCFG_DMA_EN                  (1 << 5)
> +#define GAHBCFG_GLBL_INTR_EN            (1 << 0)
> +#define GAHBCFG_CTRL_MASK               (GAHBCFG_P_TXF_EMP_LVL | \
> +                                         GAHBCFG_NP_TXF_EMP_LVL | \
> +                                         GAHBCFG_DMA_EN | \
> +                                         GAHBCFG_GLBL_INTR_EN)
> +
> +#define GUSBCFG                         0x00C
> +#define GRSTCTL                         0x010
> +#define GRSTCTL_AHBIDLE                 (1 << 31)
> +#define GRSTCTL_CSFTRST                 (1 << 0)
> +
> +#define GINTSTS                         0x014
> +#define GINTSTS_WKUPINT                 BIT31
> +#define GINTSTS_SESSREGINT              BIT30
> +#define GINTSTS_DISCONNINT              BIT29
> +#define GINTSTS_CONIDSTSCHNG            BIT28
> +#define GINTSTS_LPMTRANRCVD             BIT27
> +#define GINTSTS_PTXFEMP                 BIT26
> +#define GINTSTS_HCHINT                  BIT25
> +#define GINTSTS_PRTINT                  BIT24
> +#define GINTSTS_RESETDET                BIT23
> +#define GINTSTS_FET_SUSP                BIT22
> +#define GINTSTS_INCOMPL_IP              BIT21
> +#define GINTSTS_INCOMPL_SOIN            BIT20
> +#define GINTSTS_OEPINT                  BIT19
> +#define GINTSTS_IEPINT                  BIT18
> +#define GINTSTS_EPMIS                   BIT17
> +#define GINTSTS_RESTOREDONE             BIT16
> +#define GINTSTS_EOPF                    BIT15
> +#define GINTSTS_ISOUTDROP               BIT14
> +#define GINTSTS_ENUMDONE                BIT13
> +#define GINTSTS_USBRST                  BIT12
> +#define GINTSTS_USBSUSP                 BIT11
> +#define GINTSTS_ERLYSUSP                BIT10
> +#define GINTSTS_I2CINT                  BIT9
> +#define GINTSTS_ULPI_CK_INT             BIT8
> +#define GINTSTS_GOUTNAKEFF              BIT7
> +#define GINTSTS_GINNAKEFF               BIT6
> +#define GINTSTS_NPTXFEMP                BIT5
> +#define GINTSTS_RXFLVL                  BIT4
> +#define GINTSTS_SOF                     BIT3
> +#define GINTSTS_OTGINT                  BIT2
> +#define GINTSTS_MODEMIS                 BIT1
> +#define GINTSTS_CURMODE_HOST            BIT0
> +
> +#define GINTMSK                         0x018
> +#define GRXSTSR                         0x01C
> +#define GRXSTSP                         0x020
> +#define GRXFSIZ                         0x024
> +#define GNPTXFSIZ                       0x028
> +#define GNPTXSTS                        0x02C
> +
> +#define GHWCFG1                         0x044
> +#define GHWCFG2                         0x048
> +#define GHWCFG3                         0x04c
> +#define GHWCFG4                         0x050
> +#define GLPMCFG                         0x054
> +
> +#define GDFIFOCFG                       0x05c
> +
> +#define HPTXFSIZ                        0x100
> +#define DIEPTXF(x)                      (0x100 + 4 * (x))
> +#define DIEPTXF1                        0x104
> +#define DIEPTXF2                        0x108
> +#define DIEPTXF3                        0x10C
> +#define DIEPTXF4                        0x110
> +#define DIEPTXF5                        0x114
> +#define DIEPTXF6                        0x118
> +#define DIEPTXF7                        0x11C
> +#define DIEPTXF8                        0x120
> +#define DIEPTXF9                        0x124
> +#define DIEPTXF10                       0x128
> +#define DIEPTXF11                       0x12C
> +#define DIEPTXF12                       0x130
> +#define DIEPTXF13                       0x134
> +#define DIEPTXF14                       0x138
> +#define DIEPTXF15                       0x13C
> +
> +/*** HOST MODE REGISTERS ***/
> +/* Host Global Registers */
> +#define HCFG                            0x400
> +#define HFIR                            0x404
> +#define HFNUM                           0x408
> +#define HPTXSTS                         0x410
> +#define HAINT                           0x414
> +#define HAINTMSK                        0x418
> +
> +/* Host Port Control and Status Registers */
> +#define HPRT                            0x440
> +
> +/* Host Channel-Specific Registers */
> +#define HCCHAR(x)                       (0x500 + 0x20 * (x))
> +#define HCSPLT(x)                       (0x504 + 0x20 * (x))
> +#define HCINT(x)                        (0x508 + 0x20 * (x))
> +#define HCINTMSK(x)                     (0x50C + 0x20 * (x))
> +#define HCTSIZ(x)                       (0x510 + 0x20 * (x))
> +#define HCDMA(x)                        (0x514 + 0x20 * (x))
> +#define HCCHAR0                         0x500
> +#define HCSPLT0                         0x504
> +#define HCINT0                          0x508
> +#define HCINTMSK0                       0x50C
> +#define HCTSIZ0                         0x510
> +#define HCDMA0                          0x514
> +#define HCCHAR1                         0x520
> +#define HCSPLT1                         0x524
> +#define HCINT1                          0x528
> +#define HCINTMSK1                       0x52C
> +#define HCTSIZ1                         0x530
> +#define HCDMA1                          0x534
> +#define HCCHAR2                         0x540
> +#define HCSPLT2                         0x544
> +#define HCINT2                          0x548
> +#define HCINTMSK2                       0x54C
> +#define HCTSIZ2                         0x550
> +#define HCDMA2                          0x554
> +#define HCCHAR3                         0x560
> +#define HCSPLT3                         0x564
> +#define HCINT3                          0x568
> +#define HCINTMSK3                       0x56C
> +#define HCTSIZ3                         0x570
> +#define HCDMA3                          0x574
> +#define HCCHAR4                         0x580
> +#define HCSPLT4                         0x584
> +#define HCINT4                          0x588
> +#define HCINTMSK4                       0x58C
> +#define HCTSIZ4                         0x590
> +#define HCDMA4                          0x594
> +#define HCCHAR5                         0x5A0
> +#define HCSPLT5                         0x5A4
> +#define HCINT5                          0x5A8
> +#define HCINTMSK5                       0x5AC
> +#define HCTSIZ5                         0x5B0
> +#define HCDMA5                          0x5B4
> +#define HCCHAR6                         0x5C0
> +#define HCSPLT6                         0x5C4
> +#define HCINT6                          0x5C8
> +#define HCINTMSK6                       0x5CC
> +#define HCTSIZ6                         0x5D0
> +#define HCDMA6                          0x5D4
> +#define HCCHAR7                         0x5E0
> +#define HCSPLT7                         0x5E4
> +#define HCINT7                          0x5E8
> +#define HCINTMSK7                       0x5EC
> +#define HCTSIZ7                         0x5F0
> +#define HCDMA7                          0x5F4
> +#define HCCHAR8                         0x600
> +#define HCSPLT8                         0x604
> +#define HCINT8                          0x608
> +#define HCINTMSK8                       0x60C
> +#define HCTSIZ8                         0x610
> +#define HCDMA8                          0x614
> +#define HCCHAR9                         0x620
> +#define HCSPLT9                         0x624
> +#define HCINT9                          0x628
> +#define HCINTMSK9                       0x62C
> +#define HCTSIZ9                         0x630
> +#define HCDMA9                          0x634
> +#define HCCHAR10                        0x640
> +#define HCSPLT10                        0x644
> +#define HCINT10                         0x648
> +#define HCINTMSK10                      0x64C
> +#define HCTSIZ10                        0x650
> +#define HCDMA10                         0x654
> +#define HCCHAR11                        0x660
> +#define HCSPLT11                        0x664
> +#define HCINT11                         0x668
> +#define HCINTMSK11                      0x66C
> +#define HCTSIZ11                        0x670
> +#define HCDMA11                         0x674
> +#define HCCHAR12                        0x680
> +#define HCSPLT12                        0x684
> +#define HCINT12                         0x688
> +#define HCINTMSK12                      0x68C
> +#define HCTSIZ12                        0x690
> +#define HCDMA12                         0x694
> +#define HCCHAR13                        0x6A0
> +#define HCSPLT13                        0x6A4
> +#define HCINT13                         0x6A8
> +#define HCINTMSK13                      0x6AC
> +#define HCTSIZ13                        0x6B0
> +#define HCDMA13                         0x6B4
> +#define HCCHAR14                        0x6C0
> +#define HCSPLT14                        0x6C4
> +#define HCINT14                         0x6C8
> +#define HCINTMSK14                      0x6CC
> +#define HCTSIZ14                        0x6D0
> +#define HCDMA14                         0x6D4
> +#define HCCHAR15                        0x6E0
> +#define HCSPLT15                        0x6E4
> +#define HCINT15                         0x6E8
> +#define HCINTMSK15                      0x6EC
> +#define HCTSIZ15                        0x6F0
> +#define HCDMA15                         0x6F4
> +
> +/*** DEVICE MODE REGISTERS ***/
> +/* Device Global Registers */
> +#define DCFG                            0x800
> +#define DCFG_DESCDMA                    BIT23
> +#define DCFG_EPMISCNT_MASK              (0x1F << 18)
> +#define DCFG_EPMISCNT_SHIFT             18
> +#define DCFG_DEVADDR_MASK               (0x7F << 4)
> +#define DCFG_DEVADDR_SHIFT              4
> +#define DCFG_DEVADDR(x)                 (((x) << DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK)
> +#define DCFG_NZ_STS_OUT_HSHK            BIT2
> +
> +#define DCTL                            0x804
> +#define DCTL_PWRONPRGDONE               BIT11
> +#define DCTL_GNPINNAKSTS                BIT2
> +#define DCTL_SFTDISCON                  BIT1
> +
> +#define DSTS                            0x808
> +#define DIEPMSK                         0x810
> +#define DOEPMSK                         0x814
> +
> +#define DXEPMSK_TIMEOUTMSK              BIT3
> +#define DXEPMSK_AHBERMSK                BIT2
> +#define DXEPMSK_XFERCOMPLMSK            BIT0
> +
> +#define DAINT                           0x818
> +#define DAINTMSK                        0x81C
> +
> +#define DAINTMSK_OUTEPMSK_SHIFT         16
> +#define DAINTMSK_INEPMSK_SHIFT          0
> +
> +#define DTKNQR1                         0x820
> +#define DTKNQR2                         0x824
> +#define DVBUSDIS                        0x828
> +#define DVBUSPULSE                      0x82C
> +#define DTHRCTL                         0x830
> +
> +/* Device Logical IN Endpoint-Specific Registers */
> +#define DIEPCTL(x)                      (0x900 + 0x20 * (x))
> +#define DIEPINT(x)                      (0x908 + 0x20 * (x))
> +#define DIEPTSIZ(x)                     (0x910 + 0x20 * (x))
> +#define DIEPDMA(x)                      (0x914 + 0x20 * (x))
> +#define DTXFSTS(x)                      (0x918 + 0x20 * (x))
> +
> +#define DIEPCTL0                        0x900
> +#define DIEPINT0                        0x908
> +#define DIEPTSIZ0                       0x910
> +#define DIEPDMA0                        0x914
> +#define DIEPCTL1                        0x920
> +#define DIEPINT1                        0x928
> +#define DIEPTSIZ1                       0x930
> +#define DIEPDMA1                        0x934
> +#define DIEPCTL2                        0x940
> +#define DIEPINT2                        0x948
> +#define DIEPTSIZ2                       0x950
> +#define DIEPDMA2                        0x954
> +#define DIEPCTL3                        0x960
> +#define DIEPINT3                        0x968
> +#define DIEPTSIZ3                       0x970
> +#define DIEPDMA3                        0x974
> +#define DIEPCTL4                        0x980
> +#define DIEPINT4                        0x988
> +#define DIEPTSIZ4                       0x990
> +#define DIEPDMA4                        0x994
> +#define DIEPCTL5                        0x9A0
> +#define DIEPINT5                        0x9A8
> +#define DIEPTSIZ5                       0x9B0
> +#define DIEPDMA5                        0x9B4
> +#define DIEPCTL6                        0x9C0
> +#define DIEPINT6                        0x9C8
> +#define DIEPTSIZ6                       0x9D0
> +#define DIEPDMA6                        0x9D4
> +#define DIEPCTL7                        0x9E0
> +#define DIEPINT7                        0x9E8
> +#define DIEPTSIZ7                       0x9F0
> +#define DIEPDMA7                        0x9F4
> +#define DIEPCTL8                        0xA00
> +#define DIEPINT8                        0xA08
> +#define DIEPTSIZ8                       0xA10
> +#define DIEPDMA8                        0xA14
> +#define DIEPCTL9                        0xA20
> +#define DIEPINT9                        0xA28
> +#define DIEPTSIZ9                       0xA30
> +#define DIEPDMA9                        0xA34
> +#define DIEPCTL10                       0xA40
> +#define DIEPINT10                       0xA48
> +#define DIEPTSIZ10                      0xA50
> +#define DIEPDMA10                       0xA54
> +#define DIEPCTL11                       0xA60
> +#define DIEPINT11                       0xA68
> +#define DIEPTSIZ11                      0xA70
> +#define DIEPDMA11                       0xA74
> +#define DIEPCTL12                       0xA80
> +#define DIEPINT12                       0xA88
> +#define DIEPTSIZ12                      0xA90
> +#define DIEPDMA12                       0xA94
> +#define DIEPCTL13                       0xAA0
> +#define DIEPINT13                       0xAA8
> +#define DIEPTSIZ13                      0xAB0
> +#define DIEPDMA13                       0xAB4
> +#define DIEPCTL14                       0xAC0
> +#define DIEPINT14                       0xAC8
> +#define DIEPTSIZ14                      0xAD0
> +#define DIEPDMA14                       0xAD4
> +#define DIEPCTL15                       0xAE0
> +#define DIEPINT15                       0xAE8
> +#define DIEPTSIZ15                      0xAF0
> +#define DIEPDMA15                       0xAF4
> +
> +/* Device Logical OUT Endpoint-Specific Registers */
> +#define DOEPCTL(x)                      (0xB00 + 0x20 * (x))
> +#define DOEPINT(x)                      (0xB08 + 0x20 * (x))
> +#define DOEPTSIZ(x)                     (0xB10 + 0x20 * (x))
> +
> +#define DXEPINT_EPDISBLD                BIT1
> +#define DXEPINT_XFERCOMPL               BIT0
> +
> +#define DXEPTSIZ_SUPCNT(x)              (((x) & 0x3) << 29)
> +#define DXEPTSIZ_PKTCNT(x)              (((x) & 0x3) << 19)
> +#define DXEPTSIZ_XFERSIZE(x)            ((x) & 0x7F)
> +
> +#define DOEPDMA(x)                      (0xB14 + 0x20 * (x))
> +#define DOEPCTL0                        0xB00
> +#define DOEPINT0                        0xB08
> +#define DOEPTSIZ0                       0xB10
> +#define DOEPDMA0                        0xB14
> +#define DOEPCTL1                        0xB20
> +#define DOEPINT1                        0xB28
> +#define DOEPTSIZ1                       0xB30
> +#define DOEPDMA1                        0xB34
> +#define DOEPCTL2                        0xB40
> +#define DOEPINT2                        0xB48
> +#define DOEPTSIZ2                       0xB50
> +#define DOEPDMA2                        0xB54
> +#define DOEPCTL3                        0xB60
> +#define DOEPINT3                        0xB68
> +#define DOEPTSIZ3                       0xB70
> +#define DOEPDMA3                        0xB74
> +#define DOEPCTL4                        0xB80
> +#define DOEPINT4                        0xB88
> +#define DOEPTSIZ4                       0xB90
> +#define DOEPDMA4                        0xB94
> +#define DOEPCTL5                        0xBA0
> +#define DOEPINT5                        0xBA8
> +#define DOEPTSIZ5                       0xBB0
> +#define DOEPDMA5                        0xBB4
> +#define DOEPCTL6                        0xBC0
> +#define DOEPINT6                        0xBC8
> +#define DOEPTSIZ6                       0xBD0
> +#define DOEPDMA6                        0xBD4
> +#define DOEPCTL7                        0xBE0
> +#define DOEPINT7                        0xBE8
> +#define DOEPTSIZ7                       0xBF0
> +#define DOEPDMA7                        0xBF4
> +#define DOEPCTL8                        0xC00
> +#define DOEPINT8                        0xC08
> +#define DOEPTSIZ8                       0xC10
> +#define DOEPDMA8                        0xC14
> +#define DOEPCTL9                        0xC20
> +#define DOEPINT9                        0xC28
> +#define DOEPTSIZ9                       0xC30
> +#define DOEPDMA9                        0xC34
> +#define DOEPCTL10                       0xC40
> +#define DOEPINT10                       0xC48
> +#define DOEPTSIZ10                      0xC50
> +#define DOEPDMA10                       0xC54
> +#define DOEPCTL11                       0xC60
> +#define DOEPINT11                       0xC68
> +#define DOEPTSIZ11                      0xC70
> +#define DOEPDMA11                       0xC74
> +#define DOEPCTL12                       0xC80
> +#define DOEPINT12                       0xC88
> +#define DOEPTSIZ12                      0xC90
> +#define DOEPDMA12                       0xC94
> +#define DOEPCTL13                       0xCA0
> +#define DOEPINT13                       0xCA8
> +#define DOEPTSIZ13                      0xCB0
> +#define DOEPDMA13                       0xCB4
> +#define DOEPCTL14                       0xCC0
> +#define DOEPINT14                       0xCC8
> +#define DOEPTSIZ14                      0xCD0
> +#define DOEPDMA14                       0xCD4
> +#define DOEPCTL15                       0xCE0
> +#define DOEPINT15                       0xCE8
> +#define DOEPTSIZ15                      0xCF0
> +#define DOEPDMA15                       0xCF4
> +
> +#define DXEPCTL_EPENA                   BIT31
> +#define DXEPCTL_SNAK                    BIT27
> +#define DXEPCTL_CNAK                    BIT26
> +#define DXEPCTL_STALL                   BIT21
> +#define DXEPCTL_EPTYPE_MASK             (BIT19 | BIT18)
> +#define DXEPCTL_NAKSTS                  BIT17
> +#define DXEPCTL_USBACTEP                BIT15
> +#define DXEPCTL_MPS_MASK                0x7FF
> +
> +/* Power and Clock Gating Register */
> +#define PCGCCTL                         0xE00
> +
> +#define EP0FIFO                         0x1000
> +
> +/**
> + * This union represents the bit fields in the DMA Descriptor
> + * status quadlet. Read the quadlet into the <i>d32</i> member then
> + * set/clear the bits using the <i>b</i>it, <i>b_iso_out</i> and
> + * <i>b_iso_in</i> elements.
> + */
> +typedef union {
> +  /** raw register data */
> +  UINT32 d32;
> +    /** quadlet bits */
> +  struct {
> +    /** Received number of bytes */
> +    unsigned bytes:16;
> +    /** NAK bit - only for OUT EPs */
> +    unsigned nak:1;
> +    unsigned reserved17_22:6;
> +    /** Multiple Transfer - only for OUT EPs */
> +    unsigned mtrf:1;
> +    /** Setup Packet received - only for OUT EPs */
> +    unsigned sr:1;
> +    /** Interrupt On Complete */
> +    unsigned ioc:1;
> +    /** Short Packet */
> +    unsigned sp:1;
> +    /** Last */
> +    unsigned l:1;
> +    /** Receive Status */
> +    unsigned sts:2;
> +    /** Buffer Status */
> +    unsigned bs:2;
> +  } b;
> +} dev_dma_desc_sts_t;
> +
> +/**
> + * DMA Descriptor structure
> + *
> + * DMA Descriptor structure contains two quadlets:
> + * Status quadlet and Data buffer pointer.
> + */
> +typedef struct {
> +  /** DMA Descriptor status quadlet */
> +  dev_dma_desc_sts_t status;
> +  /** DMA Descriptor data buffer pointer */
> +  UINT32 buf;
> +} dwc_otg_dev_dma_desc_t;

The above union and struct do not conform to EDK2 coding style.

> +
> +#endif //ifndef __DW_USB_DXE_H__
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
> new file mode 100644
> index 0000000..fc1e988
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
> @@ -0,0 +1,52 @@
> +#/** @file
> +#
> +#  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +#
> +#  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.
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = DwUsbDxe
> +  FILE_GUID                      = 72d78ea6-4dee-11e3-8100-f3842a48d0a0
> +  MODULE_TYPE                    = UEFI_DRIVER

I don't believe the driver in its current state can be called a
UEFI_DRIVER. Combined with the ordering issue, should this be a
DXE_DRIVER instead?

> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = DwUsbEntryPoint
> +
> +[Sources.common]
> +  DwUsbDxe.c
> +
> +[LibraryClasses]
> +  ArmLib
> +  DebugLib
> +  IoLib
> +  MemoryAllocationLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UncachedMemoryAllocationLib
> +  CacheMaintenanceLib

Please sort LibraryClasses.

> +
> +[Protocols]
> +  gEfiDriverBindingProtocolGuid
> +  gUsbDeviceProtocolGuid
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec

Please sort Packages.

/
    Leif

> +
> +[Pcd]
> +  gDwUsbDxeTokenSpaceGuid.PcdDwUsbDxeBaseAddress
> +
> +[Depex]
> +  BEFORE gAndroidFastbootUsbGuid
> diff --git a/Include/Protocol/DwUsb.h b/Include/Protocol/DwUsb.h
> new file mode 100644
> index 0000000..040e126
> --- /dev/null
> +++ b/Include/Protocol/DwUsb.h
> @@ -0,0 +1,81 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 __DW_USB_H__
> +#define __DW_USB_H__
> +
> +//
> +// Protocol GUID
> +//
> +#define DW_USB_PROTOCOL_GUID { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _DW_USB_PROTOCOL  DW_USB_PROTOCOL;
> +
> +#define USB_HOST_MODE                 0
> +#define USB_DEVICE_MODE               1
> +#define USB_CABLE_NOT_ATTACHED        2
> +
> +#define LANG_LENGTH                   8
> +#define MANU_FACTURER_STRING_LENGTH   32
> +#define PRODUCT_STRING_LENGTH         32
> +#define SERIAL_STRING_LENGTH          17
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_LANG) (
> +  OUT CHAR16                           *Lang,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_MANU_FACTURER) (
> +  OUT CHAR16                           *ManuFacturer,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_PRODUCT) (
> +  OUT CHAR16                           *Product,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_SERIAL_NO) (
> +  OUT CHAR16                           *SerialNo,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_PHY_INIT) (
> +  IN UINT8                             Mode
> +  );
> +
> +struct _DW_USB_PROTOCOL {
> +  DW_USB_GET_LANG                      GetLang;
> +  DW_USB_GET_MANU_FACTURER             GetManuFacturer;
> +  DW_USB_GET_PRODUCT                   GetProduct;
> +  DW_USB_GET_SERIAL_NO                 GetSerialNo;
> +  DW_USB_PHY_INIT                      PhyInit;
> +};
> +
> +extern EFI_GUID gDwUsbProtocolGuid;
> +
> +#endif
> -- 
> 2.7.4
>

On 4 February 2017 at 05:13, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
> Support Designware USB controller.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.c   | 796 ++++++++++++++++++++++++++++++++++++++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.dec |  46 +++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.h   | 627 ++++++++++++++++++++++++++++++
>  Drivers/Usb/DwUsbDxe/DwUsbDxe.inf |  52 +++
>  Include/Protocol/DwUsb.h          |  81 ++++
>  5 files changed, 1602 insertions(+)
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.c
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.h
>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
>  create mode 100644 Include/Protocol/DwUsb.h
>
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.c b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
> new file mode 100644
> index 0000000..bdd8405
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
> @@ -0,0 +1,796 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 <IndustryStandard/Usb.h>
> +#include <Library/ArmLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UncachedMemoryAllocationLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/BaseLib.h>
> +#include <Protocol/DwUsb.h>
> +#include <Protocol/UsbDevice.h>
> +
> +#include "DwUsbDxe.h"
> +
> +#define USB_TYPE_LENGTH               16
> +#define USB_BLOCK_HIGH_SPEED_SIZE     512
> +#define DATA_SIZE                     32768
> +#define CMD_SIZE                      512
> +#define MATCH_CMD_LITERAL(Cmd, Buf)   !AsciiStrnCmp (Cmd, Buf, sizeof (Cmd) - 1)
> +
> +// The time between interrupt polls, in units of 100 nanoseconds
> +// 10 Microseconds
> +#define DW_INTERRUPT_POLL_PERIOD      10000
> +
> +EFI_GUID  gDwUsbProtocolGuid = DW_USB_PROTOCOL_GUID;
> +
> +STATIC dwc_otg_dev_dma_desc_t *gDmaDesc,*gDmaDescEp0,*gDmaDescIn;
> +STATIC USB_DEVICE_REQUEST *gCtrlReq;
> +STATIC VOID *RxBuf;
> +STATIC UINTN RxDescBytes = 0;
> +STATIC UINTN mNumDataBytes;
> +
> +STATIC DW_USB_PROTOCOL          *DwUsb;
> +
> +STATIC USB_DEVICE_DESCRIPTOR    *mDeviceDescriptor;
> +
> +// The config descriptor, interface descriptor, and endpoint descriptors in a
> +// buffer (in that order)
> +STATIC VOID                     *mDescriptors;
> +// Convenience pointers to those descriptors inside the buffer:
> +STATIC USB_INTERFACE_DESCRIPTOR *mInterfaceDescriptor;
> +STATIC USB_CONFIG_DESCRIPTOR    *mConfigDescriptor;
> +STATIC USB_ENDPOINT_DESCRIPTOR  *mEndpointDescriptors;
> +
> +STATIC USB_DEVICE_RX_CALLBACK   mDataReceivedCallback;
> +STATIC USB_DEVICE_TX_CALLBACK   mDataSentCallback;
> +
> +
> +/* To detect which mode was run, high speed or full speed */
> +STATIC
> +UINTN
> +UsbDrvPortSpeed (
> +  VOID
> +  )
> +{
> +  /*
> +  * 2'b00: High speed (PHY clock is running at 30 or 60 MHz)
> +  */
> +  UINT32          Val = READ_REG32 (DSTS) & 2;
> +  return (!Val);
> +}
> +
> +STATIC
> +VOID
> +ResetEndpoints (
> +  VOID
> +  )
> +{
> +  /* EP0 IN ACTIVE NEXT=1 */
> +  WRITE_REG32 (DIEPCTL0, DXEPCTL_USBACTEP | BIT11);
> +
> +  /* EP0 OUT ACTIVE */
> +  WRITE_REG32 (DOEPCTL0, DXEPCTL_USBACTEP);
> +
> +  /* Clear any pending OTG Interrupts */
> +  WRITE_REG32 (GOTGINT, ~0);
> +
> +  /* Clear any pending interrupts */
> +  WRITE_REG32 (GINTSTS, ~0);
> +  WRITE_REG32 (DIEPINT0, ~0);
> +  WRITE_REG32 (DOEPINT0, ~0);
> +  WRITE_REG32 (DIEPINT1, ~0);
> +  WRITE_REG32 (DOEPINT1, ~0);
> +
> +  /* IN EP interrupt mask */
> +  WRITE_REG32 (DIEPMSK, DXEPMSK_TIMEOUTMSK | DXEPMSK_AHBERMSK | DXEPMSK_XFERCOMPLMSK);
> +  /* OUT EP interrupt mask */
> +  WRITE_REG32 (DOEPMSK, DXEPMSK_TIMEOUTMSK | DXEPMSK_AHBERMSK | DXEPMSK_XFERCOMPLMSK);
> +  /* Enable interrupts on Ep0 */
> +  WRITE_REG32 (DAINTMSK, (1 << DAINTMSK_OUTEPMSK_SHIFT) | (1 << DAINTMSK_INEPMSK_SHIFT));
> +
> +  /* EP0 OUT Transfer Size:64 Bytes, 1 Packet, 3 Setup Packet, Read to receive setup packet*/
> +  WRITE_REG32 (DOEPTSIZ0, DXEPTSIZ_SUPCNT(3) | DXEPTSIZ_PKTCNT(1) | DXEPTSIZ_XFERSIZE(64));
> +
> +  //notes that:the compulsive conversion is expectable.
> +  gDmaDescEp0->status.b.bs = 0x3;
> +  gDmaDescEp0->status.b.mtrf = 0;
> +  gDmaDescEp0->status.b.sr = 0;
> +  gDmaDescEp0->status.b.l = 1;
> +  gDmaDescEp0->status.b.ioc = 1;
> +  gDmaDescEp0->status.b.sp = 0;
> +  gDmaDescEp0->status.b.bytes = 64;
> +  gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);
> +  gDmaDescEp0->status.b.sts = 0;
> +  gDmaDescEp0->status.b.bs = 0x0;
> +  WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
> +  /* EP0 OUT ENABLE CLEARNAK */
> +  WRITE_REG32 (DOEPCTL0, (READ_REG32 (DOEPCTL0) | DXEPCTL_EPENA | DXEPCTL_CNAK));
> +}
> +
> +STATIC
> +VOID
> +EpTx (
> +  IN UINT8          Ep,
> +  IN CONST VOID    *Ptr,
> +  IN UINTN          Len
> +  )
> +{
> +    UINT32          BlockSize;
> +    UINT32          Packets;
> +
> +    /* EPx OUT ACTIVE */
> +    WRITE_REG32 (DIEPCTL (Ep), (READ_REG32 (DIEPCTL (Ep))) | DXEPCTL_USBACTEP);
> +    if (!Ep) {
> +        BlockSize = 64;
> +    } else {
> +        BlockSize = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;
> +    }
> +    Packets = (Len + BlockSize - 1) / BlockSize;
> +
> +    if (!Len) {
> +      /* send one empty packet */
> +      gDmaDescIn->status.b.bs = 0x3;
> +      gDmaDescIn->status.b.l = 1;
> +      gDmaDescIn->status.b.ioc = 1;
> +      gDmaDescIn->status.b.sp = 1;
> +      gDmaDescIn->status.b.bytes = 0;
> +      gDmaDescIn->buf = 0;
> +      gDmaDescIn->status.b.sts = 0;
> +      gDmaDescIn->status.b.bs = 0x0;
> +
> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));             // DMA Address (DMAAddr) is zero
> +    } else {
> +      WRITE_REG32 (DIEPTSIZ (Ep), Len | (Packets << 19));
> +
> +      //flush cache
> +      WriteBackDataCacheRange ((VOID *)Ptr, Len);
> +
> +      gDmaDescIn->status.b.bs = 0x3;
> +      gDmaDescIn->status.b.l = 1;
> +      gDmaDescIn->status.b.ioc = 1;
> +      gDmaDescIn->status.b.sp = 1;
> +      gDmaDescIn->status.b.bytes = Len;
> +      gDmaDescIn->buf = (UINT32)((UINTN)Ptr);
> +      gDmaDescIn->status.b.sts = 0;
> +      gDmaDescIn->status.b.bs = 0x0;
> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));         // Ptr is DMA address
> +    }
> +    ArmDataSynchronizationBarrier ();
> +    /* epena & cnak */
> +    WRITE_REG32 (DIEPCTL (Ep), READ_REG32 (DIEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK | BIT11);
> +}
> +
> +STATIC
> +VOID
> +EpRx (
> +  IN UINTN            Ep,
> +  IN UINTN            Len
> +  )
> +{
> +  /* EPx UNSTALL */
> +  WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & (~DXEPCTL_STALL)));
> +  /* EPx OUT ACTIVE */
> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_USBACTEP));
> +
> +  if (Len >= DATA_SIZE) {
> +    RxDescBytes = DATA_SIZE;
> +  } else {
> +    RxDescBytes = Len;
> +  }
> +
> +  RxBuf = AllocatePool (DATA_SIZE);
> +  ASSERT (RxBuf != NULL);
> +
> +  InvalidateDataCacheRange (RxBuf, Len);
> +
> +  gDmaDesc->status.b.bs = 0x3;
> +  gDmaDesc->status.b.mtrf = 0;
> +  gDmaDesc->status.b.sr = 0;
> +  gDmaDesc->status.b.l = 1;
> +  gDmaDesc->status.b.ioc = 1;
> +  gDmaDesc->status.b.sp = 0;
> +  gDmaDesc->status.b.bytes = (UINT32)RxDescBytes;
> +  gDmaDesc->buf = (UINT32)((UINTN)RxBuf);
> +  gDmaDesc->status.b.sts = 0;
> +  gDmaDesc->status.b.bs = 0x0;
> +
> +  ArmDataSynchronizationBarrier ();
> +  WRITE_REG32 (DOEPDMA (Ep), (UINT32)((UINTN)gDmaDesc));
> +  /* EPx OUT ENABLE CLEARNAK */
> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK));
> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleGetDescriptor (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  UINT8       DescriptorType;
> +  UINTN       ResponseSize;
> +  VOID       *ResponseData;
> +  EFI_USB_STRING_DESCRIPTOR        *Descriptor = NULL;
> +  UINTN                             DescriptorSize;
> +
> +  ResponseSize = 0;
> +  ResponseData = NULL;
> +
> +  // Pretty confused if bmRequestType is anything but this:
> +  ASSERT (Request->RequestType == USB_DEV_GET_DESCRIPTOR_REQ_TYPE);
> +
> +  // Choose the response
> +  DescriptorType = Request->Value >> 8;
> +  switch (DescriptorType) {
> +  case USB_DESC_TYPE_DEVICE:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for device descriptor\n"));
> +    ResponseSize = sizeof (USB_DEVICE_DESCRIPTOR);
> +    ResponseData = mDeviceDescriptor;
> +    break;
> +  case USB_DESC_TYPE_CONFIG:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for config descriptor\n"));
> +    ResponseSize = mConfigDescriptor->TotalLength;
> +    ResponseData = mDescriptors;
> +    break;
> +  case USB_DESC_TYPE_STRING:
> +    DEBUG ((DEBUG_INFO, "USB: Got a request for String descriptor %d\n", Request->Value & 0xFF));
> +    switch (Request->Value & 0xff) {
> +    case 0:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       LANG_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = LANG_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetLang (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 1:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetManuFacturer (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 2:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       PRODUCT_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = PRODUCT_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetProduct (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    case 3:
> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
> +                       SERIAL_STRING_LENGTH * sizeof (CHAR16) + 1;
> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
> +      ASSERT (Descriptor != NULL);
> +      Descriptor->Length = SERIAL_STRING_LENGTH * sizeof (CHAR16);
> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
> +      DwUsb->GetSerialNo (Descriptor->String, &Descriptor->Length);
> +      ResponseSize = Descriptor->Length;
> +      ResponseData = Descriptor;
> +      break;
> +    }
> +    break;
> +  default:
> +    DEBUG ((DEBUG_INFO, "USB: Didn't understand request for descriptor 0x%04x\n", Request->Value));
> +    break;
> +  }
> +
> +  // Send the response
> +  if (ResponseData) {
> +    ASSERT (ResponseSize != 0);
> +
> +    if (Request->Length < ResponseSize) {
> +      // Truncate response
> +      ResponseSize = Request->Length;
> +    } else if (Request->Length > ResponseSize) {
> +      DEBUG ((DEBUG_INFO, "USB: Info: ResponseSize < wLength\n"));
> +    }
> +
> +    EpTx (0, ResponseData, ResponseSize);
> +  }
> +  if (Descriptor) {
> +    FreePool (Descriptor);
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleSetAddress (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  // Pretty confused if bmRequestType is anything but this:
> +  ASSERT (Request->RequestType == USB_DEV_SET_ADDRESS_REQ_TYPE);
> +  DEBUG ((DEBUG_INFO, "USB: Setting address to %d\n", Request->Value));
> +  ResetEndpoints ();
> +
> +  WRITE_REG32 (DCFG, (READ_REG32 (DCFG) & ~DCFG_DEVADDR_MASK) | DCFG_DEVADDR(Request->Value));
> +  EpTx (0, 0, 0);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +UINTN
> +UsbDrvRequestEndpoint (
> +  IN UINTN           Type,
> +  IN UINTN           Dir
> +  )
> +{
> +  UINTN              Ep = 1;
> +  UINTN              Ret, NewBits;
> +
> +  Ret = Ep | Dir;
> +  NewBits = (Type << 18) | 0x10000000;
> +
> +  /*
> +   * (Type << 18):Endpoint Type (EPType)
> +   * 0x10000000:Endpoint Enable (EPEna)
> +   * 0x000C000:Endpoint Type (EPType);Hardcoded to 00 for control.
> +   * (ep<<22):TxFIFO Number (TxFNum)
> +   * 0x20000:NAK Status (NAKSts);The core is transmitting NAK handshakes on this endpoint.
> +   */
> +  if (Dir) {  // IN: to host
> +    WRITE_REG32 (DIEPCTL (Ep), ((READ_REG32 (DIEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits | (Ep << 22) | DXEPCTL_NAKSTS);
> +  } else {    // OUT: to device
> +    WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits);
> +  }
> +
> +  return Ret;
> +}
> +
> +STATIC
> +EFI_STATUS
> +HandleSetConfiguration (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  ASSERT (Request->RequestType == USB_DEV_SET_CONFIGURATION_REQ_TYPE);
> +
> +  // Cancel all transfers
> +  ResetEndpoints ();
> +
> +  UsbDrvRequestEndpoint (2, 0);
> +  UsbDrvRequestEndpoint (2, 0x80);
> +
> +  WRITE_REG32 (DIEPCTL1, (READ_REG32 (DIEPCTL1)) | BIT28 | BIT19 | DXEPCTL_USBACTEP | BIT11);
> +
> +  /* Enable interrupts on all endpoints */
> +  WRITE_REG32 (DAINTMSK, ~0);
> +
> +  EpRx (1, CMD_SIZE);
> +  EpTx (0, 0, 0);
> +  return EFI_SUCCESS;
> +}
> +
> +
> +STATIC
> +EFI_STATUS
> +HandleDeviceRequest (
> +  IN USB_DEVICE_REQUEST  *Request
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  switch (Request->Request) {
> +  case USB_DEV_GET_DESCRIPTOR:
> +    Status = HandleGetDescriptor (Request);
> +    break;
> +  case USB_DEV_SET_ADDRESS:
> +    Status = HandleSetAddress (Request);
> +    break;
> +  case USB_DEV_SET_CONFIGURATION:
> +    Status = HandleSetConfiguration (Request);
> +    break;
> +  default:
> +    DEBUG ((DEBUG_ERROR,
> +      "Didn't understand RequestType 0x%x Request 0x%x\n",
> +      Request->RequestType, Request->Request));
> +      Status = EFI_INVALID_PARAMETER;
> +    break;
> +  }
> +
> +  return Status;
> +}
> +
> +
> +// Instead of actually registering interrupt handlers, we poll the controller's
> +//  interrupt source register in this function.
> +STATIC
> +VOID
> +CheckInterrupts (
> +  IN EFI_EVENT        Event,
> +  IN VOID            *Context
> +  )
> +{
> +  UINT32              Ints, EpInts;
> +
> +
> +  // interrupt register
> +  Ints = READ_REG32 (GINTSTS);
> +
> +  /*
> +   * bus reset
> +   * The core sets this bit to indicate that a reset is detected on the USB.
> +   */
> +  if (Ints & GINTSTS_USBRST) {
> +    WRITE_REG32 (DCFG, DCFG_DESCDMA | DCFG_NZ_STS_OUT_HSHK);
> +    ResetEndpoints ();
> +  }
> +
> +  /*
> +   * enumeration done, we now know the speed
> +   * The core sets this bit to indicate that speed enumeration is complete. The
> +   * application must read the Device Status (DSTS) register to obtain the
> +   * enumerated speed.
> +   */
> +  if (Ints & GINTSTS_ENUMDONE) {
> +    /* Set up the maximum packet sizes accordingly */
> +    UINTN MaxPacket = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;
> +    //Set Maximum In Packet Size (MPS)
> +    WRITE_REG32 (DIEPCTL1, ((READ_REG32 (DIEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);
> +    //Set Maximum Out Packet Size (MPS)
> +    WRITE_REG32 (DOEPCTL1, ((READ_REG32 (DOEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);
> +  }
> +
> +  /*
> +   * IN EP event
> +   * The core sets this bit to indicate that an interrupt is pending on one of the IN
> +   * endpoInts of the core (in Device mode). The application must read the
> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
> +   * number of the IN endpoint on which the interrupt occurred, and then read
> +   * the corresponding Device IN Endpoint-n Interrupt (DIEPINTn) register to
> +   * determine the exact cause of the interrupt. The application must clear the
> +   * appropriate status bit in the corresponding DIEPINTn register to clear this bit.
> +   */
> +  if (Ints & GINTSTS_IEPINT) {
> +    EpInts = READ_REG32 (DIEPINT0);
> +    WRITE_REG32 (DIEPINT0, EpInts);
> +    if (EpInts & DXEPINT_XFERCOMPL) {
> +      DEBUG ((DEBUG_INFO, "INT: IN TX completed.DIEPTSIZ (0) = 0x%x.\n", READ_REG32 (DIEPTSIZ0)));
> +    }
> +
> +    EpInts = READ_REG32 (DIEPINT1);
> +    WRITE_REG32 (DIEPINT1, EpInts);
> +    if (EpInts & DXEPINT_XFERCOMPL) {
> +      DEBUG ((DEBUG_INFO, "ep1: IN TX completed\n"));
> +    }
> +  }
> +
> +  /*
> +   * OUT EP event
> +   * The core sets this bit to indicate that an interrupt is pending on one of the
> +   * OUT endpoints of the core (in Device mode). The application must read the
> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
> +   * number of the OUT endpoint on which the interrupt occurred, and then read
> +   * the corresponding Device OUT Endpoint-n Interrupt (DOEPINTn) register
> +   * to determine the exact cause of the interrupt. The application must clear the
> +   * appropriate status bit in the corresponding DOEPINTn register to clear this bit.
> +   */
> +  if (Ints & GINTSTS_OEPINT) {
> +    /* indicates the status of an endpoint
> +     * with respect to USB- and AHB-related events. */
> +    EpInts = READ_REG32 (DOEPINT0);
> +    if (EpInts) {
> +      WRITE_REG32 (DOEPINT0, EpInts);
> +      if (EpInts & DXEPINT_XFERCOMPL) {
> +        DEBUG ((DEBUG_INFO, "INT: EP0 RX completed. DOEPTSIZ(0) = 0x%x.\n", READ_REG32 (DOEPTSIZ0)));
> +      }
> +      /*
> +       *
> +       IN Token Received When TxFIFO is Empty (INTknTXFEmp)
> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
> +       * was received.
> +       */
> +      if (EpInts & BIT3) { /* SETUP phase done */
> +        WRITE_REG32 (DIEPCTL0, READ_REG32 (DIEPCTL0) | DXEPCTL_SNAK);
> +        WRITE_REG32 (DOEPCTL0, READ_REG32 (DOEPCTL0) | DXEPCTL_SNAK);
> +        /*clear IN EP intr*/
> +        WRITE_REG32 (DIEPINT0, ~0);
> +        HandleDeviceRequest((USB_DEVICE_REQUEST *)gCtrlReq);
> +      }
> +
> +      /* Make sure EP0 OUT is set up to accept the next request */
> +      WRITE_REG32 (DOEPTSIZ0, DXEPTSIZ_SUPCNT(3) | DXEPTSIZ_PKTCNT(1) | DXEPTSIZ_XFERSIZE(64));
> +      /*
> +       * IN Token Received When TxFIFO is Empty (INTknTXFEmp)
> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
> +       * was received.
> +       */
> +      gDmaDescEp0->status.b.bs = 0x3;
> +      gDmaDescEp0->status.b.mtrf = 0;
> +      gDmaDescEp0->status.b.sr = 0;
> +      gDmaDescEp0->status.b.l = 1;
> +      gDmaDescEp0->status.b.ioc = 1;
> +      gDmaDescEp0->status.b.sp = 0;
> +      gDmaDescEp0->status.b.bytes = 64;
> +      gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);
> +      gDmaDescEp0->status.b.sts = 0;
> +      gDmaDescEp0->status.b.bs = 0x0;
> +      WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
> +      // endpoint enable; clear NAK
> +      WRITE_REG32 (DOEPCTL0, DXEPCTL_EPENA | DXEPCTL_CNAK);
> +    }
> +
> +    EpInts = (READ_REG32 (DOEPINT1));
> +    if (EpInts) {
> +      WRITE_REG32 (DOEPINT1, EpInts);
> +      /* Transfer Completed Interrupt (XferCompl);Transfer completed */
> +      if (EpInts & DXEPINT_XFERCOMPL) {
> +
> +        UINTN Bytes = RxDescBytes - gDmaDesc->status.b.bytes;
> +        UINTN Len = 0;
> +
> +        ArmDataSynchronizationBarrier ();
> +        if (MATCH_CMD_LITERAL ("download", RxBuf)) {
> +          mNumDataBytes = AsciiStrHexToUint64 (RxBuf + sizeof ("download"));
> +        } else {
> +          if (mNumDataBytes != 0) {
> +            mNumDataBytes -= Bytes;
> +          }
> +        }
> +
> +        mDataReceivedCallback (Bytes, RxBuf);
> +
> +        if (mNumDataBytes == 0) {
> +          Len = CMD_SIZE;
> +        } else if (mNumDataBytes > DATA_SIZE) {
> +          Len = DATA_SIZE;
> +        } else {
> +          Len = mNumDataBytes;
> +        }
> +
> +        EpRx (1, Len);
> +      }
> +    }
> +  }
> +
> +  //WRITE_REG32 clear ints
> +  WRITE_REG32 (GINTSTS, Ints);
> +}
> +
> +EFI_STATUS
> +DwUsbSend (
> +  IN        UINT8  EndpointIndex,
> +  IN        UINTN  Size,
> +  IN  CONST VOID  *Buffer
> +  )
> +{
> +    EpTx (EndpointIndex, Buffer, Size);
> +    return EFI_SUCCESS;
> +}
> +
> +STATIC
> +VOID
> +DwUsbInit (
> +  VOID
> +  )
> +{
> +  VOID     *Buf;
> +  UINT32   Data;
> +
> +  Buf = UncachedAllocatePages (16);
> +  gDmaDesc = Buf;
> +  gDmaDescEp0 = gDmaDesc + sizeof (dwc_otg_dev_dma_desc_t);
> +  gDmaDescIn = gDmaDescEp0 + sizeof (dwc_otg_dev_dma_desc_t);
> +  gCtrlReq = (USB_DEVICE_REQUEST *)gDmaDescIn + sizeof (dwc_otg_dev_dma_desc_t);
> +
> +  ZeroMem (gDmaDesc, sizeof (dwc_otg_dev_dma_desc_t));
> +  ZeroMem (gDmaDescEp0, sizeof (dwc_otg_dev_dma_desc_t));
> +  ZeroMem (gDmaDescIn, sizeof (dwc_otg_dev_dma_desc_t));
> +
> +  /*Reset usb controller.*/
> +  /* Wait for OTG AHB master idle */
> +  do {
> +    Data = READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE;
> +  } while (Data == 0);
> +
> +  /* OTG: Assert Software Reset */
> +  WRITE_REG32 (GRSTCTL, GRSTCTL_CSFTRST);
> +
> +  /* Wait for OTG to ack reset */
> +  while (READ_REG32 (GRSTCTL) & GRSTCTL_CSFTRST);
> +
> +  /* Wait for OTG AHB master idle */
> +  while ((READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE) == 0);
> +
> +  WRITE_REG32 (GDFIFOCFG, DATA_FIFO_CONFIG);
> +  WRITE_REG32 (GRXFSIZ, RX_SIZE);
> +  WRITE_REG32 (GNPTXFSIZ, ENDPOINT_TX_SIZE);
> +  WRITE_REG32 (DIEPTXF1, DATA_IN_ENDPOINT_TX_FIFO1);
> +  WRITE_REG32 (DIEPTXF2, DATA_IN_ENDPOINT_TX_FIFO2);
> +  WRITE_REG32 (DIEPTXF3, DATA_IN_ENDPOINT_TX_FIFO3);
> +  WRITE_REG32 (DIEPTXF4, DATA_IN_ENDPOINT_TX_FIFO4);
> +  WRITE_REG32 (DIEPTXF5, DATA_IN_ENDPOINT_TX_FIFO5);
> +  WRITE_REG32 (DIEPTXF6, DATA_IN_ENDPOINT_TX_FIFO6);
> +  WRITE_REG32 (DIEPTXF7, DATA_IN_ENDPOINT_TX_FIFO7);
> +  WRITE_REG32 (DIEPTXF8, DATA_IN_ENDPOINT_TX_FIFO8);
> +  WRITE_REG32 (DIEPTXF9, DATA_IN_ENDPOINT_TX_FIFO9);
> +  WRITE_REG32 (DIEPTXF10, DATA_IN_ENDPOINT_TX_FIFO10);
> +  WRITE_REG32 (DIEPTXF11, DATA_IN_ENDPOINT_TX_FIFO11);
> +  WRITE_REG32 (DIEPTXF12, DATA_IN_ENDPOINT_TX_FIFO12);
> +  WRITE_REG32 (DIEPTXF13, DATA_IN_ENDPOINT_TX_FIFO13);
> +  WRITE_REG32 (DIEPTXF14, DATA_IN_ENDPOINT_TX_FIFO14);
> +  WRITE_REG32 (DIEPTXF15, DATA_IN_ENDPOINT_TX_FIFO15);
> +
> +  /*
> +   * set Periodic TxFIFO Empty Level,
> +   * Non-Periodic TxFIFO Empty Level,
> +   * Enable DMA, Unmask Global Intr
> +   */
> +  WRITE_REG32 (GAHBCFG, GAHBCFG_CTRL_MASK);
> +
> +  /*select 8bit UTMI+, ULPI Inerface*/
> +  WRITE_REG32 (GUSBCFG, 0x2400);
> +
> +  /* Detect usb work mode,host or device? */
> +  do {
> +    Data = READ_REG32 (GINTSTS);
> +  } while (Data & GINTSTS_CURMODE_HOST);
> +  MicroSecondDelay (3);
> +
> +  /*Init global and device mode csr register.*/
> +  /*set Non-Zero-Length status out handshake */
> +  Data = (0x20 << DCFG_EPMISCNT_SHIFT) | DCFG_NZ_STS_OUT_HSHK;
> +  WRITE_REG32 (DCFG, Data);
> +
> +  /* Interrupt unmask: IN event, OUT event, bus reset */
> +  Data = GINTSTS_OEPINT | GINTSTS_IEPINT | GINTSTS_ENUMDONE | GINTSTS_USBRST;
> +  WRITE_REG32 (GINTMSK, Data);
> +
> +  do {
> +    Data = READ_REG32 (GINTSTS) & GINTSTS_ENUMDONE;
> +  } while (Data);
> +
> +  /* Clear any pending OTG Interrupts */
> +  WRITE_REG32 (GOTGINT, ~0);
> +  /* Clear any pending interrupts */
> +  WRITE_REG32 (GINTSTS, ~0);
> +  WRITE_REG32 (GINTMSK, ~0);
> +  Data = READ_REG32 (GOTGINT);
> +  Data &= ~0x3000;
> +  WRITE_REG32 (GOTGINT, Data);
> +
> +  /* endpoint settings cfg */
> +  ResetEndpoints ();
> +  MicroSecondDelay (1);
> +
> +  /* init finish. and ready to transfer data */
> +
> +  /* Soft Disconnect */
> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE | DCTL_SFTDISCON);
> +  MicroSecondDelay (10000);
> +
> +  /* Soft Reconnect */
> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +DwUsbStart (
> +  IN USB_DEVICE_DESCRIPTOR   *DeviceDescriptor,
> +  IN VOID                   **Descriptors,
> +  IN USB_DEVICE_RX_CALLBACK   RxCallback,
> +  IN USB_DEVICE_TX_CALLBACK   TxCallback
> +  )
> +{
> +  UINT8                    *Ptr;
> +  EFI_STATUS                Status;
> +  EFI_EVENT                 TimerEvent;
> +
> +  ASSERT (DeviceDescriptor != NULL);
> +  ASSERT (Descriptors[0] != NULL);
> +  ASSERT (RxCallback != NULL);
> +  ASSERT (TxCallback != NULL);
> +
> +  DwUsbInit();
> +
> +  mDeviceDescriptor = DeviceDescriptor;
> +  mDescriptors = Descriptors[0];
> +
> +  // Right now we just support one configuration
> +  ASSERT (mDeviceDescriptor->NumConfigurations == 1);
> +  mDeviceDescriptor->StrManufacturer = 1;
> +  mDeviceDescriptor->StrProduct = 2;
> +  mDeviceDescriptor->StrSerialNumber = 3;
> +  // ... and one interface
> +  mConfigDescriptor = (USB_CONFIG_DESCRIPTOR *)mDescriptors;
> +  ASSERT (mConfigDescriptor->NumInterfaces == 1);
> +
> +  Ptr = ((UINT8 *) mDescriptors) + sizeof (USB_CONFIG_DESCRIPTOR);
> +  mInterfaceDescriptor = (USB_INTERFACE_DESCRIPTOR *) Ptr;
> +  Ptr += sizeof (USB_INTERFACE_DESCRIPTOR);
> +
> +  mEndpointDescriptors = (USB_ENDPOINT_DESCRIPTOR *) Ptr;
> +
> +  mDataReceivedCallback = RxCallback;
> +  mDataSentCallback = TxCallback;
> +
> +  // Register a timer event so CheckInterupts gets called periodically
> +  Status = gBS->CreateEvent (
> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  CheckInterrupts,
> +                  NULL,
> +                  &TimerEvent
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = gBS->SetTimer (
> +                  TimerEvent,
> +                  TimerPeriodic,
> +                  DW_INTERRUPT_POLL_PERIOD
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
> +USB_DEVICE_PROTOCOL mUsbDevice = {
> +  DwUsbStart,
> +  DwUsbSend
> +};
> +
> +
> +EFI_STATUS
> +EFIAPI
> +DwUsbEntryPoint (
> +  IN EFI_HANDLE                            ImageHandle,
> +  IN EFI_SYSTEM_TABLE                      *SystemTable
> +  )
> +{
> +  EFI_STATUS      Status;
> +
> +  Status = gBS->LocateProtocol (&gDwUsbProtocolGuid, NULL, (VOID **) &DwUsb);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = DwUsb->PhyInit(USB_DEVICE_MODE);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return gBS->InstallProtocolInterface (
> +                &ImageHandle,
> +                &gUsbDeviceProtocolGuid,
> +                EFI_NATIVE_INTERFACE,
> +                &mUsbDevice
> +                );
> +}
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> new file mode 100644
> index 0000000..f991492
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> @@ -0,0 +1,46 @@
> +#/** @file
> +# Framework Module Development Environment Industry Standards
> +#
> +# This Package provides headers and libraries that conform to EFI/PI Industry standards.
> +# Copyright (c) 2007, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2012-2014, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2015-2017, Linaro. All rights reserved.<BR>
> +#
> +#    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.
> +#
> +#**/
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 0x00010019
> +  PACKAGE_NAME                   = OpenPlatformDriversUsbDwUsbDxePkg
> +  PACKAGE_GUID                   = 114a3be9-10f7-4bf1-81ca-09ac52d4c3d5
> +  PACKAGE_VERSION                = 0.1
> +
> +
> +################################################################################
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#                   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +################################################################################
> +
> +[Guids.common]
> +  gAndroidFastbootUsbGuid       = { 0xf6bec3fe, 0x88fb, 0x11e3, { 0xae, 0x84, 0xe7, 0x3b, 0x77, 0x56, 0x1c, 0x35 }}
> +  gDwUsbDxeTokenSpaceGuid       = { 0x131c4d02, 0x9449, 0x4ee9, { 0xba, 0x3d, 0x69, 0x50, 0x21, 0x89, 0x26, 0x0b }}
> +
> +[Protocols.common]
> +  gDwUsbProtocolGuid            = { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> +
> +[PcdsFixedAtBuild.common]
> +  # DwUsb Driver PCDs
> +  gDwUsbDxeTokenSpaceGuid.PcdDwUsbDxeBaseAddress|0x0|UINT32|0x00000001
> +  gDwUsbDxeTokenSpaceGuid.PcdSysCtrlBaseAddress|0x0|UINT32|0x00000002
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.h b/Drivers/Usb/DwUsbDxe/DwUsbDxe.h
> new file mode 100644
> index 0000000..7f909c3
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.h
> @@ -0,0 +1,627 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 __DW_USB_DXE_H__
> +#define __DW_USB_DXE_H__
> +
> +#define DW_USB_BASE                     FixedPcdGet32 (PcdDwUsbDxeBaseAddress)
> +
> +#define READ_REG32(Offset)              MmioRead32 (DW_USB_BASE + Offset)
> +#define READ_REG16(Offset)              (UINT16)READ_REG32 (Offset)
> +#define WRITE_REG32(Offset, Val)        MmioWrite32 (DW_USB_BASE + Offset, Val)
> +#define WRITE_REG16(Offset, Val)        MmioWrite32 (DW_USB_BASE + Offset, (UINT32) Val)
> +#define WRITE_REG8(Offset, Val)         MmioWrite32 (DW_USB_BASE + Offset, (UINT32) Val)
> +
> +// Max packet size in bytes (For Full Speed USB 64 is the only valid value)
> +#define MAX_PACKET_SIZE_CONTROL         64
> +
> +#define MAX_PACKET_SIZE_BULK            512
> +
> +// 8 Endpoints, in and out. Don't count the Endpoint 0 setup buffer
> +#define DW_NUM_ENDPOINTS                16
> +
> +// Endpoint Indexes
> +#define DW_EP0SETUP                     0x20
> +#define DW_EP0RX                        0x00
> +#define DW_EP0TX                        0x01
> +#define DW_EP1RX                        0x02
> +#define DW_EP1TX                        0x03
> +
> +// DcInterrupt bits
> +#define DW_DC_INTERRUPT_EP1TX           BIT13
> +#define DW_DC_INTERRUPT_EP1RX           BIT12
> +#define DW_DC_INTERRUPT_EP0TX           BIT11
> +#define DW_DC_INTERRUPT_EP0RX           BIT10
> +#define DW_DC_INTERRUPT_EP0SETUP        BIT8
> +#define DW_DC_INTERRUPT_VBUS            BIT7
> +#define DW_DC_INTERRUPT_DMA             BIT6
> +#define DW_DC_INTERRUPT_HS_STAT         BIT5
> +#define DW_DC_INTERRUPT_RESUME          BIT4
> +#define DW_DC_INTERRUPT_SUSP            BIT3
> +#define DW_DC_INTERRUPT_PSOF            BIT2
> +#define DW_DC_INTERRUPT_SOF             BIT1
> +#define DW_DC_INTERRUPT_BRESET          BIT0
> +// All valid peripheral controller int  rrupts
> +#define DW_DC_INTERRUPT_MASK            0x003FFFDFF
> +
> +#define DW_ADDRESS                      0x200
> +#define DW_ADDRESS_DEVEN                BIT7
> +
> +#define DW_MODE                         0x20C
> +#define DW_MODE_DATA_BUS_WIDTH          BIT8
> +#define DW_MODE_CLKAON                  BIT7
> +#define DW_MODE_SFRESET                 BIT4
> +#define DW_MODE_WKUPCS                  BIT2
> +
> +#define DW_ENDPOINT_MAX_PACKET_SIZE     0x204
> +
> +#define DW_ENDPOINT_TYPE                0x208
> +#define DW_ENDPOINT_TYPE_NOEMPKT        BIT4
> +#define DW_ENDPOINT_TYPE_ENABLE         BIT3
> +
> +#define DW_INTERRUPT_CONFIG             0x210
> +// Interrupt config value to only interrupt on ACK of IN and OUT tokens
> +#define DW_INTERRUPT_CONFIG_ACK_ONLY    (BIT2 | BIT5 | BIT6)
> +
> +#define DW_DC_INTERRUPT                 0x218
> +#define DW_DC_INTERRUPT_ENABLE          0x214
> +
> +#define DW_CTRL_FUNCTION                0x228
> +#define DW_CTRL_FUNCTION_VENDP          BIT3
> +#define DW_CTRL_FUNCTION_DSEN           BIT2
> +#define DW_CTRL_FUNCTION_STATUS         BIT1
> +
> +#define DW_DEVICE_UNLOCK                0x27C
> +#define DW_DEVICE_UNLOCK_MAGIC          0xAA37
> +
> +#define DW_SW_RESET_REG                 0x30C
> +#define DW_SW_RESET_ALL                 BIT0
> +
> +#define DW_DEVICE_ID                    0x370
> +
> +#define DW_OTG_CTRL_SET                 0x374
> +#define DW_OTG_CTRL_CLR                 (OTG_CTRL_SET + 2)
> +#define DW_OTG_CTRL_OTG_DISABLE         BIT10
> +#define DW_OTG_CTRL_VBUS_CHRG           BIT6
> +#define DW_OTG_CTRL_VBUS_DISCHRG        BIT5
> +#define DW_OTG_CTRL_DM_PULLDOWN         BIT2
> +#define DW_OTG_CTRL_DP_PULLDOWN         BIT1
> +#define DW_OTG_CTRL_DP_PULLUP           BIT0
> +
> +#define DW_OTG_STATUS                   0x378
> +#define DW_OTG_STATUS_B_SESS_END        BIT7
> +#define DW_OTG_STATUS_A_B_SESS_VLD      BIT1
> +
> +#define DW_OTG_INTERRUPT_LATCH_SET      0x37C
> +#define DW_OTG_INTERRUPT_LATCH_CLR      0x37E
> +#define DW_OTG_INTERRUPT_ENABLE_RISE    0x384
> +
> +#define DW_DMA_ENDPOINT_INDEX           0x258
> +
> +#define DW_ENDPOINT_INDEX               0x22c
> +#define DW_DATA_PORT                    0x220
> +#define DW_BUFFER_LENGTH                0x21c
> +
> +// Device ID Values
> +#define PHILLIPS_VENDOR_ID_VAL          0x04cc
> +#define DW_PRODUCT_ID_VAL               0x1761
> +#define DW_DEVICE_ID_VAL                ((ISP1761_PRODUCT_ID_VAL << 16) | \
> +                                         PHILLIPS_VENDOR_ID_VAL)
> +
> +#define DWC_OTG_BASE                    DW_USB_BASE
> +
> +#define USB_NUM_ENDPOINTS               2
> +#define MAX_EPS_CHANNELS                16
> +
> +#define BULK_OUT_EP                     1
> +#define BULK_IN_EP                      1
> +
> +#define RX_REQ_LEN                      512
> +#define MAX_PACKET_LEN                  512
> +
> +#define DATA_FIFO_CONFIG                0x0F801000
> +/* RX FIFO: 2048 bytes */
> +#define RX_SIZE                         0x00000200
> +/* Non-periodic TX FIFO: 128 bytes. start address: 0x200 * 4. */
> +#define ENDPOINT_TX_SIZE                0x01000200
> +
> +/* EP1  TX FIFO: 1024 bytes. start address: 0x220 * 4. */
> +/* EP2  TX FIFO: 1024 bytes. start address: 0x320 * 4. */
> +/* EP3  TX FIFO: 1024 bytes. start address: 0x420 * 4. */
> +/* EP4  TX FIFO: 1024 bytes. start address: 0x520 * 4. */
> +/* EP5  TX FIFO: 128 bytes. start address: 0x620 * 4. */
> +/* EP6  TX FIFO: 128 bytes. start address: 0x640 * 4. */
> +/* EP7  TX FIFO: 128 bytes. start address: 0x660 * 4. */
> +/* EP8  TX FIFO: 128 bytes. start address: 0x680 * 4. */
> +/* EP9  TX FIFO: 128 bytes. start address: 0x6A0 * 4. */
> +/* EP10 TX FIFO: 128 bytes. start address: 0x6C0 * 4. */
> +/* EP11 TX FIFO: 128  bytes. start address: 0x6E0 * 4. */
> +/* EP12 TX FIFO: 128  bytes. start address: 0x700 * 4. */
> +/* EP13 TX FIFO: 128  bytes. start address: 0x720 * 4. */
> +/* EP14 TX FIFO: 128  bytes. start address: 0x740 * 4. */
> +/* EP15 TX FIFO: 128  bytes. start address: 0x760 * 4. */
> +
> +#define DATA_IN_ENDPOINT_TX_FIFO1       0x01000220
> +#define DATA_IN_ENDPOINT_TX_FIFO2       0x01000320
> +#define DATA_IN_ENDPOINT_TX_FIFO3       0x01000420
> +#define DATA_IN_ENDPOINT_TX_FIFO4       0x01000520
> +#define DATA_IN_ENDPOINT_TX_FIFO5       0x00200620
> +#define DATA_IN_ENDPOINT_TX_FIFO6       0x00200640
> +#define DATA_IN_ENDPOINT_TX_FIFO7       0x00200680
> +#define DATA_IN_ENDPOINT_TX_FIFO8       0x002006A0
> +#define DATA_IN_ENDPOINT_TX_FIFO9       0x002006C0
> +#define DATA_IN_ENDPOINT_TX_FIFO10      0x002006E0
> +#define DATA_IN_ENDPOINT_TX_FIFO11      0x00200700
> +#define DATA_IN_ENDPOINT_TX_FIFO12      0x00200720
> +#define DATA_IN_ENDPOINT_TX_FIFO13      0x00200740
> +#define DATA_IN_ENDPOINT_TX_FIFO14      0x00200760
> +#define DATA_IN_ENDPOINT_TX_FIFO15      0x00200F00
> +
> +/*DWC_OTG regsiter descriptor*/
> +/*Device mode CSR MAP*/
> +#define DEVICE_CSR_BASE                 0x800
> +/*Device mode CSR MAP*/
> +#define DEVICE_INEP_BASE                0x900
> +/*Device mode CSR MAP*/
> +#define DEVICE_OUTEP_BASE               0xB00
> +
> +/*** OTG LINK CORE REGISTERS ***/
> +/* Core Global Registers */
> +#define GOTGCTL                         0x000
> +#define GOTGINT                         0x004
> +#define GAHBCFG                         0x008
> +#define GAHBCFG_P_TXF_EMP_LVL           (1 << 8)
> +#define GAHBCFG_NP_TXF_EMP_LVL          (1 << 7)
> +#define GAHBCFG_DMA_EN                  (1 << 5)
> +#define GAHBCFG_GLBL_INTR_EN            (1 << 0)
> +#define GAHBCFG_CTRL_MASK               (GAHBCFG_P_TXF_EMP_LVL | \
> +                                         GAHBCFG_NP_TXF_EMP_LVL | \
> +                                         GAHBCFG_DMA_EN | \
> +                                         GAHBCFG_GLBL_INTR_EN)
> +
> +#define GUSBCFG                         0x00C
> +#define GRSTCTL                         0x010
> +#define GRSTCTL_AHBIDLE                 (1 << 31)
> +#define GRSTCTL_CSFTRST                 (1 << 0)
> +
> +#define GINTSTS                         0x014
> +#define GINTSTS_WKUPINT                 BIT31
> +#define GINTSTS_SESSREGINT              BIT30
> +#define GINTSTS_DISCONNINT              BIT29
> +#define GINTSTS_CONIDSTSCHNG            BIT28
> +#define GINTSTS_LPMTRANRCVD             BIT27
> +#define GINTSTS_PTXFEMP                 BIT26
> +#define GINTSTS_HCHINT                  BIT25
> +#define GINTSTS_PRTINT                  BIT24
> +#define GINTSTS_RESETDET                BIT23
> +#define GINTSTS_FET_SUSP                BIT22
> +#define GINTSTS_INCOMPL_IP              BIT21
> +#define GINTSTS_INCOMPL_SOIN            BIT20
> +#define GINTSTS_OEPINT                  BIT19
> +#define GINTSTS_IEPINT                  BIT18
> +#define GINTSTS_EPMIS                   BIT17
> +#define GINTSTS_RESTOREDONE             BIT16
> +#define GINTSTS_EOPF                    BIT15
> +#define GINTSTS_ISOUTDROP               BIT14
> +#define GINTSTS_ENUMDONE                BIT13
> +#define GINTSTS_USBRST                  BIT12
> +#define GINTSTS_USBSUSP                 BIT11
> +#define GINTSTS_ERLYSUSP                BIT10
> +#define GINTSTS_I2CINT                  BIT9
> +#define GINTSTS_ULPI_CK_INT             BIT8
> +#define GINTSTS_GOUTNAKEFF              BIT7
> +#define GINTSTS_GINNAKEFF               BIT6
> +#define GINTSTS_NPTXFEMP                BIT5
> +#define GINTSTS_RXFLVL                  BIT4
> +#define GINTSTS_SOF                     BIT3
> +#define GINTSTS_OTGINT                  BIT2
> +#define GINTSTS_MODEMIS                 BIT1
> +#define GINTSTS_CURMODE_HOST            BIT0
> +
> +#define GINTMSK                         0x018
> +#define GRXSTSR                         0x01C
> +#define GRXSTSP                         0x020
> +#define GRXFSIZ                         0x024
> +#define GNPTXFSIZ                       0x028
> +#define GNPTXSTS                        0x02C
> +
> +#define GHWCFG1                         0x044
> +#define GHWCFG2                         0x048
> +#define GHWCFG3                         0x04c
> +#define GHWCFG4                         0x050
> +#define GLPMCFG                         0x054
> +
> +#define GDFIFOCFG                       0x05c
> +
> +#define HPTXFSIZ                        0x100
> +#define DIEPTXF(x)                      (0x100 + 4 * (x))
> +#define DIEPTXF1                        0x104
> +#define DIEPTXF2                        0x108
> +#define DIEPTXF3                        0x10C
> +#define DIEPTXF4                        0x110
> +#define DIEPTXF5                        0x114
> +#define DIEPTXF6                        0x118
> +#define DIEPTXF7                        0x11C
> +#define DIEPTXF8                        0x120
> +#define DIEPTXF9                        0x124
> +#define DIEPTXF10                       0x128
> +#define DIEPTXF11                       0x12C
> +#define DIEPTXF12                       0x130
> +#define DIEPTXF13                       0x134
> +#define DIEPTXF14                       0x138
> +#define DIEPTXF15                       0x13C
> +
> +/*** HOST MODE REGISTERS ***/
> +/* Host Global Registers */
> +#define HCFG                            0x400
> +#define HFIR                            0x404
> +#define HFNUM                           0x408
> +#define HPTXSTS                         0x410
> +#define HAINT                           0x414
> +#define HAINTMSK                        0x418
> +
> +/* Host Port Control and Status Registers */
> +#define HPRT                            0x440
> +
> +/* Host Channel-Specific Registers */
> +#define HCCHAR(x)                       (0x500 + 0x20 * (x))
> +#define HCSPLT(x)                       (0x504 + 0x20 * (x))
> +#define HCINT(x)                        (0x508 + 0x20 * (x))
> +#define HCINTMSK(x)                     (0x50C + 0x20 * (x))
> +#define HCTSIZ(x)                       (0x510 + 0x20 * (x))
> +#define HCDMA(x)                        (0x514 + 0x20 * (x))
> +#define HCCHAR0                         0x500
> +#define HCSPLT0                         0x504
> +#define HCINT0                          0x508
> +#define HCINTMSK0                       0x50C
> +#define HCTSIZ0                         0x510
> +#define HCDMA0                          0x514
> +#define HCCHAR1                         0x520
> +#define HCSPLT1                         0x524
> +#define HCINT1                          0x528
> +#define HCINTMSK1                       0x52C
> +#define HCTSIZ1                         0x530
> +#define HCDMA1                          0x534
> +#define HCCHAR2                         0x540
> +#define HCSPLT2                         0x544
> +#define HCINT2                          0x548
> +#define HCINTMSK2                       0x54C
> +#define HCTSIZ2                         0x550
> +#define HCDMA2                          0x554
> +#define HCCHAR3                         0x560
> +#define HCSPLT3                         0x564
> +#define HCINT3                          0x568
> +#define HCINTMSK3                       0x56C
> +#define HCTSIZ3                         0x570
> +#define HCDMA3                          0x574
> +#define HCCHAR4                         0x580
> +#define HCSPLT4                         0x584
> +#define HCINT4                          0x588
> +#define HCINTMSK4                       0x58C
> +#define HCTSIZ4                         0x590
> +#define HCDMA4                          0x594
> +#define HCCHAR5                         0x5A0
> +#define HCSPLT5                         0x5A4
> +#define HCINT5                          0x5A8
> +#define HCINTMSK5                       0x5AC
> +#define HCTSIZ5                         0x5B0
> +#define HCDMA5                          0x5B4
> +#define HCCHAR6                         0x5C0
> +#define HCSPLT6                         0x5C4
> +#define HCINT6                          0x5C8
> +#define HCINTMSK6                       0x5CC
> +#define HCTSIZ6                         0x5D0
> +#define HCDMA6                          0x5D4
> +#define HCCHAR7                         0x5E0
> +#define HCSPLT7                         0x5E4
> +#define HCINT7                          0x5E8
> +#define HCINTMSK7                       0x5EC
> +#define HCTSIZ7                         0x5F0
> +#define HCDMA7                          0x5F4
> +#define HCCHAR8                         0x600
> +#define HCSPLT8                         0x604
> +#define HCINT8                          0x608
> +#define HCINTMSK8                       0x60C
> +#define HCTSIZ8                         0x610
> +#define HCDMA8                          0x614
> +#define HCCHAR9                         0x620
> +#define HCSPLT9                         0x624
> +#define HCINT9                          0x628
> +#define HCINTMSK9                       0x62C
> +#define HCTSIZ9                         0x630
> +#define HCDMA9                          0x634
> +#define HCCHAR10                        0x640
> +#define HCSPLT10                        0x644
> +#define HCINT10                         0x648
> +#define HCINTMSK10                      0x64C
> +#define HCTSIZ10                        0x650
> +#define HCDMA10                         0x654
> +#define HCCHAR11                        0x660
> +#define HCSPLT11                        0x664
> +#define HCINT11                         0x668
> +#define HCINTMSK11                      0x66C
> +#define HCTSIZ11                        0x670
> +#define HCDMA11                         0x674
> +#define HCCHAR12                        0x680
> +#define HCSPLT12                        0x684
> +#define HCINT12                         0x688
> +#define HCINTMSK12                      0x68C
> +#define HCTSIZ12                        0x690
> +#define HCDMA12                         0x694
> +#define HCCHAR13                        0x6A0
> +#define HCSPLT13                        0x6A4
> +#define HCINT13                         0x6A8
> +#define HCINTMSK13                      0x6AC
> +#define HCTSIZ13                        0x6B0
> +#define HCDMA13                         0x6B4
> +#define HCCHAR14                        0x6C0
> +#define HCSPLT14                        0x6C4
> +#define HCINT14                         0x6C8
> +#define HCINTMSK14                      0x6CC
> +#define HCTSIZ14                        0x6D0
> +#define HCDMA14                         0x6D4
> +#define HCCHAR15                        0x6E0
> +#define HCSPLT15                        0x6E4
> +#define HCINT15                         0x6E8
> +#define HCINTMSK15                      0x6EC
> +#define HCTSIZ15                        0x6F0
> +#define HCDMA15                         0x6F4
> +
> +/*** DEVICE MODE REGISTERS ***/
> +/* Device Global Registers */
> +#define DCFG                            0x800
> +#define DCFG_DESCDMA                    BIT23
> +#define DCFG_EPMISCNT_MASK              (0x1F << 18)
> +#define DCFG_EPMISCNT_SHIFT             18
> +#define DCFG_DEVADDR_MASK               (0x7F << 4)
> +#define DCFG_DEVADDR_SHIFT              4
> +#define DCFG_DEVADDR(x)                 (((x) << DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK)
> +#define DCFG_NZ_STS_OUT_HSHK            BIT2
> +
> +#define DCTL                            0x804
> +#define DCTL_PWRONPRGDONE               BIT11
> +#define DCTL_GNPINNAKSTS                BIT2
> +#define DCTL_SFTDISCON                  BIT1
> +
> +#define DSTS                            0x808
> +#define DIEPMSK                         0x810
> +#define DOEPMSK                         0x814
> +
> +#define DXEPMSK_TIMEOUTMSK              BIT3
> +#define DXEPMSK_AHBERMSK                BIT2
> +#define DXEPMSK_XFERCOMPLMSK            BIT0
> +
> +#define DAINT                           0x818
> +#define DAINTMSK                        0x81C
> +
> +#define DAINTMSK_OUTEPMSK_SHIFT         16
> +#define DAINTMSK_INEPMSK_SHIFT          0
> +
> +#define DTKNQR1                         0x820
> +#define DTKNQR2                         0x824
> +#define DVBUSDIS                        0x828
> +#define DVBUSPULSE                      0x82C
> +#define DTHRCTL                         0x830
> +
> +/* Device Logical IN Endpoint-Specific Registers */
> +#define DIEPCTL(x)                      (0x900 + 0x20 * (x))
> +#define DIEPINT(x)                      (0x908 + 0x20 * (x))
> +#define DIEPTSIZ(x)                     (0x910 + 0x20 * (x))
> +#define DIEPDMA(x)                      (0x914 + 0x20 * (x))
> +#define DTXFSTS(x)                      (0x918 + 0x20 * (x))
> +
> +#define DIEPCTL0                        0x900
> +#define DIEPINT0                        0x908
> +#define DIEPTSIZ0                       0x910
> +#define DIEPDMA0                        0x914
> +#define DIEPCTL1                        0x920
> +#define DIEPINT1                        0x928
> +#define DIEPTSIZ1                       0x930
> +#define DIEPDMA1                        0x934
> +#define DIEPCTL2                        0x940
> +#define DIEPINT2                        0x948
> +#define DIEPTSIZ2                       0x950
> +#define DIEPDMA2                        0x954
> +#define DIEPCTL3                        0x960
> +#define DIEPINT3                        0x968
> +#define DIEPTSIZ3                       0x970
> +#define DIEPDMA3                        0x974
> +#define DIEPCTL4                        0x980
> +#define DIEPINT4                        0x988
> +#define DIEPTSIZ4                       0x990
> +#define DIEPDMA4                        0x994
> +#define DIEPCTL5                        0x9A0
> +#define DIEPINT5                        0x9A8
> +#define DIEPTSIZ5                       0x9B0
> +#define DIEPDMA5                        0x9B4
> +#define DIEPCTL6                        0x9C0
> +#define DIEPINT6                        0x9C8
> +#define DIEPTSIZ6                       0x9D0
> +#define DIEPDMA6                        0x9D4
> +#define DIEPCTL7                        0x9E0
> +#define DIEPINT7                        0x9E8
> +#define DIEPTSIZ7                       0x9F0
> +#define DIEPDMA7                        0x9F4
> +#define DIEPCTL8                        0xA00
> +#define DIEPINT8                        0xA08
> +#define DIEPTSIZ8                       0xA10
> +#define DIEPDMA8                        0xA14
> +#define DIEPCTL9                        0xA20
> +#define DIEPINT9                        0xA28
> +#define DIEPTSIZ9                       0xA30
> +#define DIEPDMA9                        0xA34
> +#define DIEPCTL10                       0xA40
> +#define DIEPINT10                       0xA48
> +#define DIEPTSIZ10                      0xA50
> +#define DIEPDMA10                       0xA54
> +#define DIEPCTL11                       0xA60
> +#define DIEPINT11                       0xA68
> +#define DIEPTSIZ11                      0xA70
> +#define DIEPDMA11                       0xA74
> +#define DIEPCTL12                       0xA80
> +#define DIEPINT12                       0xA88
> +#define DIEPTSIZ12                      0xA90
> +#define DIEPDMA12                       0xA94
> +#define DIEPCTL13                       0xAA0
> +#define DIEPINT13                       0xAA8
> +#define DIEPTSIZ13                      0xAB0
> +#define DIEPDMA13                       0xAB4
> +#define DIEPCTL14                       0xAC0
> +#define DIEPINT14                       0xAC8
> +#define DIEPTSIZ14                      0xAD0
> +#define DIEPDMA14                       0xAD4
> +#define DIEPCTL15                       0xAE0
> +#define DIEPINT15                       0xAE8
> +#define DIEPTSIZ15                      0xAF0
> +#define DIEPDMA15                       0xAF4
> +
> +/* Device Logical OUT Endpoint-Specific Registers */
> +#define DOEPCTL(x)                      (0xB00 + 0x20 * (x))
> +#define DOEPINT(x)                      (0xB08 + 0x20 * (x))
> +#define DOEPTSIZ(x)                     (0xB10 + 0x20 * (x))
> +
> +#define DXEPINT_EPDISBLD                BIT1
> +#define DXEPINT_XFERCOMPL               BIT0
> +
> +#define DXEPTSIZ_SUPCNT(x)              (((x) & 0x3) << 29)
> +#define DXEPTSIZ_PKTCNT(x)              (((x) & 0x3) << 19)
> +#define DXEPTSIZ_XFERSIZE(x)            ((x) & 0x7F)
> +
> +#define DOEPDMA(x)                      (0xB14 + 0x20 * (x))
> +#define DOEPCTL0                        0xB00
> +#define DOEPINT0                        0xB08
> +#define DOEPTSIZ0                       0xB10
> +#define DOEPDMA0                        0xB14
> +#define DOEPCTL1                        0xB20
> +#define DOEPINT1                        0xB28
> +#define DOEPTSIZ1                       0xB30
> +#define DOEPDMA1                        0xB34
> +#define DOEPCTL2                        0xB40
> +#define DOEPINT2                        0xB48
> +#define DOEPTSIZ2                       0xB50
> +#define DOEPDMA2                        0xB54
> +#define DOEPCTL3                        0xB60
> +#define DOEPINT3                        0xB68
> +#define DOEPTSIZ3                       0xB70
> +#define DOEPDMA3                        0xB74
> +#define DOEPCTL4                        0xB80
> +#define DOEPINT4                        0xB88
> +#define DOEPTSIZ4                       0xB90
> +#define DOEPDMA4                        0xB94
> +#define DOEPCTL5                        0xBA0
> +#define DOEPINT5                        0xBA8
> +#define DOEPTSIZ5                       0xBB0
> +#define DOEPDMA5                        0xBB4
> +#define DOEPCTL6                        0xBC0
> +#define DOEPINT6                        0xBC8
> +#define DOEPTSIZ6                       0xBD0
> +#define DOEPDMA6                        0xBD4
> +#define DOEPCTL7                        0xBE0
> +#define DOEPINT7                        0xBE8
> +#define DOEPTSIZ7                       0xBF0
> +#define DOEPDMA7                        0xBF4
> +#define DOEPCTL8                        0xC00
> +#define DOEPINT8                        0xC08
> +#define DOEPTSIZ8                       0xC10
> +#define DOEPDMA8                        0xC14
> +#define DOEPCTL9                        0xC20
> +#define DOEPINT9                        0xC28
> +#define DOEPTSIZ9                       0xC30
> +#define DOEPDMA9                        0xC34
> +#define DOEPCTL10                       0xC40
> +#define DOEPINT10                       0xC48
> +#define DOEPTSIZ10                      0xC50
> +#define DOEPDMA10                       0xC54
> +#define DOEPCTL11                       0xC60
> +#define DOEPINT11                       0xC68
> +#define DOEPTSIZ11                      0xC70
> +#define DOEPDMA11                       0xC74
> +#define DOEPCTL12                       0xC80
> +#define DOEPINT12                       0xC88
> +#define DOEPTSIZ12                      0xC90
> +#define DOEPDMA12                       0xC94
> +#define DOEPCTL13                       0xCA0
> +#define DOEPINT13                       0xCA8
> +#define DOEPTSIZ13                      0xCB0
> +#define DOEPDMA13                       0xCB4
> +#define DOEPCTL14                       0xCC0
> +#define DOEPINT14                       0xCC8
> +#define DOEPTSIZ14                      0xCD0
> +#define DOEPDMA14                       0xCD4
> +#define DOEPCTL15                       0xCE0
> +#define DOEPINT15                       0xCE8
> +#define DOEPTSIZ15                      0xCF0
> +#define DOEPDMA15                       0xCF4
> +
> +#define DXEPCTL_EPENA                   BIT31
> +#define DXEPCTL_SNAK                    BIT27
> +#define DXEPCTL_CNAK                    BIT26
> +#define DXEPCTL_STALL                   BIT21
> +#define DXEPCTL_EPTYPE_MASK             (BIT19 | BIT18)
> +#define DXEPCTL_NAKSTS                  BIT17
> +#define DXEPCTL_USBACTEP                BIT15
> +#define DXEPCTL_MPS_MASK                0x7FF
> +
> +/* Power and Clock Gating Register */
> +#define PCGCCTL                         0xE00
> +
> +#define EP0FIFO                         0x1000
> +
> +/**
> + * This union represents the bit fields in the DMA Descriptor
> + * status quadlet. Read the quadlet into the <i>d32</i> member then
> + * set/clear the bits using the <i>b</i>it, <i>b_iso_out</i> and
> + * <i>b_iso_in</i> elements.
> + */
> +typedef union {
> +  /** raw register data */
> +  UINT32 d32;
> +    /** quadlet bits */
> +  struct {
> +    /** Received number of bytes */
> +    unsigned bytes:16;
> +    /** NAK bit - only for OUT EPs */
> +    unsigned nak:1;
> +    unsigned reserved17_22:6;
> +    /** Multiple Transfer - only for OUT EPs */
> +    unsigned mtrf:1;
> +    /** Setup Packet received - only for OUT EPs */
> +    unsigned sr:1;
> +    /** Interrupt On Complete */
> +    unsigned ioc:1;
> +    /** Short Packet */
> +    unsigned sp:1;
> +    /** Last */
> +    unsigned l:1;
> +    /** Receive Status */
> +    unsigned sts:2;
> +    /** Buffer Status */
> +    unsigned bs:2;
> +  } b;
> +} dev_dma_desc_sts_t;
> +
> +/**
> + * DMA Descriptor structure
> + *
> + * DMA Descriptor structure contains two quadlets:
> + * Status quadlet and Data buffer pointer.
> + */
> +typedef struct {
> +  /** DMA Descriptor status quadlet */
> +  dev_dma_desc_sts_t status;
> +  /** DMA Descriptor data buffer pointer */
> +  UINT32 buf;
> +} dwc_otg_dev_dma_desc_t;
> +
> +#endif //ifndef __DW_USB_DXE_H__
> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
> new file mode 100644
> index 0000000..fc1e988
> --- /dev/null
> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
> @@ -0,0 +1,52 @@
> +#/** @file
> +#
> +#  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +#
> +#  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.
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = DwUsbDxe
> +  FILE_GUID                      = 72d78ea6-4dee-11e3-8100-f3842a48d0a0

Please use a fresh GUID rather than the one from

EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf:  FILE_GUID
              = 72d78ea6-4dee-11e3-8100-f3842a48d0a0


> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = DwUsbEntryPoint
> +
> +[Sources.common]
> +  DwUsbDxe.c
> +
> +[LibraryClasses]
> +  ArmLib
> +  DebugLib
> +  IoLib
> +  MemoryAllocationLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UncachedMemoryAllocationLib
> +  CacheMaintenanceLib
> +
> +[Protocols]
> +  gEfiDriverBindingProtocolGuid
> +  gUsbDeviceProtocolGuid
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> +
> +[Pcd]
> +  gDwUsbDxeTokenSpaceGuid.PcdDwUsbDxeBaseAddress
> +
> +[Depex]
> +  BEFORE gAndroidFastbootUsbGuid

This is the wrong way around.

If the Android fastboot driver depends on a protocol produced by this
driver, it should depex on that.

Using depexes like this completely breaks the modular nature of UEFI,
and makes it very difficult to create variations of a platform by
selectively changing drivers.


> diff --git a/Include/Protocol/DwUsb.h b/Include/Protocol/DwUsb.h
> new file mode 100644
> index 0000000..040e126
> --- /dev/null
> +++ b/Include/Protocol/DwUsb.h
> @@ -0,0 +1,81 @@
> +/** @file
> +
> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
> +
> +  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 __DW_USB_H__
> +#define __DW_USB_H__
> +
> +//
> +// Protocol GUID
> +//
> +#define DW_USB_PROTOCOL_GUID { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _DW_USB_PROTOCOL  DW_USB_PROTOCOL;
> +
> +#define USB_HOST_MODE                 0
> +#define USB_DEVICE_MODE               1
> +#define USB_CABLE_NOT_ATTACHED        2
> +
> +#define LANG_LENGTH                   8
> +#define MANU_FACTURER_STRING_LENGTH   32
> +#define PRODUCT_STRING_LENGTH         32
> +#define SERIAL_STRING_LENGTH          17
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_LANG) (
> +  OUT CHAR16                           *Lang,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_MANU_FACTURER) (
> +  OUT CHAR16                           *ManuFacturer,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_PRODUCT) (
> +  OUT CHAR16                           *Product,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_SERIAL_NO) (
> +  OUT CHAR16                           *SerialNo,
> +  OUT UINT8                            *Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_PHY_INIT) (
> +  IN UINT8                             Mode
> +  );
> +
> +struct _DW_USB_PROTOCOL {
> +  DW_USB_GET_LANG                      GetLang;
> +  DW_USB_GET_MANU_FACTURER             GetManuFacturer;
> +  DW_USB_GET_PRODUCT                   GetProduct;
> +  DW_USB_GET_SERIAL_NO                 GetSerialNo;
> +  DW_USB_PHY_INIT                      PhyInit;
> +};
> +
> +extern EFI_GUID gDwUsbProtocolGuid;
> +
> +#endif
> --
> 2.7.4
>

On 8 February 2017 at 00:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Sat, Feb 04, 2017 at 01:13:48PM +0800, Haojian Zhuang wrote:
>> Support Designware USB controller.
>
> A bit more detailed commit message please - which USB controller,
> which USB standards does it support?
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>>  Drivers/Usb/DwUsbDxe/DwUsbDxe.c   | 796 ++++++++++++++++++++++++++++++++++++++
>>  Drivers/Usb/DwUsbDxe/DwUsbDxe.dec |  46 +++
>>  Drivers/Usb/DwUsbDxe/DwUsbDxe.h   | 627 ++++++++++++++++++++++++++++++
>>  Drivers/Usb/DwUsbDxe/DwUsbDxe.inf |  52 +++
>>  Include/Protocol/DwUsb.h          |  81 ++++
>>  5 files changed, 1602 insertions(+)
>>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.c
>>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.h
>>  create mode 100644 Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
>>  create mode 100644 Include/Protocol/DwUsb.h
>>
>> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.c b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
>> new file mode 100644
>> index 0000000..bdd8405
>> --- /dev/null
>> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.c
>> @@ -0,0 +1,796 @@
>> +/** @file
>> +
>> +  Copyright (c) 2015-2017, Linaro. All rights reserved.
>> +
>> +  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 <IndustryStandard/Usb.h>
>> +#include <Library/ArmLib.h>
>> +#include <Library/TimerLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiDriverEntryPoint.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/UncachedMemoryAllocationLib.h>
>> +#include <Library/CacheMaintenanceLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/BaseLib.h>
>> +#include <Protocol/DwUsb.h>
>> +#include <Protocol/UsbDevice.h>
>
> Please sort include files alphabetically.
>

Fixed in v4 already.

>> +
>> +#include "DwUsbDxe.h"
>> +
>> +#define USB_TYPE_LENGTH               16
>> +#define USB_BLOCK_HIGH_SPEED_SIZE     512
>> +#define DATA_SIZE                     32768
>> +#define CMD_SIZE                      512
>> +#define MATCH_CMD_LITERAL(Cmd, Buf)   !AsciiStrnCmp (Cmd, Buf, sizeof (Cmd) - 1)
>
> Parentheses around all Cmd and Buf instances in macro please?
>

Fixed in v4 already.

>> +
>> +// The time between interrupt polls, in units of 100 nanoseconds
>> +// 10 Microseconds
>> +#define DW_INTERRUPT_POLL_PERIOD      10000
>> +
>> +EFI_GUID  gDwUsbProtocolGuid = DW_USB_PROTOCOL_GUID;
>> +
>> +STATIC dwc_otg_dev_dma_desc_t *gDmaDesc,*gDmaDescEp0,*gDmaDescIn;
>> +STATIC USB_DEVICE_REQUEST *gCtrlReq;
>> +STATIC VOID *RxBuf;
>> +STATIC UINTN RxDescBytes = 0;
>
> Static varibles are automatically initialised to 0.
>

Fixed in v4 already.

>> +STATIC UINTN mNumDataBytes;
>> +
>> +STATIC DW_USB_PROTOCOL          *DwUsb;
>> +
>> +STATIC USB_DEVICE_DESCRIPTOR    *mDeviceDescriptor;
>> +
>> +// The config descriptor, interface descriptor, and endpoint descriptors in a
>> +// buffer (in that order)
>> +STATIC VOID                     *mDescriptors;
>> +// Convenience pointers to those descriptors inside the buffer:
>> +STATIC USB_INTERFACE_DESCRIPTOR *mInterfaceDescriptor;
>> +STATIC USB_CONFIG_DESCRIPTOR    *mConfigDescriptor;
>> +STATIC USB_ENDPOINT_DESCRIPTOR  *mEndpointDescriptors;
>> +
>> +STATIC USB_DEVICE_RX_CALLBACK   mDataReceivedCallback;
>> +STATIC USB_DEVICE_TX_CALLBACK   mDataSentCallback;
>
> This is a huge amount of global static variables - could they be
> wrapped in a struct please? Also, this makes the driver hard-wired to
> only be able to support a single controller.
>

Actually it's impossible to run two or more usb devices at the same time.
It's different from USB host.

>> +
>> +
>> +/* To detect which mode was run, high speed or full speed */
>> +STATIC
>> +UINTN
>> +UsbDrvPortSpeed (
>> +  VOID
>> +  )
>> +{
>> +  /*
>> +  * 2'b00: High speed (PHY clock is running at 30 or 60 MHz)
>> +  */
>> +  UINT32          Val = READ_REG32 (DSTS) & 2;
>
> No magic numbers in code plese.
>

Fixed in v4 already.

>> +  return (!Val);
>
> No need for the temporary variable.
>

Fixed in v4 already.

>> +}
>> +
>> +STATIC
>> +VOID
>> +ResetEndpoints (
>> +  VOID
>> +  )
>> +{
>> +  /* EP0 IN ACTIVE NEXT=1 */
>> +  WRITE_REG32 (DIEPCTL0, DXEPCTL_USBACTEP | BIT11);
>
> Can you create a more descriptive #define for that BIT11?
>

I don't know this bit. Nobody share the bit definition to me.

>> +
>> +  //notes that:the compulsive conversion is expectable.
>> +  gDmaDescEp0->status.b.bs = 0x3;
>> +  gDmaDescEp0->status.b.mtrf = 0;
>> +  gDmaDescEp0->status.b.sr = 0;
>> +  gDmaDescEp0->status.b.l = 1;
>> +  gDmaDescEp0->status.b.ioc = 1;
>> +  gDmaDescEp0->status.b.sp = 0;
>> +  gDmaDescEp0->status.b.bytes = 64;
>> +  gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);
>
> Please drop the brackets around gCtrlReg.
> (Also future instances, including gDmaDescEp0 below.)

Fixed in v4 already.

>
>> +  gDmaDescEp0->status.b.sts = 0;
>> +  gDmaDescEp0->status.b.bs = 0x0;
>
> A lot of magic numbers above - please replace with #defines.

I don't have the details on these bits.

>
>> +  WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
>> +  /* EP0 OUT ENABLE CLEARNAK */
>> +  WRITE_REG32 (DOEPCTL0, (READ_REG32 (DOEPCTL0) | DXEPCTL_EPENA | DXEPCTL_CNAK));
>> +}
>> +
>> +STATIC
>> +VOID
>> +EpTx (
>> +  IN UINT8          Ep,
>
> Is Ep shohrt for EndPoint? If so, please expand to the longer version.
> Preferably also in the function name. And for other functions in this
> driver.
>

Fixed in v4 already.

>> +  IN CONST VOID    *Ptr,
>> +  IN UINTN          Len
>> +  )
>> +{
>> +    UINT32          BlockSize;
>> +    UINT32          Packets;
>> +
>> +    /* EPx OUT ACTIVE */
>> +    WRITE_REG32 (DIEPCTL (Ep), (READ_REG32 (DIEPCTL (Ep))) | DXEPCTL_USBACTEP);
>> +    if (!Ep) {
>> +        BlockSize = 64;
>> +    } else {
>> +        BlockSize = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;
>> +    }
>> +    Packets = (Len + BlockSize - 1) / BlockSize;
>> +
>> +    if (!Len) {
>> +      /* send one empty packet */
>> +      gDmaDescIn->status.b.bs = 0x3;
>> +      gDmaDescIn->status.b.l = 1;
>> +      gDmaDescIn->status.b.ioc = 1;
>> +      gDmaDescIn->status.b.sp = 1;
>> +      gDmaDescIn->status.b.bytes = 0;
>> +      gDmaDescIn->buf = 0;
>> +      gDmaDescIn->status.b.sts = 0;
>> +      gDmaDescIn->status.b.bs = 0x0;
>
> More magic numbers here, please replace with #defines.
>

I don't have any details on these.

>> +
>> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));             // DMA Address (DMAAddr) is zero
>> +    } else {
>> +      WRITE_REG32 (DIEPTSIZ (Ep), Len | (Packets << 19));
>
> "Len | (Packets << 19)" would be better to replace with a macro with a
> desriptive name.
>

Fixed in v4 already.

>> +
>> +      //flush cache
>> +      WriteBackDataCacheRange ((VOID *)Ptr, Len);
>> +
>> +      gDmaDescIn->status.b.bs = 0x3;
>> +      gDmaDescIn->status.b.l = 1;
>> +      gDmaDescIn->status.b.ioc = 1;
>> +      gDmaDescIn->status.b.sp = 1;
>> +      gDmaDescIn->status.b.bytes = Len;
>> +      gDmaDescIn->buf = (UINT32)((UINTN)Ptr);
>> +      gDmaDescIn->status.b.sts = 0;
>> +      gDmaDescIn->status.b.bs = 0x0;
>
> Magic numbers.
>

Yes, but I don't have any details.

>> +      WRITE_REG32 (DIEPDMA (Ep), (UINT32)(UINTN)(gDmaDescIn));         // Ptr is DMA address
>> +    }
>> +    ArmDataSynchronizationBarrier ();
>> +    /* epena & cnak */
>> +    WRITE_REG32 (DIEPCTL (Ep), READ_REG32 (DIEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK | BIT11);
>> +}
>> +
>> +STATIC
>> +VOID
>> +EpRx (
>> +  IN UINTN            Ep,
>> +  IN UINTN            Len
>> +  )
>> +{
>> +  /* EPx UNSTALL */
>> +  WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & (~DXEPCTL_STALL)));
>> +  /* EPx OUT ACTIVE */
>> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_USBACTEP));
>
> Some macros would help make the above more readable.
>
>> +
>> +  if (Len >= DATA_SIZE) {
>> +    RxDescBytes = DATA_SIZE;
>> +  } else {
>> +    RxDescBytes = Len;
>> +  }
>> +
>> +  RxBuf = AllocatePool (DATA_SIZE);
>> +  ASSERT (RxBuf != NULL);
>
> Surely this should return rather than progress if running a RELEASE
> build?
>

Fixed in v4 already.

>> +
>> +  InvalidateDataCacheRange (RxBuf, Len);
>> +
>> +  gDmaDesc->status.b.bs = 0x3;
>> +  gDmaDesc->status.b.mtrf = 0;
>> +  gDmaDesc->status.b.sr = 0;
>> +  gDmaDesc->status.b.l = 1;
>> +  gDmaDesc->status.b.ioc = 1;
>> +  gDmaDesc->status.b.sp = 0;
>> +  gDmaDesc->status.b.bytes = (UINT32)RxDescBytes;
>> +  gDmaDesc->buf = (UINT32)((UINTN)RxBuf);
>> +  gDmaDesc->status.b.sts = 0;
>> +  gDmaDesc->status.b.bs = 0x0;
>
> Lot of magic values.
>

Yes, but I don't have the details on these bits.

>> +
>> +  ArmDataSynchronizationBarrier ();
>> +  WRITE_REG32 (DOEPDMA (Ep), (UINT32)((UINTN)gDmaDesc));
>> +  /* EPx OUT ENABLE CLEARNAK */
>> +  WRITE_REG32 (DOEPCTL (Ep), (READ_REG32 (DOEPCTL (Ep)) | DXEPCTL_EPENA | DXEPCTL_CNAK));
>
> Macros would help again, here.
>
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +HandleGetDescriptor (
>> +  IN USB_DEVICE_REQUEST  *Request
>> +  )
>> +{
>> +  UINT8       DescriptorType;
>> +  UINTN       ResponseSize;
>> +  VOID       *ResponseData;
>> +  EFI_USB_STRING_DESCRIPTOR        *Descriptor = NULL;
>> +  UINTN                             DescriptorSize;
>> +
>> +  ResponseSize = 0;
>> +  ResponseData = NULL;
>> +
>> +  // Pretty confused if bmRequestType is anything but this:
>> +  ASSERT (Request->RequestType == USB_DEV_GET_DESCRIPTOR_REQ_TYPE);
>
> Return error instead?
>

Fixed in v4 already.

>> +
>> +  // Choose the response
>> +  DescriptorType = Request->Value >> 8;
>> +  switch (DescriptorType) {
>> +  case USB_DESC_TYPE_DEVICE:
>> +    DEBUG ((DEBUG_INFO, "USB: Got a request for device descriptor\n"));
>
> "USB" is too generic an identifier for an error message.
> "DwUsbDxe" would be more appropriate for this driver, but preferably
> as a #define, and maybe hidden behind a local macro.
> (Comment applies globally to debug printouts in this driver.)
>

Fixed in v4 already.

>> +    ResponseSize = sizeof (USB_DEVICE_DESCRIPTOR);
>> +    ResponseData = mDeviceDescriptor;
>> +    break;
>> +  case USB_DESC_TYPE_CONFIG:
>> +    DEBUG ((DEBUG_INFO, "USB: Got a request for config descriptor\n"));
>> +    ResponseSize = mConfigDescriptor->TotalLength;
>> +    ResponseData = mDescriptors;
>> +    break;
>> +  case USB_DESC_TYPE_STRING:
>> +    DEBUG ((DEBUG_INFO, "USB: Got a request for String descriptor %d\n", Request->Value & 0xFF));
>> +    switch (Request->Value & 0xff) {
>
> ((x)->Value & 0xff) as a macro?
>

Fixed in v4 already.

>> +    case 0:
>
> Please use defines rather than magic numbers.
>

OK

>> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
>> +                       LANG_LENGTH * sizeof (CHAR16) + 1;
>
> Why "+ 1"? If CHAR16, should it not be "+ 2", or even better
> "+ sizeof (CHAR16)"? Same pattern repeats in many places in the file.
>

OK

>> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
>> +      ASSERT (Descriptor != NULL);
>
> return error
>

Fixed in v4 already.

>> +      Descriptor->Length = LANG_LENGTH * sizeof (CHAR16);
>> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
>> +      DwUsb->GetLang (Descriptor->String, &Descriptor->Length);
>> +      ResponseSize = Descriptor->Length;
>> +      ResponseData = Descriptor;
>> +      break;
>> +    case 1:
>> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
>> +                       MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16) + 1;
>> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
>> +      ASSERT (Descriptor != NULL);
>
> return error
> I will stop repeating myself here. But please address: all ASSERTs
> that check whether a pointer is NULL before dereferencing it must
> return an error instead of triggering a synchronous exception at
> runtime in RELEASE builds.
>

Fixed in v4 already.

>> +      Descriptor->Length = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16);
>> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
>> +      DwUsb->GetManuFacturer (Descriptor->String, &Descriptor->Length);
>> +      ResponseSize = Descriptor->Length;
>> +      ResponseData = Descriptor;
>> +      break;
>> +    case 2:
>> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
>> +                       PRODUCT_STRING_LENGTH * sizeof (CHAR16) + 1;
>> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
>> +      ASSERT (Descriptor != NULL);
>> +      Descriptor->Length = PRODUCT_STRING_LENGTH * sizeof (CHAR16);
>> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
>> +      DwUsb->GetProduct (Descriptor->String, &Descriptor->Length);
>> +      ResponseSize = Descriptor->Length;
>> +      ResponseData = Descriptor;
>> +      break;
>> +    case 3:
>> +      DescriptorSize = sizeof (EFI_USB_STRING_DESCRIPTOR) +
>> +                       SERIAL_STRING_LENGTH * sizeof (CHAR16) + 1;
>> +      Descriptor = (EFI_USB_STRING_DESCRIPTOR *)AllocateZeroPool (DescriptorSize);
>> +      ASSERT (Descriptor != NULL);
>> +      Descriptor->Length = SERIAL_STRING_LENGTH * sizeof (CHAR16);
>> +      Descriptor->DescriptorType = USB_DESC_TYPE_STRING;
>> +      DwUsb->GetSerialNo (Descriptor->String, &Descriptor->Length);
>> +      ResponseSize = Descriptor->Length;
>> +      ResponseData = Descriptor;
>> +      break;
>> +    }
>> +    break;
>> +  default:
>> +    DEBUG ((DEBUG_INFO, "USB: Didn't understand request for descriptor 0x%04x\n", Request->Value));
>> +    break;
>> +  }
>> +
>> +  // Send the response
>> +  if (ResponseData) {
>> +    ASSERT (ResponseSize != 0);
>
> ResponseSize is set explicitly by the switch statement above.
> Rather than ASSERTing that it went wrong, why not return an error from
> the default target?
> You could then also drop the "if (ResponseData)" test, and the "if
> (Descriptor)" test below.
>

Fixed in v4 already.

>> +
>> +    if (Request->Length < ResponseSize) {
>> +      // Truncate response
>> +      ResponseSize = Request->Length;
>> +    } else if (Request->Length > ResponseSize) {
>> +      DEBUG ((DEBUG_INFO, "USB: Info: ResponseSize < wLength\n"));
>> +    }
>> +
>> +    EpTx (0, ResponseData, ResponseSize);
>> +  }
>> +  if (Descriptor) {
>
> Here.
>

Descriptor is only allocated when it's USB_DESC_TYPE_STRING. If drop
Descriptor check at here, it'll cause error to execute FreePool().

>> +    FreePool (Descriptor);
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +HandleSetAddress (
>> +  IN USB_DEVICE_REQUEST  *Request
>> +  )
>> +{
>> +  // Pretty confused if bmRequestType is anything but this:
>
> So is there any reason not to return an error?
>

Fixed in v4 already.

>> +  ASSERT (Request->RequestType == USB_DEV_SET_ADDRESS_REQ_TYPE);
>> +  DEBUG ((DEBUG_INFO, "USB: Setting address to %d\n", Request->Value));
>> +  ResetEndpoints ();
>> +
>> +  WRITE_REG32 (DCFG, (READ_REG32 (DCFG) & ~DCFG_DEVADDR_MASK) | DCFG_DEVADDR(Request->Value));
>> +  EpTx (0, 0, 0);
>
> What is the significance of transmitting 0 bytes of data?
> Probably this should be replaced by a macro. And that macro should use
> NULL, not 0, for the invalid pointer.
>

I don't know. Synoposys makes their spec as secret and they don't make
any byte as public.
I can only guess and guess.

>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +UINTN
>> +UsbDrvRequestEndpoint (
>> +  IN UINTN           Type,
>> +  IN UINTN           Dir
>> +  )
>> +{
>> +  UINTN              Ep = 1;
>> +  UINTN              Ret, NewBits;
>> +
>> +  Ret = Ep | Dir;
>> +  NewBits = (Type << 18) | 0x10000000;
>
> No magic values, please. Replace with macro(s).
>

I don't like magic number too. But it's also secret to me.

>> +
>> +  /*
>> +   * (Type << 18):Endpoint Type (EPType)
>> +   * 0x10000000:Endpoint Enable (EPEna)
>> +   * 0x000C000:Endpoint Type (EPType);Hardcoded to 00 for control.
>> +   * (ep<<22):TxFIFO Number (TxFNum)
>> +   * 0x20000:NAK Status (NAKSts);The core is transmitting NAK handshakes on this endpoint.
>> +   */
>> +  if (Dir) {  // IN: to host
>> +    WRITE_REG32 (DIEPCTL (Ep), ((READ_REG32 (DIEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits | (Ep << 22) | DXEPCTL_NAKSTS);
>> +  } else {    // OUT: to device
>> +    WRITE_REG32 (DOEPCTL (Ep), ((READ_REG32 (DOEPCTL (Ep))) & ~DXEPCTL_EPTYPE_MASK) | NewBits);
>
> More macros, please. There WRITE_REG32 lines are unreadable.
>

Fixed in v4 already.

>> +  }
>> +
>> +  return Ret;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +HandleSetConfiguration (
>> +  IN USB_DEVICE_REQUEST  *Request
>> +  )
>> +{
>> +  ASSERT (Request->RequestType == USB_DEV_SET_CONFIGURATION_REQ_TYPE);
>> +
>> +  // Cancel all transfers
>> +  ResetEndpoints ();
>> +
>> +  UsbDrvRequestEndpoint (2, 0);
>> +  UsbDrvRequestEndpoint (2, 0x80);
>
> No magic values, please.
>

Fixed in v4 already.

>> +
>> +  WRITE_REG32 (DIEPCTL1, (READ_REG32 (DIEPCTL1)) | BIT28 | BIT19 | DXEPCTL_USBACTEP | BIT11);
>
> What is this operation doing?
> Please simplify with a temporary variable and some more descriptive
> #defines.
>

Fixed in v4 already.

>> +
>> +  /* Enable interrupts on all endpoints */
>> +  WRITE_REG32 (DAINTMSK, ~0);
>> +
>> +  EpRx (1, CMD_SIZE);
>
> The global variables also makes this code less readable.
>
>> +  EpTx (0, 0, 0);
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +
>> +STATIC
>> +EFI_STATUS
>> +HandleDeviceRequest (
>> +  IN USB_DEVICE_REQUEST  *Request
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +
>> +  switch (Request->Request) {
>> +  case USB_DEV_GET_DESCRIPTOR:
>> +    Status = HandleGetDescriptor (Request);
>> +    break;
>> +  case USB_DEV_SET_ADDRESS:
>> +    Status = HandleSetAddress (Request);
>> +    break;
>> +  case USB_DEV_SET_CONFIGURATION:
>> +    Status = HandleSetConfiguration (Request);
>> +    break;
>> +  default:
>> +    DEBUG ((DEBUG_ERROR,
>> +      "Didn't understand RequestType 0x%x Request 0x%x\n",
>> +      Request->RequestType, Request->Request));
>> +      Status = EFI_INVALID_PARAMETER;
>> +    break;
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +
>> +// Instead of actually registering interrupt handlers, we poll the controller's
>> +//  interrupt source register in this function.
>> +STATIC
>> +VOID
>> +CheckInterrupts (
>> +  IN EFI_EVENT        Event,
>> +  IN VOID            *Context
>> +  )
>> +{
>> +  UINT32              Ints, EpInts;
>> +
>> +
>> +  // interrupt register
>> +  Ints = READ_REG32 (GINTSTS);
>> +
>> +  /*
>> +   * bus reset
>> +   * The core sets this bit to indicate that a reset is detected on the USB.
>> +   */
>> +  if (Ints & GINTSTS_USBRST) {
>> +    WRITE_REG32 (DCFG, DCFG_DESCDMA | DCFG_NZ_STS_OUT_HSHK);
>> +    ResetEndpoints ();
>> +  }
>> +
>> +  /*
>> +   * enumeration done, we now know the speed
>> +   * The core sets this bit to indicate that speed enumeration is complete. The
>> +   * application must read the Device Status (DSTS) register to obtain the
>> +   * enumerated speed.
>> +   */
>> +  if (Ints & GINTSTS_ENUMDONE) {
>> +    /* Set up the maximum packet sizes accordingly */
>> +    UINTN MaxPacket = UsbDrvPortSpeed () ? USB_BLOCK_HIGH_SPEED_SIZE : 64;
>
> A #define for that 64 please.
>

Fixed in v4 already.


>> +    //Set Maximum In Packet Size (MPS)
>> +    WRITE_REG32 (DIEPCTL1, ((READ_REG32 (DIEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);
>> +    //Set Maximum Out Packet Size (MPS)
>> +    WRITE_REG32 (DOEPCTL1, ((READ_REG32 (DOEPCTL1)) & ~DXEPCTL_MPS_MASK) | MaxPacket);
>
> This seems to be a recurring pattern, so should there be a macro
> UPDATE_REG32 (Regname, Mask, Value) ?
>
> Alternatively, please use a temporary variable to do the read separately.
>

Fixed in v4 already.

>> +  }
>> +
>> +  /*
>> +   * IN EP event
>> +   * The core sets this bit to indicate that an interrupt is pending on one of the IN
>> +   * endpoInts of the core (in Device mode). The application must read the
>> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
>> +   * number of the IN endpoint on which the interrupt occurred, and then read
>> +   * the corresponding Device IN Endpoint-n Interrupt (DIEPINTn) register to
>> +   * determine the exact cause of the interrupt. The application must clear the
>> +   * appropriate status bit in the corresponding DIEPINTn register to clear this bit.
>> +   */
>> +  if (Ints & GINTSTS_IEPINT) {
>> +    EpInts = READ_REG32 (DIEPINT0);
>> +    WRITE_REG32 (DIEPINT0, EpInts);
>> +    if (EpInts & DXEPINT_XFERCOMPL) {
>> +      DEBUG ((DEBUG_INFO, "INT: IN TX completed.DIEPTSIZ (0) = 0x%x.\n", READ_REG32 (DIEPTSIZ0)));
>> +    }
>> +
>> +    EpInts = READ_REG32 (DIEPINT1);
>> +    WRITE_REG32 (DIEPINT1, EpInts);
>> +    if (EpInts & DXEPINT_XFERCOMPL) {
>> +      DEBUG ((DEBUG_INFO, "ep1: IN TX completed\n"));
>> +    }
>> +  }
>
> Turn into helper function, please.
>

Fixed in v4 already.

>> +
>> +  /*
>> +   * OUT EP event
>> +   * The core sets this bit to indicate that an interrupt is pending on one of the
>> +   * OUT endpoints of the core (in Device mode). The application must read the
>> +   * Device All EndpoInts Interrupt (DAINT) register to determine the exact
>> +   * number of the OUT endpoint on which the interrupt occurred, and then read
>> +   * the corresponding Device OUT Endpoint-n Interrupt (DOEPINTn) register
>> +   * to determine the exact cause of the interrupt. The application must clear the
>> +   * appropriate status bit in the corresponding DOEPINTn register to clear this bit.
>> +   */
>> +  if (Ints & GINTSTS_OEPINT) {
>> +    /* indicates the status of an endpoint
>> +     * with respect to USB- and AHB-related events. */
>> +    EpInts = READ_REG32 (DOEPINT0);
>> +    if (EpInts) {
>> +      WRITE_REG32 (DOEPINT0, EpInts);
>> +      if (EpInts & DXEPINT_XFERCOMPL) {
>> +        DEBUG ((DEBUG_INFO, "INT: EP0 RX completed. DOEPTSIZ(0) = 0x%x.\n", READ_REG32 (DOEPTSIZ0)));
>> +      }
>> +      /*
>> +       *
>> +       IN Token Received When TxFIFO is Empty (INTknTXFEmp)
>> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
>> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
>> +       * was received.
>> +       */
>> +      if (EpInts & BIT3) { /* SETUP phase done */
>> +        WRITE_REG32 (DIEPCTL0, READ_REG32 (DIEPCTL0) | DXEPCTL_SNAK);
>> +        WRITE_REG32 (DOEPCTL0, READ_REG32 (DOEPCTL0) | DXEPCTL_SNAK);
>> +        /*clear IN EP intr*/
>> +        WRITE_REG32 (DIEPINT0, ~0);
>> +        HandleDeviceRequest((USB_DEVICE_REQUEST *)gCtrlReq);
>> +      }
>> +
>> +      /* Make sure EP0 OUT is set up to accept the next request */
>> +      WRITE_REG32 (DOEPTSIZ0, DXEPTSIZ_SUPCNT(3) | DXEPTSIZ_PKTCNT(1) | DXEPTSIZ_XFERSIZE(64));
>> +      /*
>> +       * IN Token Received When TxFIFO is Empty (INTknTXFEmp)
>> +       * Indicates that an IN token was received when the associated TxFIFO (periodic/nonperiodic)
>> +       * was empty. This interrupt is asserted on the endpoint for which the IN token
>> +       * was received.
>> +       */
>> +      gDmaDescEp0->status.b.bs = 0x3;
>> +      gDmaDescEp0->status.b.mtrf = 0;
>> +      gDmaDescEp0->status.b.sr = 0;
>> +      gDmaDescEp0->status.b.l = 1;
>> +      gDmaDescEp0->status.b.ioc = 1;
>> +      gDmaDescEp0->status.b.sp = 0;
>> +      gDmaDescEp0->status.b.bytes = 64;
>> +      gDmaDescEp0->buf = (UINT32)(UINTN)(gCtrlReq);
>> +      gDmaDescEp0->status.b.sts = 0;
>> +      gDmaDescEp0->status.b.bs = 0x0;
>> +      WRITE_REG32 (DOEPDMA0, (UINT32)(UINTN)(gDmaDescEp0));
>> +      // endpoint enable; clear NAK
>> +      WRITE_REG32 (DOEPCTL0, DXEPCTL_EPENA | DXEPCTL_CNAK);
>> +    }
>> +
>> +    EpInts = (READ_REG32 (DOEPINT1));
>> +    if (EpInts) {
>> +      WRITE_REG32 (DOEPINT1, EpInts);
>> +      /* Transfer Completed Interrupt (XferCompl);Transfer completed */
>> +      if (EpInts & DXEPINT_XFERCOMPL) {
>> +
>> +        UINTN Bytes = RxDescBytes - gDmaDesc->status.b.bytes;
>> +        UINTN Len = 0;
>> +
>> +        ArmDataSynchronizationBarrier ();
>> +        if (MATCH_CMD_LITERAL ("download", RxBuf)) {
>> +          mNumDataBytes = AsciiStrHexToUint64 (RxBuf + sizeof ("download"));
>> +        } else {
>> +          if (mNumDataBytes != 0) {
>> +            mNumDataBytes -= Bytes;
>> +          }
>> +        }
>> +
>> +        mDataReceivedCallback (Bytes, RxBuf);
>> +
>> +        if (mNumDataBytes == 0) {
>> +          Len = CMD_SIZE;
>> +        } else if (mNumDataBytes > DATA_SIZE) {
>> +          Len = DATA_SIZE;
>> +        } else {
>> +          Len = mNumDataBytes;
>> +        }
>> +
>> +        EpRx (1, Len);
>> +      }
>> +    }
>> +  }
>
> Turn into helper function, please.
>

Fixed in v4 already.

>> +
>> +  //WRITE_REG32 clear ints
>> +  WRITE_REG32 (GINTSTS, Ints);
>> +}
>> +
>> +EFI_STATUS
>> +DwUsbSend (
>> +  IN        UINT8  EndpointIndex,
>> +  IN        UINTN  Size,
>> +  IN  CONST VOID  *Buffer
>> +  )
>> +{
>> +    EpTx (EndpointIndex, Buffer, Size);
>> +    return EFI_SUCCESS;
>> +}
>
> What is the purpose of this function?
>

Just want to use unified function name.

>> +
>> +STATIC
>> +VOID
>> +DwUsbInit (
>> +  VOID
>> +  )
>> +{
>> +  VOID     *Buf;
>> +  UINT32   Data;
>> +
>> +  Buf = UncachedAllocatePages (16);
>
> No magic values, please.
>

Fixed in v4 already.

>> +  gDmaDesc = Buf;
>> +  gDmaDescEp0 = gDmaDesc + sizeof (dwc_otg_dev_dma_desc_t);
>> +  gDmaDescIn = gDmaDescEp0 + sizeof (dwc_otg_dev_dma_desc_t);
>> +  gCtrlReq = (USB_DEVICE_REQUEST *)gDmaDescIn + sizeof (dwc_otg_dev_dma_desc_t);
>> +
>> +  ZeroMem (gDmaDesc, sizeof (dwc_otg_dev_dma_desc_t));
>> +  ZeroMem (gDmaDescEp0, sizeof (dwc_otg_dev_dma_desc_t));
>> +  ZeroMem (gDmaDescIn, sizeof (dwc_otg_dev_dma_desc_t));
>
> Why not zero all of Buf once?
>

Fixed in v4 already.

>> +
>> +  /*Reset usb controller.*/
>> +  /* Wait for OTG AHB master idle */
>> +  do {
>> +    Data = READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE;
>> +  } while (Data == 0);
>> +
>> +  /* OTG: Assert Software Reset */
>> +  WRITE_REG32 (GRSTCTL, GRSTCTL_CSFTRST);
>> +
>> +  /* Wait for OTG to ack reset */
>> +  while (READ_REG32 (GRSTCTL) & GRSTCTL_CSFTRST);
>> +
>> +  /* Wait for OTG AHB master idle */
>> +  while ((READ_REG32 (GRSTCTL) & GRSTCTL_AHBIDLE) == 0);
>
> Second use of an explicit loop on this in this function - and coded
> differently to the first one. Turn into a macro instead?
>

OK

>> +
>> +  WRITE_REG32 (GDFIFOCFG, DATA_FIFO_CONFIG);
>> +  WRITE_REG32 (GRXFSIZ, RX_SIZE);
>> +  WRITE_REG32 (GNPTXFSIZ, ENDPOINT_TX_SIZE);
>> +  WRITE_REG32 (DIEPTXF1, DATA_IN_ENDPOINT_TX_FIFO1);
>> +  WRITE_REG32 (DIEPTXF2, DATA_IN_ENDPOINT_TX_FIFO2);
>> +  WRITE_REG32 (DIEPTXF3, DATA_IN_ENDPOINT_TX_FIFO3);
>> +  WRITE_REG32 (DIEPTXF4, DATA_IN_ENDPOINT_TX_FIFO4);
>> +  WRITE_REG32 (DIEPTXF5, DATA_IN_ENDPOINT_TX_FIFO5);
>> +  WRITE_REG32 (DIEPTXF6, DATA_IN_ENDPOINT_TX_FIFO6);
>> +  WRITE_REG32 (DIEPTXF7, DATA_IN_ENDPOINT_TX_FIFO7);
>> +  WRITE_REG32 (DIEPTXF8, DATA_IN_ENDPOINT_TX_FIFO8);
>> +  WRITE_REG32 (DIEPTXF9, DATA_IN_ENDPOINT_TX_FIFO9);
>> +  WRITE_REG32 (DIEPTXF10, DATA_IN_ENDPOINT_TX_FIFO10);
>> +  WRITE_REG32 (DIEPTXF11, DATA_IN_ENDPOINT_TX_FIFO11);
>> +  WRITE_REG32 (DIEPTXF12, DATA_IN_ENDPOINT_TX_FIFO12);
>> +  WRITE_REG32 (DIEPTXF13, DATA_IN_ENDPOINT_TX_FIFO13);
>> +  WRITE_REG32 (DIEPTXF14, DATA_IN_ENDPOINT_TX_FIFO14);
>> +  WRITE_REG32 (DIEPTXF15, DATA_IN_ENDPOINT_TX_FIFO15);
>> +
>> +  /*
>> +   * set Periodic TxFIFO Empty Level,
>> +   * Non-Periodic TxFIFO Empty Level,
>> +   * Enable DMA, Unmask Global Intr
>> +   */
>> +  WRITE_REG32 (GAHBCFG, GAHBCFG_CTRL_MASK);
>> +
>> +  /*select 8bit UTMI+, ULPI Inerface*/
>> +  WRITE_REG32 (GUSBCFG, 0x2400);
>
> No magic numbers, please.
>

Fixed in v4 already.

>> +
>> +  /* Detect usb work mode,host or device? */
>> +  do {
>> +    Data = READ_REG32 (GINTSTS);
>> +  } while (Data & GINTSTS_CURMODE_HOST);
>> +  MicroSecondDelay (3);
>
> Why a delay? Why 3us? Is a barrier needed?
>

Fixed in v4 already.

>> +
>> +  /*Init global and device mode csr register.*/
>> +  /*set Non-Zero-Length status out handshake */
>> +  Data = (0x20 << DCFG_EPMISCNT_SHIFT) | DCFG_NZ_STS_OUT_HSHK;
>
> No magic values, please.
>

It's also secret to me.

>> +  WRITE_REG32 (DCFG, Data);
>> +
>> +  /* Interrupt unmask: IN event, OUT event, bus reset */
>
> (And enumeration complete?)
>

OK

>> +  Data = GINTSTS_OEPINT | GINTSTS_IEPINT | GINTSTS_ENUMDONE | GINTSTS_USBRST;
>> +  WRITE_REG32 (GINTMSK, Data);
>> +
>> +  do {
>> +    Data = READ_REG32 (GINTSTS) & GINTSTS_ENUMDONE;
>> +  } while (Data);
>> +
>> +  /* Clear any pending OTG Interrupts */
>> +  WRITE_REG32 (GOTGINT, ~0);
>> +  /* Clear any pending interrupts */
>> +  WRITE_REG32 (GINTSTS, ~0);
>> +  WRITE_REG32 (GINTMSK, ~0);
>> +  Data = READ_REG32 (GOTGINT);
>> +  Data &= ~0x3000;
>
> No magic values, please.
>

I don't know this too.

>> +  WRITE_REG32 (GOTGINT, Data);
>> +
>> +  /* endpoint settings cfg */
>> +  ResetEndpoints ();
>> +  MicroSecondDelay (1);
>
> Why a delay? Why 1us? Is a barrier needed?
>

I think it's related to clock, not barrier. But I don't have the details.

>> +
>> +  /* init finish. and ready to transfer data */
>> +
>> +  /* Soft Disconnect */
>> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE | DCTL_SFTDISCON);
>> +  MicroSecondDelay (10000);
>
> Why a delay? Why 10ms? Is a barrier needed?
>

Nothing related to barrier. But it's black box to me.

>> +
>> +  /* Soft Reconnect */
>> +  WRITE_REG32 (DCTL, DCTL_PWRONPRGDONE);
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +DwUsbStart (
>> +  IN USB_DEVICE_DESCRIPTOR   *DeviceDescriptor,
>> +  IN VOID                   **Descriptors,
>> +  IN USB_DEVICE_RX_CALLBACK   RxCallback,
>> +  IN USB_DEVICE_TX_CALLBACK   TxCallback
>> +  )
>> +{
>> +  UINT8                    *Ptr;
>> +  EFI_STATUS                Status;
>> +  EFI_EVENT                 TimerEvent;
>> +
>> +  ASSERT (DeviceDescriptor != NULL);
>> +  ASSERT (Descriptors[0] != NULL);
>> +  ASSERT (RxCallback != NULL);
>> +  ASSERT (TxCallback != NULL);
>
> Can we do anything sane if any of these are NULL?
> If not, return error.
>

Fixed in v4 already.

>> +
>> +  DwUsbInit();
>> +
>> +  mDeviceDescriptor = DeviceDescriptor;
>> +  mDescriptors = Descriptors[0];
>> +
>> +  // Right now we just support one configuration
>> +  ASSERT (mDeviceDescriptor->NumConfigurations == 1);
>> +  mDeviceDescriptor->StrManufacturer = 1;
>> +  mDeviceDescriptor->StrProduct = 2;
>> +  mDeviceDescriptor->StrSerialNumber = 3;
>
> No magic values, please.
> Presumably these are just indexes into a string table, but use
> #defines please.
>

Fixed in v4 already.

>> +  // ... and one interface
>> +  mConfigDescriptor = (USB_CONFIG_DESCRIPTOR *)mDescriptors;
>> +  ASSERT (mConfigDescriptor->NumInterfaces == 1);
>> +
>> +  Ptr = ((UINT8 *) mDescriptors) + sizeof (USB_CONFIG_DESCRIPTOR);
>> +  mInterfaceDescriptor = (USB_INTERFACE_DESCRIPTOR *) Ptr;
>> +  Ptr += sizeof (USB_INTERFACE_DESCRIPTOR);
>> +
>> +  mEndpointDescriptors = (USB_ENDPOINT_DESCRIPTOR *) Ptr;
>> +
>> +  mDataReceivedCallback = RxCallback;
>> +  mDataSentCallback = TxCallback;
>> +
>> +  // Register a timer event so CheckInterupts gets called periodically
>> +  Status = gBS->CreateEvent (
>> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
>> +                  TPL_CALLBACK,
>> +                  CheckInterrupts,
>> +                  NULL,
>> +                  &TimerEvent
>> +                  );
>> +  ASSERT_EFI_ERROR (Status);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = gBS->SetTimer (
>> +                  TimerEvent,
>> +                  TimerPeriodic,
>> +                  DW_INTERRUPT_POLL_PERIOD
>> +                  );
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  return Status;
>> +}
>> +
>> +USB_DEVICE_PROTOCOL mUsbDevice = {
>> +  DwUsbStart,
>> +  DwUsbSend
>> +};
>> +
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +DwUsbEntryPoint (
>> +  IN EFI_HANDLE                            ImageHandle,
>> +  IN EFI_SYSTEM_TABLE                      *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS      Status;
>> +
>> +  Status = gBS->LocateProtocol (&gDwUsbProtocolGuid, NULL, (VOID **) &DwUsb);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = DwUsb->PhyInit(USB_DEVICE_MODE);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  return gBS->InstallProtocolInterface (
>> +                &ImageHandle,
>> +                &gUsbDeviceProtocolGuid,
>> +                EFI_NATIVE_INTERFACE,
>> +                &mUsbDevice
>> +                );
>> +}
>> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>> new file mode 100644
>> index 0000000..f991492
>> --- /dev/null
>> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>> @@ -0,0 +1,46 @@
>> +#/** @file
>> +# Framework Module Development Environment Industry Standards
>> +#
>> +# This Package provides headers and libraries that conform to EFI/PI Industry standards.
>> +# Copyright (c) 2007, Intel Corporation. All rights reserved.<BR>
>> +# Copyright (c) 2012-2014, ARM Ltd. All rights reserved.<BR>
>> +# Copyright (c) 2015-2017, Linaro. All rights reserved.<BR>
>> +#
>> +#    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.
>> +#
>> +#**/
>> +
>> +[Defines]
>> +  DEC_SPECIFICATION              = 0x00010019
>> +  PACKAGE_NAME                   = OpenPlatformDriversUsbDwUsbDxePkg
>
> I think DwUsbDxePkg would be sufficient.
>

Fixed in v4 already.

>> +  PACKAGE_GUID                   = 114a3be9-10f7-4bf1-81ca-09ac52d4c3d5
>> +  PACKAGE_VERSION                = 0.1
>> +
>> +
>> +################################################################################
>> +#
>> +# Include Section - list of Include Paths that are provided by this package.
>> +#                   Comments are used for Keywords and Module Types.
>> +#
>> +# Supported Module Types:
>> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
>> +#
>> +################################################################################
>> +
>> +[Guids.common]
>> +  gAndroidFastbootUsbGuid       = { 0xf6bec3fe, 0x88fb, 0x11e3, { 0xae, 0x84, 0xe7, 0x3b, 0x77, 0x56, 0x1c, 0x35 }}
>
> Is this included only for the DEPEX?
> This does not feel like the right way of doing this.
>

Fixed in v4 already.

>> +  gDwUsbDxeTokenSpaceGuid       = { 0x131c4d02, 0x9449, 0x4ee9, { 0xba, 0x3d, 0x69, 0x50, 0x21, 0x89, 0x26, 0x0b }}
>> +
>> +[Protocols.common]
>> +  gDwUsbProtocolGuid            = { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
>> +
>> +[PcdsFixedAtBuild.common]
>> +  # DwUsb Driver PCDs
>> +  gDwUsbDxeTokenSpaceGuid.PcdDwUsbDxeBaseAddress|0x0|UINT32|0x00000001
>> +  gDwUsbDxeTokenSpaceGuid.PcdSysCtrlBaseAddress|0x0|UINT32|0x00000002
>
> I am really not happy about adding additional drivers with hard-coded
> base addresses. Especially not ones that claim to be DXE/UEFI drivers.
>

But I realy don't know how to make it as dynamic.

>> +/**
>> + * DMA Descriptor structure
>> + *
>> + * DMA Descriptor structure contains two quadlets:
>> + * Status quadlet and Data buffer pointer.
>> + */
>> +typedef struct {
>> +  /** DMA Descriptor status quadlet */
>> +  dev_dma_desc_sts_t status;
>> +  /** DMA Descriptor data buffer pointer */
>> +  UINT32 buf;
>> +} dwc_otg_dev_dma_desc_t;
>
> The above union and struct do not conform to EDK2 coding style.
>

Fixed in v4 already.

>> +
>> +#endif //ifndef __DW_USB_DXE_H__
>> diff --git a/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
>> new file mode 100644
>> index 0000000..fc1e988
>> --- /dev/null
>> +++ b/Drivers/Usb/DwUsbDxe/DwUsbDxe.inf
>> @@ -0,0 +1,52 @@
>> +#/** @file
>> +#
>> +#  Copyright (c) 2015-2017, Linaro. All rights reserved.
>> +#
>> +#  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.
>> +#
>> +#
>> +#**/
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010019
>> +  BASE_NAME                      = DwUsbDxe
>> +  FILE_GUID                      = 72d78ea6-4dee-11e3-8100-f3842a48d0a0
>> +  MODULE_TYPE                    = UEFI_DRIVER
>
> I don't believe the driver in its current state can be called a
> UEFI_DRIVER. Combined with the ordering issue, should this be a
> DXE_DRIVER instead?
>

It'll be called by App. When App is running, DXE driver should end its
life cycle. Is it right?

>> +  VERSION_STRING                 = 1.0
>> +  ENTRY_POINT                    = DwUsbEntryPoint
>> +
>> +[Sources.common]
>> +  DwUsbDxe.c
>> +
>> +[LibraryClasses]
>> +  ArmLib
>> +  DebugLib
>> +  IoLib
>> +  MemoryAllocationLib
>> +  TimerLib
>> +  UefiBootServicesTableLib
>> +  UefiDriverEntryPoint
>> +  UncachedMemoryAllocationLib
>> +  CacheMaintenanceLib
>
> Please sort LibraryClasses.
>

Fixed in v4 already.

>> +
>> +[Protocols]
>> +  gEfiDriverBindingProtocolGuid
>> +  gUsbDeviceProtocolGuid
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>
> Please sort Packages.

Fixed in v4 already.

>
> /
>     Leif
>