Re: [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

Re: [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Leif Lindholm]

You've reworked this to be targeted for edk2-platforms, which is
excellent! However:
- Please update subject line to match.
- Please also cc edk2-devel@lists.01.org (since you are with Linaro,
  it makes sense to still cc linaro-uefi)

To get the appropriate subject line for edk2-devel, as desribed by
https://github.com/tianocore/edk2-platforms/blob/about/Readme.md it
would also be helpful to add
--subject-prefix="edk2-platforms/master][PATCH" to git format-patch
command line (unless Laszlo can point out a better way of achieving
the same).

On Wed, Aug 09, 2017 at 10:12:36PM +0800, Jun Nie wrote:
> Add Sanchip Zx296718 basic library files for Zx296718 SoC
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../Library/Zx296718EvbLib/Zx296718Evb.c           | 132 +++++++++++++++++++++
>  .../Library/Zx296718EvbLib/Zx296718EvbHelper.S     |  50 ++++++++
>  .../Library/Zx296718EvbLib/Zx296718EvbLib.inf      |  53 +++++++++
>  .../Library/Zx296718EvbLib/Zx296718EvbMem.c        | 107 +++++++++++++++++
>  Silicon/Sanchip/Zx296718/Include/Zx296718.h        |  30 +++++
>  Silicon/Sanchip/Zx296718/Zx296718.dec              |  32 +++++

It is more clear (especially when submitting new ports) to generate
the patches with --stat=1000, to make the full file paths visible.

See Laszlo's guide for helpful steps when submitting patches:
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

>  6 files changed, 404 insertions(+)
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c
>  create mode 100644 Silicon/Sanchip/Zx296718/Include/Zx296718.h
>  create mode 100644 Silicon/Sanchip/Zx296718/Zx296718.dec

Since this ends up with a .dec, please rename
Silicon/Sanchip/Zx296718/ -> Silicon/Sanchip/Zx296718Pkg/.

That said, I don't see this .dec file providing any information that
is subsequently used enywhere. Is this in preparation for further
patches coming after this initial platform support?

> diff --git a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
> new file mode 100644
> index 0000000..4e4eb54
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
> @@ -0,0 +1,132 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.

Just a clarification: Sanchip/Sanechips?
"The Internet" suggests Sanechips is the appropriate company name, so
should the directory name change?

> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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/IoLib.h>
> +#include <Library/ArmPlatformLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>

Please sort include files alphabetically.

> +
> +#include <Ppi/ArmMpCoreInfo.h>
> +
> +#include <Zx296718.h>
> +
> +ARM_CORE_INFO mZx296718EvbInfoTable[] = {
> +  {
> +    // Cluster 0, Core 0
> +    0x0, 0x0,
> +
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (UINT64)0xFFFFFFFF
> +  },
> +  {
> +    // Cluster 0, Core 1
> +    0x0, 0x1,
> +
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (UINT64)0xFFFFFFFF
> +  },
> +  {
> +    // Cluster 0, Core 2
> +    0x0, 0x2,
> +
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (UINT64)0xFFFFFFFF
> +  },
> +  {
> +    // Cluster 0, Core 3
> +    0x0, 0x3,
> +
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (UINT64)0xFFFFFFFF
> +  },
> +};
> +
> +/**
> +  Return the current Boot Mode
> +
> +  This function returns the boot reason on the platform
> +
> +  @return   Return the current Boot Mode of the platform
> +
> +**/
> +EFI_BOOT_MODE
> +ArmPlatformGetBootMode (
> +  VOID
> +  )
> +{
> +  return BOOT_WITH_FULL_CONFIGURATION;
> +}
> +
> +/**
> +  Initialize controllers that must setup in the normal world
> +
> +  This function is called by the ArmPlatformPkg/Pei or ArmPlatformPkg/Pei/PlatformPeim
> +  in the PEI phase.
> +
> +**/
> +RETURN_STATUS
> +ArmPlatformInitialize (
> +  IN  UINTN                     MpId
> +  )
> +{
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Initialize the system (or sometimes called permanent) memory
> +
> +  This memory is generally represented by the DRAM.
> +
> +**/
> +VOID
> +ArmPlatformInitializeSystemMemory (
> +  VOID
> +  )
> +{
> +}
> +
> +EFI_STATUS
> +PrePeiCoreGetMpCoreInfo (
> +  OUT UINTN                   *CoreCount,
> +  OUT ARM_CORE_INFO           **ArmCoreTable
> +  )
> +{
> +  // Only support one cluster
> +  *CoreCount    = sizeof(mZx296718EvbInfoTable) / sizeof(ARM_CORE_INFO);

Do a global search and replace of "sizeof(" to "sizeof (".

> +  *ArmCoreTable = mZx296718EvbInfoTable;
> +  return EFI_SUCCESS;
> +}
> +
> +// Needs to be declared in the file. Otherwise gArmMpCoreInfoPpiGuid is undefined in the contect of PrePeiCore
> +EFI_GUID mArmMpCoreInfoPpiGuid = ARM_MP_CORE_INFO_PPI_GUID;
> +ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
> +
> +EFI_PEI_PPI_DESCRIPTOR      gPlatformPpiTable[] = {
> +  {
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
> +    &mArmMpCoreInfoPpiGuid,
> +    &mMpCoreInfoPpi
> +  }
> +};
> +
> +VOID
> +ArmPlatformGetPlatformPpiList (
> +  OUT UINTN                   *PpiListSize,
> +  OUT EFI_PEI_PPI_DESCRIPTOR  **PpiList
> +  )
> +{
> +  *PpiListSize = sizeof(gPlatformPpiTable);
> +  *PpiList = gPlatformPpiTable;
> +}
> diff --git a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S
> new file mode 100644
> index 0000000..829d9ef
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S
> @@ -0,0 +1,50 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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 <AsmMacroIoLibV8.h>
> +#include <Library/ArmLib.h>
> +
> +ASM_FUNC(ArmPlatformPeiBootAction)
> +  ret
> +
> +//UINTN
> +//ArmPlatformGetCorePosition (
> +//  IN UINTN MpId
> +//  );
> +// With this function: CorePos = (ClusterId * 4) + CoreId
> +ASM_FUNC(ArmPlatformGetCorePosition)
> +  and   x1, x0, #ARM_CORE_MASK
> +  and   x0, x0, #ARM_CLUSTER_MASK
> +  add   x0, x1, x0, LSR #6
> +  ret
> +
> +//UINTN
> +//ArmPlatformGetPrimaryCoreMpId (
> +//  VOID
> +//  );
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  MOV32 (w0, FixedPcdGet32 (PcdArmPrimaryCore))
> +  ret
> +
> +//UINTN
> +//ArmPlatformIsPrimaryCore (
> +//  IN UINTN MpId
> +//  );
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
> +  and   x0, x0, x1
> +  MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCore))
> +  cmp   w0, w1
> +  cset  x0, eq
> +  ret
> diff --git a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
> new file mode 100644
> index 0000000..18b383b
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
> @@ -0,0 +1,53 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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                    = 0x00010005

0x00010019 is the current version.

> +  BASE_NAME                      = Zx296718EvbLib
> +  FILE_GUID                      = e1903cfc-f842-4f9a-a6ed-856bae38a902
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmPlatformLib
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

M (MdeM) before P (MdeP).

> +  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> +  Silicon/Sanchip/Zx296718/Zx296718.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  IoLib
> +  HobLib

H before I.

> +  MemoryAllocationLib
> +  SerialPortLib
> +
> +[Sources.common]
> +  Zx296718Evb.c
> +  Zx296718EvbMem.c
> +
> +[Sources.AARCH64]
> +  Zx296718EvbHelper.S | GCC
> +
> +[FeaturePcd]
> +  gEmbeddedTokenSpaceGuid.PcdCacheEnable
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdArmPrimaryCore
> +  gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> diff --git a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c
> new file mode 100644
> index 0000000..c319f4d
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c
> @@ -0,0 +1,107 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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/ArmPlatformLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>

Please sort includes alphabetically.

> +
> +#include <Zx296718.h>
> +
> +// The total number of descriptors, including the final "end-of-table" descriptor.
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 5
> +
> +// DDR attributes
> +#define DDR_ATTRIBUTES_CACHED           ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> +#define DDR_ATTRIBUTES_UNCACHED         ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED
> +
> +/**
> +  Return the Virtual Memory Map of your platform
> +
> +  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU on your platform.
> +
> +  @param[out]   VirtualMemoryMap    Array of ARM_MEMORY_REGION_DESCRIPTOR describing a Physical-to-
> +                                    Virtual Memory mapping. This array must be ended by a zero-filled
> +                                    entry
> +
> +**/
> +VOID
> +ArmPlatformGetVirtualMemoryMap (
> +  IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
> +  )
> +{
> +  ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
> +  UINTN                         Index = 0;
> +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> +
> +  ResourceAttributes = (
> +      EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_TESTED
> +  );
> +
> +  // Create initial Base Hob for system memory.
> +  BuildResourceDescriptorHob (
> +      EFI_RESOURCE_SYSTEM_MEMORY,
> +      ResourceAttributes,
> +      PcdGet64 (PcdSystemMemoryBase),
> +      PcdGet64 (PcdSystemMemorySize)
> +  );
> +
> +  ASSERT (VirtualMemoryMap != NULL);
> +  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages(
> +      EFI_SIZE_TO_PAGES (sizeof(ARM_MEMORY_REGION_DESCRIPTOR) *
> +                         MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> +  if (VirtualMemoryTable == NULL) {
> +      return;
> +  }
> +
> +  if (FeaturePcdGet(PcdCacheEnable) == TRUE) {
> +      CacheAttributes = DDR_ATTRIBUTES_CACHED;
> +  } else {
> +      CacheAttributes = DDR_ATTRIBUTES_UNCACHED;
> +  }
> +
> +  Index = 0;
> +
> +  // Zx296718 SOC peripherals
> +  VirtualMemoryTable[Index].PhysicalBase    = ZX296718_PERIPH_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = ZX296718_PERIPH_BASE;
> +  VirtualMemoryTable[Index].Length          = ZX296718_PERIPH_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // DDR - predefined 1GB size
> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[Index].Length          = PcdGet64 (PcdSystemMemorySize);
> +  VirtualMemoryTable[Index].Attributes      = CacheAttributes;
> +
> +  // End of Table
> +  VirtualMemoryTable[++Index].PhysicalBase  = 0;
> +  VirtualMemoryTable[Index].VirtualBase     = 0;
> +  VirtualMemoryTable[Index].Length          = 0;
> +  VirtualMemoryTable[Index].Attributes      = (ARM_MEMORY_REGION_ATTRIBUTES)0;
> +
> +  ASSERT((Index + 1) <= MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
> +}
> diff --git a/Silicon/Sanchip/Zx296718/Include/Zx296718.h b/Silicon/Sanchip/Zx296718/Include/Zx296718.h
> new file mode 100644
> index 0000000..3ace9ab
> --- /dev/null
> +++ b/Silicon/Sanchip/Zx296718/Include/Zx296718.h
> @@ -0,0 +1,30 @@
> +/** @file
> +*
> +*  Copyright (c) 2016, Linaro Limited. All rights reserved.

Guess 2016-2017 by now?

> +*
> +*  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 __ZX296718_H__
> +#define __ZX296718_H__

Coding style specifies:
"Every header file must have a ‘#ifndef FILE_NAME_H_’ and ‘#endif’ guard" 
so you could drop 3 of those underscores.

> +
> +/***********************************************************************************
> +// Platform Memory Map
> +************************************************************************************/
> +
> +// SOC peripherals (UART, I2C, I2S, USB, etc)
> +#define ZX296718_PERIPH_BASE         0x110000
> +#define ZX296718_PERIPH_SZ           0x3000000
> +
> +#define SYS_CFG_BASE_ADDR            0x01463000
> +#define TOP_CRM_BASE_ADDR            0x01461000
> +#define AON_SYS_CTRL_BASE_ADDR       0x116000
> +
> +#endif /* __ZX296718_H__ */
> diff --git a/Silicon/Sanchip/Zx296718/Zx296718.dec b/Silicon/Sanchip/Zx296718/Zx296718.dec
> new file mode 100644
> index 0000000..9ecd11d
> --- /dev/null
> +++ b/Silicon/Sanchip/Zx296718/Zx296718.dec
> @@ -0,0 +1,32 @@
> +#
> +#  Copyright (c) 2016, 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]
> +  DEC_SPECIFICATION              = 0x00010005
> +  PACKAGE_NAME                   = Zx296718
> +  PACKAGE_GUID                   = 6b180aeb-7bf7-4cf1-9310-dcd403d5dfcb
> +  PACKAGE_VERSION                = 0.1
> +
> +################################################################################
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#                   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +################################################################################
> +[Includes.common]
> +  Include                        # Root include for the package

This path does not exist, which breaks the build.

/
    Leif

> +
> +[Guids.common]
> +  gZx296718TokenSpaceGuid      =  { 0x7827738f, 0x0651, 0x48ab, {0xad, 0x5c, 0x8d, 0x3d, 0xad, 0x58, 0x0d, 0xa9} }
> -- 
> 1.9.1
> 

Re: [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Laszlo Ersek]

Thanks for the CC

On 08/10/17 15:04, Leif Lindholm wrote:
> You've reworked this to be targeted for edk2-platforms, which is
> excellent! However:
> - Please update subject line to match.
> - Please also cc edk2-devel@lists.01.org (since you are with Linaro,
>   it makes sense to still cc linaro-uefi)
> 
> To get the appropriate subject line for edk2-devel, as desribed by
> https://github.com/tianocore/edk2-platforms/blob/about/Readme.md it
> would also be helpful to add
> --subject-prefix="edk2-platforms/master][PATCH" to git format-patch
> command line (unless Laszlo can point out a better way of achieving
> the same).

I seem to recall a discussion (not necessarily related to the
edk2-platforms tree, but to another edk2-related tree, still posted to
edk2-devel) that we're fine if you put the tree identified right after
the PATCH word, between the same brackets. So just use

  --subject-prefix="PATCH edk2-platforms/master"

or even (if "master" is quite obvious)

  --subject-prefix="PATCH edk2-platforms"

> 
> On Wed, Aug 09, 2017 at 10:12:36PM +0800, Jun Nie wrote:
>> Add Sanchip Zx296718 basic library files for Zx296718 SoC
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  .../Library/Zx296718EvbLib/Zx296718Evb.c           | 132 +++++++++++++++++++++
>>  .../Library/Zx296718EvbLib/Zx296718EvbHelper.S     |  50 ++++++++
>>  .../Library/Zx296718EvbLib/Zx296718EvbLib.inf      |  53 +++++++++
>>  .../Library/Zx296718EvbLib/Zx296718EvbMem.c        | 107 +++++++++++++++++
>>  Silicon/Sanchip/Zx296718/Include/Zx296718.h        |  30 +++++
>>  Silicon/Sanchip/Zx296718/Zx296718.dec              |  32 +++++
> 
> It is more clear (especially when submitting new ports) to generate
> the patches with --stat=1000, to make the full file paths visible.

Yes, please -- I recommend:

  --stat=1000 --stat-graph-width=20

("--stat=1000" without "--stat-graph-width=20" won't be pleasant.)

Thanks!
Laszlo

Re: [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Leif Lindholm]

On Thu, Aug 17, 2017 at 11:42:23PM +0800, Jun Nie wrote:
> On 2017年08月10日 21:04, Leif Lindholm wrote:
> > > diff --git a/Silicon/Sanchip/Zx296718/Include/Zx296718.h b/Silicon/Sanchip/Zx296718/Include/Zx296718.h
> > > new file mode 100644
> > > index 0000000..3ace9ab
> > > --- /dev/null
> > > +++ b/Silicon/Sanchip/Zx296718/Include/Zx296718.h
> > > @@ -0,0 +1,30 @@
> > > +/** @file
> > > +*
> > > +*  Copyright (c) 2016, Linaro Limited. All rights reserved.
> > 
> > Guess 2016-2017 by now?
> > 
> > > +*
> > > +*  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 __ZX296718_H__
> > > +#define __ZX296718_H__
> > 
> > Coding style specifies:
> > "Every header file must have a ‘#ifndef FILE_NAME_H_’ and ‘#endif’ guard"
> > so you could drop 3 of those underscores.
> 
> Will change it. Is there any coding style readme file? Because I find most
> of header file use __FILE_NAME_H__ sytle macro.

Ah, yes - it helps knowing it exists :)

It is the C Coding Standards document on
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications
You will potentially find other documents there useful as well.

/
    Leif

Re: [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:
> Runtime service is not supported yet.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 +++++++++++++++++++++
>  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++++++
>  .../Zx296718RealTimeClock.inf                      |  42 +++
>  3 files changed, 520 insertions(+)
>  create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
>  create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
>  create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> 
> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> new file mode 100644
> index 0000000..af6e5bd
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> @@ -0,0 +1,376 @@
> +/** @file
> +  Implement EFI RealTimeClock runtime services via RTC Lib.
> +
> +  Currently this driver does not support runtime virtual calling.
> +
> +  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +  Copyright (c) 2017, Linaro Limited.
> +
> +  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.
> +
> +  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <PiDxe.h>

P before U.

> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +// Use EfiAtRuntime to check stage
> +#include <Library/UefiRuntimeLib.h>

L (UefiRuntimeLib) before S (UefiRuntimeServices...).
No need for a comment explaining why we include headers.

> +#include <Protocol/RealTimeClock.h>
> +#include "Zx296718RealTimeClock.h"
> +
> +STATIC UINTN       RtcBase;

mRtcBase.

> +STATIC BOOLEAN     RTCInitialized = FALSE;

mRTCInitialized.

> +
> +BOOLEAN
> +EFIAPI
> +IsTimeValid(
> +  IN EFI_TIME *Time
> +  )
> +{
> +  // Check the input parameters are within the range specified by UEFI
> +  if ((Time->Year  < 2000) ||
> +     (Time->Year   > 2099) ||
> +     (Time->Month  < 1   ) ||
> +     (Time->Month  > 12  ) ||
> +     (Time->Day    < 1   ) ||
> +     (Time->Day    > 31  ) ||
> +     (Time->Hour   > 23  ) ||
> +     (Time->Minute > 59  ) ||
> +     (Time->Second > 59  ) ||
> +     (Time->Nanosecond > 999999999) ||
> +     (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) && (Time->TimeZone <= 1440)))) ||
> +     (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT)))
> +  ) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}

This appears a direct copy of the version in
EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library
dependency to decrease duplication?
(This may not have existed as a standalone library at the time you
started this port.)

> +
> +VOID

A lot of the functions in this file could do with a STATIC prefix,
since they are only used internally.

> +Wait4Busy (

Semantically, this name is incorrect.
The function is waiting _while_ the state is RTC_BUSY, so seems to me
it should be called WaitBusy.

> +  VOID
> +  )
> +{
> +  UINT32 Val, Retry = 1000;
> +  do {
> +    MicroSecondDelay (200);

Why 200?

> +    Val = MmioRead32 (RtcBase + RTCSTS);

MmioRead32 does not imply any barrier semantics.
If this component will only ever be supported on ARM platforms, you
could insert an ArmDataMemoryBarrier (). Otherwise, MemoryFence ().

> +  } while(Val & RTC_BUSY && Retry--);

Space before (.

> +
> +  if (!Retry)
> +    DEBUG((DEBUG_ERROR, "%a Rtc busy retry timeout\n", __func__));

Space after DEBUG.

> +}
> +
> +VOID
> +RTCWriteReg (
> +  IN UINT32 Reg,
> +  IN UINT32 Val
> +  )
> +{
> +  Wait4Busy ();
> +  MmioWrite32 (RtcBase + RTCCFGID, CONFIG_PARMETER);
> +  Wait4Busy ();
> +  MmioWrite32 (RtcBase + Reg, Val);
> +}
> +
> +UINT32
> +RTCReadReg (
> +  IN UINT32 Reg
> +  )
> +{
> +  Wait4Busy ();
> +  return MmioRead32 (RtcBase + Reg);
> +}
> +
> +VOID
> +InitializeRTC (
> +  VOID
> +  )
> +{
> +  UINTN Val = (UINTN)FixedPcdGet64 (PcdZxRtcClockFreq);
> +
> +  RTCWriteReg (RTCCLKCNT, Val - 1);
> +  Val = RTCReadReg (RTCPOWERINIT1);
> +  if (RTC_POWER_INI1_PARA != Val) {
> +    RTCWriteReg (RTCCTL, 0);
> +    MicroSecondDelay (INIT_DELAY);
> +    Val = RTCReadReg (RTCCTL);
> +    Val |= RTC_CTRL_BIT6_1;
> +    RTCWriteReg (RTCCTL, Val);
> +    Val = RTCReadReg (RTCCTL);
> +    Val &= RTC_CTRL_MODULE24 | RTC_CTRL_STOP;
> +    RTCWriteReg (RTCCTL, Val);
> +  }
> +  Val = RTCReadReg (RTCINT);
> +  Val &= ~RTC_IT_MASK;
> +  RTCWriteReg (RTCINT, Val);
> +  Val = RTCReadReg (RTCSTS);
> +  Val |= (RTC_POWER_UP | RTC_ALARM | RTC_TIMER);
> +  RTCWriteReg (RTCSTS, Val);
> +  //Wait4Busy ();

No disabled code, please.

> +  // TODO: write 0x6 to AON int clear

TODO is fine for early pass of review to get some overall feedback,
but this needs to be resolved before anything is merged.

> +  RTCWriteReg (RTCPOWERINIT1, RTC_POWER_INI1_PARA);
> +  RTCWriteReg (RTCPOWERINIT2, RTC_POWER_INI2_PARA);
> +  Val = RTC_CTRL_MODULE24 | RTC_CTRL_RUN | RTC_CTRL_CLK_32K_OUTEN
> +        | RTC_CTRL_BIT6_1;

Could there be a more human readable alias for RTC_CTRL_BIT6_1
(describing what setting that bit actually does)?

> +  RTCWriteReg (RTCCTL, Val);
> +
> +  RTCInitialized = TRUE;
> +}
> +
> +/**
> +  Returns the current time and date information, and the time-keeping capabilities
> +  of the hardware platform.
> +
> +  @param  Time                   A pointer to storage to receive a snapshot of the current time.
> +  @param  Capabilities           An optional pointer to a buffer to receive the real time clock
> +                                 device's capabilities.
> +
> +  @retval EFI_SUCCESS            The operation completed successfully.
> +  @retval EFI_INVALID_PARAMETER  Time is NULL.
> +  @retval EFI_DEVICE_ERROR       The time could not be retrieved due to hardware error.
> +  @retval EFI_SECURITY_VIOLATION The time could not be retrieved due to an authentication failure.
> +**/
> +EFI_STATUS
> +EFIAPI
> +LibGetTime (
> +  OUT EFI_TIME                *Time,
> +  OUT EFI_TIME_CAPABILITIES   *Capabilities
> +  )
> +{
> +  EFI_STATUS  Status = EFI_SUCCESS;
> +
> +  // Ensure Time is a valid pointer
> +  if (NULL == Time) {

No jeopardy comparisons please.
if (Time == NULL)

> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Initialize the hardware if not already done
> +  if (!RTCInitialized) {
> +    InitializeRTC ();
> +  }
> +
> +#if 0
> +      /* fake time */
> +      Time->Year          = 2015;
> +      Time->Month         = 1;
> +      Time->Day           = 1;
> +      Time->Hour          = 0;
> +      Time->Minute        = 0;
> +      Time->Second        = 0;
> +      Time->Nanosecond    = 0;
> +#endif

No disabled code blocks, please.

> +
> +  RTCWriteReg (RTCGETTIME, 0);
> +  Time->Year = BCD4_2_BIN (RTCReadReg (RTCYEAR));
> +  Time->Year += TM_YEAR_START;
> +
> +  Time->Month = BCD4_2_BIN (RTCReadReg (RTCMONT));
> +  Time->Day = BCD4_2_BIN (RTCReadReg (RTCDAY));
> +  Time->Hour = BCD4_2_BIN (RTCReadReg (RTCHOUR));
> +  Time->Minute = BCD4_2_BIN (RTCReadReg (RTCMIN));
> +  Time->Second = BCD4_2_BIN (RTCReadReg (RTCSEC));
> +  Time->Nanosecond = 0;
> +  Time->Daylight = 0;
> +  Time->TimeZone = EFI_UNSPECIFIED_TIMEZONE;
> +
> +  if(!IsTimeValid (Time)) {
> +    Status = EFI_UNSUPPORTED;
> +  }
> +
> +  return Status;
> +
> +}
> +
> +
> +/**
> +  Sets the current local time and date information.
> +
> +  @param  Time                  A pointer to the current time.
> +
> +  @retval EFI_SUCCESS           The operation completed successfully.
> +  @retval EFI_INVALID_PARAMETER A time field is out of range.
> +  @retval EFI_DEVICE_ERROR      The time could not be set due due to hardware error.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +LibSetTime (
> +  IN  EFI_TIME                *Time
> +  )
> +{
> +  EFI_STATUS  Status = EFI_SUCCESS;
> +
> +  // Check the input parameters are within the range specified by UEFI
> +  if(!IsTimeValid (Time)){
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Initialize the hardware if not already done
> +  if (!RTCInitialized) {
> +    InitializeRTC ();
> +  }
> +
> +  RTCWriteReg (RTCSEC, BIN2BCD (Time->Second));
> +  RTCWriteReg (RTCMIN, BIN2BCD (Time->Minute));
> +  RTCWriteReg (RTCHOUR, BIN2BCD (Time->Hour));
> +  RTCWriteReg (RTCDAY, BIN2BCD (Time->Day));
> +  RTCWriteReg (RTCMONT, BIN2BCD (Time->Month));
> +  RTCWriteReg (RTCYEAR, BIN2BCD (Time->Year - TM_YEAR_START));
> +  return Status;
> +}
> +
> +
> +/**
> +  Returns the current wakeup alarm clock setting.
> +
> +  @param  Enabled               Indicates if the alarm is currently enabled or disabled.
> +  @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
> +  @param  Time                  The current alarm setting.
> +
> +  @retval EFI_SUCCESS           The alarm settings were returned.
> +  @retval EFI_INVALID_PARAMETER Any parameter is NULL.
> +  @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +LibGetWakeupTime (
> +  OUT BOOLEAN     *Enabled,
> +  OUT BOOLEAN     *Pending,
> +  OUT EFI_TIME    *Time
> +  )
> +{
> +  // Not a required feature
> +  return EFI_UNSUPPORTED;
> +}
> +
> +
> +/**
> +  Sets the system wakeup alarm clock time.
> +
> +  @param  Enabled               Enable or disable the wakeup alarm.
> +  @param  Time                  If Enable is TRUE, the time to set the wakeup alarm for.
> +
> +  @retval EFI_SUCCESS           If Enable is TRUE, then the wakeup alarm was enabled. If
> +                                Enable is FALSE, then the wakeup alarm was disabled.
> +  @retval EFI_INVALID_PARAMETER A time field is out of range.
> +  @retval EFI_DEVICE_ERROR      The wakeup time could not be set due to a hardware error.
> +  @retval EFI_UNSUPPORTED       A wakeup timer is not supported on this platform.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +LibSetWakeupTime (
> +  IN BOOLEAN      Enabled,
> +  OUT EFI_TIME    *Time
> +  )
> +{
> +  // Not a required feature
> +  return EFI_UNSUPPORTED;
> +}
> +
> +
> +
> +/**
> +  This is the declaration of an EFI image entry point. This can be the entry point to an application
> +  written to this specification, an EFI boot service driver, or an EFI runtime driver.
> +
> +  @param  ImageHandle           Handle that identifies the loaded image.
> +  @param  SystemTable           System Table for this image.
> +
> +  @retval EFI_SUCCESS           The operation completed successfully.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +LibRtcInitialize (
> +  IN EFI_HANDLE                            ImageHandle,
> +  IN EFI_SYSTEM_TABLE                      *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +  EFI_HANDLE    Handle;
> +
> +

Why blank lines?

> +  EFI_TIME      EfiTime;
> +
> +  // Setup the setters and getters
> +  gRT->GetTime       = LibGetTime;
> +  gRT->SetTime       = LibSetTime;
> +  gRT->GetWakeupTime = LibGetWakeupTime;
> +  gRT->SetWakeupTime = LibSetWakeupTime;
> +
> +

Just one blank line is fine.

> +  RtcBase = (UINTN)FixedPcdGet64 (PcdZxRtcClockBase);
> +
> +  (VOID)gRT->GetTime (&EfiTime, NULL);
> +  if ((EfiTime.Year < 2015) || (EfiTime.Year > 2099)){
> +      EfiTime.Year          = 2015;
> +      EfiTime.Month         = 1;
> +      EfiTime.Day           = 1;
> +      EfiTime.Hour          = 0;
> +      EfiTime.Minute        = 0;
> +      EfiTime.Second        = 0;
> +      EfiTime.Nanosecond    = 0;
> +      Status = gRT->SetTime (&EfiTime);
> +      if (EFI_ERROR (Status))
> +      {
> +        DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Status : %r\n", __FUNCTION__,
> +               __LINE__, Status));
> +      }
> +  }
> +
> +  // Install the protocol
> +  Handle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                  &Handle,
> +                  &gEfiRealTimeClockArchProtocolGuid,  NULL,
> +                  NULL
> +                 );
> +
> +  return Status;
> +}
> +
> +
> +/**
> +  Fixup internal data so that EFI can be call in virtual mode.
> +  Call the passed in Child Notify event and convert any pointers in
> +  lib to virtual mode.
> +
> +  @param[in]    Event   The Event that is being processed
> +  @param[in]    Context Event Context
> +**/
> +VOID
> +EFIAPI
> +LibRtcVirtualNotifyEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  //
> +  // Only needed if you are going to support the OS calling RTC functions in virtual mode.
> +  // You will need to call EfiConvertPointer (). To convert any stored physical addresses
> +  // to virtual address. After the OS transitions to calling in virtual mode, all future
> +  // runtime calls will be made in virtual mode.
> +  //
> +  return;
> +}
> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
> new file mode 100644
> index 0000000..3b5a4d4
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
> @@ -0,0 +1,102 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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.
> +*
> +*  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
> +**/
> +
> +
> +#ifndef __DS3231_REAL_TIME_CLOCK_H__
> +#define __DS3231_REAL_TIME_CLOCK_H__
> +
> +#define RTC_POWER_INI1_PARA     (0xCDBC)
> +#define RTC_POWER_INI2_PARA     (0xCFCC)
> +#define CONFIG_PARMETER         (0xC1CD)
> +
> +#define ZX_RTC_CMP_VALUE        (0x3FFF)
> +#define WAIT_FOR_COUNT          (2000)
> +#define INIT_DELAY              (100)
> +
> +
> +/* RTC Control register description */
> +#define RTC_CTRL_STOP                   (~(0x1 << 0))
> +#define RTC_CTRL_RUN                    (0x1 << 0)
> +#define RTC_CTRL_ROUND30S               (0x1 << 1)
> +#define RTC_CTRL_AUTO_COMPENSATION      (0x1 << 2)
> +#define RTC_CTRL_MODULE12               (0x1 << 3)
> +#define RTC_CTRL_MODULE24               (~(0x1 << 3))
> +#define RTC_CTRL_SET_32_COUNTER         (0x1 << 5)
> +#define RTC_CTRL_SOFT_RESET             (0x1 << 6)
> +#define RTC_CTRL_CLK_32K_OUTEN          (0x1 << 8)
> +
> +#define RTC_CTRL_BIT6_0                 ( ~(0x1 << 6))
> +#define RTC_CTRL_BIT6_1                 (0x1 << 6)
> +
> +
> +
> +/* RTC Interrupt register description */
> +#define RTC_EVERY_MASK          (0x3 << 0)
> +#define RTC_EVERY_SEC           0x00                    /* second periodic intrrupt */
> +#define RTC_EVERY_MIN           0x01                    /* minute periodic interrupt */
> +#define RTC_EVERY_HR            0x02                    /* hour periodic interrupt */
> +#define RTC_EVERY_DAY           0x03                    /* day periodic interrupt */
> +#define RTC_IT_TIMER            (0x1 << 2)              /* Enable periodic interrupt */
> +#define RTC_IT_ALARM            (0x1 << 3)              /* Enable alarm clock interrupt */
> +#define RTC_IT_MASK             (0x3 << 2)
> +
> +/* RTC Status register description */
> +#define RTC_BUSY                (0x1 << 0)              /* Read-only, indicate refresh*/
> +#define RTC_RUN                 (0x1 << 1)              /* Read-only, RTC is running */
> +#define RTC_ALARM               (0x1 << 6)              /* Read/Write, Alarm interrupt has been generated */
> +#define RTC_TIMER               (0x1 << 7)              /* Read/Write, Timer interrupt has been generated */
> +#define RTC_POWER_UP            (0x1 << 8)              /* Read/Write, Reset */
> +
> +#define TM_YEAR_START               1900
> +
> +#define TM_MONTH_OFFSET             1
> +
> +#define TM_WDAY_SUNDAY              0
> +#define ZX_RTC_SUNDAY               7
> +
> +#define BCD2BIN(val)    (((val) & 0x0f) + ((val) >> 4) * 10)
> +#define BIN2BCD(val)    ((((val) / 10) << 4) + (val) % 10)
> +
> +#define BCD4_2_BIN(x)   (( (x) & 0x0F) +                    \
> +                        ((((x) & 0x0F0) >>   4) * 10)  +    \
> +                        ((((x) & 0xF00) >>   8) * 100) +    \
> +                        ((((x) & 0xF000) >> 12) * 1000))
> +
> +
> +#define BIN_2_BCD4(x)   (((x % 10) & 0x0F)       |          \
> +                        (((x /10 ) % 10)  <<  4) |          \
> +                        (((x /100) % 10)  <<  8) |          \
> +                        (((x /1000) % 10) << 12))
> +
> +/* RTC register offset */
> +#define RTCVER          0
> +#define RTCSEC          4
> +#define RTCMIN          8
> +#define RTCHOUR         0xc
> +#define RTCDAY          0x10
> +#define RTCMONT         0x14
> +#define RTCYEAR         0x18
> +#define RTCWEEK         0x1c
> +#define RTCCTL          0x38
> +#define RTCSTS          0x3c
> +#define RTCINT          0x40
> +#define RTCCFGID        0x48
> +#define RTCPOWERINIT1   0x4c
> +#define RTCPOWERINIT2   0x50
> +#define RTCGETTIME      0x54
> +#define RTCCLKCNT       0x58
> +
> +#endif
> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> new file mode 100644
> index 0000000..0a6852b
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> @@ -0,0 +1,42 @@
> +#/** @file
> +#
> +# Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution. The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

0x00010019.
(Or 1.25.)

> +  BASE_NAME                      = Zx296718RealTimeClockLib
> +  FILE_GUID                      = 4e1aaa26-597c-4f01-a8fc-fdf1adc1400f
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = RealTimeClockLib
> +
> +[Sources.common]
> +  Zx296718RealTimeClock.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  Silicon/Sanchip/SanchipPkg.dec

Please sort alphabetically.

> +
> +[LibraryClasses]
> +  IoLib
> +  UefiLib
> +  DebugLib
> +  PcdLib
> +  TimerLib
> +# Use EFiAtRuntime to check stage
> +  UefiRuntimeLib

Please sort alphabetically.

> +
> +[FixedPcd]
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq
> -- 
> 1.9.1
> 

Re: [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Jun Nie]

On 2017年08月10日 22:03, Leif Lindholm wrote:
> On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:
>> Runtime service is not supported yet.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>   .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 +++++++++++++++++++++
>>   .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++++++
>>   .../Zx296718RealTimeClock.inf                      |  42 +++
>>   3 files changed, 520 insertions(+)
>>   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
>>   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
>>   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
>>
>> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
>> new file mode 100644
>> index 0000000..af6e5bd
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
>> @@ -0,0 +1,376 @@
>> +/** @file
>> +  Implement EFI RealTimeClock runtime services via RTC Lib.
>> +
>> +  Currently this driver does not support runtime virtual calling.
>> +
>> +  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +  Copyright (c) 2017, Linaro Limited.
>> +
>> +  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.
>> +
>> +  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
>> +
>> +**/
>> +
>> +#include <Uefi.h>
>> +#include <PiDxe.h>
> 
> P before U.
> 
>> +#include <Library/BaseLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/TimerLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +// Use EfiAtRuntime to check stage
>> +#include <Library/UefiRuntimeLib.h>
> 
> L (UefiRuntimeLib) before S (UefiRuntimeServices...).
> No need for a comment explaining why we include headers.
> 
>> +#include <Protocol/RealTimeClock.h>
>> +#include "Zx296718RealTimeClock.h"
>> +
>> +STATIC UINTN       RtcBase;
> 
> mRtcBase.
> 
>> +STATIC BOOLEAN     RTCInitialized = FALSE;
> 
> mRTCInitialized.
> 
>> +
>> +BOOLEAN
>> +EFIAPI
>> +IsTimeValid(
>> +  IN EFI_TIME *Time
>> +  )
>> +{
>> +  // Check the input parameters are within the range specified by UEFI
>> +  if ((Time->Year  < 2000) ||
>> +     (Time->Year   > 2099) ||
>> +     (Time->Month  < 1   ) ||
>> +     (Time->Month  > 12  ) ||
>> +     (Time->Day    < 1   ) ||
>> +     (Time->Day    > 31  ) ||
>> +     (Time->Hour   > 23  ) ||
>> +     (Time->Minute > 59  ) ||
>> +     (Time->Second > 59  ) ||
>> +     (Time->Nanosecond > 999999999) ||
>> +     (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) && (Time->TimeZone <= 1440)))) ||
>> +     (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT)))
>> +  ) {
>> +    return FALSE;
>> +  }
>> +
>> +  return TRUE;
>> +}
> 
> This appears a direct copy of the version in
> EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library
> dependency to decrease duplication?
> (This may not have existed as a standalone library at the time you
> started this port.)
> 
>> +
>> +VOID
> 
> A lot of the functions in this file could do with a STATIC prefix,
> since they are only used internally.
> 
>> +Wait4Busy (
> 
> Semantically, this name is incorrect.
> The function is waiting _while_ the state is RTC_BUSY, so seems to me
> it should be called WaitBusy.
> 
>> +  VOID
>> +  )
>> +{
>> +  UINT32 Val, Retry = 1000;
>> +  do {
>> +    MicroSecondDelay (200);
> 
> Why 200?

It is just a suggested delay time according to RTC clock frequency. You 
want a RTC_WAIT_DELAY macro here for better understanding, right?
> 
>> +    Val = MmioRead32 (RtcBase + RTCSTS);
> 
> MmioRead32 does not imply any barrier semantics.
> If this component will only ever be supported on ARM platforms, you
> could insert an ArmDataMemoryBarrier (). Otherwise, MemoryFence ().
> 
>> +  } while(Val & RTC_BUSY && Retry--);
> 
> Space before (.
> 
>> +
>> +  if (!Retry)
>> +    DEBUG((DEBUG_ERROR, "%a Rtc busy retry timeout\n", __func__));
> 
> Space after DEBUG.
> 
>> +}
>> +
>> +VOID
>> +RTCWriteReg (
>> +  IN UINT32 Reg,
>> +  IN UINT32 Val
>> +  )
>> +{
>> +  Wait4Busy ();
>> +  MmioWrite32 (RtcBase + RTCCFGID, CONFIG_PARMETER);
>> +  Wait4Busy ();
>> +  MmioWrite32 (RtcBase + Reg, Val);
>> +}
>> +
>> +UINT32
>> +RTCReadReg (
>> +  IN UINT32 Reg
>> +  )
>> +{
>> +  Wait4Busy ();
>> +  return MmioRead32 (RtcBase + Reg);
>> +}
>> +
>> +VOID
>> +InitializeRTC (
>> +  VOID
>> +  )
>> +{
>> +  UINTN Val = (UINTN)FixedPcdGet64 (PcdZxRtcClockFreq);
>> +
>> +  RTCWriteReg (RTCCLKCNT, Val - 1);
>> +  Val = RTCReadReg (RTCPOWERINIT1);
>> +  if (RTC_POWER_INI1_PARA != Val) {
>> +    RTCWriteReg (RTCCTL, 0);
>> +    MicroSecondDelay (INIT_DELAY);
>> +    Val = RTCReadReg (RTCCTL);
>> +    Val |= RTC_CTRL_BIT6_1;
>> +    RTCWriteReg (RTCCTL, Val);
>> +    Val = RTCReadReg (RTCCTL);
>> +    Val &= RTC_CTRL_MODULE24 | RTC_CTRL_STOP;
>> +    RTCWriteReg (RTCCTL, Val);
>> +  }
>> +  Val = RTCReadReg (RTCINT);
>> +  Val &= ~RTC_IT_MASK;
>> +  RTCWriteReg (RTCINT, Val);
>> +  Val = RTCReadReg (RTCSTS);
>> +  Val |= (RTC_POWER_UP | RTC_ALARM | RTC_TIMER);
>> +  RTCWriteReg (RTCSTS, Val);
>> +  //Wait4Busy ();
> 
> No disabled code, please.
> 
>> +  // TODO: write 0x6 to AON int clear
> 
> TODO is fine for early pass of review to get some overall feedback,
> but this needs to be resolved before anything is merged.
> 
>> +  RTCWriteReg (RTCPOWERINIT1, RTC_POWER_INI1_PARA);
>> +  RTCWriteReg (RTCPOWERINIT2, RTC_POWER_INI2_PARA);
>> +  Val = RTC_CTRL_MODULE24 | RTC_CTRL_RUN | RTC_CTRL_CLK_32K_OUTEN
>> +        | RTC_CTRL_BIT6_1;
> 
> Could there be a more human readable alias for RTC_CTRL_BIT6_1
> (describing what setting that bit actually does)?

sure.
> 
>> +  RTCWriteReg (RTCCTL, Val);
>> +
>> +  RTCInitialized = TRUE;
>> +}
>> +
>> +/**
>> +  Returns the current time and date information, and the time-keeping capabilities
>> +  of the hardware platform.
>> +
>> +  @param  Time                   A pointer to storage to receive a snapshot of the current time.
>> +  @param  Capabilities           An optional pointer to a buffer to receive the real time clock
>> +                                 device's capabilities.
>> +
>> +  @retval EFI_SUCCESS            The operation completed successfully.
>> +  @retval EFI_INVALID_PARAMETER  Time is NULL.
>> +  @retval EFI_DEVICE_ERROR       The time could not be retrieved due to hardware error.
>> +  @retval EFI_SECURITY_VIOLATION The time could not be retrieved due to an authentication failure.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +LibGetTime (
>> +  OUT EFI_TIME                *Time,
>> +  OUT EFI_TIME_CAPABILITIES   *Capabilities
>> +  )
>> +{
>> +  EFI_STATUS  Status = EFI_SUCCESS;
>> +
>> +  // Ensure Time is a valid pointer
>> +  if (NULL == Time) {
> 
> No jeopardy comparisons please.
> if (Time == NULL)
> 
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  // Initialize the hardware if not already done
>> +  if (!RTCInitialized) {
>> +    InitializeRTC ();
>> +  }
>> +
>> +#if 0
>> +      /* fake time */
>> +      Time->Year          = 2015;
>> +      Time->Month         = 1;
>> +      Time->Day           = 1;
>> +      Time->Hour          = 0;
>> +      Time->Minute        = 0;
>> +      Time->Second        = 0;
>> +      Time->Nanosecond    = 0;
>> +#endif
> 
> No disabled code blocks, please.
> 
>> +
>> +  RTCWriteReg (RTCGETTIME, 0);
>> +  Time->Year = BCD4_2_BIN (RTCReadReg (RTCYEAR));
>> +  Time->Year += TM_YEAR_START;
>> +
>> +  Time->Month = BCD4_2_BIN (RTCReadReg (RTCMONT));
>> +  Time->Day = BCD4_2_BIN (RTCReadReg (RTCDAY));
>> +  Time->Hour = BCD4_2_BIN (RTCReadReg (RTCHOUR));
>> +  Time->Minute = BCD4_2_BIN (RTCReadReg (RTCMIN));
>> +  Time->Second = BCD4_2_BIN (RTCReadReg (RTCSEC));
>> +  Time->Nanosecond = 0;
>> +  Time->Daylight = 0;
>> +  Time->TimeZone = EFI_UNSPECIFIED_TIMEZONE;
>> +
>> +  if(!IsTimeValid (Time)) {
>> +    Status = EFI_UNSUPPORTED;
>> +  }
>> +
>> +  return Status;
>> +
>> +}
>> +
>> +
>> +/**
>> +  Sets the current local time and date information.
>> +
>> +  @param  Time                  A pointer to the current time.
>> +
>> +  @retval EFI_SUCCESS           The operation completed successfully.
>> +  @retval EFI_INVALID_PARAMETER A time field is out of range.
>> +  @retval EFI_DEVICE_ERROR      The time could not be set due due to hardware error.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +LibSetTime (
>> +  IN  EFI_TIME                *Time
>> +  )
>> +{
>> +  EFI_STATUS  Status = EFI_SUCCESS;
>> +
>> +  // Check the input parameters are within the range specified by UEFI
>> +  if(!IsTimeValid (Time)){
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  // Initialize the hardware if not already done
>> +  if (!RTCInitialized) {
>> +    InitializeRTC ();
>> +  }
>> +
>> +  RTCWriteReg (RTCSEC, BIN2BCD (Time->Second));
>> +  RTCWriteReg (RTCMIN, BIN2BCD (Time->Minute));
>> +  RTCWriteReg (RTCHOUR, BIN2BCD (Time->Hour));
>> +  RTCWriteReg (RTCDAY, BIN2BCD (Time->Day));
>> +  RTCWriteReg (RTCMONT, BIN2BCD (Time->Month));
>> +  RTCWriteReg (RTCYEAR, BIN2BCD (Time->Year - TM_YEAR_START));
>> +  return Status;
>> +}
>> +
>> +
>> +/**
>> +  Returns the current wakeup alarm clock setting.
>> +
>> +  @param  Enabled               Indicates if the alarm is currently enabled or disabled.
>> +  @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
>> +  @param  Time                  The current alarm setting.
>> +
>> +  @retval EFI_SUCCESS           The alarm settings were returned.
>> +  @retval EFI_INVALID_PARAMETER Any parameter is NULL.
>> +  @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +LibGetWakeupTime (
>> +  OUT BOOLEAN     *Enabled,
>> +  OUT BOOLEAN     *Pending,
>> +  OUT EFI_TIME    *Time
>> +  )
>> +{
>> +  // Not a required feature
>> +  return EFI_UNSUPPORTED;
>> +}
>> +
>> +
>> +/**
>> +  Sets the system wakeup alarm clock time.
>> +
>> +  @param  Enabled               Enable or disable the wakeup alarm.
>> +  @param  Time                  If Enable is TRUE, the time to set the wakeup alarm for.
>> +
>> +  @retval EFI_SUCCESS           If Enable is TRUE, then the wakeup alarm was enabled. If
>> +                                Enable is FALSE, then the wakeup alarm was disabled.
>> +  @retval EFI_INVALID_PARAMETER A time field is out of range.
>> +  @retval EFI_DEVICE_ERROR      The wakeup time could not be set due to a hardware error.
>> +  @retval EFI_UNSUPPORTED       A wakeup timer is not supported on this platform.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +LibSetWakeupTime (
>> +  IN BOOLEAN      Enabled,
>> +  OUT EFI_TIME    *Time
>> +  )
>> +{
>> +  // Not a required feature
>> +  return EFI_UNSUPPORTED;
>> +}
>> +
>> +
>> +
>> +/**
>> +  This is the declaration of an EFI image entry point. This can be the entry point to an application
>> +  written to this specification, an EFI boot service driver, or an EFI runtime driver.
>> +
>> +  @param  ImageHandle           Handle that identifies the loaded image.
>> +  @param  SystemTable           System Table for this image.
>> +
>> +  @retval EFI_SUCCESS           The operation completed successfully.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +LibRtcInitialize (
>> +  IN EFI_HANDLE                            ImageHandle,
>> +  IN EFI_SYSTEM_TABLE                      *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS    Status;
>> +  EFI_HANDLE    Handle;
>> +
>> +
> 
> Why blank lines?
> 
>> +  EFI_TIME      EfiTime;
>> +
>> +  // Setup the setters and getters
>> +  gRT->GetTime       = LibGetTime;
>> +  gRT->SetTime       = LibSetTime;
>> +  gRT->GetWakeupTime = LibGetWakeupTime;
>> +  gRT->SetWakeupTime = LibSetWakeupTime;
>> +
>> +
> 
> Just one blank line is fine.
> 
>> +  RtcBase = (UINTN)FixedPcdGet64 (PcdZxRtcClockBase);
>> +
>> +  (VOID)gRT->GetTime (&EfiTime, NULL);
>> +  if ((EfiTime.Year < 2015) || (EfiTime.Year > 2099)){
>> +      EfiTime.Year          = 2015;
>> +      EfiTime.Month         = 1;
>> +      EfiTime.Day           = 1;
>> +      EfiTime.Hour          = 0;
>> +      EfiTime.Minute        = 0;
>> +      EfiTime.Second        = 0;
>> +      EfiTime.Nanosecond    = 0;
>> +      Status = gRT->SetTime (&EfiTime);
>> +      if (EFI_ERROR (Status))
>> +      {
>> +        DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Status : %r\n", __FUNCTION__,
>> +               __LINE__, Status));
>> +      }
>> +  }
>> +
>> +  // Install the protocol
>> +  Handle = NULL;
>> +  Status = gBS->InstallMultipleProtocolInterfaces (
>> +                  &Handle,
>> +                  &gEfiRealTimeClockArchProtocolGuid,  NULL,
>> +                  NULL
>> +                 );
>> +
>> +  return Status;
>> +}
>> +
>> +
>> +/**
>> +  Fixup internal data so that EFI can be call in virtual mode.
>> +  Call the passed in Child Notify event and convert any pointers in
>> +  lib to virtual mode.
>> +
>> +  @param[in]    Event   The Event that is being processed
>> +  @param[in]    Context Event Context
>> +**/
>> +VOID
>> +EFIAPI
>> +LibRtcVirtualNotifyEvent (
>> +  IN EFI_EVENT        Event,
>> +  IN VOID             *Context
>> +  )
>> +{
>> +  //
>> +  // Only needed if you are going to support the OS calling RTC functions in virtual mode.
>> +  // You will need to call EfiConvertPointer (). To convert any stored physical addresses
>> +  // to virtual address. After the OS transitions to calling in virtual mode, all future
>> +  // runtime calls will be made in virtual mode.
>> +  //
>> +  return;
>> +}
>> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
>> new file mode 100644
>> index 0000000..3b5a4d4
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
>> @@ -0,0 +1,102 @@
>> +/** @file
>> +*
>> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +*  Copyright (c) 2017, Linaro Ltd.
>> +*
>> +*  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.
>> +*
>> +*  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
>> +**/
>> +
>> +
>> +#ifndef __DS3231_REAL_TIME_CLOCK_H__
>> +#define __DS3231_REAL_TIME_CLOCK_H__
>> +
>> +#define RTC_POWER_INI1_PARA     (0xCDBC)
>> +#define RTC_POWER_INI2_PARA     (0xCFCC)
>> +#define CONFIG_PARMETER         (0xC1CD)
>> +
>> +#define ZX_RTC_CMP_VALUE        (0x3FFF)
>> +#define WAIT_FOR_COUNT          (2000)
>> +#define INIT_DELAY              (100)
>> +
>> +
>> +/* RTC Control register description */
>> +#define RTC_CTRL_STOP                   (~(0x1 << 0))
>> +#define RTC_CTRL_RUN                    (0x1 << 0)
>> +#define RTC_CTRL_ROUND30S               (0x1 << 1)
>> +#define RTC_CTRL_AUTO_COMPENSATION      (0x1 << 2)
>> +#define RTC_CTRL_MODULE12               (0x1 << 3)
>> +#define RTC_CTRL_MODULE24               (~(0x1 << 3))
>> +#define RTC_CTRL_SET_32_COUNTER         (0x1 << 5)
>> +#define RTC_CTRL_SOFT_RESET             (0x1 << 6)
>> +#define RTC_CTRL_CLK_32K_OUTEN          (0x1 << 8)
>> +
>> +#define RTC_CTRL_BIT6_0                 ( ~(0x1 << 6))
>> +#define RTC_CTRL_BIT6_1                 (0x1 << 6)
>> +
>> +
>> +
>> +/* RTC Interrupt register description */
>> +#define RTC_EVERY_MASK          (0x3 << 0)
>> +#define RTC_EVERY_SEC           0x00                    /* second periodic intrrupt */
>> +#define RTC_EVERY_MIN           0x01                    /* minute periodic interrupt */
>> +#define RTC_EVERY_HR            0x02                    /* hour periodic interrupt */
>> +#define RTC_EVERY_DAY           0x03                    /* day periodic interrupt */
>> +#define RTC_IT_TIMER            (0x1 << 2)              /* Enable periodic interrupt */
>> +#define RTC_IT_ALARM            (0x1 << 3)              /* Enable alarm clock interrupt */
>> +#define RTC_IT_MASK             (0x3 << 2)
>> +
>> +/* RTC Status register description */
>> +#define RTC_BUSY                (0x1 << 0)              /* Read-only, indicate refresh*/
>> +#define RTC_RUN                 (0x1 << 1)              /* Read-only, RTC is running */
>> +#define RTC_ALARM               (0x1 << 6)              /* Read/Write, Alarm interrupt has been generated */
>> +#define RTC_TIMER               (0x1 << 7)              /* Read/Write, Timer interrupt has been generated */
>> +#define RTC_POWER_UP            (0x1 << 8)              /* Read/Write, Reset */
>> +
>> +#define TM_YEAR_START               1900
>> +
>> +#define TM_MONTH_OFFSET             1
>> +
>> +#define TM_WDAY_SUNDAY              0
>> +#define ZX_RTC_SUNDAY               7
>> +
>> +#define BCD2BIN(val)    (((val) & 0x0f) + ((val) >> 4) * 10)
>> +#define BIN2BCD(val)    ((((val) / 10) << 4) + (val) % 10)
>> +
>> +#define BCD4_2_BIN(x)   (( (x) & 0x0F) +                    \
>> +                        ((((x) & 0x0F0) >>   4) * 10)  +    \
>> +                        ((((x) & 0xF00) >>   8) * 100) +    \
>> +                        ((((x) & 0xF000) >> 12) * 1000))
>> +
>> +
>> +#define BIN_2_BCD4(x)   (((x % 10) & 0x0F)       |          \
>> +                        (((x /10 ) % 10)  <<  4) |          \
>> +                        (((x /100) % 10)  <<  8) |          \
>> +                        (((x /1000) % 10) << 12))
>> +
>> +/* RTC register offset */
>> +#define RTCVER          0
>> +#define RTCSEC          4
>> +#define RTCMIN          8
>> +#define RTCHOUR         0xc
>> +#define RTCDAY          0x10
>> +#define RTCMONT         0x14
>> +#define RTCYEAR         0x18
>> +#define RTCWEEK         0x1c
>> +#define RTCCTL          0x38
>> +#define RTCSTS          0x3c
>> +#define RTCINT          0x40
>> +#define RTCCFGID        0x48
>> +#define RTCPOWERINIT1   0x4c
>> +#define RTCPOWERINIT2   0x50
>> +#define RTCGETTIME      0x54
>> +#define RTCCLKCNT       0x58
>> +
>> +#endif
>> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
>> new file mode 100644
>> index 0000000..0a6852b
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
>> @@ -0,0 +1,42 @@
>> +#/** @file
>> +#
>> +# Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution. The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +#
>> +#**/
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010005
> 
> 0x00010019.
> (Or 1.25.)
> 
>> +  BASE_NAME                      = Zx296718RealTimeClockLib
>> +  FILE_GUID                      = 4e1aaa26-597c-4f01-a8fc-fdf1adc1400f
>> +  MODULE_TYPE                    = BASE
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = RealTimeClockLib
>> +
>> +[Sources.common]
>> +  Zx296718RealTimeClock.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  Silicon/Sanchip/SanchipPkg.dec
> 
> Please sort alphabetically.
> 
>> +
>> +[LibraryClasses]
>> +  IoLib
>> +  UefiLib
>> +  DebugLib
>> +  PcdLib
>> +  TimerLib
>> +# Use EFiAtRuntime to check stage
>> +  UefiRuntimeLib
> 
> Please sort alphabetically >
>> +
>> +[FixedPcd]
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq
>> -- 
>> 1.9.1
>>

All other coding style and inf version comments will be addressed.

Re: [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Leif Lindholm]

On Thu, Aug 17, 2017 at 11:43:37PM +0800, Jun Nie wrote:
> On 2017年08月10日 22:03, Leif Lindholm wrote:
> > On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:
> > > Runtime service is not supported yet.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > >   .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 +++++++++++++++++++++
> > >   .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++++++
> > >   .../Zx296718RealTimeClock.inf                      |  42 +++
> > >   3 files changed, 520 insertions(+)
> > >   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> > >   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
> > >   create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> > > 
> > > diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> > > new file mode 100644
> > > index 0000000..af6e5bd
> > > --- /dev/null
> > > +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> > > @@ -0,0 +1,376 @@
> > > +/** @file
> > > +  Implement EFI RealTimeClock runtime services via RTC Lib.
> > > +
> > > +  Currently this driver does not support runtime virtual calling.
> > > +
> > > +  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> > > +  Copyright (c) 2017, Linaro Limited.
> > > +
> > > +  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.
> > > +
> > > +  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
> > > +
> > > +**/
> > > +
> > > +#include <Uefi.h>
> > > +#include <PiDxe.h>
> > 
> > P before U.
> > 
> > > +#include <Library/BaseLib.h>
> > > +#include <Library/BaseMemoryLib.h>
> > > +#include <Library/DebugLib.h>
> > > +#include <Library/IoLib.h>
> > > +#include <Library/MemoryAllocationLib.h>
> > > +#include <Library/PcdLib.h>
> > > +#include <Library/TimerLib.h>
> > > +#include <Library/UefiBootServicesTableLib.h>
> > > +#include <Library/UefiLib.h>
> > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > +// Use EfiAtRuntime to check stage
> > > +#include <Library/UefiRuntimeLib.h>
> > 
> > L (UefiRuntimeLib) before S (UefiRuntimeServices...).
> > No need for a comment explaining why we include headers.
> > 
> > > +#include <Protocol/RealTimeClock.h>
> > > +#include "Zx296718RealTimeClock.h"
> > > +
> > > +STATIC UINTN       RtcBase;
> > 
> > mRtcBase.
> > 
> > > +STATIC BOOLEAN     RTCInitialized = FALSE;
> > 
> > mRTCInitialized.
> > 
> > > +
> > > +BOOLEAN
> > > +EFIAPI
> > > +IsTimeValid(
> > > +  IN EFI_TIME *Time
> > > +  )
> > > +{
> > > +  // Check the input parameters are within the range specified by UEFI
> > > +  if ((Time->Year  < 2000) ||
> > > +     (Time->Year   > 2099) ||
> > > +     (Time->Month  < 1   ) ||
> > > +     (Time->Month  > 12  ) ||
> > > +     (Time->Day    < 1   ) ||
> > > +     (Time->Day    > 31  ) ||
> > > +     (Time->Hour   > 23  ) ||
> > > +     (Time->Minute > 59  ) ||
> > > +     (Time->Second > 59  ) ||
> > > +     (Time->Nanosecond > 999999999) ||
> > > +     (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) && (Time->TimeZone <= 1440)))) ||
> > > +     (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT)))
> > > +  ) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  return TRUE;
> > > +}
> > 
> > This appears a direct copy of the version in
> > EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library
> > dependency to decrease duplication?
> > (This may not have existed as a standalone library at the time you
> > started this port.)
> > 
> > > +
> > > +VOID
> > 
> > A lot of the functions in this file could do with a STATIC prefix,
> > since they are only used internally.
> > 
> > > +Wait4Busy (
> > 
> > Semantically, this name is incorrect.
> > The function is waiting _while_ the state is RTC_BUSY, so seems to me
> > it should be called WaitBusy.
> > 
> > > +  VOID
> > > +  )
> > > +{
> > > +  UINT32 Val, Retry = 1000;
> > > +  do {
> > > +    MicroSecondDelay (200);
> > 
> > Why 200?
> 
> It is just a suggested delay time according to RTC clock frequency. You want
> a RTC_WAIT_DELAY macro here for better understanding, right?

That would be more clear, yes.
It is also good if next to that definition there is some description
of how the value was detemined. (I.e., are we waiting for a particular
number of cycles, or is this simply a determined to be "good enough".)

/
    Leif

Re: [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Leif Lindholm]

Just another thought.

On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:
> diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
> new file mode 100644
> index 0000000..3b5a4d4
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
> @@ -0,0 +1,102 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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.
> +*
> +*  Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
> +**/
> +
> +
> +#ifndef __DS3231_REAL_TIME_CLOCK_H__
> +#define __DS3231_REAL_TIME_CLOCK_H__
> +
> +#define RTC_POWER_INI1_PARA     (0xCDBC)
> +#define RTC_POWER_INI2_PARA     (0xCFCC)
> +#define CONFIG_PARMETER         (0xC1CD)
> +
> +#define ZX_RTC_CMP_VALUE        (0x3FFF)
> +#define WAIT_FOR_COUNT          (2000)
> +#define INIT_DELAY              (100)
> +
> +
> +/* RTC Control register description */
> +#define RTC_CTRL_STOP                   (~(0x1 << 0))
> +#define RTC_CTRL_RUN                    (0x1 << 0)
> +#define RTC_CTRL_ROUND30S               (0x1 << 1)
> +#define RTC_CTRL_AUTO_COMPENSATION      (0x1 << 2)
> +#define RTC_CTRL_MODULE12               (0x1 << 3)
> +#define RTC_CTRL_MODULE24               (~(0x1 << 3))
> +#define RTC_CTRL_SET_32_COUNTER         (0x1 << 5)
> +#define RTC_CTRL_SOFT_RESET             (0x1 << 6)
> +#define RTC_CTRL_CLK_32K_OUTEN          (0x1 << 8)
> +
> +#define RTC_CTRL_BIT6_0                 ( ~(0x1 << 6))
> +#define RTC_CTRL_BIT6_1                 (0x1 << 6)
> +
> +
> +
> +/* RTC Interrupt register description */
> +#define RTC_EVERY_MASK          (0x3 << 0)
> +#define RTC_EVERY_SEC           0x00                    /* second periodic intrrupt */
> +#define RTC_EVERY_MIN           0x01                    /* minute periodic interrupt */
> +#define RTC_EVERY_HR            0x02                    /* hour periodic interrupt */
> +#define RTC_EVERY_DAY           0x03                    /* day periodic interrupt */
> +#define RTC_IT_TIMER            (0x1 << 2)              /* Enable periodic interrupt */
> +#define RTC_IT_ALARM            (0x1 << 3)              /* Enable alarm clock interrupt */
> +#define RTC_IT_MASK             (0x3 << 2)
> +
> +/* RTC Status register description */
> +#define RTC_BUSY                (0x1 << 0)              /* Read-only, indicate refresh*/
> +#define RTC_RUN                 (0x1 << 1)              /* Read-only, RTC is running */
> +#define RTC_ALARM               (0x1 << 6)              /* Read/Write, Alarm interrupt has been generated */
> +#define RTC_TIMER               (0x1 << 7)              /* Read/Write, Timer interrupt has been generated */
> +#define RTC_POWER_UP            (0x1 << 8)              /* Read/Write, Reset */
> +
> +#define TM_YEAR_START               1900
> +
> +#define TM_MONTH_OFFSET             1
> +
> +#define TM_WDAY_SUNDAY              0
> +#define ZX_RTC_SUNDAY               7
> +
> +#define BCD2BIN(val)    (((val) & 0x0f) + ((val) >> 4) * 10)
> +#define BIN2BCD(val)    ((((val) / 10) << 4) + (val) % 10)

Are these not equivalent to DecimalToBcd8/BcdToDecimal8 in BaseLib?
If so, could we drop these and use the BasLib versions in the code?

> +
> +#define BCD4_2_BIN(x)   (( (x) & 0x0F) +                    \
> +                        ((((x) & 0x0F0) >>   4) * 10)  +    \
> +                        ((((x) & 0xF00) >>   8) * 100) +    \
> +                        ((((x) & 0xF000) >> 12) * 1000))
> +
> +
> +#define BIN_2_BCD4(x)   (((x % 10) & 0x0F)       |          \
> +                        (((x /10 ) % 10)  <<  4) |          \
> +                        (((x /100) % 10)  <<  8) |          \
> +                        (((x /1000) % 10) << 12))
> +

And would these not be DecimalToBcd16/BcdToDecimal16? Should we add
those to BaseLib?

/
    Leif

Re: [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:
> Add boot manager lib and entries, including Android and Grub.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c        | 105 ++++++
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf      |  66 ++++
>  .../Library/PlatformBootManagerLib/PlatformBm.c    | 404 +++++++++++++++++++++
>  .../Library/PlatformBootManagerLib/PlatformBm.h    |  30 ++
>  .../PlatformBootManagerLib.inf                     |  91 +++++
>  Silicon/Sanchip/SanchipPkg.dec                     |  29 ++
>  6 files changed, 725 insertions(+)
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 Silicon/Sanchip/SanchipPkg.dec
> 
> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> new file mode 100644
> index 0000000..47d02bf
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> @@ -0,0 +1,105 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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/BdsLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/AndroidBootImg.h>
> +#include <Protocol/BlockIo.h>

Which aspect of BlockIo is used here?
On the whole, several of the above includes look unused - could you
prune them back a bit?

> +
> +#include <libfdt.h>
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgAppendKernelArgs (
> +  IN CHAR16            *Args,
> +  IN UINTN              Size
> +  )
> +{
> +  UnicodeSPrint (
> +    Args + StrLen (Args), Size - StrLen (Args), L" efi=noruntime");

I am not sure what your intent is here, but I am entirely convinced
there is a better way to achieve it. Can you give some background,
please?

> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgUpdateDtb (
> +  IN  EFI_PHYSICAL_ADDRESS        OrigFdtBase,
> +  OUT EFI_PHYSICAL_ADDRESS       *NewFdtBase
> +  )
> +{
> +  UINTN             FdtSize, NumPages;
> +  INTN              err;
> +  EFI_STATUS        Status;
> +
> +  //
> +  // Store the FDT as Runtime Service Data to prevent the Kernel from
> +  // overwritting its data.
> +  //

This should really not be necessary and has never been needed
elsewhere. Have you seen this cause any change in behaviour in an
actual system?

> +  FdtSize = fdt_totalsize ((VOID *)(UINTN)OrigFdtBase);
> +  NumPages = EFI_SIZE_TO_PAGES (FdtSize) + 20;
> +  Status = gBS->AllocatePages (
> +                  AllocateAnyPages, EfiRuntimeServicesData,
> +                  NumPages, NewFdtBase);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  CopyMem (
> +    (VOID*)(UINTN)*NewFdtBase,
> +    (VOID*)(UINTN)OrigFdtBase,
> +    FdtSize
> +    );
> +
> +  fdt_pack ((VOID*)(UINTN)*NewFdtBase);
> +  err = fdt_check_header ((VOID*)(UINTN)*NewFdtBase);

This does not feel like it belongs here. Check it when you first load
it, by all means, but there is no need to keep checking it did not get
destroyed by normal execution.

> +  if (err != 0) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (err:%d)\n", err));
> +    gBS->FreePages (*NewFdtBase, NumPages);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +ANDROID_BOOTIMG_PROTOCOL mAndroidBootImg = {
> +  AndroidBootImgAppendKernelArgs,
> +  AndroidBootImgUpdateDtb
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +Zx296718EvbEntryPoint (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS           Status;
> +
> +  Status = gBS->InstallProtocolInterface (
> +                  &ImageHandle,
> +                  &gAndroidBootImgProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mAndroidBootImg
> +                  );
> +  return Status;
> +}
> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> new file mode 100644
> index 0000000..03982d1
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> @@ -0,0 +1,66 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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                    = 0x00010005

0x00010019/1.25.

> +  BASE_NAME                      = Zx296718EvbDxe
> +  FILE_GUID                      = db154b2a-031f-4f3c-9315-c4697ff33e6c
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = Zx296718EvbEntryPoint
> +
> +[Sources.common]
> +  Zx296718EvbDxe.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

M (MdeM) before P (MdeP).

> +  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  BdsLib
> +  CacheMaintenanceLib
> +  DebugLib
> +  DxeServicesLib
> +  DxeServicesTableLib
> +  FdtLib
> +  IoLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiRuntimeServicesTableLib
> +  UefiLib

L before R.

> +  UefiDriverEntryPoint

But D before L.

But also, how many of these LibraryClasses are actually used by this
driver?

> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiFileInfoGuid
> +  gEfiGlobalVariableGuid
> +  gFdtTableGuid
> +  gFdtVariableGuid

How many of these Guids are actually used by this driver?

> +
> +[Protocols]
> +  gAndroidBootImgProtocolGuid
> +  gEfiBlockIoProtocolGuid
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiDevicePathToTextProtocolGuid
> +  gEfiDevicePathProtocolGuid
> +  gEfiSimpleFileSystemProtocolGuid

How many of these protocols are actually used by this driver?

> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c

It looks to me like this implementation (perhaps with a few small
modifications) should be reusable across multiple implementations.

That does not need to happen for this to be merged, but it would be
good if you and Haojian could discuss between yourselves. (Also,
perhaps we should all sit down and discuss this at Linaro Connect next
month.)

> new file mode 100644
> index 0000000..41861e2
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -0,0 +1,404 @@
> +/** @file
> +  Implementation for PlatformBootManagerLib library class interfaces.
> +
> +  Copyright (C) 2015-2016, Red Hat, Inc.
> +  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/DevicePathLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +#include <Library/UefiLib.h>
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/DevicePathFromText.h>
> +#include <Protocol/DevicePathToText.h>
> +#include <Protocol/LoadedImage.h>
> +#include <Guid/EventGroup.h>
> +#include <Guid/TtyTerm.h>

Can you verify all of the above are actually used by this file?

> +
> +#include "PlatformBm.h"
> +
> +#define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
> +
> +#define GRUB_FILE_NAME       L"\\EFI\\BOOT\\GRUBAA64.EFI"
> +
> +
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH         SerialDxe;
> +  UART_DEVICE_PATH           Uart;
> +  VENDOR_DEFINED_DEVICE_PATH TermType;
> +  EFI_DEVICE_PATH_PROTOCOL   End;
> +} PLATFORM_SERIAL_CONSOLE;
> +#pragma pack ()
> +
> +#define SERIAL_DXE_FILE_GUID { \
> +          0xD3987D4B, 0x971A, 0x435F, \
> +          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
> +          }
> +
> +STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
> +  //
> +  // VENDOR_DEVICE_PATH SerialDxe
> +  //
> +  {
> +    { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, DP_NODE_LEN (VENDOR_DEVICE_PATH) },
> +    SERIAL_DXE_FILE_GUID
> +  },
> +
> +  //
> +  // UART_DEVICE_PATH Uart
> +  //
> +  {
> +    { MESSAGING_DEVICE_PATH, MSG_UART_DP, DP_NODE_LEN (UART_DEVICE_PATH) },
> +    0,                                      // Reserved
> +    FixedPcdGet64 (PcdUartDefaultBaudRate), // BaudRate
> +    FixedPcdGet8 (PcdUartDefaultDataBits),  // DataBits
> +    FixedPcdGet8 (PcdUartDefaultParity),    // Parity
> +    FixedPcdGet8 (PcdUartDefaultStopBits)   // StopBits
> +  },
> +
> +  //
> +  // VENDOR_DEFINED_DEVICE_PATH TermType
> +  //
> +  {
> +    {
> +      MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
> +      DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
> +    }
> +    //
> +    // Guid to be filled in dynamically
> +    //
> +  },
> +
> +  //
> +  // EFI_DEVICE_PATH_PROTOCOL End
> +  //
> +  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    DP_NODE_LEN (EFI_DEVICE_PATH_PROTOCOL)
> +  }
> +};
> +
> +/**
> +  Check if the handle satisfies a particular condition.
> +
> +  @param[in] Handle      The handle to check.
> +  @param[in] ReportText  A caller-allocated string passed in for reporting
> +                         purposes. It must never be NULL.
> +
> +  @retval TRUE   The condition is satisfied.
> +  @retval FALSE  Otherwise. This includes the case when the condition could not
> +                 be fully evaluated due to an error.
> +**/
> +typedef
> +BOOLEAN
> +(EFIAPI *FILTER_FUNCTION) (
> +  IN EFI_HANDLE   Handle,
> +  IN CONST CHAR16 *ReportText
> +  );
> +
> +
> +/**
> +  Process a handle.
> +
> +  @param[in] Handle      The handle to process.
> +  @param[in] ReportText  A caller-allocated string passed in for reporting
> +                         purposes. It must never be NULL.
> +**/
> +typedef
> +VOID
> +(EFIAPI *CALLBACK_FUNCTION)  (
> +  IN EFI_HANDLE   Handle,
> +  IN CONST CHAR16 *ReportText
> +  );
> +
> +STATIC
> +VOID
> +PlatformRegisterFvBootOption (
> +  EFI_GUID                         *FileGuid,
> +  CHAR16                           *Description,
> +  UINT32                           Attributes
> +  )
> +{
> +  EFI_STATUS                        Status;
> +  INTN                              OptionIndex;
> +  EFI_BOOT_MANAGER_LOAD_OPTION      NewOption;
> +  EFI_BOOT_MANAGER_LOAD_OPTION      *BootOptions;
> +  UINTN                             BootOptionCount;
> +  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
> +  EFI_LOADED_IMAGE_PROTOCOL         *LoadedImage;
> +  EFI_DEVICE_PATH_PROTOCOL          *DevicePath;
> +
> +  Status = gBS->HandleProtocol (
> +                  gImageHandle,
> +                  &gEfiLoadedImageProtocolGuid,
> +                  (VOID **) &LoadedImage
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
> +  DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
> +  ASSERT (DevicePath != NULL);
> +  DevicePath = AppendDevicePathNode (
> +                 DevicePath,
> +                 (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
> +                 );
> +  ASSERT (DevicePath != NULL);
> +
> +  Status = EfiBootManagerInitializeLoadOption (
> +             &NewOption,
> +             LoadOptionNumberUnassigned,
> +             LoadOptionTypeBoot,
> +             Attributes,
> +             Description,
> +             DevicePath,
> +             NULL,
> +             0
> +             );
> +  ASSERT_EFI_ERROR (Status);
> +  FreePool (DevicePath);
> +
> +  BootOptions = EfiBootManagerGetLoadOptions (
> +                  &BootOptionCount, LoadOptionTypeBoot
> +                  );
> +
> +  OptionIndex = EfiBootManagerFindLoadOption (
> +                  &NewOption, BootOptions, BootOptionCount
> +                  );
> +
> +  if (OptionIndex == -1) {
> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  EfiBootManagerFreeLoadOption (&NewOption);
> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +}
> +
> +
> +STATIC
> +VOID
> +PlatformRegisterBootGrub (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                           Status;
> +  CHAR16                              *BootPathStr;
> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
> +  EFI_DEVICE_PATH                     *DevicePath;
> +  EFI_DEVICE_PATH                     *FileDevicePath;
> +  FILEPATH_DEVICE_PATH                *FilePath;
> +  UINTN                                Size;
> +  EFI_BOOT_MANAGER_LOAD_OPTION         NewOption;
> +  EFI_BOOT_MANAGER_LOAD_OPTION        *BootOptions;
> +  UINTN                                BootOptionCount;
> +  INTN                                 OptionIndex;
> +
> +  //
> +  // Get PcdAndroidBootDevicePath
> +  //
> +  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
> +  ASSERT (BootPathStr != NULL);
> +  Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, NULL,
> +                                (VOID **)&EfiDevicePathFromTextProtocol);
> +  ASSERT_EFI_ERROR(Status);
> +  DevicePath = (EFI_DEVICE_PATH *)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
> +  ASSERT (DevicePath != NULL);
> +
> +  Size = StrSize (GRUB_FILE_NAME);
> +  FileDevicePath = AllocatePool (Size + SIZE_OF_FILEPATH_DEVICE_PATH + END_DEVICE_PATH_LENGTH);
> +  if (FileDevicePath != NULL) {
> +    FilePath = (FILEPATH_DEVICE_PATH *) FileDevicePath;
> +    FilePath->Header.Type    = MEDIA_DEVICE_PATH;
> +    FilePath->Header.SubType = MEDIA_FILEPATH_DP;
> +    CopyMem (&FilePath->PathName, GRUB_FILE_NAME, Size);
> +    SetDevicePathNodeLength (&FilePath->Header, Size + SIZE_OF_FILEPATH_DEVICE_PATH);
> +    SetDevicePathEndNode (NextDevicePathNode (&FilePath->Header));
> +
> +    DevicePath = AppendDevicePath (DevicePath, FileDevicePath);
> +    FreePool (FileDevicePath);
> +  }
> +  Status = EfiBootManagerInitializeLoadOption (
> +             &NewOption,
> +             LoadOptionNumberUnassigned,
> +             LoadOptionTypeBoot,
> +             LOAD_OPTION_ACTIVE,
> +             L"Grub",
> +             DevicePath,
> +             NULL,
> +             0
> +             );
> +  ASSERT_EFI_ERROR (Status);
> +  FreePool (DevicePath);
> +
> +  BootOptions = EfiBootManagerGetLoadOptions (
> +                  &BootOptionCount, LoadOptionTypeBoot
> +                  );
> +
> +  OptionIndex = EfiBootManagerFindLoadOption (
> +                  &NewOption, BootOptions, BootOptionCount
> +                  );
> +
> +  if (OptionIndex == -1) {
> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  EfiBootManagerFreeLoadOption (&NewOption);
> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +
> +}
> +
> +STATIC
> +VOID
> +PlatformRegisterOptionsAndKeys (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                           Status;
> +  EFI_INPUT_KEY                        Enter;
> +  EFI_INPUT_KEY                        Esc;
> +  EFI_BOOT_MANAGER_LOAD_OPTION         BootOption;
> +
> +  //
> +  // Register Boot. OptionNumber is 1.
> +  //
> +  PlatformRegisterBootGrub ();
> +
> +  //
> +  // Register Android Boot. OptionNumber is 2.
> +  //
> +  PlatformRegisterFvBootOption (
> +    PcdGetPtr (PcdAndroidBootFile), L"Android Boot", LOAD_OPTION_ACTIVE
> +    );
> +
> +  //
> +  // Register ENTER as CONTINUE key
> +  //
> +  Enter.ScanCode    = SCAN_NULL;
> +  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
> +  Status = EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Map ESC to Boot Manager Menu
> +  //
> +  Esc.ScanCode    = SCAN_ESC;
> +  Esc.UnicodeChar = CHAR_NULL;
> +  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
> +  ASSERT_EFI_ERROR (Status);
> +  Status = EfiBootManagerAddKeyOptionVariable (
> +             NULL, (UINT16) BootOption.OptionNumber, 0, &Esc, NULL
> +             );
> +  ASSERT (Status == EFI_SUCCESS || Status == EFI_ALREADY_STARTED);
> +}
> +
> +
> +//
> +// BDS Platform Functions
> +//
> +/**
> +  Do the platform init, can be customized by OEM/IBV
> +  Possible things that can be done in PlatformBootManagerBeforeConsole:
> +  > Update console variable: 1. include hot-plug devices;
> +  >                          2. Clear ConIn and add SOL for AMT
> +  > Register new Driver#### or Boot####
> +  > Register new Key####: e.g.: F12
> +  > Signal ReadyToLock event
> +  > Authentication action: 1. connect Auth devices;
> +  >                        2. Identify auto logon user.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerBeforeConsole (
> +  VOID
> +  )
> +{
> +  //
> +  // Signal EndOfDxe PI Event
> +  //
> +  EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
> +
> +  //
> +  // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
> +  //
> +  ASSERT (FixedPcdGet8 (PcdDefaultTerminalType) == 4);
> +  CopyGuid (&mSerialConsole.TermType.Guid, &gEfiTtyTermGuid);
> +
> +  EfiBootManagerUpdateConsoleVariable (ConIn,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +
> +  //
> +  // Register platform-specific boot options and keyboard shortcuts.
> +  //
> +  PlatformRegisterOptionsAndKeys ();
> +}
> +
> +/**
> +  Do the platform specific action after the console is ready
> +  Possible things that can be done in PlatformBootManagerAfterConsole:
> +  > Console post action:
> +    > Dynamically switch output mode from 100x31 to 80x25 for certain senarino
> +    > Signal console ready platform customized event
> +  > Run diagnostics like memory testing
> +  > Connect certain devices
> +  > Dispatch aditional option roms
> +  > Special boot
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerAfterConsole (
> +  VOID
> +  )
> +{
> +  Print (L"Press ESCAPE for boot options ");
> +
> +  //
> +  // Connect the rest of the devices.
> +  //
> +  EfiBootManagerConnectAll ();
> +
> +  //
> +  // Enumerate all possible boot options.
> +  //
> +  EfiBootManagerRefreshAllBootOption ();
> +
> +  //
> +  // Register UEFI Shell
> +  //
> +  PlatformRegisterFvBootOption (
> +    PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE
> +    );
> +}
> +
> +/**
> +  This function is called each second during the boot manager waits the
> +  timeout.
> +
> +  @param TimeoutRemain  The remaining timeout.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerWaitCallback (
> +  UINT16          TimeoutRemain
> +  )
> +{
> +  Print (L".");
> +}
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
> new file mode 100644
> index 0000000..de52a9c
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
> @@ -0,0 +1,30 @@
> +/** @file
> +  Head file for BDS Platform specific code
> +
> +  Copyright (C) 2015-2016, Red Hat, Inc.
> +  Copyright (c) 2004 - 2008, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef _PLATFORM_BM_H_
> +#define _PLATFORM_BM_H_
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>

If all this holds is a set of include files for a single source file,
may they not just go directly into the .c file?

> +
> +#endif // _PLATFORM_BM_H_
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> new file mode 100644
> index 0000000..b9c6aaa
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -0,0 +1,91 @@
> +## @file
> +#  Implementation for PlatformBootManagerLib library class interfaces.
> +#
> +#  Copyright (C) 2015-2016, Red Hat, Inc.
> +#  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials are licensed and made available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution. The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> +#  IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

0x00010019/1.25.

> +  BASE_NAME                      = PlatformBootManagerLib
> +  FILE_GUID                      = 9001ec12-8757-4638-b6d0-e0863dddddb0
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformBootManagerLib|DXE_DRIVER
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = ARM AARCH64
> +#
> +
> +[Sources]
> +  PlatformBm.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Sanchip/SanchipPkg.dec

Are all of these actually used?

> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DevicePathLib
> +  DxeServicesLib
> +  MemoryAllocationLib
> +  PcdLib
> +  PrintLib
> +  UefiBootManagerLib
> +  UefiBootServicesTableLib
> +  UefiLib

Are all of these actually used?

> +
> +[FeaturePcd]
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootlogoOnlyEnable
> +  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport

Are all of these actually used?

> +
> +[FixedPcd]
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType

Are all of these actually used?

> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile

Are all of these actually used?

> +
> +[Guids]
> +  gEfiFileInfoGuid
> +  gEfiFileSystemInfoGuid
> +  gEfiFileSystemVolumeLabelInfoIdGuid
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiTtyTermGuid

Are all of these actually used?

> +
> +[Protocols]
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiDevicePathProtocolGuid
> +  gEfiLoadedImageProtocolGuid
> +  gEfiOEMBadgingProtocolGuid
> +  gEfiPciRootBridgeIoProtocolGuid
> +  gEfiSimpleFileSystemProtocolGuid
> +  gEfiDevicePathToTextProtocolGuid

Are all of these actually used?

> +
> +[Pcd]
> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath
> diff --git a/Silicon/Sanchip/SanchipPkg.dec b/Silicon/Sanchip/SanchipPkg.dec
> new file mode 100644
> index 0000000..92d439d
> --- /dev/null
> +++ b/Silicon/Sanchip/SanchipPkg.dec
> @@ -0,0 +1,29 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 0x00010005

0x00010019/1.25.

> +  PACKAGE_NAME                   = SanchipPkg
> +  PACKAGE_GUID                   = f991248f-9e21-4e4f-b344-eaad28e42ec0
> +  PACKAGE_VERSION                = 0.1
> +
> +[Includes.common]
> +  Include                        # Root include for the package

This directory does not exist, causing compliation to fail.

> +
> +[Guids.common]
> +  gSanchipTokenSpaceGuid          =  { 0x9589ba86, 0x58e6, 0x426c, { 0xbb, 0x39, 0x21, 0xf5, 0x68, 0x78, 0xe4, 0x27 } }
> +
> +[PcdsFixedAtBuild.common]
> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000003

What?

> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase|0|UINT64|0x00000002
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq|24000|UINT64|0x00000001
> -- 
> 1.9.1
> 

Re: [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Jun Nie]

On 2017年08月10日 22:41, Leif Lindholm wrote:
> On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:
>> Add boot manager lib and entries, including Android and Grub.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>   .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c        | 105 ++++++
>>   .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf      |  66 ++++
>>   .../Library/PlatformBootManagerLib/PlatformBm.c    | 404 +++++++++++++++++++++
>>   .../Library/PlatformBootManagerLib/PlatformBm.h    |  30 ++
>>   .../PlatformBootManagerLib.inf                     |  91 +++++
>>   Silicon/Sanchip/SanchipPkg.dec                     |  29 ++
>>   6 files changed, 725 insertions(+)
>>   create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>>   create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>>   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
>>   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>>   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>>   create mode 100644 Silicon/Sanchip/SanchipPkg.dec
>>
>> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>> new file mode 100644
>> index 0000000..47d02bf
>> --- /dev/null
>> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>> @@ -0,0 +1,105 @@
>> +/** @file
>> +*
>> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +*  Copyright (c) 2017, Linaro Ltd.
>> +*
>> +*  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/BdsLib.h>
>> +#include <Library/CacheMaintenanceLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/PrintLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +
>> +#include <Protocol/AndroidBootImg.h>
>> +#include <Protocol/BlockIo.h>
> 
> Which aspect of BlockIo is used here?
> On the whole, several of the above includes look unused - could you
> prune them back a bit?

Sure.
> 
>> +
>> +#include <libfdt.h>
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +AndroidBootImgAppendKernelArgs (
>> +  IN CHAR16            *Args,
>> +  IN UINTN              Size
>> +  )
>> +{
>> +  UnicodeSPrint (
>> +    Args + StrLen (Args), Size - StrLen (Args), L" efi=noruntime");
> 
> I am not sure what your intent is here, but I am entirely convinced
> there is a better way to achieve it. Can you give some background,
> please?

RTC driver is lack of runtime service, so register access result kernel 
panic. After RTC driver support runtime service, I already delete this 
function  in later version.
> 
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +AndroidBootImgUpdateDtb (
>> +  IN  EFI_PHYSICAL_ADDRESS        OrigFdtBase,
>> +  OUT EFI_PHYSICAL_ADDRESS       *NewFdtBase
>> +  )
>> +{
>> +  UINTN             FdtSize, NumPages;
>> +  INTN              err;
>> +  EFI_STATUS        Status;
>> +
>> +  //
>> +  // Store the FDT as Runtime Service Data to prevent the Kernel from
>> +  // overwritting its data.
>> +  //
> 
> This should really not be necessary and has never been needed
> elsewhere. Have you seen this cause any change in behaviour in an
> actual system?

This is copied from Hisilicon code and I had thought it is for safety. I 
already delete this function in later version.
> 
>> +  FdtSize = fdt_totalsize ((VOID *)(UINTN)OrigFdtBase);
>> +  NumPages = EFI_SIZE_TO_PAGES (FdtSize) + 20;
>> +  Status = gBS->AllocatePages (
>> +                  AllocateAnyPages, EfiRuntimeServicesData,
>> +                  NumPages, NewFdtBase);
>> +  if (EFI_ERROR (Status)) {
>> +    return EFI_BUFFER_TOO_SMALL;
>> +  }
>> +
>> +  CopyMem (
>> +    (VOID*)(UINTN)*NewFdtBase,
>> +    (VOID*)(UINTN)OrigFdtBase,
>> +    FdtSize
>> +    );
>> +
>> +  fdt_pack ((VOID*)(UINTN)*NewFdtBase);
>> +  err = fdt_check_header ((VOID*)(UINTN)*NewFdtBase);
> 
> This does not feel like it belongs here. Check it when you first load
> it, by all means, but there is no need to keep checking it did not get
> destroyed by normal execution.

Yes, it can be removed after check logic is added in library.
> 
>> +  if (err != 0) {
>> +    DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (err:%d)\n", err));
>> +    gBS->FreePages (*NewFdtBase, NumPages);
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +ANDROID_BOOTIMG_PROTOCOL mAndroidBootImg = {
>> +  AndroidBootImgAppendKernelArgs,
>> +  AndroidBootImgUpdateDtb
>> +};
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +Zx296718EvbEntryPoint (
>> +  IN EFI_HANDLE         ImageHandle,
>> +  IN EFI_SYSTEM_TABLE   *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS           Status;
>> +
>> +  Status = gBS->InstallProtocolInterface (
>> +                  &ImageHandle,
>> +                  &gAndroidBootImgProtocolGuid,
>> +                  EFI_NATIVE_INTERFACE,
>> +                  &mAndroidBootImg
>> +                  );
>> +  return Status;
>> +}
>> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>> new file mode 100644
>> index 0000000..03982d1
>> --- /dev/null
>> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>> @@ -0,0 +1,66 @@
>> +#
>> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +#  Copyright (c) 2017, Linaro Ltd.
>> +#
>> +#  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                    = 0x00010005
> 
> 0x00010019/1.25.
> 
>> +  BASE_NAME                      = Zx296718EvbDxe
>> +  FILE_GUID                      = db154b2a-031f-4f3c-9315-c4697ff33e6c
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  ENTRY_POINT                    = Zx296718EvbEntryPoint
>> +
>> +[Sources.common]
>> +  Zx296718EvbDxe.c
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
> 
> M (MdeM) before P (MdeP).
> 
>> +  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>> +
>> +[LibraryClasses]
>> +  BaseMemoryLib
>> +  BdsLib
>> +  CacheMaintenanceLib
>> +  DebugLib
>> +  DxeServicesLib
>> +  DxeServicesTableLib
>> +  FdtLib
>> +  IoLib
>> +  PcdLib
>> +  PrintLib
>> +  SerialPortLib
>> +  TimerLib
>> +  UefiBootServicesTableLib
>> +  UefiRuntimeServicesTableLib
>> +  UefiLib
> 
> L before R.
> 
>> +  UefiDriverEntryPoint
> 
> But D before L.
> 
> But also, how many of these LibraryClasses are actually used by this
> driver?
> 
>> +
>> +[Guids]
>> +  gEfiEndOfDxeEventGroupGuid
>> +  gEfiFileInfoGuid
>> +  gEfiGlobalVariableGuid
>> +  gFdtTableGuid
>> +  gFdtVariableGuid
> 
> How many of these Guids are actually used by this driver?
> 
>> +
>> +[Protocols]
>> +  gAndroidBootImgProtocolGuid
>> +  gEfiBlockIoProtocolGuid
>> +  gEfiDevicePathFromTextProtocolGuid
>> +  gEfiDevicePathToTextProtocolGuid
>> +  gEfiDevicePathProtocolGuid
>> +  gEfiSimpleFileSystemProtocolGuid
> 
> How many of these protocols are actually used by this driver?

Lots of code are removed from previous version. So most of them can be 
deleted now.

> 
>> +
>> +[Depex]
>> +  TRUE
>> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
> 
> It looks to me like this implementation (perhaps with a few small
> modifications) should be reusable across multiple implementations.
> 
> That does not need to happen for this to be merged, but it would be
> good if you and Haojian could discuss between yourselves. (Also,
> perhaps we should all sit down and discuss this at Linaro Connect next
> month.)

It is good idea and I will collect your comments later when code is ready.
> 
>> new file mode 100644
>> index 0000000..41861e2
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
>> @@ -0,0 +1,404 @@
>> +/** @file
>> +  Implementation for PlatformBootManagerLib library class interfaces.
>> +
>> +  Copyright (C) 2015-2016, Red Hat, Inc.
>> +  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
>> +  Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution. The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
>> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/UefiBootManagerLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Protocol/BlockIo.h>
>> +#include <Protocol/DevicePath.h>
>> +#include <Protocol/DevicePathFromText.h>
>> +#include <Protocol/DevicePathToText.h>
>> +#include <Protocol/LoadedImage.h>
>> +#include <Guid/EventGroup.h>
>> +#include <Guid/TtyTerm.h>
> 
> Can you verify all of the above are actually used by this file?

Sure.
> 
>> +
>> +#include "PlatformBm.h"
>> +
>> +#define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
>> +
>> +#define GRUB_FILE_NAME       L"\\EFI\\BOOT\\GRUBAA64.EFI"
>> +
>> +
>> +#pragma pack (1)
>> +typedef struct {
>> +  VENDOR_DEVICE_PATH         SerialDxe;
>> +  UART_DEVICE_PATH           Uart;
>> +  VENDOR_DEFINED_DEVICE_PATH TermType;
>> +  EFI_DEVICE_PATH_PROTOCOL   End;
>> +} PLATFORM_SERIAL_CONSOLE;
>> +#pragma pack ()
>> +
>> +#define SERIAL_DXE_FILE_GUID { \
>> +          0xD3987D4B, 0x971A, 0x435F, \
>> +          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
>> +          }
>> +
>> +STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
>> +  //
>> +  // VENDOR_DEVICE_PATH SerialDxe
>> +  //
>> +  {
>> +    { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, DP_NODE_LEN (VENDOR_DEVICE_PATH) },
>> +    SERIAL_DXE_FILE_GUID
>> +  },
>> +
>> +  //
>> +  // UART_DEVICE_PATH Uart
>> +  //
>> +  {
>> +    { MESSAGING_DEVICE_PATH, MSG_UART_DP, DP_NODE_LEN (UART_DEVICE_PATH) },
>> +    0,                                      // Reserved
>> +    FixedPcdGet64 (PcdUartDefaultBaudRate), // BaudRate
>> +    FixedPcdGet8 (PcdUartDefaultDataBits),  // DataBits
>> +    FixedPcdGet8 (PcdUartDefaultParity),    // Parity
>> +    FixedPcdGet8 (PcdUartDefaultStopBits)   // StopBits
>> +  },
>> +
>> +  //
>> +  // VENDOR_DEFINED_DEVICE_PATH TermType
>> +  //
>> +  {
>> +    {
>> +      MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
>> +      DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
>> +    }
>> +    //
>> +    // Guid to be filled in dynamically
>> +    //
>> +  },
>> +
>> +  //
>> +  // EFI_DEVICE_PATH_PROTOCOL End
>> +  //
>> +  {
>> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> +    DP_NODE_LEN (EFI_DEVICE_PATH_PROTOCOL)
>> +  }
>> +};
>> +
>> +/**
>> +  Check if the handle satisfies a particular condition.
>> +
>> +  @param[in] Handle      The handle to check.
>> +  @param[in] ReportText  A caller-allocated string passed in for reporting
>> +                         purposes. It must never be NULL.
>> +
>> +  @retval TRUE   The condition is satisfied.
>> +  @retval FALSE  Otherwise. This includes the case when the condition could not
>> +                 be fully evaluated due to an error.
>> +**/
>> +typedef
>> +BOOLEAN
>> +(EFIAPI *FILTER_FUNCTION) (
>> +  IN EFI_HANDLE   Handle,
>> +  IN CONST CHAR16 *ReportText
>> +  );
>> +
>> +
>> +/**
>> +  Process a handle.
>> +
>> +  @param[in] Handle      The handle to process.
>> +  @param[in] ReportText  A caller-allocated string passed in for reporting
>> +                         purposes. It must never be NULL.
>> +**/
>> +typedef
>> +VOID
>> +(EFIAPI *CALLBACK_FUNCTION)  (
>> +  IN EFI_HANDLE   Handle,
>> +  IN CONST CHAR16 *ReportText
>> +  );
>> +
>> +STATIC
>> +VOID
>> +PlatformRegisterFvBootOption (
>> +  EFI_GUID                         *FileGuid,
>> +  CHAR16                           *Description,
>> +  UINT32                           Attributes
>> +  )
>> +{
>> +  EFI_STATUS                        Status;
>> +  INTN                              OptionIndex;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION      NewOption;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION      *BootOptions;
>> +  UINTN                             BootOptionCount;
>> +  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
>> +  EFI_LOADED_IMAGE_PROTOCOL         *LoadedImage;
>> +  EFI_DEVICE_PATH_PROTOCOL          *DevicePath;
>> +
>> +  Status = gBS->HandleProtocol (
>> +                  gImageHandle,
>> +                  &gEfiLoadedImageProtocolGuid,
>> +                  (VOID **) &LoadedImage
>> +                  );
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
>> +  DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
>> +  ASSERT (DevicePath != NULL);
>> +  DevicePath = AppendDevicePathNode (
>> +                 DevicePath,
>> +                 (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
>> +                 );
>> +  ASSERT (DevicePath != NULL);
>> +
>> +  Status = EfiBootManagerInitializeLoadOption (
>> +             &NewOption,
>> +             LoadOptionNumberUnassigned,
>> +             LoadOptionTypeBoot,
>> +             Attributes,
>> +             Description,
>> +             DevicePath,
>> +             NULL,
>> +             0
>> +             );
>> +  ASSERT_EFI_ERROR (Status);
>> +  FreePool (DevicePath);
>> +
>> +  BootOptions = EfiBootManagerGetLoadOptions (
>> +                  &BootOptionCount, LoadOptionTypeBoot
>> +                  );
>> +
>> +  OptionIndex = EfiBootManagerFindLoadOption (
>> +                  &NewOption, BootOptions, BootOptionCount
>> +                  );
>> +
>> +  if (OptionIndex == -1) {
>> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>> +
>> +  EfiBootManagerFreeLoadOption (&NewOption);
>> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
>> +}
>> +
>> +
>> +STATIC
>> +VOID
>> +PlatformRegisterBootGrub (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS                           Status;
>> +  CHAR16                              *BootPathStr;
>> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
>> +  EFI_DEVICE_PATH                     *DevicePath;
>> +  EFI_DEVICE_PATH                     *FileDevicePath;
>> +  FILEPATH_DEVICE_PATH                *FilePath;
>> +  UINTN                                Size;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION         NewOption;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION        *BootOptions;
>> +  UINTN                                BootOptionCount;
>> +  INTN                                 OptionIndex;
>> +
>> +  //
>> +  // Get PcdAndroidBootDevicePath
>> +  //
>> +  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
>> +  ASSERT (BootPathStr != NULL);
>> +  Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, NULL,
>> +                                (VOID **)&EfiDevicePathFromTextProtocol);
>> +  ASSERT_EFI_ERROR(Status);
>> +  DevicePath = (EFI_DEVICE_PATH *)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
>> +  ASSERT (DevicePath != NULL);
>> +
>> +  Size = StrSize (GRUB_FILE_NAME);
>> +  FileDevicePath = AllocatePool (Size + SIZE_OF_FILEPATH_DEVICE_PATH + END_DEVICE_PATH_LENGTH);
>> +  if (FileDevicePath != NULL) {
>> +    FilePath = (FILEPATH_DEVICE_PATH *) FileDevicePath;
>> +    FilePath->Header.Type    = MEDIA_DEVICE_PATH;
>> +    FilePath->Header.SubType = MEDIA_FILEPATH_DP;
>> +    CopyMem (&FilePath->PathName, GRUB_FILE_NAME, Size);
>> +    SetDevicePathNodeLength (&FilePath->Header, Size + SIZE_OF_FILEPATH_DEVICE_PATH);
>> +    SetDevicePathEndNode (NextDevicePathNode (&FilePath->Header));
>> +
>> +    DevicePath = AppendDevicePath (DevicePath, FileDevicePath);
>> +    FreePool (FileDevicePath);
>> +  }
>> +  Status = EfiBootManagerInitializeLoadOption (
>> +             &NewOption,
>> +             LoadOptionNumberUnassigned,
>> +             LoadOptionTypeBoot,
>> +             LOAD_OPTION_ACTIVE,
>> +             L"Grub",
>> +             DevicePath,
>> +             NULL,
>> +             0
>> +             );
>> +  ASSERT_EFI_ERROR (Status);
>> +  FreePool (DevicePath);
>> +
>> +  BootOptions = EfiBootManagerGetLoadOptions (
>> +                  &BootOptionCount, LoadOptionTypeBoot
>> +                  );
>> +
>> +  OptionIndex = EfiBootManagerFindLoadOption (
>> +                  &NewOption, BootOptions, BootOptionCount
>> +                  );
>> +
>> +  if (OptionIndex == -1) {
>> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
>> +    ASSERT_EFI_ERROR (Status);
>> +  }
>> +
>> +  EfiBootManagerFreeLoadOption (&NewOption);
>> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
>> +
>> +}
>> +
>> +STATIC
>> +VOID
>> +PlatformRegisterOptionsAndKeys (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS                           Status;
>> +  EFI_INPUT_KEY                        Enter;
>> +  EFI_INPUT_KEY                        Esc;
>> +  EFI_BOOT_MANAGER_LOAD_OPTION         BootOption;
>> +
>> +  //
>> +  // Register Boot. OptionNumber is 1.
>> +  //
>> +  PlatformRegisterBootGrub ();
>> +
>> +  //
>> +  // Register Android Boot. OptionNumber is 2.
>> +  //
>> +  PlatformRegisterFvBootOption (
>> +    PcdGetPtr (PcdAndroidBootFile), L"Android Boot", LOAD_OPTION_ACTIVE
>> +    );
>> +
>> +  //
>> +  // Register ENTER as CONTINUE key
>> +  //
>> +  Enter.ScanCode    = SCAN_NULL;
>> +  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
>> +  Status = EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  //
>> +  // Map ESC to Boot Manager Menu
>> +  //
>> +  Esc.ScanCode    = SCAN_ESC;
>> +  Esc.UnicodeChar = CHAR_NULL;
>> +  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
>> +  ASSERT_EFI_ERROR (Status);
>> +  Status = EfiBootManagerAddKeyOptionVariable (
>> +             NULL, (UINT16) BootOption.OptionNumber, 0, &Esc, NULL
>> +             );
>> +  ASSERT (Status == EFI_SUCCESS || Status == EFI_ALREADY_STARTED);
>> +}
>> +
>> +
>> +//
>> +// BDS Platform Functions
>> +//
>> +/**
>> +  Do the platform init, can be customized by OEM/IBV
>> +  Possible things that can be done in PlatformBootManagerBeforeConsole:
>> +  > Update console variable: 1. include hot-plug devices;
>> +  >                          2. Clear ConIn and add SOL for AMT
>> +  > Register new Driver#### or Boot####
>> +  > Register new Key####: e.g.: F12
>> +  > Signal ReadyToLock event
>> +  > Authentication action: 1. connect Auth devices;
>> +  >                        2. Identify auto logon user.
>> +**/
>> +VOID
>> +EFIAPI
>> +PlatformBootManagerBeforeConsole (
>> +  VOID
>> +  )
>> +{
>> +  //
>> +  // Signal EndOfDxe PI Event
>> +  //
>> +  EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
>> +
>> +  //
>> +  // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
>> +  //
>> +  ASSERT (FixedPcdGet8 (PcdDefaultTerminalType) == 4);
>> +  CopyGuid (&mSerialConsole.TermType.Guid, &gEfiTtyTermGuid);
>> +
>> +  EfiBootManagerUpdateConsoleVariable (ConIn,
>> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
>> +  EfiBootManagerUpdateConsoleVariable (ConOut,
>> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
>> +  EfiBootManagerUpdateConsoleVariable (ErrOut,
>> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
>> +
>> +  //
>> +  // Register platform-specific boot options and keyboard shortcuts.
>> +  //
>> +  PlatformRegisterOptionsAndKeys ();
>> +}
>> +
>> +/**
>> +  Do the platform specific action after the console is ready
>> +  Possible things that can be done in PlatformBootManagerAfterConsole:
>> +  > Console post action:
>> +    > Dynamically switch output mode from 100x31 to 80x25 for certain senarino
>> +    > Signal console ready platform customized event
>> +  > Run diagnostics like memory testing
>> +  > Connect certain devices
>> +  > Dispatch aditional option roms
>> +  > Special boot
>> +**/
>> +VOID
>> +EFIAPI
>> +PlatformBootManagerAfterConsole (
>> +  VOID
>> +  )
>> +{
>> +  Print (L"Press ESCAPE for boot options ");
>> +
>> +  //
>> +  // Connect the rest of the devices.
>> +  //
>> +  EfiBootManagerConnectAll ();
>> +
>> +  //
>> +  // Enumerate all possible boot options.
>> +  //
>> +  EfiBootManagerRefreshAllBootOption ();
>> +
>> +  //
>> +  // Register UEFI Shell
>> +  //
>> +  PlatformRegisterFvBootOption (
>> +    PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE
>> +    );
>> +}
>> +
>> +/**
>> +  This function is called each second during the boot manager waits the
>> +  timeout.
>> +
>> +  @param TimeoutRemain  The remaining timeout.
>> +**/
>> +VOID
>> +EFIAPI
>> +PlatformBootManagerWaitCallback (
>> +  UINT16          TimeoutRemain
>> +  )
>> +{
>> +  Print (L".");
>> +}
>> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>> new file mode 100644
>> index 0000000..de52a9c
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>> @@ -0,0 +1,30 @@
>> +/** @file
>> +  Head file for BDS Platform specific code
>> +
>> +  Copyright (C) 2015-2016, Red Hat, Inc.
>> +  Copyright (c) 2004 - 2008, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution. The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
>> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#ifndef _PLATFORM_BM_H_
>> +#define _PLATFORM_BM_H_
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
> 
> If all this holds is a set of include files for a single source file,
> may they not just go directly into the .c file?

Sure.
> 
>> +
>> +#endif // _PLATFORM_BM_H_
>> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> new file mode 100644
>> index 0000000..b9c6aaa
>> --- /dev/null
>> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> @@ -0,0 +1,91 @@
>> +## @file
>> +#  Implementation for PlatformBootManagerLib library class interfaces.
>> +#
>> +#  Copyright (C) 2015-2016, Red Hat, Inc.
>> +#  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
>> +#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
>> +#  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
>> +#
>> +#  This program and the accompanying materials are licensed and made available
>> +#  under the terms and conditions of the BSD License which accompanies this
>> +#  distribution. The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> +#  IMPLIED.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010005
> 
> 0x00010019/1.25.
> 
>> +  BASE_NAME                      = PlatformBootManagerLib
>> +  FILE_GUID                      = 9001ec12-8757-4638-b6d0-e0863dddddb0
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = PlatformBootManagerLib|DXE_DRIVER
>> +
>> +#
>> +# The following information is for reference only and not required by the build tools.
>> +#
>> +#  VALID_ARCHITECTURES           = ARM AARCH64
>> +#
>> +
>> +[Sources]
>> +  PlatformBm.c
>> +
>> +[Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  Silicon/Sanchip/SanchipPkg.dec
> 
> Are all of these actually used?
> 
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  BaseMemoryLib
>> +  DebugLib
>> +  DevicePathLib
>> +  DxeServicesLib
>> +  MemoryAllocationLib
>> +  PcdLib
>> +  PrintLib
>> +  UefiBootManagerLib
>> +  UefiBootServicesTableLib
>> +  UefiLib
> 
> Are all of these actually used?
> 
>> +
>> +[FeaturePcd]
>> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootlogoOnlyEnable
>> +  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
> 
> Are all of these actually used?
> 
>> +
>> +[FixedPcd]
>> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile
>> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
>> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType
> 
> Are all of these actually used?
> 
>> +
>> +[Pcd]
>> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
>> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile
> 
> Are all of these actually used?
> 
>> +
>> +[Guids]
>> +  gEfiFileInfoGuid
>> +  gEfiFileSystemInfoGuid
>> +  gEfiFileSystemVolumeLabelInfoIdGuid
>> +  gEfiEndOfDxeEventGroupGuid
>> +  gEfiTtyTermGuid
> 
> Are all of these actually used?
> 
>> +
>> +[Protocols]
>> +  gEfiDevicePathFromTextProtocolGuid
>> +  gEfiDevicePathProtocolGuid
>> +  gEfiLoadedImageProtocolGuid
>> +  gEfiOEMBadgingProtocolGuid
>> +  gEfiPciRootBridgeIoProtocolGuid
>> +  gEfiSimpleFileSystemProtocolGuid
>> +  gEfiDevicePathToTextProtocolGuid
> 
> Are all of these actually used?

Will clean it after this library rewrite for sharing.
> 
>> +
>> +[Pcd]
>> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath
>> diff --git a/Silicon/Sanchip/SanchipPkg.dec b/Silicon/Sanchip/SanchipPkg.dec
>> new file mode 100644
>> index 0000000..92d439d
>> --- /dev/null
>> +++ b/Silicon/Sanchip/SanchipPkg.dec
>> @@ -0,0 +1,29 @@
>> +#
>> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +#  Copyright (c) 2017, Linaro Ltd.
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +
>> +[Defines]
>> +  DEC_SPECIFICATION              = 0x00010005
> 
> 0x00010019/1.25.
> 
>> +  PACKAGE_NAME                   = SanchipPkg
>> +  PACKAGE_GUID                   = f991248f-9e21-4e4f-b344-eaad28e42ec0
>> +  PACKAGE_VERSION                = 0.1
>> +
>> +[Includes.common]
>> +  Include                        # Root include for the package
> 
> This directory does not exist, causing compliation to fail.

Test shows that this include entry is unnecessary.
> 
>> +
>> +[Guids.common]
>> +  gSanchipTokenSpaceGuid          =  { 0x9589ba86, 0x58e6, 0x426c, { 0xbb, 0x39, 0x21, 0xf5, 0x68, 0x78, 0xe4, 0x27 } }
>> +
>> +[PcdsFixedAtBuild.common]
>> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000003
> 
> What?

PcdAndroidBootFile is FileGuid for AndroidBoot app file, as PcdShellFile 
Guid in other places. Do you want to hardcode the Guid in PlatformBm.c 
library code?
> 
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase|0|UINT64|0x00000002
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq|24000|UINT64|0x00000001
>> -- 
>> 1.9.1
>>

Re: [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Leif Lindholm]

On Thu, Aug 17, 2017 at 11:45:52PM +0800, Jun Nie wrote:
> On 2017年08月10日 22:41, Leif Lindholm wrote:
> > On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:
> > > Add boot manager lib and entries, including Android and Grub.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > >   .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c        | 105 ++++++
> > >   .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf      |  66 ++++
> > >   .../Library/PlatformBootManagerLib/PlatformBm.c    | 404 +++++++++++++++++++++
> > >   .../Library/PlatformBootManagerLib/PlatformBm.h    |  30 ++
> > >   .../PlatformBootManagerLib.inf                     |  91 +++++
> > >   Silicon/Sanchip/SanchipPkg.dec                     |  29 ++
> > >   6 files changed, 725 insertions(+)
> > >   create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> > >   create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> > >   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
> > >   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
> > >   create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> > >   create mode 100644 Silicon/Sanchip/SanchipPkg.dec
> > > 

> > > diff --git a/Silicon/Sanchip/SanchipPkg.dec b/Silicon/Sanchip/SanchipPkg.dec
> > > new file mode 100644
> > > index 0000000..92d439d
> > > --- /dev/null
> > > +++ b/Silicon/Sanchip/SanchipPkg.dec
> > > @@ -0,0 +1,29 @@
> > > +#
> > > +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> > > +#  Copyright (c) 2017, Linaro Ltd.
> > > +#
> > > +#  This program and the accompanying materials
> > > +#  are licensed and made available under the terms and conditions of the BSD License
> > > +#  which accompanies this distribution.  The full text of the license may be found at
> > > +#  http://opensource.org/licenses/bsd-license.php
> > > +#
> > > +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > +#
> > > +
> > > +[Defines]
> > > +  DEC_SPECIFICATION              = 0x00010005
> > 
> > 0x00010019/1.25.
> > 
> > > +  PACKAGE_NAME                   = SanchipPkg
> > > +  PACKAGE_GUID                   = f991248f-9e21-4e4f-b344-eaad28e42ec0
> > > +  PACKAGE_VERSION                = 0.1
> > > +
> > > +[Includes.common]
> > > +  Include                        # Root include for the package
> > 
> > This directory does not exist, causing compliation to fail.
> 
> Test shows that this include entry is unnecessary.
> > 
> > > +
> > > +[Guids.common]
> > > +  gSanchipTokenSpaceGuid          =  { 0x9589ba86, 0x58e6, 0x426c, { 0xbb, 0x39, 0x21, 0xf5, 0x68, 0x78, 0xe4, 0x27 } }
> > > +
> > > +[PcdsFixedAtBuild.common]
> > > +  gSanchipTokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000003
> > 
> > What?
> 
> PcdAndroidBootFile is FileGuid for AndroidBoot app file, as PcdShellFile
> Guid in other places. Do you want to hardcode the Guid in PlatformBm.c
> library code?

But you are not setting it in your platform description file, so
what's the point of declaring it here?

Also, it does not feel like something that belongs in a
platform-specific .dec. It would make a lot more sense to me in edk2
EmbeddedPkg/EmbeddedPkg.dec.

/
    Leif

Re: [PATCH 4/4] Platforms/zx: Add platform build system files

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:39PM +0800, Jun Nie wrote:
> Add platform build system files, including *.dsc *.fdf *.dec
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec |  33 ++
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc | 467 +++++++++++++++++++++++++++
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf | 331 +++++++++++++++++++
>  3 files changed, 831 insertions(+)
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> 
> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> new file mode 100644
> index 0000000..078cfdf
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> @@ -0,0 +1,33 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 0x00010005

0x00010019/1.25.

> +  PACKAGE_NAME                   = Zx296718Evb
> +  PACKAGE_GUID                   = d6db414d-ea67-4312-94d5-9c9e5b224d25
> +  PACKAGE_VERSION                = 0.1
> +
> +################################################################################
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#                   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +################################################################################
> +[Includes.common]
> +  Include                        # Root include for the package

This directory does not exist, breaking compilation.

> +
> +[Guids.common]
> +  gZx296718EvbTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }

This does not appear to actually be used anywhere.

> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
> new file mode 100644
> index 0000000..d104e7c
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
> @@ -0,0 +1,467 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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 Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = Zx296718Evb
> +  PLATFORM_GUID                  = 8edf1480-da5c-4857-bc02-7530bd8e7b7a
> +  PLATFORM_VERSION               = 0.2
> +  DSC_SPECIFICATION              = 0x00010005
> +  OUTPUT_DIRECTORY               = Build/Zx296718Evb
> +  SUPPORTED_ARCHITECTURES        = AARCH64
> +  BUILD_TARGETS                  = DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> +
> +[LibraryClasses.common]
> +!if $(TARGET) == RELEASE
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +!else
> +  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> +!endif
> +  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> +
> +  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
> +  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> +  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> +  ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> +
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +  ArmPlatformLib|Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
> +  ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> +  ArmPlatformSysConfigLib|ArmPlatformPkg/Library/ArmPlatformSysConfigLibNull/ArmPlatformSysConfigLibNull.inf
> +
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
> +  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
> +  CpuExceptionHandlerLib|ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> +  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
> +  DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
> +  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +  DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
> +  EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> +  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +
> +  BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf
> +  AndroidBootImgLib|EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> +  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +  PlatformBootManagerLib|Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> +
> +  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> +  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
> +  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
> +
> +  # UiApp dependencies
> +  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> +  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +
> +  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> +  BasePeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +  HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
> +  UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
> +  RealTimeClockLib|Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> +  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> +  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
> +  UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
> +
> +  PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
> +  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +
> +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> +
> +  #
> +  # Assume everything is fixed at build
> +  #
> +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +
> +  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> +  PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
> +  PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
> +  PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
> +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> +  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
> +  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> +  UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> +  UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
> +
> +  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> +
> +  # Network Libraries
> +  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
> +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> +  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> +  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> +  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> +
> +  # It is not possible to prevent compilers from generating calls to generic
> +  # intrinsic functions. This library provides the intrinsic functions
> +  # generated by a given compiler.
> +  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> +
> +  # Add support for GCC stack protector
> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +
> +[LibraryClasses.common.SEC]
> +  PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> +  ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> +  LzmaDecompressLib|IntelFrameworkModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> +  MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> +  HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
> +  PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> +  PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
> +
> +[LibraryClasses.common.DXE_CORE]
> +  DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +  ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
> +  HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
> +  MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
> +
> +[LibraryClasses.common.UEFI_DRIVER]
> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
> +
> +[LibraryClasses.common.DXE_DRIVER]
> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
> +  SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> +
> +[LibraryClasses.common.DXE_RUNTIME_DRIVER]
> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> +  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
> +
> +[BuildOptions]
> +  GCC:*_*_*_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Sanchip/Zx296718/Include -I$(WORKSPACE)/Platform/Sanchip/Zx296718Evb/Include

Platform/Sanchip/Zx296718Evb/Include does not exist.
Silicon/Sanchip/Zx296718 should be automatically added by including
Silicon/Sanchip/Zx296718/Zx296718.dec where required.

> +
> +[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> +  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
> +
> +################################################################################
> +#
> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +#
> +################################################################################
> +
> +[PcdsFeatureFlag.common]
> +  gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable|TRUE
> +  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnosticsDisable|TRUE
> +  gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
> +  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
> +
> +  #
> +  # Control what commands are supported from the UI
> +  # Turn these on and off to add features or save size
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
> +
> +  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
> +
> +  # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
> +  gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
> +
> +  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|TRUE
> +
> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryInitializeInSec|TRUE
> +
> +  ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
> +  #  It could be set FALSE to save size.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
> +
> +[PcdsFixedAtBuild.common]
> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
> +  gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
> +  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
> +  gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
> +  gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
> +
> +  # DEBUG_ASSERT_ENABLED       0x01
> +  # DEBUG_PRINT_ENABLED        0x02
> +  # DEBUG_CODE_ENABLED         0x04
> +  # CLEAR_MEMORY_ENABLED       0x08
> +  # ASSERT_BREAKPOINT_ENABLED  0x10
> +  # ASSERT_DEADLOOP_ENABLED    0x20
> +!if $(TARGET) == RELEASE
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x21
> +!else
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
> +!endif
> +
> +  #  DEBUG_INIT      0x00000001  // Initialization
> +  #  DEBUG_WARN      0x00000002  // Warnings
> +  #  DEBUG_LOAD      0x00000004  // Load events
> +  #  DEBUG_FS        0x00000008  // EFI File system
> +  #  DEBUG_POOL      0x00000010  // Alloc & Free's
> +  #  DEBUG_PAGE      0x00000020  // Alloc & Free's
> +  #  DEBUG_INFO      0x00000040  // Verbose
> +  #  DEBUG_DISPATCH  0x00000080  // PEI/DXE Dispatchers
> +  #  DEBUG_VARIABLE  0x00000100  // Variable
> +  #  DEBUG_BM        0x00000400  // Boot Manager
> +  #  DEBUG_BLKIO     0x00001000  // BlkIo Driver
> +  #  DEBUG_NET       0x00004000  // SNI Driver
> +  #  DEBUG_UNDI      0x00010000  // UNDI Driver
> +  #  DEBUG_LOADFILE  0x00020000  // UNDI Driver
> +  #  DEBUG_EVENT     0x00080000  // Event messages
> +  #  DEBUG_GCD       0x00100000  // Global Coherency Database changes
> +  #  DEBUG_CACHE     0x00200000  // Memory range cachability changes
> +  #  DEBUG_ERROR     0x80000000  // Error
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
> +
> +  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> +
> +  #
> +  # Optional feature to help prevent EFI memory map fragments
> +  # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
> +  # Values are in EFI Pages (4K). DXE Core will make sure that
> +  # at least this much of each type of memory can be allocated
> +  # from a single memory range. This way you only end up with
> +  # maximum of two fragements for each type in the memory map
> +  # (the memory used, and the free memory that was prereserved
> +  # but not used).
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|80
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|65
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|400
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
> +
> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
> +
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
> +
> +  gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"ZTE"
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedPrompt|"Zx296718Evb"
> +
> +  # System Memory (1GB)
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
> +  gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
> +
> +  # Zx296718 Cluster profile
> +  gArmPlatformTokenSpaceGuid.PcdCoreCount|4
> +  gArmPlatformTokenSpaceGuid.PcdClusterCount|1
> +
> +  gArmTokenSpaceGuid.PcdVFPEnabled|1
> +
> +  #
> +  # ARM PrimeCell
> +  #
> +
> +  ## PL011 - Serial Terminal
> +  DEFINE SERIAL_BASE = 0x11F000 # UART0
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|$(SERIAL_BASE)
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> +  gArmPlatformTokenSpaceGuid.PL011UartRegOffsetVariant|1
> +#  gArmPlatformTokenSpaceGuid.PL011UartInteger|10
> +#  gArmPlatformTokenSpaceGuid.PL011UartFractional|26

If these are not needed, please delete them rather than commenting
out.

> +
> +  ## RealTimeClock
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase|0x115000
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq|24000
> +
> +  #
> +  # ARM General Interrupt Controller
> +  #
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x02a00000
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x2b00000
> +
> +  # Use the serial console (ConIn & ConOut)
> +  gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenPcAnsi();VenHw(CE660500-824D-11E0-AC72-0002A5D5C51B)"
> +  gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenPcAnsi()"
> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|1
> +
> +  # GUID of the UEFI Shell
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
> +
> +  # GUID of the UI app
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> +
> +  #
> +  # ARM Architectural Timer Frequency
> +  #
> +  gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|24000000
> +  gEmbeddedTokenSpaceGuid.PcdMetronomeTickPeriod|1000
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000 # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
> +
> +  #
> +  # DW MMC/SD card controller
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x1470000
> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|198000000
> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|26000000
> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth|128
> +
> +  #
> +  # Android Loader
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(8,GPT,A092C620-D178-4CA7-B540-C4E26BD6D2E2,0x228000,0x10001)"
> +
> +################################################################################
> +#
> +# Components Section - list of all EDK II Modules needed by this Platform
> +#
> +################################################################################
> +[Components.common]
> +  #
> +  # PEI Phase modules
> +  #
> +  ArmPlatformPkg/PrePi/PeiUniCore.inf
> +
> +  #
> +  # DXE
> +  #
> +  MdeModulePkg/Core/Dxe/DxeMain.inf {
> +    <LibraryClasses>
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +      NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +  }
> +
> +  #
> +  # Architectural Protocols
> +  #
> +  ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +  EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> +  EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
> +  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> +  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
> +
> +  MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> +  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> +  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +
> +  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
> +
> +  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +  ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> +
> +  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> +
> +  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> +
> +  Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> +
> +  #
> +  # MMC/SD
> +  #
> +  EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
> +  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
> +
> +  #
> +  # Android Loader
> +  #
> +  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf
> +  EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf {
> +    <LibraryClasses>
> +      BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf

This is already getting globally resolved to the same version.

> +  }
> +
> +  #
> +  # UEFI Network Stack
> +  #
> +  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> +  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> +  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> +  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> +  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> +  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> +  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> +  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> +  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +
> +  #
> +  # FAT filesystem + GPT/MBR partitioning
> +  #
> +  MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
> +  MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> +  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> +  FatPkg/EnhancedFatDxe/Fat.inf
> +
> +  #
> +  # Bds
> +  #
> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> +  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +  MdeModulePkg/Application/UiApp/UiApp.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
> +      NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
> +      NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
> +      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> +  }
> +  ShellPkg/Application/Shell/Shell.inf {
> +    <LibraryClasses>
> +      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> +      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf
> +      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> +      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> +      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> +    <PcdsFixedAtBuild>
> +      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> +      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> +      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> +  }
> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> new file mode 100644
> index 0000000..bbafe45
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> @@ -0,0 +1,331 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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.
> +#
> +
> +################################################################################
> +#
> +# FD Section
> +# The [FD] Section is made up of the definition statements and a
> +# description of what goes into  the Flash Device Image.  Each FD section
> +# defines one flash "device" image.  A flash device image may be one of
> +# the following: Removable media bootable image (like a boot floppy
> +# image,) an Option ROM image (that would be "flashed" into an add-in
> +# card,) a System "Flash"  image (that would be burned into a system's
> +# flash) or an Update ("Capsule") image that will be used to update and
> +# existing system flash.
> +#
> +################################################################################
> +
> +[FD.BL33_AP_UEFI]
> +BaseAddress   = 0x4F000000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
> +Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
> +ErasePolarity = 1
> +
> +# This one is tricky, it must be: BlockSize * NumBlocks = Size
> +BlockSize     = 0x00001000
> +NumBlocks     = 0xF0
> +
> +################################################################################
> +#
> +# Following are lists of FD Region layout which correspond to the locations of different
> +# images within the flash device.
> +#
> +# Regions must be defined in ascending order and may not overlap.
> +#
> +# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
> +# the pipe "|" character, followed by the size of the region, also in hex with the leading
> +# "0x" characters. Like:
> +# Offset|Size
> +# PcdOffsetCName|PcdSizeCName
> +# RegionType <FV, DATA, or FILE>
> +#
> +################################################################################
> +
> +0x00000000|0x000F0000
> +gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> +FV = FVMAIN_COMPACT
> +
> +
> +################################################################################
> +#
> +# FV Section
> +#
> +# [FV] section is used to define what components or modules are placed within a flash
> +# device file.  This section also defines order the components and modules are positioned
> +# within the image.  The [FV] section consists of define statements, set statements and
> +# module statements.
> +#
> +################################################################################
> +
> +[FV.FvMain]
> +BlockSize          = 0x40
> +NumBlocks          = 0         # This FV gets compressed so make it just big enough
> +FvAlignment        = 8         # FV alignment and FV attributes setting.
> +ERASE_POLARITY     = 1
> +MEMORY_MAPPED      = TRUE
> +STICKY_WRITE       = TRUE
> +LOCK_CAP           = TRUE
> +LOCK_STATUS        = TRUE
> +WRITE_DISABLED_CAP = TRUE
> +WRITE_ENABLED_CAP  = TRUE
> +WRITE_STATUS       = TRUE
> +WRITE_LOCK_CAP     = TRUE
> +WRITE_LOCK_STATUS  = TRUE
> +READ_DISABLED_CAP  = TRUE
> +READ_ENABLED_CAP   = TRUE
> +READ_STATUS        = TRUE
> +READ_LOCK_CAP      = TRUE
> +READ_LOCK_STATUS   = TRUE
> +
> +  INF MdeModulePkg/Core/Dxe/DxeMain.inf
> +
> +  #
> +  # PI DXE Drivers producing Architectural Protocols (EFI Services)
> +  #
> +  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> +  INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +  INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +  INF EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> +  INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
> +  INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> +  INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
> +
> +  #
> +  # Multiple Console IO support
> +  #
> +  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> +  INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +  INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +
> +  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
> +  INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> +
> +  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> +
> +  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> +
> +  #
> +  # Multimedia Card Interface
> +  #
> +  INF EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
> +  INF EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
> +
> +  #
> +  # Android Loader
> +  #
> +  INF EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
> +
> +  #
> +  # UEFI Network Stack
> +  #
> +  INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> +  INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> +  INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> +  INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> +  INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> +  INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> +  INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> +  INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> +  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +
> +  #
> +  # FAT filesystem + GPT/MBR partitioning
> +  #
> +  INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
> +  INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> +  INF FatPkg/EnhancedFatDxe/Fat.inf
> +  INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> +
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +  INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
> +
> +  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +
> +  #
> +  # UEFI applications
> +  #
> +  INF ShellPkg/Application/Shell/Shell.inf
> +
> +  #
> +  # Bds
> +  #
> +  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> +  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +  INF MdeModulePkg/Application/UiApp/UiApp.inf
> +
> +  #
> +  # Zx296718Evb Platform
> +  #
> +  INF Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> +
> +[FV.FVMAIN_COMPACT]
> +FvAlignment        = 8
> +ERASE_POLARITY     = 1
> +MEMORY_MAPPED      = TRUE
> +STICKY_WRITE       = TRUE
> +LOCK_CAP           = TRUE
> +LOCK_STATUS        = TRUE
> +WRITE_DISABLED_CAP = TRUE
> +WRITE_ENABLED_CAP  = TRUE
> +WRITE_STATUS       = TRUE
> +WRITE_LOCK_CAP     = TRUE
> +WRITE_LOCK_STATUS  = TRUE
> +READ_DISABLED_CAP  = TRUE
> +READ_ENABLED_CAP   = TRUE
> +READ_STATUS        = TRUE
> +READ_LOCK_CAP      = TRUE
> +READ_LOCK_STATUS   = TRUE
> +
> +  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
> +
> +  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> +    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> +      SECTION FV_IMAGE = FVMAIN
> +    }
> +  }
> +
> +
> +################################################################################
> +#
> +# Rules are use with the [FV] section's module INF type to define
> +# how an FFS file is created for a given INF file. The following Rule are the default
> +# rules for the different module type. User can add the customized rules to define the
> +# content of the FFS file.
> +#
> +################################################################################
> +
> +
> +############################################################################
> +# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #
> +############################################################################
> +#
> +#[Rule.Common.DXE_DRIVER]
> +#  FILE DRIVER = $(NAMED_GUID) {
> +#    DXE_DEPEX    DXE_DEPEX               Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> +#    COMPRESS PI_STD {
> +#      GUIDED {
> +#        PE32     PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
> +#        UI       STRING="$(MODULE_NAME)" Optional
> +#        VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> +#      }
> +#    }
> +#  }
> +#
> +############################################################################
> +
> +#
> +# These SEC rules are used for ArmPlatformPkg/PrePi module.
> +# ArmPlatformPkg/PrePi is declared as a SEC module to make GenFv patch the
> +# UEFI Firmware to jump to ArmPlatformPkg/PrePi entrypoint
> +#
> +[Rule.ARM.SEC]
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> +    TE  TE    Align = 32                $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +[Rule.AARCH64.SEC]
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> +    TE  TE    Align = 4K                $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +# A shim specific rule is required to ensure the alignment is 4K.
> +# Otherwise BaseTools pick up the AArch32 alignment (ie: 32)
> +[Rule.ARM.SEC.SHIM]
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> +    TE  TE    Align = 4K                $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +[Rule.Common.PEI_CORE]
> +  FILE PEI_CORE = $(NAMED_GUID) {
> +    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    UI     STRING ="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.PEIM]
> +  FILE PEIM = $(NAMED_GUID) {
> +     PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
> +     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
> +     UI       STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.PEIM.TIANOCOMPRESSED]
> +  FILE PEIM = $(NAMED_GUID) DEBUG_MYTOOLS_IA32 {
> +    PEI_DEPEX PEI_DEPEX Optional        $(INF_OUTPUT)/$(MODULE_NAME).depex
> +    GUIDED A31280AD-481E-41B6-95E8-127F4C984779 PROCESSING_REQUIRED = TRUE {
> +      PE32      PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
> +      UI        STRING="$(MODULE_NAME)" Optional
> +    }
> +  }
> +
> +[Rule.Common.DXE_CORE]
> +  FILE DXE_CORE = $(NAMED_GUID) {
> +    PE32     PE32                       $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    UI       STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.UEFI_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    UI           STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.DXE_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    UI           STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.DXE_DRIVER.BINARY]
> +  FILE DRIVER = $(NAMED_GUID) {
> +  DXE_DEPEX    DXE_DEPEX              Optional |.depex
> +  PE32         PE32                   |.efi
> +  UI           STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.DXE_RUNTIME_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    UI           STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.UEFI_APPLICATION]
> +  FILE APPLICATION = $(NAMED_GUID) {
> +    UI     STRING ="$(MODULE_NAME)" Optional
> +    PE32   PE32                         $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +[Rule.Common.UEFI_DRIVER.BINARY]
> +  FILE DRIVER = $(NAMED_GUID) {
> +    DXE_DEPEX DXE_DEPEX Optional      |.depex
> +    PE32      PE32                    |.efi
> +    UI        STRING="$(MODULE_NAME)" Optional
> +    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> +  }
> +
> +[Rule.Common.UEFI_APPLICATION.BINARY]
> +  FILE APPLICATION = $(NAMED_GUID) {
> +    PE32      PE32                    |.efi
> +    UI        STRING="$(MODULE_NAME)" Optional
> +    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> +  }
> +
> +[Rule.Common.USER_DEFINED.ACPITABLE]

Are you planning on adding ACPI support?

> +  FILE FREEFORM = $(NAMED_GUID) {
> +    RAW ACPI               |.acpi
> +    RAW ASL                |.aml
> +  }
> -- 
> 1.9.1
> 

Re: [PATCH 4/4] Platforms/zx: Add platform build system files

[Jun Nie]

On 2017年08月10日 23:00, Leif Lindholm wrote:
> On Wed, Aug 09, 2017 at 10:12:39PM +0800, Jun Nie wrote:
>> Add platform build system files, including *.dsc *.fdf *.dec
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>   Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec |  33 ++
>>   Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc | 467 +++++++++++++++++++++++++++
>>   Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf | 331 +++++++++++++++++++
>>   3 files changed, 831 insertions(+)
>>   create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>>   create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
>>   create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
>>
>> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>> new file mode 100644
>> index 0000000..078cfdf
>> --- /dev/null
>> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>> @@ -0,0 +1,33 @@
>> +#
>> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +#  Copyright (c) 2017, Linaro Ltd.
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +
>> +[Defines]
>> +  DEC_SPECIFICATION              = 0x00010005
> 
> 0x00010019/1.25.
> 
>> +  PACKAGE_NAME                   = Zx296718Evb
>> +  PACKAGE_GUID                   = d6db414d-ea67-4312-94d5-9c9e5b224d25
>> +  PACKAGE_VERSION                = 0.1
>> +
>> +################################################################################
>> +#
>> +# Include Section - list of Include Paths that are provided by this package.
>> +#                   Comments are used for Keywords and Module Types.
>> +#
>> +# Supported Module Types:
>> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
>> +#
>> +################################################################################
>> +[Includes.common]
>> +  Include                        # Root include for the package
> 
> This directory does not exist, breaking compilation. >
>> +
>> +[Guids.common]
>> +  gZx296718EvbTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> 
> This does not appear to actually be used anywhere.

I already remove this whole file in next version as Ard suggested.
> 
>> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
>> new file mode 100644
>> index 0000000..d104e7c
>> --- /dev/null
>> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
>> @@ -0,0 +1,467 @@
>> +#
>> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +#  Copyright (c) 2017, Linaro Ltd.
>> +#
>> +#  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 Section - statements that will be processed to create a Makefile.
>> +#
>> +################################################################################
>> +[Defines]
>> +  PLATFORM_NAME                  = Zx296718Evb
>> +  PLATFORM_GUID                  = 8edf1480-da5c-4857-bc02-7530bd8e7b7a
>> +  PLATFORM_VERSION               = 0.2
>> +  DSC_SPECIFICATION              = 0x00010005
>> +  OUTPUT_DIRECTORY               = Build/Zx296718Evb
>> +  SUPPORTED_ARCHITECTURES        = AARCH64
>> +  BUILD_TARGETS                  = DEBUG|RELEASE
>> +  SKUID_IDENTIFIER               = DEFAULT
>> +  FLASH_DEFINITION               = Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
>> +
>> +[LibraryClasses.common]
>> +!if $(TARGET) == RELEASE
>> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
>> +!else
>> +  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>> +!endif
>> +  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>> +
>> +  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
>> +  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> +  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
>> +  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
>> +  ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>> +
>> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>> +  ArmPlatformLib|Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
>> +  ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
>> +  ArmPlatformSysConfigLib|ArmPlatformPkg/Library/ArmPlatformSysConfigLibNull/ArmPlatformSysConfigLibNull.inf
>> +
>> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
>> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>> +  CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
>> +  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
>> +  CpuExceptionHandlerLib|ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
>> +  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
>> +  DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
>> +  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
>> +  DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>> +  EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>> +  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>> +
>> +  BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf
>> +  AndroidBootImgLib|EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
>> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> +  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>> +  PlatformBootManagerLib|Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> +  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
>> +
>> +  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>> +  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>> +  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
>> +
>> +  # UiApp dependencies
>> +  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>> +  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> +
>> +  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>> +  BasePeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> +  HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
>> +  UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
>> +  RealTimeClockLib|Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
>> +  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
>> +  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
>> +  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
>> +  UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
>> +
>> +  PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
>> +  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>> +
>> +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
>> +
>> +  #
>> +  # Assume everything is fixed at build
>> +  #
>> +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>> +
>> +  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>> +  PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
>> +  PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
>> +  PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
>> +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>> +  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
>> +  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
>> +  UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>> +  UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
>> +
>> +  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> +
>> +  # Network Libraries
>> +  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>> +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
>> +  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
>> +  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
>> +  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
>> +
>> +  # It is not possible to prevent compilers from generating calls to generic
>> +  # intrinsic functions. This library provides the intrinsic functions
>> +  # generated by a given compiler.
>> +  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>> +
>> +  # Add support for GCC stack protector
>> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> +
>> +[LibraryClasses.common.SEC]
>> +  PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
>> +  ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
>> +  LzmaDecompressLib|IntelFrameworkModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
>> +  MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>> +  HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
>> +  PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>> +  PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>> +  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> +  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>> +
>> +[LibraryClasses.common.DXE_CORE]
>> +  DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> +  ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
>> +  HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>> +  MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
>> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>> +
>> +[LibraryClasses.common.UEFI_DRIVER]
>> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>> +
>> +[LibraryClasses.common.DXE_DRIVER]
>> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>> +  SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>> +
>> +[LibraryClasses.common.DXE_RUNTIME_DRIVER]
>> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>> +  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>> +  ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>> +
>> +[BuildOptions]
>> +  GCC:*_*_*_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Sanchip/Zx296718/Include -I$(WORKSPACE)/Platform/Sanchip/Zx296718Evb/Include
> 
> Platform/Sanchip/Zx296718Evb/Include does not exist.
> Silicon/Sanchip/Zx296718 should be automatically added by including
> Silicon/Sanchip/Zx296718/Zx296718.dec where required.

Platform/Sanchip/Zx296718Evb/Include is empty folder in my env. Will 
remove it.
> 
>> +
>> +[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>> +  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>> +
>> +################################################################################
>> +#
>> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
>> +#
>> +################################################################################
>> +
>> +[PcdsFeatureFlag.common]
>> +  gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable|TRUE
>> +  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnosticsDisable|TRUE
>> +  gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE
>> +  gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE
>> +
>> +  #
>> +  # Control what commands are supported from the UI
>> +  # Turn these on and off to add features or save size
>> +  #
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE
>> +
>> +  gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE
>> +
>> +  # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress
>> +  gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE
>> +
>> +  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|TRUE
>> +
>> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryInitializeInSec|TRUE
>> +
>> +  ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
>> +  #  It could be set FALSE to save size.
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>> +
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
>> +
>> +[PcdsFixedAtBuild.common]
>> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
>> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
>> +  gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
>> +  gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
>> +  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
>> +  gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
>> +  gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
>> +
>> +  # DEBUG_ASSERT_ENABLED       0x01
>> +  # DEBUG_PRINT_ENABLED        0x02
>> +  # DEBUG_CODE_ENABLED         0x04
>> +  # CLEAR_MEMORY_ENABLED       0x08
>> +  # ASSERT_BREAKPOINT_ENABLED  0x10
>> +  # ASSERT_DEADLOOP_ENABLED    0x20
>> +!if $(TARGET) == RELEASE
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x21
>> +!else
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
>> +!endif
>> +
>> +  #  DEBUG_INIT      0x00000001  // Initialization
>> +  #  DEBUG_WARN      0x00000002  // Warnings
>> +  #  DEBUG_LOAD      0x00000004  // Load events
>> +  #  DEBUG_FS        0x00000008  // EFI File system
>> +  #  DEBUG_POOL      0x00000010  // Alloc & Free's
>> +  #  DEBUG_PAGE      0x00000020  // Alloc & Free's
>> +  #  DEBUG_INFO      0x00000040  // Verbose
>> +  #  DEBUG_DISPATCH  0x00000080  // PEI/DXE Dispatchers
>> +  #  DEBUG_VARIABLE  0x00000100  // Variable
>> +  #  DEBUG_BM        0x00000400  // Boot Manager
>> +  #  DEBUG_BLKIO     0x00001000  // BlkIo Driver
>> +  #  DEBUG_NET       0x00004000  // SNI Driver
>> +  #  DEBUG_UNDI      0x00010000  // UNDI Driver
>> +  #  DEBUG_LOADFILE  0x00020000  // UNDI Driver
>> +  #  DEBUG_EVENT     0x00080000  // Event messages
>> +  #  DEBUG_GCD       0x00100000  // Global Coherency Database changes
>> +  #  DEBUG_CACHE     0x00200000  // Memory range cachability changes
>> +  #  DEBUG_ERROR     0x80000000  // Error
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
>> +
>> +  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>> +
>> +  #
>> +  # Optional feature to help prevent EFI memory map fragments
>> +  # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
>> +  # Values are in EFI Pages (4K). DXE Core will make sure that
>> +  # at least this much of each type of memory can be allocated
>> +  # from a single memory range. This way you only end up with
>> +  # maximum of two fragements for each type in the memory map
>> +  # (the memory used, and the free memory that was prereserved
>> +  # but not used).
>> +  #
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|80
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|65
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|400
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
>> +
>> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
>> +
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
>> +
>> +  gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"ZTE"
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
>> +  gEmbeddedTokenSpaceGuid.PcdEmbeddedPrompt|"Zx296718Evb"
>> +
>> +  # System Memory (1GB)
>> +  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
>> +  gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
>> +
>> +  # Zx296718 Cluster profile
>> +  gArmPlatformTokenSpaceGuid.PcdCoreCount|4
>> +  gArmPlatformTokenSpaceGuid.PcdClusterCount|1
>> +
>> +  gArmTokenSpaceGuid.PcdVFPEnabled|1
>> +
>> +  #
>> +  # ARM PrimeCell
>> +  #
>> +
>> +  ## PL011 - Serial Terminal
>> +  DEFINE SERIAL_BASE = 0x11F000 # UART0
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|$(SERIAL_BASE)
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>> +  gArmPlatformTokenSpaceGuid.PL011UartRegOffsetVariant|1
>> +#  gArmPlatformTokenSpaceGuid.PL011UartInteger|10
>> +#  gArmPlatformTokenSpaceGuid.PL011UartFractional|26
> 
> If these are not needed, please delete them rather than commenting
> out.

sure.
> 
>> +
>> +  ## RealTimeClock
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase|0x115000
>> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq|24000
>> +
>> +  #
>> +  # ARM General Interrupt Controller
>> +  #
>> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x02a00000
>> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x2b00000
>> +
>> +  # Use the serial console (ConIn & ConOut)
>> +  gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenPcAnsi();VenHw(CE660500-824D-11E0-AC72-0002A5D5C51B)"
>> +  gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenPcAnsi()"
>> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|1
>> +
>> +  # GUID of the UEFI Shell
>> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>> +
>> +  # GUID of the UI app
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
>> +
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>> +
>> +  #
>> +  # ARM Architectural Timer Frequency
>> +  #
>> +  gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|24000000
>> +  gEmbeddedTokenSpaceGuid.PcdMetronomeTickPeriod|1000
>> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>> +  gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000 # expressed in 100ns units, 10,000 x 100 ns = 1,000,000 ns = 1 ms
>> +
>> +  #
>> +  # DW MMC/SD card controller
>> +  #
>> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x1470000
>> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|198000000
>> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|26000000
>> +  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth|128
>> +
>> +  #
>> +  # Android Loader
>> +  #
>> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(8,GPT,A092C620-D178-4CA7-B540-C4E26BD6D2E2,0x228000,0x10001)"
>> +
>> +################################################################################
>> +#
>> +# Components Section - list of all EDK II Modules needed by this Platform
>> +#
>> +################################################################################
>> +[Components.common]
>> +  #
>> +  # PEI Phase modules
>> +  #
>> +  ArmPlatformPkg/PrePi/PeiUniCore.inf
>> +
>> +  #
>> +  # DXE
>> +  #
>> +  MdeModulePkg/Core/Dxe/DxeMain.inf {
>> +    <LibraryClasses>
>> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>> +      NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
>> +  }
>> +
>> +  #
>> +  # Architectural Protocols
>> +  #
>> +  ArmPkg/Drivers/CpuDxe/CpuDxe.inf
>> +  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>> +  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>> +  EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
>> +  EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
>> +  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>> +  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>> +
>> +  MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>> +  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>> +  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
>> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
>> +
>> +  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>> +  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>> +
>> +  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>> +  ArmPkg/Drivers/TimerDxe/TimerDxe.inf
>> +
>> +  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>> +
>> +  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>> +
>> +  Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>> +
>> +  #
>> +  # MMC/SD
>> +  #
>> +  EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
>> +  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
>> +
>> +  #
>> +  # Android Loader
>> +  #
>> +  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf
>> +  EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf {
>> +    <LibraryClasses>
>> +      BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf
> 
> This is already getting globally resolved to the same version.
> 
>> +  }
>> +
>> +  #
>> +  # UEFI Network Stack
>> +  #
>> +  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
>> +  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
>> +  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
>> +  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
>> +  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
>> +  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>> +  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
>> +  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
>> +  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>> +
>> +  #
>> +  # FAT filesystem + GPT/MBR partitioning
>> +  #
>> +  MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
>> +  MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
>> +  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>> +  FatPkg/EnhancedFatDxe/Fat.inf
>> +
>> +  #
>> +  # Bds
>> +  #
>> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>> +  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>> +  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>> +  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>> +  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>> +  MdeModulePkg/Application/UiApp/UiApp.inf {
>> +    <LibraryClasses>
>> +      NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
>> +      NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
>> +      NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>> +      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>> +  }
>> +  ShellPkg/Application/Shell/Shell.inf {
>> +    <LibraryClasses>
>> +      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
>> +      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf
>> +      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
>> +      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>> +      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
>> +    <PcdsFixedAtBuild>
>> +      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
>> +      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>> +      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>> +  }
>> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
>> new file mode 100644
>> index 0000000..bbafe45
>> --- /dev/null
>> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
>> @@ -0,0 +1,331 @@
>> +#
>> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
>> +#  Copyright (c) 2017, Linaro Ltd.
>> +#
>> +#  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.
>> +#
>> +
>> +################################################################################
>> +#
>> +# FD Section
>> +# The [FD] Section is made up of the definition statements and a
>> +# description of what goes into  the Flash Device Image.  Each FD section
>> +# defines one flash "device" image.  A flash device image may be one of
>> +# the following: Removable media bootable image (like a boot floppy
>> +# image,) an Option ROM image (that would be "flashed" into an add-in
>> +# card,) a System "Flash"  image (that would be burned into a system's
>> +# flash) or an Update ("Capsule") image that will be used to update and
>> +# existing system flash.
>> +#
>> +################################################################################
>> +
>> +[FD.BL33_AP_UEFI]
>> +BaseAddress   = 0x4F000000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>> +Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
>> +ErasePolarity = 1
>> +
>> +# This one is tricky, it must be: BlockSize * NumBlocks = Size
>> +BlockSize     = 0x00001000
>> +NumBlocks     = 0xF0
>> +
>> +################################################################################
>> +#
>> +# Following are lists of FD Region layout which correspond to the locations of different
>> +# images within the flash device.
>> +#
>> +# Regions must be defined in ascending order and may not overlap.
>> +#
>> +# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
>> +# the pipe "|" character, followed by the size of the region, also in hex with the leading
>> +# "0x" characters. Like:
>> +# Offset|Size
>> +# PcdOffsetCName|PcdSizeCName
>> +# RegionType <FV, DATA, or FILE>
>> +#
>> +################################################################################
>> +
>> +0x00000000|0x000F0000
>> +gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>> +FV = FVMAIN_COMPACT
>> +
>> +
>> +################################################################################
>> +#
>> +# FV Section
>> +#
>> +# [FV] section is used to define what components or modules are placed within a flash
>> +# device file.  This section also defines order the components and modules are positioned
>> +# within the image.  The [FV] section consists of define statements, set statements and
>> +# module statements.
>> +#
>> +################################################################################
>> +
>> +[FV.FvMain]
>> +BlockSize          = 0x40
>> +NumBlocks          = 0         # This FV gets compressed so make it just big enough
>> +FvAlignment        = 8         # FV alignment and FV attributes setting.
>> +ERASE_POLARITY     = 1
>> +MEMORY_MAPPED      = TRUE
>> +STICKY_WRITE       = TRUE
>> +LOCK_CAP           = TRUE
>> +LOCK_STATUS        = TRUE
>> +WRITE_DISABLED_CAP = TRUE
>> +WRITE_ENABLED_CAP  = TRUE
>> +WRITE_STATUS       = TRUE
>> +WRITE_LOCK_CAP     = TRUE
>> +WRITE_LOCK_STATUS  = TRUE
>> +READ_DISABLED_CAP  = TRUE
>> +READ_ENABLED_CAP   = TRUE
>> +READ_STATUS        = TRUE
>> +READ_LOCK_CAP      = TRUE
>> +READ_LOCK_STATUS   = TRUE
>> +
>> +  INF MdeModulePkg/Core/Dxe/DxeMain.inf
>> +
>> +  #
>> +  # PI DXE Drivers producing Architectural Protocols (EFI Services)
>> +  #
>> +  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
>> +  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>> +  INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>> +  INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>> +  INF EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
>> +  INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
>> +  INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>> +  INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>> +
>> +  #
>> +  # Multiple Console IO support
>> +  #
>> +  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>> +  INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
>> +  INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
>> +
>> +  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>> +  INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
>> +
>> +  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>> +
>> +  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>> +
>> +  #
>> +  # Multimedia Card Interface
>> +  #
>> +  INF EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
>> +  INF EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
>> +
>> +  #
>> +  # Android Loader
>> +  #
>> +  INF EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>> +
>> +  #
>> +  # UEFI Network Stack
>> +  #
>> +  INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
>> +  INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
>> +  INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
>> +  INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
>> +  INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
>> +  INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>> +  INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
>> +  INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
>> +  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>> +
>> +  #
>> +  # FAT filesystem + GPT/MBR partitioning
>> +  #
>> +  INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
>> +  INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
>> +  INF FatPkg/EnhancedFatDxe/Fat.inf
>> +  INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>> +
>> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>> +  INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>> +
>> +  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>> +
>> +  #
>> +  # UEFI applications
>> +  #
>> +  INF ShellPkg/Application/Shell/Shell.inf
>> +
>> +  #
>> +  # Bds
>> +  #
>> +  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>> +  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>> +  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>> +  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>> +  INF MdeModulePkg/Application/UiApp/UiApp.inf
>> +
>> +  #
>> +  # Zx296718Evb Platform
>> +  #
>> +  INF Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>> +
>> +[FV.FVMAIN_COMPACT]
>> +FvAlignment        = 8
>> +ERASE_POLARITY     = 1
>> +MEMORY_MAPPED      = TRUE
>> +STICKY_WRITE       = TRUE
>> +LOCK_CAP           = TRUE
>> +LOCK_STATUS        = TRUE
>> +WRITE_DISABLED_CAP = TRUE
>> +WRITE_ENABLED_CAP  = TRUE
>> +WRITE_STATUS       = TRUE
>> +WRITE_LOCK_CAP     = TRUE
>> +WRITE_LOCK_STATUS  = TRUE
>> +READ_DISABLED_CAP  = TRUE
>> +READ_ENABLED_CAP   = TRUE
>> +READ_STATUS        = TRUE
>> +READ_LOCK_CAP      = TRUE
>> +READ_LOCK_STATUS   = TRUE
>> +
>> +  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
>> +
>> +  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>> +    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>> +      SECTION FV_IMAGE = FVMAIN
>> +    }
>> +  }
>> +
>> +
>> +################################################################################
>> +#
>> +# Rules are use with the [FV] section's module INF type to define
>> +# how an FFS file is created for a given INF file. The following Rule are the default
>> +# rules for the different module type. User can add the customized rules to define the
>> +# content of the FFS file.
>> +#
>> +################################################################################
>> +
>> +
>> +############################################################################
>> +# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #
>> +############################################################################
>> +#
>> +#[Rule.Common.DXE_DRIVER]
>> +#  FILE DRIVER = $(NAMED_GUID) {
>> +#    DXE_DEPEX    DXE_DEPEX               Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +#    COMPRESS PI_STD {
>> +#      GUIDED {
>> +#        PE32     PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +#        UI       STRING="$(MODULE_NAME)" Optional
>> +#        VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>> +#      }
>> +#    }
>> +#  }
>> +#
>> +############################################################################
>> +
>> +#
>> +# These SEC rules are used for ArmPlatformPkg/PrePi module.
>> +# ArmPlatformPkg/PrePi is declared as a SEC module to make GenFv patch the
>> +# UEFI Firmware to jump to ArmPlatformPkg/PrePi entrypoint
>> +#
>> +[Rule.ARM.SEC]
>> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
>> +    TE  TE    Align = 32                $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +  }
>> +
>> +[Rule.AARCH64.SEC]
>> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
>> +    TE  TE    Align = 4K                $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +  }
>> +
>> +# A shim specific rule is required to ensure the alignment is 4K.
>> +# Otherwise BaseTools pick up the AArch32 alignment (ie: 32)
>> +[Rule.ARM.SEC.SHIM]
>> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
>> +    TE  TE    Align = 4K                $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +  }
>> +
>> +[Rule.Common.PEI_CORE]
>> +  FILE PEI_CORE = $(NAMED_GUID) {
>> +    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    UI     STRING ="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.PEIM]
>> +  FILE PEIM = $(NAMED_GUID) {
>> +     PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +     UI       STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.PEIM.TIANOCOMPRESSED]
>> +  FILE PEIM = $(NAMED_GUID) DEBUG_MYTOOLS_IA32 {
>> +    PEI_DEPEX PEI_DEPEX Optional        $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +    GUIDED A31280AD-481E-41B6-95E8-127F4C984779 PROCESSING_REQUIRED = TRUE {
>> +      PE32      PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +      UI        STRING="$(MODULE_NAME)" Optional
>> +    }
>> +  }
>> +
>> +[Rule.Common.DXE_CORE]
>> +  FILE DXE_CORE = $(NAMED_GUID) {
>> +    PE32     PE32                       $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    UI       STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.UEFI_DRIVER]
>> +  FILE DRIVER = $(NAMED_GUID) {
>> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    UI           STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.DXE_DRIVER]
>> +  FILE DRIVER = $(NAMED_GUID) {
>> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    UI           STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.DXE_DRIVER.BINARY]
>> +  FILE DRIVER = $(NAMED_GUID) {
>> +  DXE_DEPEX    DXE_DEPEX              Optional |.depex
>> +  PE32         PE32                   |.efi
>> +  UI           STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.DXE_RUNTIME_DRIVER]
>> +  FILE DRIVER = $(NAMED_GUID) {
>> +    DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>> +    PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    UI           STRING="$(MODULE_NAME)" Optional
>> +  }
>> +
>> +[Rule.Common.UEFI_APPLICATION]
>> +  FILE APPLICATION = $(NAMED_GUID) {
>> +    UI     STRING ="$(MODULE_NAME)" Optional
>> +    PE32   PE32                         $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +  }
>> +
>> +[Rule.Common.UEFI_DRIVER.BINARY]
>> +  FILE DRIVER = $(NAMED_GUID) {
>> +    DXE_DEPEX DXE_DEPEX Optional      |.depex
>> +    PE32      PE32                    |.efi
>> +    UI        STRING="$(MODULE_NAME)" Optional
>> +    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>> +  }
>> +
>> +[Rule.Common.UEFI_APPLICATION.BINARY]
>> +  FILE APPLICATION = $(NAMED_GUID) {
>> +    PE32      PE32                    |.efi
>> +    UI        STRING="$(MODULE_NAME)" Optional
>> +    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>> +  }
>> +
>> +[Rule.Common.USER_DEFINED.ACPITABLE]
> 
> Are you planning on adding ACPI support?
> 
>> +  FILE FREEFORM = $(NAMED_GUID) {
>> +    RAW ACPI               |.acpi
>> +    RAW ASL                |.aml
>> +  }
>> -- 
>> 1.9.1
>>