[Linaro-uefi,v4,4/5] Platforms/Hisilicon/HiKey: support designware USB controller

Support Designware USB device controller on HiKey platform.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c      | 265 +++++++++++++++++++++
 .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf    |  46 ++++
 2 files changed, 311 insertions(+)
 create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
 create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf

On Wed, Feb 15, 2017 at 10:54:58PM +0800, Haojian Zhuang wrote:
> Support Designware USB device controller on HiKey platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c      | 265 +++++++++++++++++++++
>  .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf    |  46 ++++
>  2 files changed, 311 insertions(+)
>  create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
>  create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
> 
> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
> new file mode 100644
> index 0000000..59ef57c
> --- /dev/null
> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
> @@ -0,0 +1,265 @@
> +/** @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 <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/EmbeddedGpio.h>
> +#include <Protocol/DwUsb.h>
> +
> +#include <Hi6220.h>
> +
> +
> +#define USB_SEL_GPIO0_3          3     // GPIO 0_3
> +#define USB_5V_HUB_EN            7     // GPIO 0_7
> +#define USB_ID_DET_GPIO2_5       21    // GPIO 2_5
> +#define USB_VBUS_DET_GPIO2_6     22    // GPIO 2_6
> +
> +// Jumper on pin5-6 of J15 determines whether boot to fastboot
> +#define DETECT_J15_FASTBOOT      24    // GPIO 3_0
> +
> +#define IOCG_GPIO0_BASE          0xF8001800
> +#define IOCG_GPIO0_3_OFFSET      0x1C
> +#define IOCG_GPIO0_7_OFFSET      0x2C
> +#define IOCG_GPIO2_5_OFFSET      0x64
> +#define IOCG_GPIO2_6_OFFSET      0x68
> +
> +#define IOCG_PULLUP              1
> +#define IOCG_PULLDOWN            2
> +
> +#define USB_EYE_PATTERN          0x70533483
> +
> +#define LANG_EN                  0x409
> +
> +STATIC EMBEDDED_GPIO *mGpio;
> +
> +STATIC
> +EFI_STATUS
> +HiKeyDetectUsbModeInit (
> +  IN VOID
> +  )
> +{
> +  EFI_STATUS     Status;
> +
> +  /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */
> +  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_5_OFFSET, IOCG_PULLUP);
> +  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_6_OFFSET, IOCG_PULLUP);
> +
> +  Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Can't locate gEmbeddedGpioProtocolGuid\n"));
> +    return Status;
> +  }
> +  /* power on USB HUB */
> +  Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0);
> +  ASSERT_EFI_ERROR (Status);
> +  /* start to detect USB device or host */
> +  Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  Status = mGpio->Set (mGpio, USB_ID_DET_GPIO2_5, GPIO_MODE_INPUT);
> +  ASSERT_EFI_ERROR (Status);
> +  Status = mGpio->Set (mGpio, USB_VBUS_DET_GPIO2_6, GPIO_MODE_INPUT);
> +  ASSERT_EFI_ERROR (Status);
> +  return EFI_SUCCESS;
> +}
> +
> +UINTN
> +HiKeyGetUsbMode (
> +  IN VOID
> +  )
> +{
> +  EFI_STATUS     Status;
> +  UINTN          GpioId, GpioVbus;
> +
> +  Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &GpioId);
> +  ASSERT_EFI_ERROR (Status);
> +  Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &GpioVbus);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  if ((GpioId == 1) && (GpioVbus == 0)) {
> +    return USB_DEVICE_MODE;
> +  } else if ((GpioId == 0) && (GpioVbus == 1)) {
> +    return USB_CABLE_NOT_ATTACHED;
> +  }
> +  return USB_HOST_MODE;
> +}
> +
> +EFI_STATUS
> +HiKeyUsbPhyInit (
> +  IN UINT8        Mode
> +  )
> +{
> +  UINTN          Value;
> +  UINT32         Data;
> +  EFI_STATUS     Status;
> +
> +  Status = HiKeyDetectUsbModeInit ();
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //setup clock
> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, PERIPH_CLKEN0_USBOTG);
> +  do {
> +       Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0);
> +  } while ((Value & PERIPH_CLKEN0_USBOTG) == 0);
> +
> +  //setup phy
> +  Data = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> +           RST0_USBOTG | RST0_USBOTG_32K;
> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS0, Data);
> +  do {
> +    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_RSTSTAT0);
> +    Value &= Data;
> +  } while (Value);
> +
> +  Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4);
> +  Value &= ~(CTRL4_PICO_SIDDQ | CTRL4_FPGA_EXT_PHY_SEL |
> +             CTRL4_OTG_PHY_SEL);
> +  Value |=  CTRL4_PICO_VBUSVLDEXT | CTRL4_PICO_VBUSVLDEXTSEL;
> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4, Value);
> +
> +  if (HiKeyGetUsbMode () != Mode) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  //If Mode = 1, USB in Device Mode
> +  //If Mode = 0, USB in Host Mode
> +  if (Mode == USB_DEVICE_MODE) {
> +    DEBUG ((DEBUG_INFO, "usb work as device mode.\n"));
> +
> +    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
> +    Value &= ~CTRL5_PICOPHY_BC_MODE;
> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value);
> +    /* wait for stable */
> +    MicroSecondDelay (20000);
> +  } else {
> +    DEBUG ((DEBUG_INFO, "usb work as host mode.\n"));
> +
> +    /*CTRL5*/
> +    Data = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
> +    Data &= ~CTRL5_PICOPHY_BC_MODE;
> +    Data |= CTRL5_USBOTG_RES_SEL | CTRL5_PICOPHY_ACAENB |
> +            CTRL5_PICOPHY_VDATDETENB | CTRL5_PICOPHY_DCDENB;
> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Data);
> +    /* wait for stable */
> +    MicroSecondDelay (20000);
> +    /* Set the USB phy timing with tuned value that shows an eye pattern on oscillator.  */
> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL8, USB_EYE_PATTERN);
> +    /* wait for eye pattern effective */
> +    MicroSecondDelay (5000);
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyUsbGetLang (
> +  OUT CHAR16            *Lang,
> +  OUT UINT8             *Length
> +  )
> +{
> +  if ((Lang == NULL) || (Length == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  Lang[0] = LANG_EN;
> +  *Length = sizeof (CHAR16);
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyUsbGetManufacturer (
> +  OUT CHAR16            *Manufacturer,
> +  OUT UINT8             *Length
> +  )
> +{
> +  CHAR16                 DataUnicode[] = L"96Boards";
> +
> +  if ((Manufacturer == NULL) || (Length == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  StrCpy (Manufacturer, DataUnicode);
> +  /* include '\0' for string */
> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);

This code is identical to v3.

In another thread in some other iteration, you stated
"I need the terminated '\0' character. Since it'll be sent by USB
driver. Without this, the last character will dispear."

Overall, I am very reluctant to accept this entire mechanism for
retrieving strings, unless you can explain why this extremely
roundabout way of putting a string into a buffer is necessary.

But this also requires going back and replying to my comments on 2/5.

/
    Leif

> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyUsbGetProduct (
> +  OUT CHAR16            *Product,
> +  OUT UINT8             *Length
> +  )
> +{
> +  CHAR16                 DataUnicode[] = L"HiKey";
> +
> +  if ((Product == NULL) || (Length == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  StrCpy (Product, DataUnicode);
> +  /* include '\0' for string */
> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyUsbGetSerialNo (
> +  OUT CHAR16            *SerialNo,
> +  OUT UINT8             *Length
> +  )
> +{
> +  CHAR16                 DataUnicode[] = L"0123456789abcdef";
> +
> +  if ((SerialNo == NULL) || (Length == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  StrCpy (SerialNo, DataUnicode);
> +  /* include '\0' for string */
> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
> +  return EFI_SUCCESS;
> +}
> +
> +DW_USB_PROTOCOL mDwUsbDevice = {
> +  HiKeyUsbGetLang,
> +  HiKeyUsbGetManufacturer,
> +  HiKeyUsbGetProduct,
> +  HiKeyUsbGetSerialNo,
> +  HiKeyUsbPhyInit
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyUsbEntryPoint (
> +  IN EFI_HANDLE                            ImageHandle,
> +  IN EFI_SYSTEM_TABLE                      *SystemTable
> +  )
> +{
> +  EFI_STATUS        Status;
> +
> +  Status = gBS->InstallProtocolInterface (
> +                  &ImageHandle,
> +                  &gDwUsbProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mDwUsbDevice
> +                  );
> +  return Status;
> +}
> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
> new file mode 100644
> index 0000000..57d639a
> --- /dev/null
> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
> @@ -0,0 +1,46 @@
> +#/** @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                      = HiKeyUsbDxe
> +  FILE_GUID                      = c5c7089e-9b00-448c-8b23-a552688e2833
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = HiKeyUsbEntryPoint
> +
> +[Sources.common]
> +  HiKeyUsbDxe.c
> +
> +[LibraryClasses]
> +  DebugLib
> +  IoLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gDwUsbProtocolGuid
> +  gEfiDriverBindingProtocolGuid
> +  gEmbeddedGpioProtocolGuid
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
> +  OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec
> +
> +[Depex]
> +  TRUE
> -- 
> 2.7.4
>

On 2017/3/1 23:00, Leif Lindholm wrote:
> On Wed, Feb 15, 2017 at 10:54:58PM +0800, Haojian Zhuang wrote:
>> Support Designware USB device controller on HiKey platform.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>>  .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c      | 265 +++++++++++++++++++++
>>  .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf    |  46 ++++
>>  2 files changed, 311 insertions(+)
>>  create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
>>  create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
>>
>> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
>> new file mode 100644
>> index 0000000..59ef57c
>> --- /dev/null
>> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
>> @@ -0,0 +1,265 @@
>> +/** @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 <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/TimerLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +
>> +#include <Protocol/EmbeddedGpio.h>
>> +#include <Protocol/DwUsb.h>
>> +
>> +#include <Hi6220.h>
>> +
>> +
>> +#define USB_SEL_GPIO0_3          3     // GPIO 0_3
>> +#define USB_5V_HUB_EN            7     // GPIO 0_7
>> +#define USB_ID_DET_GPIO2_5       21    // GPIO 2_5
>> +#define USB_VBUS_DET_GPIO2_6     22    // GPIO 2_6
>> +
>> +// Jumper on pin5-6 of J15 determines whether boot to fastboot
>> +#define DETECT_J15_FASTBOOT      24    // GPIO 3_0
>> +
>> +#define IOCG_GPIO0_BASE          0xF8001800
>> +#define IOCG_GPIO0_3_OFFSET      0x1C
>> +#define IOCG_GPIO0_7_OFFSET      0x2C
>> +#define IOCG_GPIO2_5_OFFSET      0x64
>> +#define IOCG_GPIO2_6_OFFSET      0x68
>> +
>> +#define IOCG_PULLUP              1
>> +#define IOCG_PULLDOWN            2
>> +
>> +#define USB_EYE_PATTERN          0x70533483
>> +
>> +#define LANG_EN                  0x409
>> +
>> +STATIC EMBEDDED_GPIO *mGpio;
>> +
>> +STATIC
>> +EFI_STATUS
>> +HiKeyDetectUsbModeInit (
>> +  IN VOID
>> +  )
>> +{
>> +  EFI_STATUS     Status;
>> +
>> +  /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */
>> +  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_5_OFFSET, IOCG_PULLUP);
>> +  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_6_OFFSET, IOCG_PULLUP);
>> +
>> +  Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "Can't locate gEmbeddedGpioProtocolGuid\n"));
>> +    return Status;
>> +  }
>> +  /* power on USB HUB */
>> +  Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0);
>> +  ASSERT_EFI_ERROR (Status);
>> +  /* start to detect USB device or host */
>> +  Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  Status = mGpio->Set (mGpio, USB_ID_DET_GPIO2_5, GPIO_MODE_INPUT);
>> +  ASSERT_EFI_ERROR (Status);
>> +  Status = mGpio->Set (mGpio, USB_VBUS_DET_GPIO2_6, GPIO_MODE_INPUT);
>> +  ASSERT_EFI_ERROR (Status);
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +UINTN
>> +HiKeyGetUsbMode (
>> +  IN VOID
>> +  )
>> +{
>> +  EFI_STATUS     Status;
>> +  UINTN          GpioId, GpioVbus;
>> +
>> +  Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &GpioId);
>> +  ASSERT_EFI_ERROR (Status);
>> +  Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &GpioVbus);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  if ((GpioId == 1) && (GpioVbus == 0)) {
>> +    return USB_DEVICE_MODE;
>> +  } else if ((GpioId == 0) && (GpioVbus == 1)) {
>> +    return USB_CABLE_NOT_ATTACHED;
>> +  }
>> +  return USB_HOST_MODE;
>> +}
>> +
>> +EFI_STATUS
>> +HiKeyUsbPhyInit (
>> +  IN UINT8        Mode
>> +  )
>> +{
>> +  UINTN          Value;
>> +  UINT32         Data;
>> +  EFI_STATUS     Status;
>> +
>> +  Status = HiKeyDetectUsbModeInit ();
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  //setup clock
>> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, PERIPH_CLKEN0_USBOTG);
>> +  do {
>> +       Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0);
>> +  } while ((Value & PERIPH_CLKEN0_USBOTG) == 0);
>> +
>> +  //setup phy
>> +  Data = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
>> +           RST0_USBOTG | RST0_USBOTG_32K;
>> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS0, Data);
>> +  do {
>> +    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_RSTSTAT0);
>> +    Value &= Data;
>> +  } while (Value);
>> +
>> +  Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4);
>> +  Value &= ~(CTRL4_PICO_SIDDQ | CTRL4_FPGA_EXT_PHY_SEL |
>> +             CTRL4_OTG_PHY_SEL);
>> +  Value |=  CTRL4_PICO_VBUSVLDEXT | CTRL4_PICO_VBUSVLDEXTSEL;
>> +  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4, Value);
>> +
>> +  if (HiKeyGetUsbMode () != Mode) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  //If Mode = 1, USB in Device Mode
>> +  //If Mode = 0, USB in Host Mode
>> +  if (Mode == USB_DEVICE_MODE) {
>> +    DEBUG ((DEBUG_INFO, "usb work as device mode.\n"));
>> +
>> +    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
>> +    Value &= ~CTRL5_PICOPHY_BC_MODE;
>> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value);
>> +    /* wait for stable */
>> +    MicroSecondDelay (20000);
>> +  } else {
>> +    DEBUG ((DEBUG_INFO, "usb work as host mode.\n"));
>> +
>> +    /*CTRL5*/
>> +    Data = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
>> +    Data &= ~CTRL5_PICOPHY_BC_MODE;
>> +    Data |= CTRL5_USBOTG_RES_SEL | CTRL5_PICOPHY_ACAENB |
>> +            CTRL5_PICOPHY_VDATDETENB | CTRL5_PICOPHY_DCDENB;
>> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Data);
>> +    /* wait for stable */
>> +    MicroSecondDelay (20000);
>> +    /* Set the USB phy timing with tuned value that shows an eye pattern on oscillator.  */
>> +    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL8, USB_EYE_PATTERN);
>> +    /* wait for eye pattern effective */
>> +    MicroSecondDelay (5000);
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +HiKeyUsbGetLang (
>> +  OUT CHAR16            *Lang,
>> +  OUT UINT8             *Length
>> +  )
>> +{
>> +  if ((Lang == NULL) || (Length == NULL)) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  Lang[0] = LANG_EN;
>> +  *Length = sizeof (CHAR16);
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +HiKeyUsbGetManufacturer (
>> +  OUT CHAR16            *Manufacturer,
>> +  OUT UINT8             *Length
>> +  )
>> +{
>> +  CHAR16                 DataUnicode[] = L"96Boards";
>> +
>> +  if ((Manufacturer == NULL) || (Length == NULL)) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  StrCpy (Manufacturer, DataUnicode);
>> +  /* include '\0' for string */
>> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
>
> This code is identical to v3.
>
> In another thread in some other iteration, you stated
> "I need the terminated '\0' character. Since it'll be sent by USB
> driver. Without this, the last character will dispear."
>
> Overall, I am very reluctant to accept this entire mechanism for
> retrieving strings, unless you can explain why this extremely
> roundabout way of putting a string into a buffer is necessary.
>
> But this also requires going back and replying to my comments on 2/5.
>

StrLen (
   IN      CONST CHAR16              *String
   )
{
   UINTN                             Length;

   ASSERT (String != NULL);
   ASSERT (((UINTN) String & BIT0) == 0);

   for (Length = 0; *String != L'\0'; String++, Length++) {
     ...
   }
   return Length;
}

We can find that it only counts the number of non-null character.
When the USB driver gets the buffer and the size, it'll only send
out all non-null characters. So the USB host will miss the null
character as terminator.

In order to avoid this issue, I accumulate string length at here.

>
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +HiKeyUsbGetProduct (
>> +  OUT CHAR16            *Product,
>> +  OUT UINT8             *Length
>> +  )
>> +{
>> +  CHAR16                 DataUnicode[] = L"HiKey";
>> +
>> +  if ((Product == NULL) || (Length == NULL)) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  StrCpy (Product, DataUnicode);
>> +  /* include '\0' for string */
>> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +HiKeyUsbGetSerialNo (
>> +  OUT CHAR16            *SerialNo,
>> +  OUT UINT8             *Length
>> +  )
>> +{
>> +  CHAR16                 DataUnicode[] = L"0123456789abcdef";
>> +
>> +  if ((SerialNo == NULL) || (Length == NULL)) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  StrCpy (SerialNo, DataUnicode);
>> +  /* include '\0' for string */
>> +  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +DW_USB_PROTOCOL mDwUsbDevice = {
>> +  HiKeyUsbGetLang,
>> +  HiKeyUsbGetManufacturer,
>> +  HiKeyUsbGetProduct,
>> +  HiKeyUsbGetSerialNo,
>> +  HiKeyUsbPhyInit
>> +};
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +HiKeyUsbEntryPoint (
>> +  IN EFI_HANDLE                            ImageHandle,
>> +  IN EFI_SYSTEM_TABLE                      *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS        Status;
>> +
>> +  Status = gBS->InstallProtocolInterface (
>> +                  &ImageHandle,
>> +                  &gDwUsbProtocolGuid,
>> +                  EFI_NATIVE_INTERFACE,
>> +                  &mDwUsbDevice
>> +                  );
>> +  return Status;
>> +}
>> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
>> new file mode 100644
>> index 0000000..57d639a
>> --- /dev/null
>> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
>> @@ -0,0 +1,46 @@
>> +#/** @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                      = HiKeyUsbDxe
>> +  FILE_GUID                      = c5c7089e-9b00-448c-8b23-a552688e2833
>> +  MODULE_TYPE                    = UEFI_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  ENTRY_POINT                    = HiKeyUsbEntryPoint
>> +
>> +[Sources.common]
>> +  HiKeyUsbDxe.c
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> +  IoLib
>> +  TimerLib
>> +  UefiBootServicesTableLib
>> +  UefiDriverEntryPoint
>> +
>> +[Protocols]
>> +  gDwUsbProtocolGuid
>> +  gEfiDriverBindingProtocolGuid
>> +  gEmbeddedGpioProtocolGuid
>> +
>> +[Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec
>> +  OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec
>> +
>> +[Depex]
>> +  TRUE
>> --
>> 2.7.4
>>

On Thu, Mar 02, 2017 at 09:45:50AM +0800, Haojian Zhuang wrote:
> On 2017/3/1 23:00, Leif Lindholm wrote:
> >On Wed, Feb 15, 2017 at 10:54:58PM +0800, Haojian Zhuang wrote:
> >>Support Designware USB device controller on HiKey platform.
> >>
> >>Contributed-under: TianoCore Contribution Agreement 1.0
> >>Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> >>---
> >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c      | 265 +++++++++++++++++++++
> >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf    |  46 ++++
> >> 2 files changed, 311 insertions(+)
> >> create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
> >> create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf
> >>
> >>diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
> >>new file mode 100644
> >>index 0000000..59ef57c
> >>--- /dev/null
> >>+++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c
> >>@@ -0,0 +1,265 @@
> >>+/** @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 <Library/BaseMemoryLib.h>
> >>+#include <Library/DebugLib.h>
> >>+#include <Library/IoLib.h>
> >>+#include <Library/TimerLib.h>
> >>+#include <Library/UefiBootServicesTableLib.h>
> >>+#include <Library/UefiLib.h>
> >>+#include <Library/UefiRuntimeServicesTableLib.h>
> >>+
> >>+#include <Protocol/EmbeddedGpio.h>
> >>+#include <Protocol/DwUsb.h>
> >>+
> >>+#include <Hi6220.h>
> >>+
> >>+
> >>+#define USB_SEL_GPIO0_3          3     // GPIO 0_3
> >>+#define USB_5V_HUB_EN            7     // GPIO 0_7
> >>+#define USB_ID_DET_GPIO2_5       21    // GPIO 2_5
> >>+#define USB_VBUS_DET_GPIO2_6     22    // GPIO 2_6
> >>+
> >>+// Jumper on pin5-6 of J15 determines whether boot to fastboot
> >>+#define DETECT_J15_FASTBOOT      24    // GPIO 3_0
> >>+
> >>+#define IOCG_GPIO0_BASE          0xF8001800
> >>+#define IOCG_GPIO0_3_OFFSET      0x1C
> >>+#define IOCG_GPIO0_7_OFFSET      0x2C
> >>+#define IOCG_GPIO2_5_OFFSET      0x64
> >>+#define IOCG_GPIO2_6_OFFSET      0x68
> >>+
> >>+#define IOCG_PULLUP              1
> >>+#define IOCG_PULLDOWN            2
> >>+
> >>+#define USB_EYE_PATTERN          0x70533483
> >>+
> >>+#define LANG_EN                  0x409
> >>+
> >>+STATIC EMBEDDED_GPIO *mGpio;
> >>+
> >>+STATIC
> >>+EFI_STATUS
> >>+HiKeyDetectUsbModeInit (
> >>+  IN VOID
> >>+  )
> >>+{
> >>+  EFI_STATUS     Status;
> >>+
> >>+  /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */
> >>+  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_5_OFFSET, IOCG_PULLUP);
> >>+  MmioWrite32 (IOCG_GPIO0_BASE + IOCG_GPIO2_6_OFFSET, IOCG_PULLUP);
> >>+
> >>+  Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio);
> >>+  if (EFI_ERROR (Status)) {
> >>+    DEBUG ((DEBUG_ERROR, "Can't locate gEmbeddedGpioProtocolGuid\n"));
> >>+    return Status;
> >>+  }
> >>+  /* power on USB HUB */
> >>+  Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+  /* start to detect USB device or host */
> >>+  Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+
> >>+  Status = mGpio->Set (mGpio, USB_ID_DET_GPIO2_5, GPIO_MODE_INPUT);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+  Status = mGpio->Set (mGpio, USB_VBUS_DET_GPIO2_6, GPIO_MODE_INPUT);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+  return EFI_SUCCESS;
> >>+}
> >>+
> >>+UINTN
> >>+HiKeyGetUsbMode (
> >>+  IN VOID
> >>+  )
> >>+{
> >>+  EFI_STATUS     Status;
> >>+  UINTN          GpioId, GpioVbus;
> >>+
> >>+  Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &GpioId);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+  Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &GpioVbus);
> >>+  ASSERT_EFI_ERROR (Status);
> >>+
> >>+  if ((GpioId == 1) && (GpioVbus == 0)) {
> >>+    return USB_DEVICE_MODE;
> >>+  } else if ((GpioId == 0) && (GpioVbus == 1)) {
> >>+    return USB_CABLE_NOT_ATTACHED;
> >>+  }
> >>+  return USB_HOST_MODE;
> >>+}
> >>+
> >>+EFI_STATUS
> >>+HiKeyUsbPhyInit (
> >>+  IN UINT8        Mode
> >>+  )
> >>+{
> >>+  UINTN          Value;
> >>+  UINT32         Data;
> >>+  EFI_STATUS     Status;
> >>+
> >>+  Status = HiKeyDetectUsbModeInit ();
> >>+  if (EFI_ERROR (Status)) {
> >>+    return Status;
> >>+  }
> >>+
> >>+  //setup clock
> >>+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, PERIPH_CLKEN0_USBOTG);
> >>+  do {
> >>+       Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0);
> >>+  } while ((Value & PERIPH_CLKEN0_USBOTG) == 0);
> >>+
> >>+  //setup phy
> >>+  Data = RST0_USBOTG_BUS | RST0_POR_PICOPHY |
> >>+           RST0_USBOTG | RST0_USBOTG_32K;
> >>+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS0, Data);
> >>+  do {
> >>+    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_RSTSTAT0);
> >>+    Value &= Data;
> >>+  } while (Value);
> >>+
> >>+  Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4);
> >>+  Value &= ~(CTRL4_PICO_SIDDQ | CTRL4_FPGA_EXT_PHY_SEL |
> >>+             CTRL4_OTG_PHY_SEL);
> >>+  Value |=  CTRL4_PICO_VBUSVLDEXT | CTRL4_PICO_VBUSVLDEXTSEL;
> >>+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL4, Value);
> >>+
> >>+  if (HiKeyGetUsbMode () != Mode) {
> >>+    return EFI_INVALID_PARAMETER;
> >>+  }
> >>+  //If Mode = 1, USB in Device Mode
> >>+  //If Mode = 0, USB in Host Mode
> >>+  if (Mode == USB_DEVICE_MODE) {
> >>+    DEBUG ((DEBUG_INFO, "usb work as device mode.\n"));
> >>+
> >>+    Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
> >>+    Value &= ~CTRL5_PICOPHY_BC_MODE;
> >>+    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value);
> >>+    /* wait for stable */
> >>+    MicroSecondDelay (20000);
> >>+  } else {
> >>+    DEBUG ((DEBUG_INFO, "usb work as host mode.\n"));
> >>+
> >>+    /*CTRL5*/
> >>+    Data = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5);
> >>+    Data &= ~CTRL5_PICOPHY_BC_MODE;
> >>+    Data |= CTRL5_USBOTG_RES_SEL | CTRL5_PICOPHY_ACAENB |
> >>+            CTRL5_PICOPHY_VDATDETENB | CTRL5_PICOPHY_DCDENB;
> >>+    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Data);
> >>+    /* wait for stable */
> >>+    MicroSecondDelay (20000);
> >>+    /* Set the USB phy timing with tuned value that shows an eye pattern on oscillator.  */
> >>+    MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL8, USB_EYE_PATTERN);
> >>+    /* wait for eye pattern effective */
> >>+    MicroSecondDelay (5000);
> >>+  }
> >>+
> >>+  return EFI_SUCCESS;
> >>+}
> >>+
> >>+EFI_STATUS
> >>+EFIAPI
> >>+HiKeyUsbGetLang (
> >>+  OUT CHAR16            *Lang,
> >>+  OUT UINT8             *Length
> >>+  )
> >>+{
> >>+  if ((Lang == NULL) || (Length == NULL)) {
> >>+    return EFI_INVALID_PARAMETER;
> >>+  }
> >>+  Lang[0] = LANG_EN;
> >>+  *Length = sizeof (CHAR16);
> >>+  return EFI_SUCCESS;
> >>+}
> >>+
> >>+EFI_STATUS
> >>+EFIAPI
> >>+HiKeyUsbGetManufacturer (
> >>+  OUT CHAR16            *Manufacturer,
> >>+  OUT UINT8             *Length
> >>+  )
> >>+{
> >>+  CHAR16                 DataUnicode[] = L"96Boards";
> >>+
> >>+  if ((Manufacturer == NULL) || (Length == NULL)) {
> >>+    return EFI_INVALID_PARAMETER;
> >>+  }
> >>+  StrCpy (Manufacturer, DataUnicode);
> >>+  /* include '\0' for string */
> >>+  *Length = (StrLen (DataUnicode) + 1) * sizeof (CHAR16);
> >
> >This code is identical to v3.
> >
> >In another thread in some other iteration, you stated
> >"I need the terminated '\0' character. Since it'll be sent by USB
> >driver. Without this, the last character will dispear."
> >
> >Overall, I am very reluctant to accept this entire mechanism for
> >retrieving strings, unless you can explain why this extremely
> >roundabout way of putting a string into a buffer is necessary.
> >
> >But this also requires going back and replying to my comments on 2/5.
> >
> 
> StrLen (
>   IN      CONST CHAR16              *String
>   )
> {
>   UINTN                             Length;
> 
>   ASSERT (String != NULL);
>   ASSERT (((UINTN) String & BIT0) == 0);
> 
>   for (Length = 0; *String != L'\0'; String++, Length++) {
>     ...
>   }
>   return Length;
> }
> 
> We can find that it only counts the number of non-null character.
> When the USB driver gets the buffer and the size, it'll only send
> out all non-null characters. So the USB host will miss the null
> character as terminator.
> 
> In order to avoid this issue, I accumulate string length at here.

The compiler knows the size of the null character, you just need to
add it to the value returned by StrSize: sizeof (L'\0').

/
    Leif