Re: [PATCH edk2-platforms 22/41] Platform/NXP: Add ArmPlatformLib for LS1046A

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Nov 28, 2018 at 08:31:36PM +0530, Meenakshi Aggarwal wrote:
> From: Vabhav <vabhav.sharma@nxp.com>
> 
> Adding support of ArmPlatformLib for NXP LS1046ARDB board
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vabhav <vabhav.sharma@nxp.com>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  .../Library/PlatformLib/ArmPlatformLib.c           | 105 ++++++++++++++
>  .../Library/PlatformLib/ArmPlatformLib.inf         |  66 +++++++++
>  .../Library/PlatformLib/NxpQoriqLsHelper.S         |  35 +++++
>  .../Library/PlatformLib/NxpQoriqLsMem.c            | 152 +++++++++++++++++++++
>  4 files changed, 358 insertions(+)
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> 
> diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.c b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
> new file mode 100644
> index 0000000..c59a06a
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
> @@ -0,0 +1,105 @@
> +/** ArmPlatformLib.c
> +*
> +*  Contains board initialization functions.
> +*
> +*  Based on BeagleBoardPkg/Library/BeagleBoardLib/BeagleBoard.c
> +*
> +*  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> +*  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> +*  Copyright 2017 NXP
> +*
> +*  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 <Ppi/ArmMpCoreInfo.h>
> +
> +extern VOID SocInit (VOID);

Please import this through an include file.

> +
> +/**
> +  Return the current Boot Mode
> +
> +  This function returns the boot reason on the platform
> +
> +**/
> +EFI_BOOT_MODE
> +ArmPlatformGetBootMode (
> +  VOID
> +  )
> +{
> +  return BOOT_WITH_FULL_CONFIGURATION;
> +}
> +
> +/**
> + Placeholder for Platform Initialization
> +**/
> +EFI_STATUS
> +ArmPlatformInitialize (
> +  IN  UINTN   MpId
> +  )
> +{
> +  SocInit ();
> +
> +  return EFI_SUCCESS;
> +}
> +
> +ARM_CORE_INFO LS1046aMpCoreInfoCTA72x4[] = {

STATIC?
m-prefix.

> +  {
> +    // Cluster 0, Core 0
> +    0x0, 0x0,
> +
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (UINT64)0xFFFFFFFF
> +  },
> +};

Move global variables before function definitions start:
https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/54_code_file_structure.html

> +
> +EFI_STATUS
> +PrePeiCoreGetMpCoreInfo (
> +  OUT UINTN                   *CoreCount,
> +  OUT ARM_CORE_INFO           **ArmCoreTable
> +  )
> +{
> +  *CoreCount    = sizeof (LS1046aMpCoreInfoCTA72x4) / sizeof (ARM_CORE_INFO);
> +  *ArmCoreTable = LS1046aMpCoreInfoCTA72x4;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };

STATIC?
Before function definitions.

> +
> +EFI_PEI_PPI_DESCRIPTOR      gPlatformPpiTable[] = {

STATIC?
m-prefix?
Before function definitions?

/
    Leif

> +  {
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
> +    &gArmMpCoreInfoPpiGuid,
> +    &mMpCoreInfoPpi
> +  }
> +};
> +
> +VOID
> +ArmPlatformGetPlatformPpiList (
> +  OUT UINTN                   *PpiListSize,
> +  OUT EFI_PEI_PPI_DESCRIPTOR  **PpiList
> +  )
> +{
> +  *PpiListSize = sizeof (gPlatformPpiTable);
> +  *PpiList = gPlatformPpiTable;
> +}
> +
> +
> +UINTN
> +ArmPlatformGetCorePosition (
> +  IN UINTN MpId
> +  )
> +{
> +  return 1;
> +}
> diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> new file mode 100644
> index 0000000..49b57fc
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> @@ -0,0 +1,66 @@
> +#  @file
> +#
> +#  Copyright 2017 NXP
> +#
> +#  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                    = 0x0001001A
> +  BASE_NAME                      = PlatformLib
> +  FILE_GUID                      = 05a9029b-266f-421d-bb46-0e8385c64aa0
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmPlatformLib
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  SocLib
> +
> +[Sources.common]
> +  NxpQoriqLsHelper.S    | GCC
> +  NxpQoriqLsMem.c
> +  ArmPlatformLib.c
> +
> +[Ppis]
> +  gArmMpCoreInfoPpiGuid
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdArmPrimaryCore
> +  gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1Size
> +  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2Size
> +  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram1BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram1Size
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram2BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram2Size
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram3BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdDram3Size
> +  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
> diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
> new file mode 100644
> index 0000000..6d54091
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
> @@ -0,0 +1,35 @@
> +#  @file
> +#
> +#  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> +#  Copyright 2017 NXP
> +#
> +#  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 <AutoGen.h>
> +
> +.text
> +.align 2
> +
> +GCC_ASM_IMPORT(ArmReadMpidr)
> +
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  tst x0, #3
> +  cset x0, eq
> +  ret
> +
> +ASM_FUNC(ArmPlatformPeiBootAction)
> +  ret
> +
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  MOV32 (x0, FixedPcdGet32(PcdArmPrimaryCore))
> +  ldrh   w0, [x0]
> +  ret
> diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> new file mode 100644
> index 0000000..64c5612
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> @@ -0,0 +1,152 @@
> +/** NxpQoriqLsMem.c
> +*
> +*  Board memory specific Library.
> +*
> +*  Based on BeagleBoardPkg/Library/BeagleBoardLib/BeagleBoardMem.c
> +*
> +*  Copyright (c) 2011, ARM Limited. All rights reserved.
> +*  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> +*  Copyright 2017 NXP
> +*
> +*  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/PcdLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          25
> +
> +/**
> +  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  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
> +  )
> +{
> +  UINTN                            Index;
> +  ARM_MEMORY_REGION_DESCRIPTOR     *VirtualMemoryTable;
> +
> +  Index = 0;
> +
> +  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;
> +  }
> +
> +  // DRAM1 (Must be 1st entry)
> +  VirtualMemoryTable[Index].PhysicalBase = FixedPcdGet64 (PcdDram1BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdDram1BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdDram1Size);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // CCSR Space
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdCcsrBaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdCcsrBaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdCcsrSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // IFC region 1
> +  //
> +  // A-009241   : Unaligned write transactions to IFC may result in corruption of data
> +  // Affects    : IFC
> +  // Description: 16 byte unaligned write from system bus to IFC may result in extra unintended
> +  //              writes on external IFC interface that can corrupt data on external flash.
> +  // Impact     : Data corruption on external flash may happen in case of unaligned writes to
> +  //              IFC memory space.
> +  // Workaround: Following are the workarounds:
> +  //             For write transactions from core, IFC interface memories (including IFC SRAM)
> +  //                should be configured as device type memory in MMU.
> +  //             For write transactions from non-core masters (like system DMA), the address
> +  //                should be 16 byte aligned and the data size should be multiple of 16 bytes.
> +  //
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdIfcRegion1BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdIfcRegion1BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdIfcRegion1Size);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // QMAN SWP
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdQmanSwpBaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdQmanSwpBaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdQmanSwpSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> +
> +  // BMAN SWP
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdBmanSwpBaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdBmanSwpBaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdBmanSwpSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> +
> +  // IFC region 2
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdIfcRegion2BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdIfcRegion2BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdIfcRegion2Size);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // DRAM2
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdDram2BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdDram2BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdDram2Size);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // PCIe1
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdPciExp1BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdPciExp1BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdPciExp1BaseSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe2
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdPciExp2BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdPciExp2BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdPciExp2BaseSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe3
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdPciExp3BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdPciExp3BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdPciExp3BaseSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // DRAM3
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdDram3BaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdDram3BaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdDram3Size);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // QSPI region
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdQspiRegionBaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdQspiRegionBaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdQspiRegionSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> +
> +  // 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;
> +}
> -- 
> 1.9.1
>