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

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%27s-unkempt-git...

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