diff mbox series

[edk2,v2,edk-platforms,2/4] Platform/Hisilicon/HiKey960: enable virtual keyboard

Message ID 1520515804-29577-1-git-send-email-haojian.zhuang@linaro.org
State Superseded
Headers show
Series enable virtual keyboards on hikey | expand

Commit Message

Haojian Zhuang March 8, 2018, 1:30 p.m. UTC
Enable virtual keyboard on HiKey960 platform. The platform
driver read pattern from memory or GPIO pin. When the value
is matched, it simulates a hotkey that is used to adjust
sequence of boot options.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

---
 Platform/Hisilicon/HiKey960/HiKey960.dsc                |  10 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf                |   7 +
 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  54 +++
 Silicon/Hisilicon/Hi3660/Include/Hi3660.h               | 147 ++++++
 Silicon/Hisilicon/Hi3660/Include/Hkadc.h                |  66 +++
 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 ++++++++++++++++++++
 6 files changed, 775 insertions(+)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm May 2, 2018, 10:49 p.m. UTC | #1
On Thu, Mar 08, 2018 at 09:30:04PM +0800, Haojian Zhuang wrote:
> Enable virtual keyboard on HiKey960 platform. The platform

> driver read pattern from memory or GPIO pin. When the value

> is matched, it simulates a hotkey that is used to adjust

> sequence of boot options.


This patch looks like it contains an awful lot more than described
here. Can it be split up?

> Cc: Leif Lindholm <leif.lindholm@linaro.org>

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> ---

>  Platform/Hisilicon/HiKey960/HiKey960.dsc                |  10 +

>  Platform/Hisilicon/HiKey960/HiKey960.fdf                |   7 +

>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  54 +++

>  Silicon/Hisilicon/Hi3660/Include/Hi3660.h               | 147 ++++++

>  Silicon/Hisilicon/Hi3660/Include/Hkadc.h                |  66 +++

>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 ++++++++++++++++++++

>  6 files changed, 775 insertions(+)

> 

> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc

> index 3da1b8556321..859ab84f8415 100644

> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc

> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc

> @@ -68,6 +68,9 @@ [LibraryClasses.common.SEC]

>    PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf

>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf

>  

> +[BuildOptions]

> +  GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi3660/Include


Please add a Hi3660/Hi3660.dec instead, like for Hi6220.

> +

>  ################################################################################

>  #

>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform

> @@ -183,6 +186,13 @@ [Components.common]

>    ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf

>  

>    #

> +  # Virtual Keyboard

> +  #

> +  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf

> +

> +  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf

> +

> +  #

>    # USB Host Support

>    #

>    MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf

> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf

> index 162dbaaf2646..d65f77878575 100644

> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf

> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf

> @@ -124,6 +124,13 @@ [FV.FvMain]

>    INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf

>  

>    #

> +  # Virtual Keyboard

> +  #

> +  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf

> +

> +  INF Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf

> +

> +  #

>    # USB Host Support

>    #

>    INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf

> diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf

> new file mode 100644

> index 000000000000..cc517656b340

> --- /dev/null

> +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf

> @@ -0,0 +1,54 @@

> +#

> +#  Copyright (c) 2018, Linaro Limited. 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


0x0001001a

> +  BASE_NAME                      = HiKey960Dxe

> +  FILE_GUID                      = 6d824b2c-640e-4643-b9f2-9c09e8bff429

> +  MODULE_TYPE                    = DXE_DRIVER

> +  VERSION_STRING                 = 1.0

> +  ENTRY_POINT                    = HiKey960EntryPoint

> +

> +[Sources.common]

> +  HiKey960Dxe.c

> +

> +[Packages]

> +  EmbeddedPkg/EmbeddedPkg.dec

> +  MdePkg/MdePkg.dec

> +  MdeModulePkg/MdeModulePkg.dec


MdeM... before MdeP...

> +

> +[LibraryClasses]

> +  BaseMemoryLib

> +  CacheMaintenanceLib

> +  DebugLib

> +  DxeServicesTableLib

> +  FdtLib

> +  IoLib

> +  PcdLib

> +  PrintLib

> +  SerialPortLib

> +  TimerLib

> +  UefiBootServicesTableLib

> +  UefiDriverEntryPoint

> +  UefiLib

> +  UefiRuntimeServicesTableLib


Are all of these really used?

> +

> +[Protocols]

> +  gEmbeddedGpioProtocolGuid

> +  gPlatformVirtualKeyboardProtocolGuid

> +

> +[Guids]

> +  gEfiEndOfDxeEventGroupGuid

> +  gEfiFileInfoGuid


I don't see gEfiFileInfoGuid referenced anywhere else in this patch?

> +

> +[Depex]

> +  TRUE

> diff --git a/Silicon/Hisilicon/Hi3660/Include/Hi3660.h b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h

> new file mode 100644

> index 000000000000..f3ce12f64ed5

> --- /dev/null

> +++ b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h

> @@ -0,0 +1,147 @@

> +/** @file

> +*

> +*  Copyright (c) 2018, Linaro Ltd. 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 __HI3660_H__

> +#define __HI3660_H__

> +

> +#define HKADC_SSI_REG_BASE                      0xE82B8000

> +

> +#define PCTRL_REG_BASE                          0xE8A09000

> +

> +#define PCTRL_CTRL3                             (PCTRL_REG_BASE + 0x010)

> +#define PCTRL_CTRL24                            (PCTRL_REG_BASE + 0x064)

> +

> +#define PCTRL_CTRL3_USB_TXCO_EN                 (1 << 1)

> +#define PCTRL_CTRL24_USB3PHY_3MUX1_SEL          (1 << 25)

> +

> +#define SCTRL_REG_BASE                          0xFFF0A000

> +

> +#define SCTRL_SCFPLLCTRL0                       (SCTRL_REG_BASE + 0x120)

> +#define SCTRL_SCFPLLCTRL0_FPLL0_EN              (1 << 0)

> +

> +#define SCTRL_BAK_DATA0                         (SCTRL_REG_BASE + 0x40C)

> +

> +#define USB3OTG_BC_REG_BASE                     0xFF200000

> +

> +#define USB3OTG_CTRL0                           (USB3OTG_BC_REG_BASE + 0x000)

> +#define USB3OTG_CTRL2                           (USB3OTG_BC_REG_BASE + 0x008)

> +#define USB3OTG_CTRL3                           (USB3OTG_BC_REG_BASE + 0x00C)

> +#define USB3OTG_CTRL4                           (USB3OTG_BC_REG_BASE + 0x010)

> +#define USB3OTG_CTRL6                           (USB3OTG_BC_REG_BASE + 0x018)

> +#define USB3OTG_CTRL7                           (USB3OTG_BC_REG_BASE + 0x01C)

> +#define USB3OTG_PHY_CR_STS                      (USB3OTG_BC_REG_BASE + 0x050)

> +#define USB3OTG_PHY_CR_CTRL                     (USB3OTG_BC_REG_BASE + 0x054)

> +

> +#define USB3OTG_CTRL0_SC_USB3PHY_ABB_GT_EN      (1 << 15)

> +#define USB3OTG_CTRL2_TEST_POWERDOWN_SSP        (1 << 1)

> +#define USB3OTG_CTRL2_TEST_POWERDOWN_HSP        (1 << 0)

> +#define USB3OTG_CTRL3_VBUSVLDEXT                (1 << 6)

> +#define USB3OTG_CTRL3_VBUSVLDEXTSEL             (1 << 5)

> +#define USB3OTG_CTRL7_REF_SSP_EN                (1 << 16)

> +#define USB3OTG_PHY_CR_DATA_OUT(x)              (((x) & 0xFFFF) << 1)

> +#define USB3OTG_PHY_CR_ACK                      (1 << 0)

> +#define USB3OTG_PHY_CR_DATA_IN(x)               (((x) & 0xFFFF) << 4)

> +#define USB3OTG_PHY_CR_WRITE                    (1 << 3)

> +#define USB3OTG_PHY_CR_READ                     (1 << 2)

> +#define USB3OTG_PHY_CR_CAP_DATA                 (1 << 1)

> +#define USB3OTG_PHY_CR_CAP_ADDR                 (1 << 0)

> +

> +#define PMU_REG_BASE                            0xFFF34000

> +#define PMIC_HARDWARE_CTRL0                     (PMU_REG_BASE + (0x0C5 << 2))

> +#define PMIC_OSC32K_ONOFF_CTRL                  (PMU_REG_BASE + (0x0CC << 2))

> +

> +#define PMIC_HARDWARE_CTRL0_WIFI_CLK            (1 << 5)

> +#define PMIC_OSC32K_ONOFF_CTRL_EN_32K           (1 << 1)

> +

> +

> +#define CRG_REG_BASE                            0xFFF35000

> +

> +#define CRG_PEREN2                              (CRG_REG_BASE + 0x020)

> +#define CRG_PERDIS2                             (CRG_REG_BASE + 0x024)

> +#define CRG_PERCLKEN2                           (CRG_REG_BASE + 0x028)

> +#define CRG_PERSTAT2                            (CRG_REG_BASE + 0x02C)

> +#define CRG_PEREN4                              (CRG_REG_BASE + 0x040)

> +#define CRG_PERDIS4                             (CRG_REG_BASE + 0x044)

> +#define CRG_PERCLKEN4                           (CRG_REG_BASE + 0x048)

> +#define CRG_PERSTAT4                            (CRG_REG_BASE + 0x04C)

> +#define CRG_PERRSTEN2                           (CRG_REG_BASE + 0x078)

> +#define CRG_PERRSTDIS2                          (CRG_REG_BASE + 0x07C)

> +#define CRG_PERRSTSTAT2                         (CRG_REG_BASE + 0x080)

> +#define CRG_PERRSTEN3                           (CRG_REG_BASE + 0x084)

> +#define CRG_PERRSTDIS3                          (CRG_REG_BASE + 0x088)

> +#define CRG_PERRSTSTAT3                         (CRG_REG_BASE + 0x08C)

> +#define CRG_PERRSTEN4                           (CRG_REG_BASE + 0x090)

> +#define CRG_PERRSTDIS4                          (CRG_REG_BASE + 0x094)

> +#define CRG_PERRSTSTAT4                         (CRG_REG_BASE + 0x098)

> +#define CRG_ISOEN                               (CRG_REG_BASE + 0x144)

> +#define CRG_ISODIS                              (CRG_REG_BASE + 0x148)

> +#define CRG_ISOSTAT                             (CRG_REG_BASE + 0x14C)

> +

> +#define PERI_UFS_BIT                            (1 << 12)

> +#define PERI_ARST_UFS_BIT                       (1 << 7)

> +

> +#define PEREN2_HKADCSSI                         BIT24

> +

> +#define PEREN4_GT_ACLK_USB3OTG                  (1 << 1)

> +#define PEREN4_GT_CLK_USB3OTG_REF               (1 << 0)

> +

> +#define PERRSTEN2_HKADCSSI                      BIT24

> +

> +#define PERRSTEN4_USB3OTG_MUX                   (1 << 8)

> +#define PERRSTEN4_USB3OTG_AHBIF                 (1 << 7)

> +#define PERRSTEN4_USB3OTG_32K                   (1 << 6)

> +#define PERRSTEN4_USB3OTG                       (1 << 5)

> +#define PERRSTEN4_USB3OTGPHY_POR                (1 << 3)

> +

> +#define PERISOEN_USB_REFCLK_ISO_EN              (1 << 25)

> +

> +#define CRG_CLKDIV16_OFFSET                     0x0E8

> +#define SC_DIV_UFSPHY_CFG_MASK                  (0x3 << 9)

> +#define SC_DIV_UFSPHY_CFG(x)                    (((x) & 0x3) << 9)

> +

> +#define CRG_CLKDIV17_OFFSET                     0x0EC

> +#define SC_DIV_UFS_PERIBUS                      (1 << 14)

> +

> +#define UFS_SYS_REG_BASE                        0xFF3B1000

> +

> +#define UFS_SYS_PSW_POWER_CTRL_OFFSET           0x004

> +#define UFS_SYS_PHY_ISO_EN_OFFSET               0x008

> +#define UFS_SYS_HC_LP_CTRL_OFFSET               0x00C

> +#define UFS_SYS_PHY_CLK_CTRL_OFFSET             0x010

> +#define UFS_SYS_PSW_CLK_CTRL_OFFSET             0x014

> +#define UFS_SYS_CLOCK_GATE_BYPASS_OFFSET        0x018

> +#define UFS_SYS_RESET_CTRL_EN_OFFSET            0x01C

> +#define UFS_SYS_MONITOR_HH_OFFSET               0x03C

> +#define UFS_SYS_UFS_SYSCTRL_OFFSET              0x05C

> +#define UFS_SYS_UFS_DEVICE_RESET_CTRL_OFFSET    0x060

> +#define UFS_SYS_UFS_APB_ADDR_MASK_OFFSET        0x064

> +

> +#define BIT_UFS_PSW_ISO_CTRL                    (1 << 16)

> +#define BIT_UFS_PSW_MTCMOS_EN                   (1 << 0)

> +#define BIT_UFS_REFCLK_ISO_EN                   (1 << 16)

> +#define BIT_UFS_PHY_ISO_CTRL                    (1 << 0)

> +#define BIT_SYSCTRL_LP_ISOL_EN                  (1 << 16)

> +#define BIT_SYSCTRL_PWR_READY                   (1 << 8)

> +#define BIT_SYSCTRL_REF_CLOCK_EN                (1 << 24)

> +#define MASK_SYSCTRL_REF_CLOCK_SEL              (3 << 8)

> +#define MASK_SYSCTRL_CFG_CLOCK_FREQ             (0xFF)

> +#define BIT_SYSCTRL_PSW_CLK_EN                  (1 << 4)

> +#define MASK_UFS_CLK_GATE_BYPASS                (0x3F)

> +#define BIT_SYSCTRL_LP_RESET_N                  (1 << 0)

> +#define BIT_UFS_REFCLK_SRC_SE1                  (1 << 0)

> +#define MASK_UFS_SYSCTRL_BYPASS                 (0x3F << 16)

> +#define MASK_UFS_DEVICE_RESET                   (1 << 16)

> +#define BIT_UFS_DEVICE_RESET                    (1 << 0)

> +

> +#endif /* __HI3660_H__ */

> diff --git a/Silicon/Hisilicon/Hi3660/Include/Hkadc.h b/Silicon/Hisilicon/Hi3660/Include/Hkadc.h

> new file mode 100644

> index 000000000000..2584256b8726

> --- /dev/null

> +++ b/Silicon/Hisilicon/Hi3660/Include/Hkadc.h


I take it HKADC is HiKey A/D Converter?
I guess ADC is a common enough term to be used abbreviated, but HiKey
should be spelled out. That includes filenames, macros and
variables/function names.

> @@ -0,0 +1,66 @@

> +/** @file

> +*

> +*  Copyright (c) 2018, Linaro Ltd. 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 __HKADC_H__

> +#define __HKADC_H__

> +

> +#include <Hi3660.h>

> +

> +#define HKADC_DSP_START                         (HKADC_SSI_REG_BASE + 0x000)

> +#define HKADC_WR_NUM                            (HKADC_SSI_REG_BASE + 0x008)

> +#define HKADC_DSP_START_CLR                     (HKADC_SSI_REG_BASE + 0x01C)

> +#define HKADC_WR01_DATA                         (HKADC_SSI_REG_BASE + 0x020)

> +

> +#define WR1_WRITE_MODE                          BIT31

> +#define WR1_READ_MODE                           (0 << 31)

> +#define WR1_ADDR(x)                             (((x) & 0x7F) << 24)

> +#define WR1_DATA(x)                             (((x) & 0xFF) << 16)

> +#define WR0_WRITE_MODE                          BIT15

> +#define WR0_READ_MODE                           (0 << 15)

> +#define WR0_ADDR(x)                             (((x) & 0x7F) << 8)

> +#define WR0_DATA(x)                             ((x) & 0xFF)

> +

> +#define HKADC_WR23_DATA                         (HKADC_SSI_REG_BASE + 0x024)

> +#define HKADC_WR45_DATA                         (HKADC_SSI_REG_BASE + 0x028)

> +#define HKADC_DELAY01                           (HKADC_SSI_REG_BASE + 0x030)

> +#define HKADC_DELAY23                           (HKADC_SSI_REG_BASE + 0x034)

> +#define HKADC_DELAY45                           (HKADC_SSI_REG_BASE + 0x038)

> +#define HKADC_DSP_RD2_DATA                      (HKADC_SSI_REG_BASE + 0x048)

> +#define HKADC_DSP_RD3_DATA                      (HKADC_SSI_REG_BASE + 0x04C)

> +

> +// HKADC Internal Registers

> +#define HKADC_CTRL_ADDR                         0x00

> +#define HKADC_START_ADDR                        0x01

> +#define HKADC_DATA1_ADDR                        0x03   // high 8 bits

> +#define HKADC_DATA0_ADDR                        0x04   // low 8 bits

> +#define HKADC_MODE_CFG                          0x0A

> +

> +#define HKADC_VALUE_HIGH                        0x0FF0

> +#define HKADC_VALUE_LOW                         0x000F

> +#define HKADC_VALID_VALUE                       0x0FFF

> +

> +#define HKADC_CHANNEL_MAX                       15

> +#define HKADC_VREF_1V8                          1800

> +#define HKADC_ACCURACY                          0x0FFF

> +

> +#define HKADC_WR01_VALUE                        ((HKADC_START_ADDR << 24) | (0x1 << 16))

> +#define HKADC_WR23_VALUE                        ((0x1 << 31) | (HKADC_DATA0_ADDR << 24) | (1 << 15) | (HKADC_DATA1_ADDR << 8))

> +#define HKADC_WR45_VALUE                        (0x80)

> +#define HKADC_CHANNEL0_DELAY01_VALUE            ((0x0700 << 16) | 0xFFFF)

> +#define HKADC_DELAY01_VALUE                     ((0x0700 << 16) | 0x0200)

> +#define HKADC_DELAY23_VALUE                     ((0x00C8 << 16) | 0x00C8)

> +#define START_DELAY_TIMEOUT                     2000

> +#define HKADC_WR_NUM_VALUE                      4

> +

> +#endif /* __HKADC_H__ */

> diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c

> new file mode 100644

> index 000000000000..473d61ed384e

> --- /dev/null

> +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c

> @@ -0,0 +1,491 @@

> +/** @file

> +*

> +*  Copyright (c) 2018, Linaro Ltd. 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 <Guid/EventGroup.h>

> +

> +#include <Hi3660.h>

> +#include <Hkadc.h>

> +#include <libfdt.h>

> +

> +#include <Library/BaseLib.h>

> +#include <Library/BaseMemoryLib.h>

> +#include <Library/CacheMaintenanceLib.h>

> +#include <Library/DebugLib.h>

> +#include <Library/DevicePathLib.h>

> +#include <Library/MemoryAllocationLib.h>

> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>

> +#include <Library/IoLib.h>

> +#include <Library/PcdLib.h>

> +#include <Library/PrintLib.h>

> +#include <Library/SerialPortLib.h>

> +#include <Library/TimerLib.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include <Library/UefiRuntimeServicesTableLib.h>


Are all of these really used?

> +

> +#include <Protocol/EmbeddedGpio.h>

> +#include <Protocol/PlatformVirtualKeyboard.h>

> +

> +#define ADC_ADCIN0                       0

> +#define ADC_ADCIN1                       1

> +#define ADC_ADCIN2                       2

> +

> +#define HKADC_DATA_GRADE0                0

> +#define HKADC_DATA_GRADE1                100

> +#define HKADC_DATA_GRADE2                300

> +#define HKADC_DATA_GRADE3                500

> +#define HKADC_DATA_GRADE4                700

> +#define HKADC_DATA_GRADE5                900

> +#define HKADC_DATA_GRADE6                1100

> +#define HKADC_DATA_GRADE7                1300

> +#define HKADC_DATA_GRADE8                1500

> +#define HKADC_DATA_GRADE9                1700

> +#define HKADC_DATA_GRADE10               1800

> +

> +#define BOARDID_VALUE0                   0

> +#define BOARDID_VALUE1                   1

> +#define BOARDID_VALUE2                   2

> +#define BOARDID_VALUE3                   3

> +#define BOARDID_VALUE4                   4

> +#define BOARDID_VALUE5                   5

> +#define BOARDID_VALUE6                   6

> +#define BOARDID_VALUE7                   7

> +#define BOARDID_VALUE8                   8

> +#define BOARDID_VALUE9                   9

> +#define BOARDID_UNKNOW                   0xF


           BOARDID_UNKNOWN

> +

> +#define BOARDID3_BASE                    5

> +

> +#define HIKEY960_BOARDID_V1              5300

> +#define HIKEY960_BOARDID_V2              5301

> +

> +#define HIKEY960_COMPATIBLE_LEDS_V1      "gpio-leds_v1"

> +#define HIKEY960_COMPATIBLE_LEDS_V2      "gpio-leds_v2"

> +#define HIKEY960_COMPATIBLE_HUB_V1       "hisilicon,gpio_hubv1"

> +#define HIKEY960_COMPATIBLE_HUB_V2       "hisilicon,gpio_hubv2"

> +

> +#define SERIAL_NUMBER_SIZE               17

> +#define SERIAL_NUMBER_BLOCK_SIZE         EFI_PAGE_SIZE

> +#define SERIAL_NUMBER_LBA                20

> +#define RANDOM_MAX                       0x7FFFFFFFFFFFFFFF


Use MAX_INTN?

> +#define RANDOM_MAGIC                     0x9A4DBEAF

> +

> +#define ADB_REBOOT_ADDRESS               0x32100000

> +#define ADB_REBOOT_BOOTLOADER            0x77665500

> +#define ADB_REBOOT_NONE                  0x77665501

> +

> +#define DETECT_SW_FASTBOOT               68        // GPIO8_4

> +

> +typedef struct {

> +  UINT64        Magic;

> +  UINT64        Data;

> +  CHAR16        UnicodeSN[SERIAL_NUMBER_SIZE];

> +} RANDOM_SERIAL_NUMBER;

> +

> +enum {

> +  BOOT_MODE_RECOVERY  = 0,

> +  BOOT_MODE_NORMAL,

> +  BOOT_MODE_MASK = 1,

> +};


Please move all of the above #defines, typedefs and enums to a local
HiKey960Dxe.h.

> +

> +STATIC UINTN    mBoardId;

> +

> +STATIC EMBEDDED_GPIO   *mGpio;

> +

> +STATIC

> +VOID

> +InitAdc (

> +  VOID

> +  )

> +{

> +  // reset hkadc

> +  MmioWrite32 (CRG_PERRSTEN2, PERRSTEN2_HKADCSSI);

> +  // wait a few clock cycles

> +  MicroSecondDelay (2);

> +  MmioWrite32 (CRG_PERRSTDIS2, PERRSTEN2_HKADCSSI);

> +  MicroSecondDelay (2);

> +  // enable hkadc clock

> +  MmioWrite32 (CRG_PERDIS2, PEREN2_HKADCSSI);

> +  MicroSecondDelay (2);

> +  MmioWrite32 (CRG_PEREN2, PEREN2_HKADCSSI);

> +  MicroSecondDelay (2);


Why the need for a few clock cycles? Why was 2 chosen? Barriers needed?

> +}

> +

> +STATIC

> +EFI_STATUS

> +AdcGetAdc (

> +  IN  UINTN         Channel,

> +  OUT UINTN         *Value

> +  )

> +{

> +  UINT32            Data;

> +  UINT16            Value1, Value0;

> +

> +  if (Channel > HKADC_CHANNEL_MAX) {

> +    DEBUG ((DEBUG_ERROR, "invalid channel:%d\n", Channel));

> +    return EFI_OUT_OF_RESOURCES;

> +  }

> +  // configure the read/write operation for external HKADC

> +  MmioWrite32 (HKADC_WR01_DATA, HKADC_WR01_VALUE | Channel);

> +  MmioWrite32 (HKADC_WR23_DATA, HKADC_WR23_VALUE);

> +  MmioWrite32 (HKADC_WR45_DATA, HKADC_WR45_VALUE);

> +  // configure the number of accessing registers

> +  MmioWrite32 (HKADC_WR_NUM, HKADC_WR_NUM_VALUE);

> +  // configure delay of accessing registers

> +  MmioWrite32 (HKADC_DELAY01, HKADC_CHANNEL0_DELAY01_VALUE);

> +  MmioWrite32 (HKADC_DELAY23, HKADC_DELAY23_VALUE);

> +

> +  // start HKADC

> +  MmioWrite32 (HKADC_DSP_START, 1);

> +  do {

> +    Data = MmioRead32 (HKADC_DSP_START);

> +  } while (Data & 1);


What is this hardcoded 1?
Can it be replaced by a #define?

> +

> +  // convert AD result

> +  Value1 = (UINT16)MmioRead32 (HKADC_DSP_RD2_DATA);

> +  Value0 = (UINT16)MmioRead32 (HKADC_DSP_RD3_DATA);

> +

> +  Data = ((Value1 << 4) & HKADC_VALUE_HIGH) | ((Value0 >> 4) & HKADC_VALUE_LOW);


The operation is clear enough, but the need for it deserves a comment.

> +  *Value = Data;

> +  return EFI_SUCCESS;

> +}

> +

> +STATIC

> +EFI_STATUS

> +AdcGetValue (

> +  IN UINTN         Channel,

> +  IN OUT UINTN     *Value

> +  )

> +{

> +  EFI_STATUS       Status;

> +  UINTN            Result;

> +

> +  Status = AdcGetAdc (Channel, Value);

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  // convert ADC value to micro-volt

> +  Result = ((*Value & HKADC_VALID_VALUE) * HKADC_VREF_1V8) / HKADC_ACCURACY;

> +  *Value = Result;

> +  return EFI_SUCCESS;

> +}

> +

> +STATIC

> +UINTN

> +AdcinDataRemap (

> +  IN UINTN           AdcinValue

> +  )

> +{

> +  UINTN              Result;

> +

> +  if (AdcinValue < HKADC_DATA_GRADE0) {


This checks if a UINTN is < 0.

> +    Result = BOARDID_UNKNOW;

> +  } else if (AdcinValue < HKADC_DATA_GRADE1) {

> +    Result = BOARDID_VALUE0;

> +  } else if (AdcinValue < HKADC_DATA_GRADE2) {

> +    Result = BOARDID_VALUE1;

> +  } else if (AdcinValue < HKADC_DATA_GRADE3) {

> +    Result = BOARDID_VALUE2;

> +  } else if (AdcinValue < HKADC_DATA_GRADE4) {

> +    Result = BOARDID_VALUE3;

> +  } else if (AdcinValue < HKADC_DATA_GRADE5) {

> +    Result = BOARDID_VALUE4;

> +  } else if (AdcinValue < HKADC_DATA_GRADE6) {

> +    Result = BOARDID_VALUE5;

> +  } else if (AdcinValue < HKADC_DATA_GRADE7) {

> +    Result = BOARDID_VALUE6;

> +  } else if (AdcinValue < HKADC_DATA_GRADE8) {

> +    Result = BOARDID_VALUE7;

> +  } else if (AdcinValue < HKADC_DATA_GRADE9) {

> +    Result = BOARDID_VALUE8;

> +  } else if (AdcinValue < HKADC_DATA_GRADE10) {

> +    Result = BOARDID_VALUE9;

> +  } else {

> +    Result = BOARDID_UNKNOW;

> +  }


This would be a lot more readable as a loop iterating over a static
struct.

> +  return Result;

> +}

> +

> +STATIC

> +EFI_STATUS

> +InitBoardId (

> +  OUT UINTN          *Id

> +  )

> +{

> +  UINTN              Adcin0, Adcin1, Adcin2;

> +  UINTN              Adcin0Remap, Adcin1Remap, Adcin2Remap;

> +

> +  InitAdc ();

> +

> +  // read ADC channel0 data

> +  AdcGetValue (ADC_ADCIN0, &Adcin0);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin0:%d\n", Adcin0));

> +  Adcin0Remap = AdcinDataRemap (Adcin0);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin0Remap:%d\n", Adcin0Remap));

> +  if (Adcin0Remap == BOARDID_UNKNOW) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +  // read ADC channel1 data

> +  AdcGetValue (ADC_ADCIN1, &Adcin1);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin1:%d\n", Adcin1));

> +  Adcin1Remap = AdcinDataRemap (Adcin1);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin1Remap:%d\n", Adcin1Remap));

> +  if (Adcin1Remap == BOARDID_UNKNOW) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +  // read ADC channel2 data

> +  AdcGetValue (ADC_ADCIN2, &Adcin2);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2:%d\n", Adcin2));

> +  Adcin2Remap = AdcinDataRemap (Adcin2);

> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2Remap:%d\n", Adcin2Remap));

> +  if (Adcin2Remap == BOARDID_UNKNOW) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +  *Id = BOARDID3_BASE * 1000 + (Adcin2Remap * 100) + (Adcin1Remap * 10) + Adcin0Remap;


1) Please use parentheses consistently.
2) This is the only use of BOARDID3_BASE - why is it being multiplied?
3) What is the format of this ID? Is it described somewhere or is it
   completely arbitrary?
4) What is the purpose of this Id? This function is used to populate
   mBoardId, but that variable is not used anywhere.

> +  DEBUG ((DEBUG_INFO, "[BDID]boardid: %d\n", *Id));

> +  return EFI_SUCCESS;

> +}

> +

> +STATIC

> +VOID

> +InitSdCard (

> +  IN VOID

> +  )

> +{

> +  UINT32        Data;

> +

> +  // LDO16

> +  Data = MmioRead32 (PMU_REG_BASE + (0x79 << 2)) & 7;

> +  Data |= 6;

> +  MmioWrite32 (PMU_REG_BASE + (0x79 << 2), Data);

> +  MmioOr32 (PMU_REG_BASE + (0x78 << 2), 2);


Please replace all live-coded values with #defines.

> +  MicroSecondDelay (100);


Why this delay?
Does this need a barrier?

> +

> +  // LDO9

> +  Data = MmioRead32 (PMU_REG_BASE + (0x6b << 2)) & 7;

> +  Data |= 5;

> +  MmioWrite32 (PMU_REG_BASE + (0x6b << 2), Data);

> +  MmioOr32 (PMU_REG_BASE + (0x6a << 2), 2);


Please replace all live-coded values with #defines.

> +  MicroSecondDelay (100);


Why this delay?
Does this need a barrier?

> +

> +  // GPIO203

> +  MmioWrite32 (0xfff11000 + (24 << 2), 0); // GPIO function

> +

> +  // SD pinmux

> +  MmioWrite32 (0xff37e000 + 0x0, 1); // SD_CLK

> +  MmioWrite32 (0xff37e000 + 0x4, 1); // SD_CMD

> +  MmioWrite32 (0xff37e000 + 0x8, 1); // SD_DATA0

> +  MmioWrite32 (0xff37e000 + 0xc, 1); // SD_DATA1

> +  MmioWrite32 (0xff37e000 + 0x10, 1); // SD_DATA2

> +  MmioWrite32 (0xff37e000 + 0x14, 1); // SD_DATA3

> +  MmioWrite32 (0xff37e800 + 0x0, 15 << 4); // SD_CLK float with 32mA

> +  MmioWrite32 (0xff37e800 + 0x4, (1 << 0) | (8 << 4)); // SD_CMD

> +  MmioWrite32 (0xff37e800 + 0x8, (1 << 0) | (8 << 4)); // SD_DATA0

> +  MmioWrite32 (0xff37e800 + 0xc, (1 << 0) | (8 << 4)); // SD_DATA1

> +  MmioWrite32 (0xff37e800 + 0x10, (1 << 0) | (8 << 4)); // SD_DATA2

> +  MmioWrite32 (0xff37e800 + 0x14, (1 << 0) | (8 << 4)); // SD_DATA3


Please replace all live-coded values with #defines.

> +

> +  do {

> +    MmioOr32 (CRG_REG_BASE + 0xb8, (1 << 6) | (1 << 6 << 16) | (0 << 4) | (3 << 4 << 16));

> +    Data = MmioRead32 (CRG_REG_BASE + 0xb8);

> +  } while ((Data & ((1 << 6) | (3 << 4))) != ((1 << 6) | (0 << 4)));


Please replace all live-coded values with #defines.

> +

> +  // Unreset SD controller

> +  MmioWrite32 (CRG_PERRSTDIS4, 1 << 18);

> +  do {

> +    Data = MmioRead32 (CRG_PERRSTSTAT4);

> +  } while ((Data & (1 << 18)) == (1 << 18));

> +  // Enable SD controller clock

> +  MmioOr32 (CRG_REG_BASE + 0, 1 << 30);

> +  MmioOr32 (CRG_REG_BASE + 0x40, 1 << 17);

> +  do {

> +    Data = MmioRead32 (CRG_REG_BASE + 0x48);

> +  } while ((Data & (1 << 17)) != (1 << 17));


Please replace all live-coded values with #defines.

> +}

> +

> +VOID

> +InitPeripherals (

> +  IN VOID

> +  )

> +{

> +  // Enable FPLL0

> +  MmioOr32 (SCTRL_SCFPLLCTRL0, SCTRL_SCFPLLCTRL0_FPLL0_EN);

> +

> +  InitSdCard ();

> +

> +  // Enable wifi clock

> +  MmioOr32 (PMIC_HARDWARE_CTRL0, PMIC_HARDWARE_CTRL0_WIFI_CLK);

> +  MmioOr32 (PMIC_OSC32K_ONOFF_CTRL, PMIC_OSC32K_ONOFF_CTRL_EN_32K);

> +}

> +

> +/**

> +  Notification function of the event defined as belonging to the

> +  EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in

> +  the entry point of the driver.

> +

> +  This function is called when an event belonging to the

> +  EFI_END_OF_DXE_EVENT_GROUP_GUID event group is signalled. Such an

> +  event is signalled once at the end of the dispatching of all

> +  drivers (end of the so called DXE phase).

> +

> +  @param[in]  Event    Event declared in the entry point of the driver whose

> +                       notification function is being invoked.

> +  @param[in]  Context  NULL

> +**/

> +STATIC

> +VOID

> +OnEndOfDxe (

> +  IN EFI_EVENT  Event,

> +  IN VOID       *Context

> +  )

> +{

> +  UINT32        BootMode;

> +

> +  BootMode = MmioRead32 (SCTRL_BAK_DATA0) & BOOT_MODE_MASK;

> +  if (BootMode == BOOT_MODE_RECOVERY) {

> +    SerialPortWrite ((UINT8 *)"WARNING: CAN NOT BOOT KERNEL IN RECOVERY MODE!\r\n", 48);

> +    SerialPortWrite ((UINT8 *)"Switch to normal boot mode, then reboot to boot kernel.\r\n", 57);


1) Why this use of SerialPortWrite instead of Print?
2) This cast to (UINT8 *) works by accident. Please don't do this.
3) Please _never_ use hand-coded values for compile-time determined
   (and known) information.

> +  }

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +VirtualKeyboardRegister (

> +  IN VOID

> +  )

> +{

> +  EFI_STATUS           Status;

> +

> +  Status = gBS->LocateProtocol (

> +                  &gEmbeddedGpioProtocolGuid,

> +                  NULL,

> +                  (VOID **) &mGpio

> +                  );

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +  return EFI_SUCCESS;

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +VirtualKeyboardReset (

> +  IN VOID

> +  )

> +{

> +  EFI_STATUS           Status;

> +

> +  if (mGpio == NULL) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +  // Configure GPIO68 as GPIO function

> +  MmioWrite32 (0xe896c108, 0);


Please replace all live-coded values with #defines.

> +  Status = mGpio->Set (mGpio, DETECT_SW_FASTBOOT, GPIO_MODE_INPUT);

> +  return Status;

> +}

> +

> +BOOLEAN

> +EFIAPI

> +VirtualKeyboardQuery (

> +  IN VIRTUAL_KBD_KEY             *VirtualKey

> +  )

> +{

> +  EFI_STATUS           Status;

> +  UINTN                Value = 0;

> +

> +  if ((VirtualKey == NULL) || (mGpio == NULL)) {

> +    return FALSE;

> +  }

> +  if (MmioRead32 (ADB_REBOOT_ADDRESS) == ADB_REBOOT_BOOTLOADER) {

> +    goto Done;

> +  } else {

> +    Status = mGpio->Get (mGpio, DETECT_SW_FASTBOOT, &Value);

> +    if (EFI_ERROR (Status) || (Value != 0)) {


Please replace all live-coded values with #defines.

/
    Leif

> +      return FALSE;

> +    }

> +  }

> +Done:

> +  VirtualKey->Signature = VIRTUAL_KEYBOARD_KEY_SIGNATURE;

> +  VirtualKey->Key.ScanCode = SCAN_NULL;

> +  VirtualKey->Key.UnicodeChar = L'f';

> +  return TRUE;

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +VirtualKeyboardClear (

> +  IN VIRTUAL_KBD_KEY            *VirtualKey

> +  )

> +{

> +  if (VirtualKey == NULL) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +  if (MmioRead32 (ADB_REBOOT_ADDRESS) == ADB_REBOOT_BOOTLOADER) {

> +    MmioWrite32 (ADB_REBOOT_ADDRESS, ADB_REBOOT_NONE);

> +    WriteBackInvalidateDataCacheRange ((VOID *)ADB_REBOOT_ADDRESS, 4);

> +  }

> +  return EFI_SUCCESS;

> +}

> +

> +PLATFORM_VIRTUAL_KBD_PROTOCOL mVirtualKeyboard = {

> +  VirtualKeyboardRegister,

> +  VirtualKeyboardReset,

> +  VirtualKeyboardQuery,

> +  VirtualKeyboardClear

> +};

> +

> +EFI_STATUS

> +EFIAPI

> +HiKey960EntryPoint (

> +  IN EFI_HANDLE         ImageHandle,

> +  IN EFI_SYSTEM_TABLE   *SystemTable

> +  )

> +{

> +  EFI_STATUS            Status;

> +  EFI_EVENT             EndOfDxeEvent;

> +

> +  Status = InitBoardId (&mBoardId);

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  InitPeripherals ();

> +

> +  //

> +  // Create an event belonging to the "gEfiEndOfDxeEventGroupGuid" group.

> +  // The "OnEndOfDxe()" function is declared as the call back function.

> +  // It will be called at the end of the DXE phase when an event of the

> +  // same group is signalled to inform about the end of the DXE phase.

> +  // Install the INSTALL_FDT_PROTOCOL protocol.

> +  //

> +  Status = gBS->CreateEventEx (

> +                  EVT_NOTIFY_SIGNAL,

> +                  TPL_CALLBACK,

> +                  OnEndOfDxe,

> +                  NULL,

> +                  &gEfiEndOfDxeEventGroupGuid,

> +                  &EndOfDxeEvent

> +                  );

> +  if (EFI_ERROR (Status)) {

> +    return Status;

> +  }

> +

> +  Status = gBS->InstallProtocolInterface (

> +                  &ImageHandle,

> +                  &gPlatformVirtualKeyboardProtocolGuid,

> +                  EFI_NATIVE_INTERFACE,

> +                  &mVirtualKeyboard

> +                  );

> +  return Status;

> +}

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Haojian Zhuang May 9, 2018, 10:59 a.m. UTC | #2
On 3 May 2018 at 06:49, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Mar 08, 2018 at 09:30:04PM +0800, Haojian Zhuang wrote:

>> Enable virtual keyboard on HiKey960 platform. The platform

>> driver read pattern from memory or GPIO pin. When the value

>> is matched, it simulates a hotkey that is used to adjust

>> sequence of boot options.

>

> This patch looks like it contains an awful lot more than described

> here. Can it be split up?

>

>> Cc: Leif Lindholm <leif.lindholm@linaro.org>

>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

>> ---

>>  Platform/Hisilicon/HiKey960/HiKey960.dsc                |  10 +

>>  Platform/Hisilicon/HiKey960/HiKey960.fdf                |   7 +

>>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  54 +++

>>  Silicon/Hisilicon/Hi3660/Include/Hi3660.h               | 147 ++++++

>>  Silicon/Hisilicon/Hi3660/Include/Hkadc.h                |  66 +++

>>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 ++++++++++++++++++++

>>  6 files changed, 775 insertions(+)

>>

>> +STATIC

>> +VOID

>> +InitAdc (

>> +  VOID

>> +  )

>> +{

>> +  // reset hkadc

>> +  MmioWrite32 (CRG_PERRSTEN2, PERRSTEN2_HKADCSSI);

>> +  // wait a few clock cycles

>> +  MicroSecondDelay (2);

>> +  MmioWrite32 (CRG_PERRSTDIS2, PERRSTEN2_HKADCSSI);

>> +  MicroSecondDelay (2);

>> +  // enable hkadc clock

>> +  MmioWrite32 (CRG_PERDIS2, PEREN2_HKADCSSI);

>> +  MicroSecondDelay (2);

>> +  MmioWrite32 (CRG_PEREN2, PEREN2_HKADCSSI);

>> +  MicroSecondDelay (2);

>

> Why the need for a few clock cycles? Why was 2 chosen? Barriers needed?

>


Need to wait hardware stabilization. Barriers don't help at here.

>> +  Adcin1Remap = AdcinDataRemap (Adcin1);

>> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin1Remap:%d\n", Adcin1Remap));

>> +  if (Adcin1Remap == BOARDID_UNKNOW) {

>> +    return EFI_INVALID_PARAMETER;

>> +  }

>> +  // read ADC channel2 data

>> +  AdcGetValue (ADC_ADCIN2, &Adcin2);

>> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2:%d\n", Adcin2));

>> +  Adcin2Remap = AdcinDataRemap (Adcin2);

>> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2Remap:%d\n", Adcin2Remap));

>> +  if (Adcin2Remap == BOARDID_UNKNOW) {

>> +    return EFI_INVALID_PARAMETER;

>> +  }

>> +  *Id = BOARDID3_BASE * 1000 + (Adcin2Remap * 100) + (Adcin1Remap * 10) + Adcin0Remap;

>

> 1) Please use parentheses consistently.

> 2) This is the only use of BOARDID3_BASE - why is it being multiplied?


It's used to generate the same prefix of board family.

> 3) What is the format of this ID? Is it described somewhere or is it

>    completely arbitrary?


BoardID is based on the voltage of a resistor. Since the resistor
value is fixed,
the BoardID is same on all HiKey960 platforms.

For example, BoardID v1 is different from BoardID v2. But the prefix is same.

> 4) What is the purpose of this Id? This function is used to populate

>    mBoardId, but that variable is not used anywhere.

>


It'll be used later. It will fill different DTB contents according to
different versions
of the HiKey960 platforms.

>> +  DEBUG ((DEBUG_INFO, "[BDID]boardid: %d\n", *Id));

>> +  return EFI_SUCCESS;

>> +}

>> +

>> +STATIC

>> +VOID

>> +InitSdCard (

>> +  IN VOID

>> +  )

>> +{

>> +  UINT32        Data;

>> +

>> +  // LDO16

>> +  Data = MmioRead32 (PMU_REG_BASE + (0x79 << 2)) & 7;

>> +  Data |= 6;

>> +  MmioWrite32 (PMU_REG_BASE + (0x79 << 2), Data);

>> +  MmioOr32 (PMU_REG_BASE + (0x78 << 2), 2);

>

> Please replace all live-coded values with #defines.


I'll try to replace the hard-coded values as I could. But I can't get everything
with document.

>

>> +  MicroSecondDelay (100);

>

> Why this delay?

> Does this need a barrier?

>


Barrier doesn't help. Just wait regulator stable.

>> +

>> +  // LDO9

>> +  Data = MmioRead32 (PMU_REG_BASE + (0x6b << 2)) & 7;

>> +  Data |= 5;

>> +  MmioWrite32 (PMU_REG_BASE + (0x6b << 2), Data);

>> +  MmioOr32 (PMU_REG_BASE + (0x6a << 2), 2);

>

> Please replace all live-coded values with #defines.

>

>> +  MicroSecondDelay (100);

>

> Why this delay?

> Does this need a barrier?

>


Barrier doesn't help. Just wait regulator stable.

Best Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm May 9, 2018, 1:32 p.m. UTC | #3
On Wed, May 09, 2018 at 06:59:53PM +0800, Haojian Zhuang wrote:
> On 3 May 2018 at 06:49, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Thu, Mar 08, 2018 at 09:30:04PM +0800, Haojian Zhuang wrote:

> >> Enable virtual keyboard on HiKey960 platform. The platform

> >> driver read pattern from memory or GPIO pin. When the value

> >> is matched, it simulates a hotkey that is used to adjust

> >> sequence of boot options.

> >

> > This patch looks like it contains an awful lot more than described

> > here. Can it be split up?

> >

> >> Cc: Leif Lindholm <leif.lindholm@linaro.org>

> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> Contributed-under: TianoCore Contribution Agreement 1.1

> >> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> >> ---

> >>  Platform/Hisilicon/HiKey960/HiKey960.dsc                |  10 +

> >>  Platform/Hisilicon/HiKey960/HiKey960.fdf                |   7 +

> >>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  54 +++

> >>  Silicon/Hisilicon/Hi3660/Include/Hi3660.h               | 147 ++++++

> >>  Silicon/Hisilicon/Hi3660/Include/Hkadc.h                |  66 +++

> >>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 ++++++++++++++++++++

> >>  6 files changed, 775 insertions(+)

> >>

> >> +STATIC

> >> +VOID

> >> +InitAdc (

> >> +  VOID

> >> +  )

> >> +{

> >> +  // reset hkadc

> >> +  MmioWrite32 (CRG_PERRSTEN2, PERRSTEN2_HKADCSSI);

> >> +  // wait a few clock cycles

> >> +  MicroSecondDelay (2);

> >> +  MmioWrite32 (CRG_PERRSTDIS2, PERRSTEN2_HKADCSSI);

> >> +  MicroSecondDelay (2);

> >> +  // enable hkadc clock

> >> +  MmioWrite32 (CRG_PERDIS2, PEREN2_HKADCSSI);

> >> +  MicroSecondDelay (2);

> >> +  MmioWrite32 (CRG_PEREN2, PEREN2_HKADCSSI);

> >> +  MicroSecondDelay (2);

> >

> > Why the need for a few clock cycles? Why was 2 chosen? Barriers needed?

> >

> 

> Need to wait hardware stabilization. Barriers don't help at here.


Understood, but please add a comment to that effect.
One comment for each block of these.

> >> +  Adcin1Remap = AdcinDataRemap (Adcin1);

> >> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin1Remap:%d\n", Adcin1Remap));

> >> +  if (Adcin1Remap == BOARDID_UNKNOW) {

> >> +    return EFI_INVALID_PARAMETER;

> >> +  }

> >> +  // read ADC channel2 data

> >> +  AdcGetValue (ADC_ADCIN2, &Adcin2);

> >> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2:%d\n", Adcin2));

> >> +  Adcin2Remap = AdcinDataRemap (Adcin2);

> >> +  DEBUG ((DEBUG_INFO, "[BDID]Adcin2Remap:%d\n", Adcin2Remap));

> >> +  if (Adcin2Remap == BOARDID_UNKNOW) {

> >> +    return EFI_INVALID_PARAMETER;

> >> +  }

> >> +  *Id = BOARDID3_BASE * 1000 + (Adcin2Remap * 100) + (Adcin1Remap * 10) + Adcin0Remap;

> >

> > 1) Please use parentheses consistently.

> > 2) This is the only use of BOARDID3_BASE - why is it being multiplied?

> 

> It's used to generate the same prefix of board family.

> 

> > 3) What is the format of this ID? Is it described somewhere or is it

> >    completely arbitrary?

> 

> BoardID is based on the voltage of a resistor. Since the resistor

> value is fixed,

> the BoardID is same on all HiKey960 platforms.

> 

> For example, BoardID v1 is different from BoardID v2. But the prefix is same.


Can you add a comment describing this before the assignment to *Id?
I would like to understand how we can ensure analog errors do not
cause misidentification.

And this would be easier with hex values and masks, but if the
algorithm is already defined and used, we could have max/min values
for each, test against them, and return error if invalid values are
found.

Trying to make sense of the code, I also think these should all be
AcdIn... rather than Acdin... (upper case I)
Functions and variables.

> > 4) What is the purpose of this Id? This function is used to populate

> >    mBoardId, but that variable is not used anywhere.

> 

> It'll be used later. It will fill different DTB contents according to

> different versions

> of the HiKey960 platforms.


Then could it be postponed until we get those patches as well, so we
can see its use and not just its definition?

> >> +  DEBUG ((DEBUG_INFO, "[BDID]boardid: %d\n", *Id));

> >> +  return EFI_SUCCESS;

> >> +}

> >> +

> >> +STATIC

> >> +VOID

> >> +InitSdCard (

> >> +  IN VOID

> >> +  )

> >> +{

> >> +  UINT32        Data;

> >> +

> >> +  // LDO16

> >> +  Data = MmioRead32 (PMU_REG_BASE + (0x79 << 2)) & 7;

> >> +  Data |= 6;

> >> +  MmioWrite32 (PMU_REG_BASE + (0x79 << 2), Data);

> >> +  MmioOr32 (PMU_REG_BASE + (0x78 << 2), 2);

> >

> > Please replace all live-coded values with #defines.

> 

> I'll try to replace the hard-coded values as I could. But I can't get everything

> with document.


Thanks.

> >> +  MicroSecondDelay (100);

> >

> > Why this delay?

> > Does this need a barrier?

> 

> Barrier doesn't help. Just wait regulator stable.


Add comment please.

> >> +

> >> +  // LDO9

> >> +  Data = MmioRead32 (PMU_REG_BASE + (0x6b << 2)) & 7;

> >> +  Data |= 5;

> >> +  MmioWrite32 (PMU_REG_BASE + (0x6b << 2), Data);

> >> +  MmioOr32 (PMU_REG_BASE + (0x6a << 2), 2);

> >

> > Please replace all live-coded values with #defines.

> >

> >> +  MicroSecondDelay (100);

> >

> > Why this delay?

> > Does this need a barrier?

> >

> 

> Barrier doesn't help. Just wait regulator stable.


Add comment please.

Regards,

Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 3da1b8556321..859ab84f8415 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -68,6 +68,9 @@  [LibraryClasses.common.SEC]
   PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
   PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
 
+[BuildOptions]
+  GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi3660/Include
+
 ################################################################################
 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
@@ -183,6 +186,13 @@  [Components.common]
   ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
+
+  #
   # USB Host Support
   #
   MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf
index 162dbaaf2646..d65f77878575 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
+++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
@@ -124,6 +124,13 @@  [FV.FvMain]
   INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  INF Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
+
+  #
   # USB Host Support
   #
   INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
new file mode 100644
index 000000000000..cc517656b340
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
@@ -0,0 +1,54 @@ 
+#
+#  Copyright (c) 2018, Linaro Limited. 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                      = HiKey960Dxe
+  FILE_GUID                      = 6d824b2c-640e-4643-b9f2-9c09e8bff429
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = HiKey960EntryPoint
+
+[Sources.common]
+  HiKey960Dxe.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  CacheMaintenanceLib
+  DebugLib
+  DxeServicesTableLib
+  FdtLib
+  IoLib
+  PcdLib
+  PrintLib
+  SerialPortLib
+  TimerLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+  UefiRuntimeServicesTableLib
+
+[Protocols]
+  gEmbeddedGpioProtocolGuid
+  gPlatformVirtualKeyboardProtocolGuid
+
+[Guids]
+  gEfiEndOfDxeEventGroupGuid
+  gEfiFileInfoGuid
+
+[Depex]
+  TRUE
diff --git a/Silicon/Hisilicon/Hi3660/Include/Hi3660.h b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h
new file mode 100644
index 000000000000..f3ce12f64ed5
--- /dev/null
+++ b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h
@@ -0,0 +1,147 @@ 
+/** @file
+*
+*  Copyright (c) 2018, Linaro Ltd. 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 __HI3660_H__
+#define __HI3660_H__
+
+#define HKADC_SSI_REG_BASE                      0xE82B8000
+
+#define PCTRL_REG_BASE                          0xE8A09000
+
+#define PCTRL_CTRL3                             (PCTRL_REG_BASE + 0x010)
+#define PCTRL_CTRL24                            (PCTRL_REG_BASE + 0x064)
+
+#define PCTRL_CTRL3_USB_TXCO_EN                 (1 << 1)
+#define PCTRL_CTRL24_USB3PHY_3MUX1_SEL          (1 << 25)
+
+#define SCTRL_REG_BASE                          0xFFF0A000
+
+#define SCTRL_SCFPLLCTRL0                       (SCTRL_REG_BASE + 0x120)
+#define SCTRL_SCFPLLCTRL0_FPLL0_EN              (1 << 0)
+
+#define SCTRL_BAK_DATA0                         (SCTRL_REG_BASE + 0x40C)
+
+#define USB3OTG_BC_REG_BASE                     0xFF200000
+
+#define USB3OTG_CTRL0                           (USB3OTG_BC_REG_BASE + 0x000)
+#define USB3OTG_CTRL2                           (USB3OTG_BC_REG_BASE + 0x008)
+#define USB3OTG_CTRL3                           (USB3OTG_BC_REG_BASE + 0x00C)
+#define USB3OTG_CTRL4                           (USB3OTG_BC_REG_BASE + 0x010)
+#define USB3OTG_CTRL6                           (USB3OTG_BC_REG_BASE + 0x018)
+#define USB3OTG_CTRL7                           (USB3OTG_BC_REG_BASE + 0x01C)
+#define USB3OTG_PHY_CR_STS                      (USB3OTG_BC_REG_BASE + 0x050)
+#define USB3OTG_PHY_CR_CTRL                     (USB3OTG_BC_REG_BASE + 0x054)
+
+#define USB3OTG_CTRL0_SC_USB3PHY_ABB_GT_EN      (1 << 15)
+#define USB3OTG_CTRL2_TEST_POWERDOWN_SSP        (1 << 1)
+#define USB3OTG_CTRL2_TEST_POWERDOWN_HSP        (1 << 0)
+#define USB3OTG_CTRL3_VBUSVLDEXT                (1 << 6)
+#define USB3OTG_CTRL3_VBUSVLDEXTSEL             (1 << 5)
+#define USB3OTG_CTRL7_REF_SSP_EN                (1 << 16)
+#define USB3OTG_PHY_CR_DATA_OUT(x)              (((x) & 0xFFFF) << 1)
+#define USB3OTG_PHY_CR_ACK                      (1 << 0)
+#define USB3OTG_PHY_CR_DATA_IN(x)               (((x) & 0xFFFF) << 4)
+#define USB3OTG_PHY_CR_WRITE                    (1 << 3)
+#define USB3OTG_PHY_CR_READ                     (1 << 2)
+#define USB3OTG_PHY_CR_CAP_DATA                 (1 << 1)
+#define USB3OTG_PHY_CR_CAP_ADDR                 (1 << 0)
+
+#define PMU_REG_BASE                            0xFFF34000
+#define PMIC_HARDWARE_CTRL0                     (PMU_REG_BASE + (0x0C5 << 2))
+#define PMIC_OSC32K_ONOFF_CTRL                  (PMU_REG_BASE + (0x0CC << 2))
+
+#define PMIC_HARDWARE_CTRL0_WIFI_CLK            (1 << 5)
+#define PMIC_OSC32K_ONOFF_CTRL_EN_32K           (1 << 1)
+
+
+#define CRG_REG_BASE                            0xFFF35000
+
+#define CRG_PEREN2                              (CRG_REG_BASE + 0x020)
+#define CRG_PERDIS2                             (CRG_REG_BASE + 0x024)
+#define CRG_PERCLKEN2                           (CRG_REG_BASE + 0x028)
+#define CRG_PERSTAT2                            (CRG_REG_BASE + 0x02C)
+#define CRG_PEREN4                              (CRG_REG_BASE + 0x040)
+#define CRG_PERDIS4                             (CRG_REG_BASE + 0x044)
+#define CRG_PERCLKEN4                           (CRG_REG_BASE + 0x048)
+#define CRG_PERSTAT4                            (CRG_REG_BASE + 0x04C)
+#define CRG_PERRSTEN2                           (CRG_REG_BASE + 0x078)
+#define CRG_PERRSTDIS2                          (CRG_REG_BASE + 0x07C)
+#define CRG_PERRSTSTAT2                         (CRG_REG_BASE + 0x080)
+#define CRG_PERRSTEN3                           (CRG_REG_BASE + 0x084)
+#define CRG_PERRSTDIS3                          (CRG_REG_BASE + 0x088)
+#define CRG_PERRSTSTAT3                         (CRG_REG_BASE + 0x08C)
+#define CRG_PERRSTEN4                           (CRG_REG_BASE + 0x090)
+#define CRG_PERRSTDIS4                          (CRG_REG_BASE + 0x094)
+#define CRG_PERRSTSTAT4                         (CRG_REG_BASE + 0x098)
+#define CRG_ISOEN                               (CRG_REG_BASE + 0x144)
+#define CRG_ISODIS                              (CRG_REG_BASE + 0x148)
+#define CRG_ISOSTAT                             (CRG_REG_BASE + 0x14C)
+
+#define PERI_UFS_BIT                            (1 << 12)
+#define PERI_ARST_UFS_BIT                       (1 << 7)
+
+#define PEREN2_HKADCSSI                         BIT24
+
+#define PEREN4_GT_ACLK_USB3OTG                  (1 << 1)
+#define PEREN4_GT_CLK_USB3OTG_REF               (1 << 0)
+
+#define PERRSTEN2_HKADCSSI                      BIT24
+
+#define PERRSTEN4_USB3OTG_MUX                   (1 << 8)
+#define PERRSTEN4_USB3OTG_AHBIF                 (1 << 7)
+#define PERRSTEN4_USB3OTG_32K                   (1 << 6)
+#define PERRSTEN4_USB3OTG                       (1 << 5)
+#define PERRSTEN4_USB3OTGPHY_POR                (1 << 3)
+
+#define PERISOEN_USB_REFCLK_ISO_EN              (1 << 25)
+
+#define CRG_CLKDIV16_OFFSET                     0x0E8
+#define SC_DIV_UFSPHY_CFG_MASK                  (0x3 << 9)
+#define SC_DIV_UFSPHY_CFG(x)                    (((x) & 0x3) << 9)
+
+#define CRG_CLKDIV17_OFFSET                     0x0EC
+#define SC_DIV_UFS_PERIBUS                      (1 << 14)
+
+#define UFS_SYS_REG_BASE                        0xFF3B1000
+
+#define UFS_SYS_PSW_POWER_CTRL_OFFSET           0x004
+#define UFS_SYS_PHY_ISO_EN_OFFSET               0x008
+#define UFS_SYS_HC_LP_CTRL_OFFSET               0x00C
+#define UFS_SYS_PHY_CLK_CTRL_OFFSET             0x010
+#define UFS_SYS_PSW_CLK_CTRL_OFFSET             0x014
+#define UFS_SYS_CLOCK_GATE_BYPASS_OFFSET        0x018
+#define UFS_SYS_RESET_CTRL_EN_OFFSET            0x01C
+#define UFS_SYS_MONITOR_HH_OFFSET               0x03C
+#define UFS_SYS_UFS_SYSCTRL_OFFSET              0x05C
+#define UFS_SYS_UFS_DEVICE_RESET_CTRL_OFFSET    0x060
+#define UFS_SYS_UFS_APB_ADDR_MASK_OFFSET        0x064
+
+#define BIT_UFS_PSW_ISO_CTRL                    (1 << 16)
+#define BIT_UFS_PSW_MTCMOS_EN                   (1 << 0)
+#define BIT_UFS_REFCLK_ISO_EN                   (1 << 16)
+#define BIT_UFS_PHY_ISO_CTRL                    (1 << 0)
+#define BIT_SYSCTRL_LP_ISOL_EN                  (1 << 16)
+#define BIT_SYSCTRL_PWR_READY                   (1 << 8)
+#define BIT_SYSCTRL_REF_CLOCK_EN                (1 << 24)
+#define MASK_SYSCTRL_REF_CLOCK_SEL              (3 << 8)
+#define MASK_SYSCTRL_CFG_CLOCK_FREQ             (0xFF)
+#define BIT_SYSCTRL_PSW_CLK_EN                  (1 << 4)
+#define MASK_UFS_CLK_GATE_BYPASS                (0x3F)
+#define BIT_SYSCTRL_LP_RESET_N                  (1 << 0)
+#define BIT_UFS_REFCLK_SRC_SE1                  (1 << 0)
+#define MASK_UFS_SYSCTRL_BYPASS                 (0x3F << 16)
+#define MASK_UFS_DEVICE_RESET                   (1 << 16)
+#define BIT_UFS_DEVICE_RESET                    (1 << 0)
+
+#endif /* __HI3660_H__ */
diff --git a/Silicon/Hisilicon/Hi3660/Include/Hkadc.h b/Silicon/Hisilicon/Hi3660/Include/Hkadc.h
new file mode 100644
index 000000000000..2584256b8726
--- /dev/null
+++ b/Silicon/Hisilicon/Hi3660/Include/Hkadc.h
@@ -0,0 +1,66 @@ 
+/** @file
+*
+*  Copyright (c) 2018, Linaro Ltd. 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 __HKADC_H__
+#define __HKADC_H__
+
+#include <Hi3660.h>
+
+#define HKADC_DSP_START                         (HKADC_SSI_REG_BASE + 0x000)
+#define HKADC_WR_NUM                            (HKADC_SSI_REG_BASE + 0x008)
+#define HKADC_DSP_START_CLR                     (HKADC_SSI_REG_BASE + 0x01C)
+#define HKADC_WR01_DATA                         (HKADC_SSI_REG_BASE + 0x020)
+
+#define WR1_WRITE_MODE                          BIT31
+#define WR1_READ_MODE                           (0 << 31)
+#define WR1_ADDR(x)                             (((x) & 0x7F) << 24)
+#define WR1_DATA(x)                             (((x) & 0xFF) << 16)
+#define WR0_WRITE_MODE                          BIT15
+#define WR0_READ_MODE                           (0 << 15)
+#define WR0_ADDR(x)                             (((x) & 0x7F) << 8)
+#define WR0_DATA(x)                             ((x) & 0xFF)
+
+#define HKADC_WR23_DATA                         (HKADC_SSI_REG_BASE + 0x024)
+#define HKADC_WR45_DATA                         (HKADC_SSI_REG_BASE + 0x028)
+#define HKADC_DELAY01                           (HKADC_SSI_REG_BASE + 0x030)
+#define HKADC_DELAY23                           (HKADC_SSI_REG_BASE + 0x034)
+#define HKADC_DELAY45                           (HKADC_SSI_REG_BASE + 0x038)
+#define HKADC_DSP_RD2_DATA                      (HKADC_SSI_REG_BASE + 0x048)
+#define HKADC_DSP_RD3_DATA                      (HKADC_SSI_REG_BASE + 0x04C)
+
+// HKADC Internal Registers
+#define HKADC_CTRL_ADDR                         0x00
+#define HKADC_START_ADDR                        0x01
+#define HKADC_DATA1_ADDR                        0x03   // high 8 bits
+#define HKADC_DATA0_ADDR                        0x04   // low 8 bits
+#define HKADC_MODE_CFG                          0x0A
+
+#define HKADC_VALUE_HIGH                        0x0FF0
+#define HKADC_VALUE_LOW                         0x000F
+#define HKADC_VALID_VALUE                       0x0FFF
+
+#define HKADC_CHANNEL_MAX                       15
+#define HKADC_VREF_1V8                          1800
+#define HKADC_ACCURACY                          0x0FFF
+
+#define HKADC_WR01_VALUE                        ((HKADC_START_ADDR << 24) | (0x1 << 16))
+#define HKADC_WR23_VALUE                        ((0x1 << 31) | (HKADC_DATA0_ADDR << 24) | (1 << 15) | (HKADC_DATA1_ADDR << 8))
+#define HKADC_WR45_VALUE                        (0x80)
+#define HKADC_CHANNEL0_DELAY01_VALUE            ((0x0700 << 16) | 0xFFFF)
+#define HKADC_DELAY01_VALUE                     ((0x0700 << 16) | 0x0200)
+#define HKADC_DELAY23_VALUE                     ((0x00C8 << 16) | 0x00C8)
+#define START_DELAY_TIMEOUT                     2000
+#define HKADC_WR_NUM_VALUE                      4
+
+#endif /* __HKADC_H__ */
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
new file mode 100644
index 000000000000..473d61ed384e
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
@@ -0,0 +1,491 @@ 
+/** @file
+*
+*  Copyright (c) 2018, Linaro Ltd. 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 <Guid/EventGroup.h>
+
+#include <Hi3660.h>
+#include <Hkadc.h>
+#include <libfdt.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/NonDiscoverableDeviceRegistrationLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/SerialPortLib.h>
+#include <Library/TimerLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Protocol/EmbeddedGpio.h>
+#include <Protocol/PlatformVirtualKeyboard.h>
+
+#define ADC_ADCIN0                       0
+#define ADC_ADCIN1                       1
+#define ADC_ADCIN2                       2
+
+#define HKADC_DATA_GRADE0                0
+#define HKADC_DATA_GRADE1                100
+#define HKADC_DATA_GRADE2                300
+#define HKADC_DATA_GRADE3                500
+#define HKADC_DATA_GRADE4                700
+#define HKADC_DATA_GRADE5                900
+#define HKADC_DATA_GRADE6                1100
+#define HKADC_DATA_GRADE7                1300
+#define HKADC_DATA_GRADE8                1500
+#define HKADC_DATA_GRADE9                1700
+#define HKADC_DATA_GRADE10               1800
+
+#define BOARDID_VALUE0                   0
+#define BOARDID_VALUE1                   1
+#define BOARDID_VALUE2                   2
+#define BOARDID_VALUE3                   3
+#define BOARDID_VALUE4                   4
+#define BOARDID_VALUE5                   5
+#define BOARDID_VALUE6                   6
+#define BOARDID_VALUE7                   7
+#define BOARDID_VALUE8                   8
+#define BOARDID_VALUE9                   9
+#define BOARDID_UNKNOW                   0xF
+
+#define BOARDID3_BASE                    5
+
+#define HIKEY960_BOARDID_V1              5300
+#define HIKEY960_BOARDID_V2              5301
+
+#define HIKEY960_COMPATIBLE_LEDS_V1      "gpio-leds_v1"
+#define HIKEY960_COMPATIBLE_LEDS_V2      "gpio-leds_v2"
+#define HIKEY960_COMPATIBLE_HUB_V1       "hisilicon,gpio_hubv1"
+#define HIKEY960_COMPATIBLE_HUB_V2       "hisilicon,gpio_hubv2"
+
+#define SERIAL_NUMBER_SIZE               17
+#define SERIAL_NUMBER_BLOCK_SIZE         EFI_PAGE_SIZE
+#define SERIAL_NUMBER_LBA                20
+#define RANDOM_MAX                       0x7FFFFFFFFFFFFFFF
+#define RANDOM_MAGIC                     0x9A4DBEAF
+
+#define ADB_REBOOT_ADDRESS               0x32100000
+#define ADB_REBOOT_BOOTLOADER            0x77665500
+#define ADB_REBOOT_NONE                  0x77665501
+
+#define DETECT_SW_FASTBOOT               68        // GPIO8_4
+
+typedef struct {
+  UINT64        Magic;
+  UINT64        Data;
+  CHAR16        UnicodeSN[SERIAL_NUMBER_SIZE];
+} RANDOM_SERIAL_NUMBER;
+
+enum {
+  BOOT_MODE_RECOVERY  = 0,
+  BOOT_MODE_NORMAL,
+  BOOT_MODE_MASK = 1,
+};
+
+STATIC UINTN    mBoardId;
+
+STATIC EMBEDDED_GPIO   *mGpio;
+
+STATIC
+VOID
+InitAdc (
+  VOID
+  )
+{
+  // reset hkadc
+  MmioWrite32 (CRG_PERRSTEN2, PERRSTEN2_HKADCSSI);
+  // wait a few clock cycles
+  MicroSecondDelay (2);
+  MmioWrite32 (CRG_PERRSTDIS2, PERRSTEN2_HKADCSSI);
+  MicroSecondDelay (2);
+  // enable hkadc clock
+  MmioWrite32 (CRG_PERDIS2, PEREN2_HKADCSSI);
+  MicroSecondDelay (2);
+  MmioWrite32 (CRG_PEREN2, PEREN2_HKADCSSI);
+  MicroSecondDelay (2);
+}
+
+STATIC
+EFI_STATUS
+AdcGetAdc (
+  IN  UINTN         Channel,
+  OUT UINTN         *Value
+  )
+{
+  UINT32            Data;
+  UINT16            Value1, Value0;
+
+  if (Channel > HKADC_CHANNEL_MAX) {
+    DEBUG ((DEBUG_ERROR, "invalid channel:%d\n", Channel));
+    return EFI_OUT_OF_RESOURCES;
+  }
+  // configure the read/write operation for external HKADC
+  MmioWrite32 (HKADC_WR01_DATA, HKADC_WR01_VALUE | Channel);
+  MmioWrite32 (HKADC_WR23_DATA, HKADC_WR23_VALUE);
+  MmioWrite32 (HKADC_WR45_DATA, HKADC_WR45_VALUE);
+  // configure the number of accessing registers
+  MmioWrite32 (HKADC_WR_NUM, HKADC_WR_NUM_VALUE);
+  // configure delay of accessing registers
+  MmioWrite32 (HKADC_DELAY01, HKADC_CHANNEL0_DELAY01_VALUE);
+  MmioWrite32 (HKADC_DELAY23, HKADC_DELAY23_VALUE);
+
+  // start HKADC
+  MmioWrite32 (HKADC_DSP_START, 1);
+  do {
+    Data = MmioRead32 (HKADC_DSP_START);
+  } while (Data & 1);
+
+  // convert AD result
+  Value1 = (UINT16)MmioRead32 (HKADC_DSP_RD2_DATA);
+  Value0 = (UINT16)MmioRead32 (HKADC_DSP_RD3_DATA);
+
+  Data = ((Value1 << 4) & HKADC_VALUE_HIGH) | ((Value0 >> 4) & HKADC_VALUE_LOW);
+  *Value = Data;
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+AdcGetValue (
+  IN UINTN         Channel,
+  IN OUT UINTN     *Value
+  )
+{
+  EFI_STATUS       Status;
+  UINTN            Result;
+
+  Status = AdcGetAdc (Channel, Value);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // convert ADC value to micro-volt
+  Result = ((*Value & HKADC_VALID_VALUE) * HKADC_VREF_1V8) / HKADC_ACCURACY;
+  *Value = Result;
+  return EFI_SUCCESS;
+}
+
+STATIC
+UINTN
+AdcinDataRemap (
+  IN UINTN           AdcinValue
+  )
+{
+  UINTN              Result;
+
+  if (AdcinValue < HKADC_DATA_GRADE0) {
+    Result = BOARDID_UNKNOW;
+  } else if (AdcinValue < HKADC_DATA_GRADE1) {
+    Result = BOARDID_VALUE0;
+  } else if (AdcinValue < HKADC_DATA_GRADE2) {
+    Result = BOARDID_VALUE1;
+  } else if (AdcinValue < HKADC_DATA_GRADE3) {
+    Result = BOARDID_VALUE2;
+  } else if (AdcinValue < HKADC_DATA_GRADE4) {
+    Result = BOARDID_VALUE3;
+  } else if (AdcinValue < HKADC_DATA_GRADE5) {
+    Result = BOARDID_VALUE4;
+  } else if (AdcinValue < HKADC_DATA_GRADE6) {
+    Result = BOARDID_VALUE5;
+  } else if (AdcinValue < HKADC_DATA_GRADE7) {
+    Result = BOARDID_VALUE6;
+  } else if (AdcinValue < HKADC_DATA_GRADE8) {
+    Result = BOARDID_VALUE7;
+  } else if (AdcinValue < HKADC_DATA_GRADE9) {
+    Result = BOARDID_VALUE8;
+  } else if (AdcinValue < HKADC_DATA_GRADE10) {
+    Result = BOARDID_VALUE9;
+  } else {
+    Result = BOARDID_UNKNOW;
+  }
+  return Result;
+}
+
+STATIC
+EFI_STATUS
+InitBoardId (
+  OUT UINTN          *Id
+  )
+{
+  UINTN              Adcin0, Adcin1, Adcin2;
+  UINTN              Adcin0Remap, Adcin1Remap, Adcin2Remap;
+
+  InitAdc ();
+
+  // read ADC channel0 data
+  AdcGetValue (ADC_ADCIN0, &Adcin0);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin0:%d\n", Adcin0));
+  Adcin0Remap = AdcinDataRemap (Adcin0);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin0Remap:%d\n", Adcin0Remap));
+  if (Adcin0Remap == BOARDID_UNKNOW) {
+    return EFI_INVALID_PARAMETER;
+  }
+  // read ADC channel1 data
+  AdcGetValue (ADC_ADCIN1, &Adcin1);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin1:%d\n", Adcin1));
+  Adcin1Remap = AdcinDataRemap (Adcin1);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin1Remap:%d\n", Adcin1Remap));
+  if (Adcin1Remap == BOARDID_UNKNOW) {
+    return EFI_INVALID_PARAMETER;
+  }
+  // read ADC channel2 data
+  AdcGetValue (ADC_ADCIN2, &Adcin2);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin2:%d\n", Adcin2));
+  Adcin2Remap = AdcinDataRemap (Adcin2);
+  DEBUG ((DEBUG_INFO, "[BDID]Adcin2Remap:%d\n", Adcin2Remap));
+  if (Adcin2Remap == BOARDID_UNKNOW) {
+    return EFI_INVALID_PARAMETER;
+  }
+  *Id = BOARDID3_BASE * 1000 + (Adcin2Remap * 100) + (Adcin1Remap * 10) + Adcin0Remap;
+  DEBUG ((DEBUG_INFO, "[BDID]boardid: %d\n", *Id));
+  return EFI_SUCCESS;
+}
+
+STATIC
+VOID
+InitSdCard (
+  IN VOID
+  )
+{
+  UINT32        Data;
+
+  // LDO16
+  Data = MmioRead32 (PMU_REG_BASE + (0x79 << 2)) & 7;
+  Data |= 6;
+  MmioWrite32 (PMU_REG_BASE + (0x79 << 2), Data);
+  MmioOr32 (PMU_REG_BASE + (0x78 << 2), 2);
+  MicroSecondDelay (100);
+
+  // LDO9
+  Data = MmioRead32 (PMU_REG_BASE + (0x6b << 2)) & 7;
+  Data |= 5;
+  MmioWrite32 (PMU_REG_BASE + (0x6b << 2), Data);
+  MmioOr32 (PMU_REG_BASE + (0x6a << 2), 2);
+  MicroSecondDelay (100);
+
+  // GPIO203
+  MmioWrite32 (0xfff11000 + (24 << 2), 0); // GPIO function
+
+  // SD pinmux
+  MmioWrite32 (0xff37e000 + 0x0, 1); // SD_CLK
+  MmioWrite32 (0xff37e000 + 0x4, 1); // SD_CMD
+  MmioWrite32 (0xff37e000 + 0x8, 1); // SD_DATA0
+  MmioWrite32 (0xff37e000 + 0xc, 1); // SD_DATA1
+  MmioWrite32 (0xff37e000 + 0x10, 1); // SD_DATA2
+  MmioWrite32 (0xff37e000 + 0x14, 1); // SD_DATA3
+  MmioWrite32 (0xff37e800 + 0x0, 15 << 4); // SD_CLK float with 32mA
+  MmioWrite32 (0xff37e800 + 0x4, (1 << 0) | (8 << 4)); // SD_CMD
+  MmioWrite32 (0xff37e800 + 0x8, (1 << 0) | (8 << 4)); // SD_DATA0
+  MmioWrite32 (0xff37e800 + 0xc, (1 << 0) | (8 << 4)); // SD_DATA1
+  MmioWrite32 (0xff37e800 + 0x10, (1 << 0) | (8 << 4)); // SD_DATA2
+  MmioWrite32 (0xff37e800 + 0x14, (1 << 0) | (8 << 4)); // SD_DATA3
+
+  do {
+    MmioOr32 (CRG_REG_BASE + 0xb8, (1 << 6) | (1 << 6 << 16) | (0 << 4) | (3 << 4 << 16));
+    Data = MmioRead32 (CRG_REG_BASE + 0xb8);
+  } while ((Data & ((1 << 6) | (3 << 4))) != ((1 << 6) | (0 << 4)));
+
+  // Unreset SD controller
+  MmioWrite32 (CRG_PERRSTDIS4, 1 << 18);
+  do {
+    Data = MmioRead32 (CRG_PERRSTSTAT4);
+  } while ((Data & (1 << 18)) == (1 << 18));
+  // Enable SD controller clock
+  MmioOr32 (CRG_REG_BASE + 0, 1 << 30);
+  MmioOr32 (CRG_REG_BASE + 0x40, 1 << 17);
+  do {
+    Data = MmioRead32 (CRG_REG_BASE + 0x48);
+  } while ((Data & (1 << 17)) != (1 << 17));
+}
+
+VOID
+InitPeripherals (
+  IN VOID
+  )
+{
+  // Enable FPLL0
+  MmioOr32 (SCTRL_SCFPLLCTRL0, SCTRL_SCFPLLCTRL0_FPLL0_EN);
+
+  InitSdCard ();
+
+  // Enable wifi clock
+  MmioOr32 (PMIC_HARDWARE_CTRL0, PMIC_HARDWARE_CTRL0_WIFI_CLK);
+  MmioOr32 (PMIC_OSC32K_ONOFF_CTRL, PMIC_OSC32K_ONOFF_CTRL_EN_32K);
+}
+
+/**
+  Notification function of the event defined as belonging to the
+  EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
+  the entry point of the driver.
+
+  This function is called when an event belonging to the
+  EFI_END_OF_DXE_EVENT_GROUP_GUID event group is signalled. Such an
+  event is signalled once at the end of the dispatching of all
+  drivers (end of the so called DXE phase).
+
+  @param[in]  Event    Event declared in the entry point of the driver whose
+                       notification function is being invoked.
+  @param[in]  Context  NULL
+**/
+STATIC
+VOID
+OnEndOfDxe (
+  IN EFI_EVENT  Event,
+  IN VOID       *Context
+  )
+{
+  UINT32        BootMode;
+
+  BootMode = MmioRead32 (SCTRL_BAK_DATA0) & BOOT_MODE_MASK;
+  if (BootMode == BOOT_MODE_RECOVERY) {
+    SerialPortWrite ((UINT8 *)"WARNING: CAN NOT BOOT KERNEL IN RECOVERY MODE!\r\n", 48);
+    SerialPortWrite ((UINT8 *)"Switch to normal boot mode, then reboot to boot kernel.\r\n", 57);
+  }
+}
+
+EFI_STATUS
+EFIAPI
+VirtualKeyboardRegister (
+  IN VOID
+  )
+{
+  EFI_STATUS           Status;
+
+  Status = gBS->LocateProtocol (
+                  &gEmbeddedGpioProtocolGuid,
+                  NULL,
+                  (VOID **) &mGpio
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+VirtualKeyboardReset (
+  IN VOID
+  )
+{
+  EFI_STATUS           Status;
+
+  if (mGpio == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+  // Configure GPIO68 as GPIO function
+  MmioWrite32 (0xe896c108, 0);
+  Status = mGpio->Set (mGpio, DETECT_SW_FASTBOOT, GPIO_MODE_INPUT);
+  return Status;
+}
+
+BOOLEAN
+EFIAPI
+VirtualKeyboardQuery (
+  IN VIRTUAL_KBD_KEY             *VirtualKey
+  )
+{
+  EFI_STATUS           Status;
+  UINTN                Value = 0;
+
+  if ((VirtualKey == NULL) || (mGpio == NULL)) {
+    return FALSE;
+  }
+  if (MmioRead32 (ADB_REBOOT_ADDRESS) == ADB_REBOOT_BOOTLOADER) {
+    goto Done;
+  } else {
+    Status = mGpio->Get (mGpio, DETECT_SW_FASTBOOT, &Value);
+    if (EFI_ERROR (Status) || (Value != 0)) {
+      return FALSE;
+    }
+  }
+Done:
+  VirtualKey->Signature = VIRTUAL_KEYBOARD_KEY_SIGNATURE;
+  VirtualKey->Key.ScanCode = SCAN_NULL;
+  VirtualKey->Key.UnicodeChar = L'f';
+  return TRUE;
+}
+
+EFI_STATUS
+EFIAPI
+VirtualKeyboardClear (
+  IN VIRTUAL_KBD_KEY            *VirtualKey
+  )
+{
+  if (VirtualKey == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+  if (MmioRead32 (ADB_REBOOT_ADDRESS) == ADB_REBOOT_BOOTLOADER) {
+    MmioWrite32 (ADB_REBOOT_ADDRESS, ADB_REBOOT_NONE);
+    WriteBackInvalidateDataCacheRange ((VOID *)ADB_REBOOT_ADDRESS, 4);
+  }
+  return EFI_SUCCESS;
+}
+
+PLATFORM_VIRTUAL_KBD_PROTOCOL mVirtualKeyboard = {
+  VirtualKeyboardRegister,
+  VirtualKeyboardReset,
+  VirtualKeyboardQuery,
+  VirtualKeyboardClear
+};
+
+EFI_STATUS
+EFIAPI
+HiKey960EntryPoint (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS            Status;
+  EFI_EVENT             EndOfDxeEvent;
+
+  Status = InitBoardId (&mBoardId);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  InitPeripherals ();
+
+  //
+  // Create an event belonging to the "gEfiEndOfDxeEventGroupGuid" group.
+  // The "OnEndOfDxe()" function is declared as the call back function.
+  // It will be called at the end of the DXE phase when an event of the
+  // same group is signalled to inform about the end of the DXE phase.
+  // Install the INSTALL_FDT_PROTOCOL protocol.
+  //
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  OnEndOfDxe,
+                  NULL,
+                  &gEfiEndOfDxeEventGroupGuid,
+                  &EndOfDxeEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gPlatformVirtualKeyboardProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mVirtualKeyboard
+                  );
+  return Status;
+}