| Message ID | 1486649511-4149-7-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | enable Android Fastboot App on Hikey | expand |
On Thu, Feb 09, 2017 at 10:11:48PM +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 | 266 +++++++++++++++++++++ > .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ > 2 files changed, 312 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..60ad4d6 > --- /dev/null > +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c > @@ -0,0 +1,266 @@ > +/** @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 > + > +STATIC EMBEDDED_GPIO *mGpio; > + > +STATIC > +VOID > +HiKeyDetectUsbModeInit ( > + IN VOID > + ) > +{ > + EFI_STATUS Status; > + > + /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */ > + MmioWrite32 (0xf8001864, 1); > + MmioWrite32 (0xf8001868, 1); No magic values please, add #defines. > + > + Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio); > + ASSERT_EFI_ERROR (Status); This should unconditionally print an error and return if failed. Also, it should probably return error and be handled at the call site, instead of continuing as if nothing had happened. > + Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0); > + ASSERT_EFI_ERROR (Status); > + Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0); > + ASSERT_EFI_ERROR (Status); > + MicroSecondDelay (1000); Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > + > + 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); > +} > + > +UINTN > +HiKeyGetUsbMode ( > + IN VOID > + ) > +{ > +#if 0 Please don't include commented-out code blocks. > + EFI_STATUS Status; > + UINTN GpioId, GpioVbus; > + UINTN Value; > + > + Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &Value); > + ASSERT_EFI_ERROR (Status); > + GpioId = Value; > + Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &Value); > + ASSERT_EFI_ERROR (Status); > + GpioVbus = Value; > + > +DEBUG ((DEBUG_ERROR, "#%a, %d, GpioId:%d, GpioVbus:%d\n", __func__, __LINE__, GpioId, GpioVbus)); > + if ((GpioId == 1) && (GpioVbus == 0)) { > + return USB_DEVICE_MODE; > + } else if ((GpioId == 0) && (GpioVbus == 1)) { > + return USB_CABLE_NOT_ATTACHED; > + } > + return USB_HOST_MODE; > +#else > + return USB_DEVICE_MODE; > +#endif > +} > + > +EFI_STATUS > +HiKeyUsbPhyInit ( > + IN UINT8 Mode > + ) > +{ > + UINTN Value; > + UINT32 Data; > + > + HiKeyDetectUsbModeInit (); > + > + //setup clock > + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, BIT4); What does BIT4 mean in this context? Can a #define with a more descriptive name be created? > + do { > + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0); > + } while ((Value & BIT4) == 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); > + MicroSecondDelay (1000); Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > + > + //If Mode = 1, USB in Device Mode > + //If Mode = 0, USB in Host Mode You have introduced #defines for these, so don't need to list. Could just say: // Verify that controller is in expected mode > + if (Mode == USB_DEVICE_MODE) { > + if (HiKeyGetUsbMode () == USB_DEVICE_MODE) { > + DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); > + } else { > + return EFI_INVALID_PARAMETER; > + } May be cleaner written as: if (HiKeyGetUsbMode () != USB_DEVICE_MODE) { return EFI_INVALID_PARAMETER; } DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); But also, please don't use DEBUG_ERROR for non-error conditions. > + > + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5); > + Value &= ~CTRL5_PICOPHY_BC_MODE; > + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value); > + MicroSecondDelay (20000); Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > + } else { > + if (HiKeyGetUsbMode () == USB_HOST_MODE) { > + DEBUG ((DEBUG_ERROR, "usb work as host mode.\n")); Please don't use DEBUG_ERROR for non-error conditions. > + } else { > + return EFI_INVALID_PARAMETER; > + } Ah, but here the pattern repeats again. So actually, you could do: if (HiKeyGetUsbMode () != Mode) { return EFI_INVALID_PARAMETER; } once and then if (Mode == USB_DEVICE_MODE) { ... } else { ... } > + > + /*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); > + MicroSecondDelay (20000); Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > + MmioWrite32 (PERI_CTRL_BASE + 0x018, 0x70533483); //EYE_PATTERN No magic values, please. (What's an eye pattern?) > + > + MicroSecondDelay (5000); Why this delay? Is a barrier needed? Why was 5ms chosen as the delay? > + } > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +HiKeyUsbGetLang ( > + OUT CHAR16 *Lang, > + OUT UINT8 *Length > + ) > +{ > + if ((Lang == NULL) || (Length == NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + Lang[0] = 0x409; No magic values, please. > + *Length = sizeof (CHAR16); > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +HiKeyUsbGetManuFacturer ( Manufacturer is a single word, so the 'f' should not be capitalised and the #define should be "MANUFACTURER_". (To be corrected also in 3/9, where I missed it on the first pass.) > + OUT CHAR16 *ManuFacturer, > + OUT UINT8 *Length > + ) > +{ > + UINTN VariableSize; > + CHAR16 DataUnicode[MANU_FACTURER_STRING_LENGTH]; > + > + if ((ManuFacturer == NULL) || (Length == NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + VariableSize = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16); > + ZeroMem (DataUnicode, MANU_FACTURER_STRING_LENGTH * sizeof(CHAR16)); > + StrCpy (DataUnicode, L"96Boards"); > + CopyMem (ManuFacturer, DataUnicode, VariableSize); > + *Length = VariableSize; > + return EFI_SUCCESS; > +} OK, the mechanism used here to set manufacturer, product and serial strings is hideously over-complicated. In fact, ignore my comment that the string lenghts should be corrected also in 3/9 - they should never exist as defines in the first place. This file holds all of the information about which strings are going into the data structure, and that should not be exported anywhere. These functions should return a pointer to a string allocated dynamically in this file (or NULL if allocation fails). Since these functions create '0'-terminated strings. the length can then be determined using StrLen. > + > +EFI_STATUS > +EFIAPI > +HiKeyUsbGetProduct ( > + OUT CHAR16 *Product, > + OUT UINT8 *Length > + ) > +{ > + UINTN VariableSize; > + CHAR16 DataUnicode[PRODUCT_STRING_LENGTH]; > + > + if ((Product == NULL) || (Length == NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + VariableSize = PRODUCT_STRING_LENGTH * sizeof (CHAR16); > + ZeroMem (DataUnicode, PRODUCT_STRING_LENGTH * sizeof(CHAR16)); > + StrCpy (DataUnicode, L"HiKey"); > + CopyMem (Product, DataUnicode, VariableSize); > + *Length = VariableSize; > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +HiKeyUsbGetSerialNo ( > + OUT CHAR16 *SerialNo, > + OUT UINT8 *Length > + ) > +{ > + UINTN VariableSize; > + CHAR16 DataUnicode[SERIAL_STRING_LENGTH]; > + > + if ((SerialNo == NULL) || (Length == NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + VariableSize = SERIAL_STRING_LENGTH * sizeof (CHAR16); > + ZeroMem (DataUnicode, SERIAL_STRING_LENGTH * sizeof(CHAR16)); > + StrCpy (DataUnicode, L"0123456789abcdef"); A hardcoded serial number will make you no friends in validation labs. I would suggest that as a start, this fake serial number is broken out to be retreived from a helper function (which for now can return only a made up one). Then at a later date, you can figure out how to provide it with some data that lets you actually tell different devices apart. > + CopyMem (SerialNo, DataUnicode, VariableSize); > + *Length = VariableSize; > + 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 > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } The code returns status anyway, so this EFI_ERROR test has no purpose. > + return Status; > +} > diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf > new file mode 100644 > index 0000000..1cb70c9 > --- /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] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec > + OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec Please sort packages alphabetically. > + > +[Depex] > + TRUE > -- > 2.7.4 >
> On 10 Feb 2017, at 11:37, Leif Lindholm <leif.lindholm@linaro.org> wrote: > >> On Thu, Feb 09, 2017 at 10:11:48PM +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 | 266 +++++++++++++++++++++ >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ >> 2 files changed, 312 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..60ad4d6 >> --- /dev/null >> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c >> @@ -0,0 +1,266 @@ >> +/** @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 >> + >> +STATIC EMBEDDED_GPIO *mGpio; >> + >> +STATIC >> +VOID >> +HiKeyDetectUsbModeInit ( >> + IN VOID >> + ) >> +{ >> + EFI_STATUS Status; >> + >> + /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */ >> + MmioWrite32 (0xf8001864, 1); >> + MmioWrite32 (0xf8001868, 1); > > No magic values please, add #defines. > >> + >> + Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio); >> + ASSERT_EFI_ERROR (Status); > > This should unconditionally print an error and return if failed. > Also, it should probably return error and be handled at the call site, > instead of continuing as if nothing had happened. > If this driver has a depex on this protocol, it is reasonable (and idiomatic in edk2) to use this pattern imo >> + Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0); >> + ASSERT_EFI_ERROR (Status); >> + Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0); >> + ASSERT_EFI_ERROR (Status); >> + MicroSecondDelay (1000); > > Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > >> + >> + 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); >> +} >> + >> +UINTN >> +HiKeyGetUsbMode ( >> + IN VOID >> + ) >> +{ >> +#if 0 > > Please don't include commented-out code blocks. > >> + EFI_STATUS Status; >> + UINTN GpioId, GpioVbus; >> + UINTN Value; >> + >> + Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &Value); >> + ASSERT_EFI_ERROR (Status); >> + GpioId = Value; >> + Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &Value); >> + ASSERT_EFI_ERROR (Status); >> + GpioVbus = Value; >> + >> +DEBUG ((DEBUG_ERROR, "#%a, %d, GpioId:%d, GpioVbus:%d\n", __func__, __LINE__, GpioId, GpioVbus)); >> + if ((GpioId == 1) && (GpioVbus == 0)) { >> + return USB_DEVICE_MODE; >> + } else if ((GpioId == 0) && (GpioVbus == 1)) { >> + return USB_CABLE_NOT_ATTACHED; >> + } >> + return USB_HOST_MODE; >> +#else >> + return USB_DEVICE_MODE; >> +#endif >> +} >> + >> +EFI_STATUS >> +HiKeyUsbPhyInit ( >> + IN UINT8 Mode >> + ) >> +{ >> + UINTN Value; >> + UINT32 Data; >> + >> + HiKeyDetectUsbModeInit (); >> + >> + //setup clock >> + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, BIT4); > > What does BIT4 mean in this context? > Can a #define with a more descriptive name be created? > >> + do { >> + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0); >> + } while ((Value & BIT4) == 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); >> + MicroSecondDelay (1000); > > Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > >> + >> + //If Mode = 1, USB in Device Mode >> + //If Mode = 0, USB in Host Mode > > You have introduced #defines for these, so don't need to list. > Could just say: > // Verify that controller is in expected mode > >> + if (Mode == USB_DEVICE_MODE) { >> + if (HiKeyGetUsbMode () == USB_DEVICE_MODE) { >> + DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); >> + } else { >> + return EFI_INVALID_PARAMETER; >> + } > > May be cleaner written as: > > if (HiKeyGetUsbMode () != USB_DEVICE_MODE) { > return EFI_INVALID_PARAMETER; > } > > DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); > > But also, please don't use DEBUG_ERROR for non-error conditions. > >> + >> + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5); >> + Value &= ~CTRL5_PICOPHY_BC_MODE; >> + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value); >> + MicroSecondDelay (20000); > > Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > >> + } else { >> + if (HiKeyGetUsbMode () == USB_HOST_MODE) { >> + DEBUG ((DEBUG_ERROR, "usb work as host mode.\n")); > > Please don't use DEBUG_ERROR for non-error conditions. > >> + } else { >> + return EFI_INVALID_PARAMETER; >> + } > > Ah, but here the pattern repeats again. > So actually, you could do: > > if (HiKeyGetUsbMode () != Mode) { > return EFI_INVALID_PARAMETER; > } > > once > > and then > if (Mode == USB_DEVICE_MODE) { > ... > } else { > ... > } > >> + >> + /*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); >> + MicroSecondDelay (20000); > > Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > >> + MmioWrite32 (PERI_CTRL_BASE + 0x018, 0x70533483); //EYE_PATTERN > > No magic values, please. > (What's an eye pattern?) > >> + >> + MicroSecondDelay (5000); > > Why this delay? Is a barrier needed? Why was 5ms chosen as the delay? > >> + } >> + >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetLang ( >> + OUT CHAR16 *Lang, >> + OUT UINT8 *Length >> + ) >> +{ >> + if ((Lang == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + Lang[0] = 0x409; > > No magic values, please. > >> + *Length = sizeof (CHAR16); >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetManuFacturer ( > > Manufacturer is a single word, so the 'f' should not be capitalised > and the #define should be "MANUFACTURER_". (To be corrected also in > 3/9, where I missed it on the first pass.) > >> + OUT CHAR16 *ManuFacturer, >> + OUT UINT8 *Length >> + ) >> +{ >> + UINTN VariableSize; >> + CHAR16 DataUnicode[MANU_FACTURER_STRING_LENGTH]; >> + >> + if ((ManuFacturer == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + VariableSize = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16); >> + ZeroMem (DataUnicode, MANU_FACTURER_STRING_LENGTH * sizeof(CHAR16)); >> + StrCpy (DataUnicode, L"96Boards"); >> + CopyMem (ManuFacturer, DataUnicode, VariableSize); >> + *Length = VariableSize; >> + return EFI_SUCCESS; >> +} > > OK, the mechanism used here to set manufacturer, product and serial > strings is hideously over-complicated. > > In fact, ignore my comment that the string lenghts should be corrected > also in 3/9 - they should never exist as defines in the first place. > > This file holds all of the information about which strings are going > into the data structure, and that should not be exported anywhere. > > These functions should return a pointer to a string allocated > dynamically in this file (or NULL if allocation fails). > Since these functions create '0'-terminated strings. the length can > then be determined using StrLen. > >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetProduct ( >> + OUT CHAR16 *Product, >> + OUT UINT8 *Length >> + ) >> +{ >> + UINTN VariableSize; >> + CHAR16 DataUnicode[PRODUCT_STRING_LENGTH]; >> + >> + if ((Product == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + VariableSize = PRODUCT_STRING_LENGTH * sizeof (CHAR16); >> + ZeroMem (DataUnicode, PRODUCT_STRING_LENGTH * sizeof(CHAR16)); >> + StrCpy (DataUnicode, L"HiKey"); >> + CopyMem (Product, DataUnicode, VariableSize); >> + *Length = VariableSize; >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetSerialNo ( >> + OUT CHAR16 *SerialNo, >> + OUT UINT8 *Length >> + ) >> +{ >> + UINTN VariableSize; >> + CHAR16 DataUnicode[SERIAL_STRING_LENGTH]; >> + >> + if ((SerialNo == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + VariableSize = SERIAL_STRING_LENGTH * sizeof (CHAR16); >> + ZeroMem (DataUnicode, SERIAL_STRING_LENGTH * sizeof(CHAR16)); >> + StrCpy (DataUnicode, L"0123456789abcdef"); > > A hardcoded serial number will make you no friends in validation labs. > I would suggest that as a start, this fake serial number is broken out > to be retreived from a helper function (which for now can return only > a made up one). Then at a later date, you can figure out how to > provide it with some data that lets you actually tell different > devices apart. > >> + CopyMem (SerialNo, DataUnicode, VariableSize); >> + *Length = VariableSize; >> + 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 >> + ); >> + if (EFI_ERROR (Status)) { >> + return Status; >> + } > > The code returns status anyway, so this EFI_ERROR test has no purpose. > >> + return Status; >> +} >> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf >> new file mode 100644 >> index 0000000..1cb70c9 >> --- /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] >> + MdePkg/MdePkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> + EmbeddedPkg/EmbeddedPkg.dec >> + OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec >> + OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec > > Please sort packages alphabetically. > >> + >> +[Depex] >> + TRUE >> -- >> 2.7.4 >>
On Fri, Feb 10, 2017 at 11:45:20AM +0000, Ard Biesheuvel wrote: > > > On 10 Feb 2017, at 11:37, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > > >> On Thu, Feb 09, 2017 at 10:11:48PM +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 | 266 +++++++++++++++++++++ > >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ > >> 2 files changed, 312 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..60ad4d6 > >> --- /dev/null > >> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c > >> @@ -0,0 +1,266 @@ > >> +/** @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 > >> + > >> +STATIC EMBEDDED_GPIO *mGpio; > >> + > >> +STATIC > >> +VOID > >> +HiKeyDetectUsbModeInit ( > >> + IN VOID > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + > >> + /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */ > >> + MmioWrite32 (0xf8001864, 1); > >> + MmioWrite32 (0xf8001868, 1); > > > > No magic values please, add #defines. > > > >> + > >> + Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio); > >> + ASSERT_EFI_ERROR (Status); > > > > This should unconditionally print an error and return if failed. > > Also, it should probably return error and be handled at the call site, > > instead of continuing as if nothing had happened. > > If this driver has a depex on this protocol, it is reasonable (and > idiomatic in edk2) to use this pattern imo Right, I didn't notice that aspect. On the basis that this is an "impossible" error? I still don't feel great about _not_ returning before dereferencing a NULL pointer, and I see both ways of dealing with this in edk2. But I think if the code stays as it is, there should at least be a comment added - I shouldn't need to refer to other files in order to determine whether a condition is impossible or not. / Leif > >> + Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0); > >> + ASSERT_EFI_ERROR (Status); > >> + Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0); > >> + ASSERT_EFI_ERROR (Status); > >> + MicroSecondDelay (1000); > > > > Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > > > >> + > >> + 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); > >> +} > >> + > >> +UINTN > >> +HiKeyGetUsbMode ( > >> + IN VOID > >> + ) > >> +{ > >> +#if 0 > > > > Please don't include commented-out code blocks. > > > >> + EFI_STATUS Status; > >> + UINTN GpioId, GpioVbus; > >> + UINTN Value; > >> + > >> + Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &Value); > >> + ASSERT_EFI_ERROR (Status); > >> + GpioId = Value; > >> + Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &Value); > >> + ASSERT_EFI_ERROR (Status); > >> + GpioVbus = Value; > >> + > >> +DEBUG ((DEBUG_ERROR, "#%a, %d, GpioId:%d, GpioVbus:%d\n", __func__, __LINE__, GpioId, GpioVbus)); > >> + if ((GpioId == 1) && (GpioVbus == 0)) { > >> + return USB_DEVICE_MODE; > >> + } else if ((GpioId == 0) && (GpioVbus == 1)) { > >> + return USB_CABLE_NOT_ATTACHED; > >> + } > >> + return USB_HOST_MODE; > >> +#else > >> + return USB_DEVICE_MODE; > >> +#endif > >> +} > >> + > >> +EFI_STATUS > >> +HiKeyUsbPhyInit ( > >> + IN UINT8 Mode > >> + ) > >> +{ > >> + UINTN Value; > >> + UINT32 Data; > >> + > >> + HiKeyDetectUsbModeInit (); > >> + > >> + //setup clock > >> + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, BIT4); > > > > What does BIT4 mean in this context? > > Can a #define with a more descriptive name be created? > > > >> + do { > >> + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0); > >> + } while ((Value & BIT4) == 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); > >> + MicroSecondDelay (1000); > > > > Why this delay? Is a barrier needed? Why was 1ms chosen as the delay? > > > >> + > >> + //If Mode = 1, USB in Device Mode > >> + //If Mode = 0, USB in Host Mode > > > > You have introduced #defines for these, so don't need to list. > > Could just say: > > // Verify that controller is in expected mode > > > >> + if (Mode == USB_DEVICE_MODE) { > >> + if (HiKeyGetUsbMode () == USB_DEVICE_MODE) { > >> + DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); > >> + } else { > >> + return EFI_INVALID_PARAMETER; > >> + } > > > > May be cleaner written as: > > > > if (HiKeyGetUsbMode () != USB_DEVICE_MODE) { > > return EFI_INVALID_PARAMETER; > > } > > > > DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); > > > > But also, please don't use DEBUG_ERROR for non-error conditions. > > > >> + > >> + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5); > >> + Value &= ~CTRL5_PICOPHY_BC_MODE; > >> + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value); > >> + MicroSecondDelay (20000); > > > > Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > > > >> + } else { > >> + if (HiKeyGetUsbMode () == USB_HOST_MODE) { > >> + DEBUG ((DEBUG_ERROR, "usb work as host mode.\n")); > > > > Please don't use DEBUG_ERROR for non-error conditions. > > > >> + } else { > >> + return EFI_INVALID_PARAMETER; > >> + } > > > > Ah, but here the pattern repeats again. > > So actually, you could do: > > > > if (HiKeyGetUsbMode () != Mode) { > > return EFI_INVALID_PARAMETER; > > } > > > > once > > > > and then > > if (Mode == USB_DEVICE_MODE) { > > ... > > } else { > > ... > > } > > > >> + > >> + /*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); > >> + MicroSecondDelay (20000); > > > > Why this delay? Is a barrier needed? Why was 20ms chosen as the delay? > > > >> + MmioWrite32 (PERI_CTRL_BASE + 0x018, 0x70533483); //EYE_PATTERN > > > > No magic values, please. > > (What's an eye pattern?) > > > >> + > >> + MicroSecondDelay (5000); > > > > Why this delay? Is a barrier needed? Why was 5ms chosen as the delay? > > > >> + } > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +HiKeyUsbGetLang ( > >> + OUT CHAR16 *Lang, > >> + OUT UINT8 *Length > >> + ) > >> +{ > >> + if ((Lang == NULL) || (Length == NULL)) { > >> + return EFI_INVALID_PARAMETER; > >> + } > >> + Lang[0] = 0x409; > > > > No magic values, please. > > > >> + *Length = sizeof (CHAR16); > >> + return EFI_SUCCESS; > >> +} > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +HiKeyUsbGetManuFacturer ( > > > > Manufacturer is a single word, so the 'f' should not be capitalised > > and the #define should be "MANUFACTURER_". (To be corrected also in > > 3/9, where I missed it on the first pass.) > > > >> + OUT CHAR16 *ManuFacturer, > >> + OUT UINT8 *Length > >> + ) > >> +{ > >> + UINTN VariableSize; > >> + CHAR16 DataUnicode[MANU_FACTURER_STRING_LENGTH]; > >> + > >> + if ((ManuFacturer == NULL) || (Length == NULL)) { > >> + return EFI_INVALID_PARAMETER; > >> + } > >> + VariableSize = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16); > >> + ZeroMem (DataUnicode, MANU_FACTURER_STRING_LENGTH * sizeof(CHAR16)); > >> + StrCpy (DataUnicode, L"96Boards"); > >> + CopyMem (ManuFacturer, DataUnicode, VariableSize); > >> + *Length = VariableSize; > >> + return EFI_SUCCESS; > >> +} > > > > OK, the mechanism used here to set manufacturer, product and serial > > strings is hideously over-complicated. > > > > In fact, ignore my comment that the string lenghts should be corrected > > also in 3/9 - they should never exist as defines in the first place. > > > > This file holds all of the information about which strings are going > > into the data structure, and that should not be exported anywhere. > > > > These functions should return a pointer to a string allocated > > dynamically in this file (or NULL if allocation fails). > > Since these functions create '0'-terminated strings. the length can > > then be determined using StrLen. > > > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +HiKeyUsbGetProduct ( > >> + OUT CHAR16 *Product, > >> + OUT UINT8 *Length > >> + ) > >> +{ > >> + UINTN VariableSize; > >> + CHAR16 DataUnicode[PRODUCT_STRING_LENGTH]; > >> + > >> + if ((Product == NULL) || (Length == NULL)) { > >> + return EFI_INVALID_PARAMETER; > >> + } > >> + VariableSize = PRODUCT_STRING_LENGTH * sizeof (CHAR16); > >> + ZeroMem (DataUnicode, PRODUCT_STRING_LENGTH * sizeof(CHAR16)); > >> + StrCpy (DataUnicode, L"HiKey"); > >> + CopyMem (Product, DataUnicode, VariableSize); > >> + *Length = VariableSize; > >> + return EFI_SUCCESS; > >> +} > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +HiKeyUsbGetSerialNo ( > >> + OUT CHAR16 *SerialNo, > >> + OUT UINT8 *Length > >> + ) > >> +{ > >> + UINTN VariableSize; > >> + CHAR16 DataUnicode[SERIAL_STRING_LENGTH]; > >> + > >> + if ((SerialNo == NULL) || (Length == NULL)) { > >> + return EFI_INVALID_PARAMETER; > >> + } > >> + VariableSize = SERIAL_STRING_LENGTH * sizeof (CHAR16); > >> + ZeroMem (DataUnicode, SERIAL_STRING_LENGTH * sizeof(CHAR16)); > >> + StrCpy (DataUnicode, L"0123456789abcdef"); > > > > A hardcoded serial number will make you no friends in validation labs. > > I would suggest that as a start, this fake serial number is broken out > > to be retreived from a helper function (which for now can return only > > a made up one). Then at a later date, you can figure out how to > > provide it with some data that lets you actually tell different > > devices apart. > > > >> + CopyMem (SerialNo, DataUnicode, VariableSize); > >> + *Length = VariableSize; > >> + 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 > >> + ); > >> + if (EFI_ERROR (Status)) { > >> + return Status; > >> + } > > > > The code returns status anyway, so this EFI_ERROR test has no purpose. > > > >> + return Status; > >> +} > >> diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf > >> new file mode 100644 > >> index 0000000..1cb70c9 > >> --- /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] > >> + MdePkg/MdePkg.dec > >> + MdeModulePkg/MdeModulePkg.dec > >> + EmbeddedPkg/EmbeddedPkg.dec > >> + OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec > >> + OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec > > > > Please sort packages alphabetically. > > > >> + > >> +[Depex] > >> + TRUE > >> -- > >> 2.7.4 > >>
On 10 February 2017 at 13:32, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Fri, Feb 10, 2017 at 11:45:20AM +0000, Ard Biesheuvel wrote: >> >> > On 10 Feb 2017, at 11:37, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > >> >> On Thu, Feb 09, 2017 at 10:11:48PM +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 | 266 +++++++++++++++++++++ >> >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ >> >> 2 files changed, 312 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..60ad4d6 >> >> --- /dev/null >> >> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c >> >> @@ -0,0 +1,266 @@ >> >> +/** @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 >> >> + >> >> +STATIC EMBEDDED_GPIO *mGpio; >> >> + >> >> +STATIC >> >> +VOID >> >> +HiKeyDetectUsbModeInit ( >> >> + IN VOID >> >> + ) >> >> +{ >> >> + EFI_STATUS Status; >> >> + >> >> + /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */ >> >> + MmioWrite32 (0xf8001864, 1); >> >> + MmioWrite32 (0xf8001868, 1); >> > >> > No magic values please, add #defines. >> > >> >> + >> >> + Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio); >> >> + ASSERT_EFI_ERROR (Status); >> > >> > This should unconditionally print an error and return if failed. >> > Also, it should probably return error and be handled at the call site, >> > instead of continuing as if nothing had happened. >> >> If this driver has a depex on this protocol, it is reasonable (and >> idiomatic in edk2) to use this pattern imo > > Right, I didn't notice that aspect. > On the basis that this is an "impossible" error? > Yes, but only if you declare the dependency in the [Depex], which this driver does not do. The DXE core will only dispatch your driver if the protocol is installed, so your firmware is *badly* broken if you end up here and the protocol is not here. > I still don't feel great about _not_ returning before dereferencing a > NULL pointer, and I see both ways of dealing with this in edk2. > > But I think if the code stays as it is, there should at least be a > comment added - I shouldn't need to refer to other files in order to > determine whether a condition is impossible or not. > I agree that it makes sense to annotate a LocateProtocol() call as involving a depex'ed upon protocol.
On 10 February 2017 at 19:37, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Feb 09, 2017 at 10:11:48PM +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 | 266 +++++++++++++++++++++ >> .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ >> 2 files changed, 312 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..60ad4d6 >> --- /dev/null >> +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetProduct ( >> + OUT CHAR16 *Product, >> + OUT UINT8 *Length >> + ) >> +{ >> + UINTN VariableSize; >> + CHAR16 DataUnicode[PRODUCT_STRING_LENGTH]; >> + >> + if ((Product == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + VariableSize = PRODUCT_STRING_LENGTH * sizeof (CHAR16); >> + ZeroMem (DataUnicode, PRODUCT_STRING_LENGTH * sizeof(CHAR16)); >> + StrCpy (DataUnicode, L"HiKey"); >> + CopyMem (Product, DataUnicode, VariableSize); >> + *Length = VariableSize; >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +HiKeyUsbGetSerialNo ( >> + OUT CHAR16 *SerialNo, >> + OUT UINT8 *Length >> + ) >> +{ >> + UINTN VariableSize; >> + CHAR16 DataUnicode[SERIAL_STRING_LENGTH]; >> + >> + if ((SerialNo == NULL) || (Length == NULL)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + VariableSize = SERIAL_STRING_LENGTH * sizeof (CHAR16); >> + ZeroMem (DataUnicode, SERIAL_STRING_LENGTH * sizeof(CHAR16)); >> + StrCpy (DataUnicode, L"0123456789abcdef"); > > A hardcoded serial number will make you no friends in validation labs. > I would suggest that as a start, this fake serial number is broken out > to be retreived from a helper function (which for now can return only > a made up one). Then at a later date, you can figure out how to > provide it with some data that lets you actually tell different > devices apart. > It's simplified at here. I'll add code to fill custom serial number later. For this patch, it's used to make fastboot app working.
diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c new file mode 100644 index 0000000..60ad4d6 --- /dev/null +++ b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c @@ -0,0 +1,266 @@ +/** @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 + +STATIC EMBEDDED_GPIO *mGpio; + +STATIC +VOID +HiKeyDetectUsbModeInit ( + IN VOID + ) +{ + EFI_STATUS Status; + + /* set pullup on both GPIO2_5 & GPIO2_6. It's required for inupt. */ + MmioWrite32 (0xf8001864, 1); + MmioWrite32 (0xf8001868, 1); + + Status = gBS->LocateProtocol (&gEmbeddedGpioProtocolGuid, NULL, (VOID **)&mGpio); + ASSERT_EFI_ERROR (Status); + Status = mGpio->Set (mGpio, USB_SEL_GPIO0_3, GPIO_MODE_OUTPUT_0); + ASSERT_EFI_ERROR (Status); + Status = mGpio->Set (mGpio, USB_5V_HUB_EN, GPIO_MODE_OUTPUT_0); + ASSERT_EFI_ERROR (Status); + MicroSecondDelay (1000); + + 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); +} + +UINTN +HiKeyGetUsbMode ( + IN VOID + ) +{ +#if 0 + EFI_STATUS Status; + UINTN GpioId, GpioVbus; + UINTN Value; + + Status = mGpio->Get (mGpio, USB_ID_DET_GPIO2_5, &Value); + ASSERT_EFI_ERROR (Status); + GpioId = Value; + Status = mGpio->Get (mGpio, USB_VBUS_DET_GPIO2_6, &Value); + ASSERT_EFI_ERROR (Status); + GpioVbus = Value; + +DEBUG ((DEBUG_ERROR, "#%a, %d, GpioId:%d, GpioVbus:%d\n", __func__, __LINE__, GpioId, GpioVbus)); + if ((GpioId == 1) && (GpioVbus == 0)) { + return USB_DEVICE_MODE; + } else if ((GpioId == 0) && (GpioVbus == 1)) { + return USB_CABLE_NOT_ATTACHED; + } + return USB_HOST_MODE; +#else + return USB_DEVICE_MODE; +#endif +} + +EFI_STATUS +HiKeyUsbPhyInit ( + IN UINT8 Mode + ) +{ + UINTN Value; + UINT32 Data; + + HiKeyDetectUsbModeInit (); + + //setup clock + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN0, BIT4); + do { + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CLKSTAT0); + } while ((Value & BIT4) == 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); + MicroSecondDelay (1000); + + //If Mode = 1, USB in Device Mode + //If Mode = 0, USB in Host Mode + if (Mode == USB_DEVICE_MODE) { + if (HiKeyGetUsbMode () == USB_DEVICE_MODE) { + DEBUG ((DEBUG_ERROR, "usb work as device mode.\n")); + } else { + return EFI_INVALID_PARAMETER; + } + + Value = MmioRead32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5); + Value &= ~CTRL5_PICOPHY_BC_MODE; + MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CTRL5, Value); + MicroSecondDelay (20000); + } else { + if (HiKeyGetUsbMode () == USB_HOST_MODE) { + DEBUG ((DEBUG_ERROR, "usb work as host mode.\n")); + } else { + return EFI_INVALID_PARAMETER; + } + + /*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); + MicroSecondDelay (20000); + MmioWrite32 (PERI_CTRL_BASE + 0x018, 0x70533483); //EYE_PATTERN + + 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] = 0x409; + *Length = sizeof (CHAR16); + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +HiKeyUsbGetManuFacturer ( + OUT CHAR16 *ManuFacturer, + OUT UINT8 *Length + ) +{ + UINTN VariableSize; + CHAR16 DataUnicode[MANU_FACTURER_STRING_LENGTH]; + + if ((ManuFacturer == NULL) || (Length == NULL)) { + return EFI_INVALID_PARAMETER; + } + VariableSize = MANU_FACTURER_STRING_LENGTH * sizeof (CHAR16); + ZeroMem (DataUnicode, MANU_FACTURER_STRING_LENGTH * sizeof(CHAR16)); + StrCpy (DataUnicode, L"96Boards"); + CopyMem (ManuFacturer, DataUnicode, VariableSize); + *Length = VariableSize; + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +HiKeyUsbGetProduct ( + OUT CHAR16 *Product, + OUT UINT8 *Length + ) +{ + UINTN VariableSize; + CHAR16 DataUnicode[PRODUCT_STRING_LENGTH]; + + if ((Product == NULL) || (Length == NULL)) { + return EFI_INVALID_PARAMETER; + } + VariableSize = PRODUCT_STRING_LENGTH * sizeof (CHAR16); + ZeroMem (DataUnicode, PRODUCT_STRING_LENGTH * sizeof(CHAR16)); + StrCpy (DataUnicode, L"HiKey"); + CopyMem (Product, DataUnicode, VariableSize); + *Length = VariableSize; + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +HiKeyUsbGetSerialNo ( + OUT CHAR16 *SerialNo, + OUT UINT8 *Length + ) +{ + UINTN VariableSize; + CHAR16 DataUnicode[SERIAL_STRING_LENGTH]; + + if ((SerialNo == NULL) || (Length == NULL)) { + return EFI_INVALID_PARAMETER; + } + VariableSize = SERIAL_STRING_LENGTH * sizeof (CHAR16); + ZeroMem (DataUnicode, SERIAL_STRING_LENGTH * sizeof(CHAR16)); + StrCpy (DataUnicode, L"0123456789abcdef"); + CopyMem (SerialNo, DataUnicode, VariableSize); + *Length = VariableSize; + 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 + ); + if (EFI_ERROR (Status)) { + return Status; + } + return Status; +} diff --git a/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf b/Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf new file mode 100644 index 0000000..1cb70c9 --- /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] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + EmbeddedPkg/EmbeddedPkg.dec + OpenPlatformPkg/Drivers/Usb/DwUsbDxe/DwUsbDxe.dec + OpenPlatformPkg/Platforms/Hisilicon/HiKey/HiKey.dec + +[Depex] + TRUE
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 | 266 +++++++++++++++++++++ .../Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf | 46 ++++ 2 files changed, 312 insertions(+) create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.c create mode 100644 Platforms/Hisilicon/HiKey/HiKeyUsbDxe/HiKeyUsbDxe.inf