From: Pankaj Bansal <pankaj.bansal@nxp.com>

LX2160A Reference Design Board (RDB) is a high-performance development
platform that supports the QorIQ LX2160A Layerscape Architecture SOCs.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    V2:
    - split Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ in two parts
      part containing gPlatformGetClockPpi is put before PL011UartClockLib
      implementation.

 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec                          |  23 +++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc                          |  46 ++++++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf                          | 168 ++++++++++++++++++++
 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf  |   3 +
 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c    |   4 +-
 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c |  54 ++++++-
 6 files changed, 296 insertions(+), 2 deletions(-)

diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec
new file mode 100644
index 000000000000..03996b07fcb9
--- /dev/null
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec
@@ -0,0 +1,23 @@
+#  LX2160aRdbPkg.dec
+#  LX2160a board package.
+#
+#  Copyright 2018, 2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+[Defines]
+  PACKAGE_NAME                   = LX2160aRdbPkg
+  PACKAGE_GUID                   = 474e0c59-5f77-4060-82dd-9025ee4f4939
+
+################################################################################
+#
+# 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
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
new file mode 100644
index 000000000000..9b3e0386c13e
--- /dev/null
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
@@ -0,0 +1,46 @@
+#  LX2160aRdbPkg.dsc
+#
+#  LX2160ARDB Board package.
+#
+#  Copyright 2018-2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+  #
+  # Defines for default states.  These can be changed on the command line.
+  # -D FLAG=VALUE
+  #
+  PLATFORM_NAME                  = LX2160aRdbPkg
+  PLATFORM_GUID                  = be06d8bc-05eb-44d6-b39f-191e93617ebd
+  OUTPUT_DIRECTORY               = Build/LX2160aRdbPkg
+  FLASH_DEFINITION               = Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
+
+!include Silicon/NXP/NxpQoriqLs.dsc.inc
+!include Silicon/NXP/LX2160A/LX2160A.dsc.inc
+
+[LibraryClasses.common]
+  ArmPlatformLib|Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
+  RealTimeClockLib|EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
+
+################################################################################
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+################################################################################
+[Components.common]
+  #
+  # Architectural Protocols
+  #
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
+    <PcdsFixedAtBuild>
+    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
+  }
+
+ ##
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
new file mode 100644
index 000000000000..19d2ca9bbe58
--- /dev/null
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
@@ -0,0 +1,168 @@
+#  LX2160aRdbPkg.fdf
+#
+#  FLASH layout file for LX2160a board.
+#
+#  Copyright 2018-2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# 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.LX2160ARDB_EFI]
+BaseAddress   = 0x82000000|gArmTokenSpaceGuid.PcdFdBaseAddress  #The base address of the FLASH Device.
+Size          = 0x00140000|gArmTokenSpaceGuid.PcdFdSize         #The size in bytes of the FLASH Device
+ErasePolarity = 1
+BlockSize     = 0x10000
+NumBlocks     = 0x14
+
+################################################################################
+#
+# 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|0x00140000
+gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
+FV = FVMAIN_COMPACT
+
+!include Platform/NXP/FVRules.fdf.inc
+################################################################################
+#
+# 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]
+FvNameGuid         = 1037c42b-8452-4c41-aac7-41e6c31468da
+BlockSize          = 0x1
+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
+  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.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 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+  INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+  INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
+  INF MdeModulePkg/Universal/Metronome/Metronome.inf
+  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+  #
+  # Multiple Console IO support
+  #
+  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+  INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+  INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.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 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.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
+
+  #
+  # UEFI application (Shell Embedded Boot Loader)
+  #
+  INF ShellPkg/Application/Shell/Shell.inf
+
+  #
+  # Bds
+  #
+  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  INF MdeModulePkg/Application/UiApp/UiApp.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/PrePeiCore/PrePeiCoreUniCore.inf
+  INF MdeModulePkg/Core/Pei/PeiMain.inf
+  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
+  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+
+  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
+    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
+      SECTION FV_IMAGE = FVMAIN
+    }
+  }
diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
index 743836e57141..692ba74ec59e 100644
--- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
+++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
@@ -19,9 +19,12 @@
   ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
   Silicon/NXP/NxpQoriqLs.dec
+  Silicon/NXP/Chassis3V2/Chassis3V2.dec
+  Silicon/NXP/LX2160A/LX2160A.dec
 
 [LibraryClasses]
   ArmLib
+  SocLib
   DebugLib
 
 [Sources.common]
diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
index 806cfd180bee..f3f1e5b3f220 100644
--- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
+++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
@@ -8,6 +8,7 @@
 
 #include <Library/ArmLib.h>
 #include <Library/ArmPlatformLib.h>
+#include <Library/SocLib.h>
 
 #include <Ppi/ArmMpCoreInfo.h>
 #include <Ppi/NxpPlatformGetClock.h>
@@ -76,6 +77,7 @@ NxpPlatformGetClock(
   case NXP_I2C_CLOCK:
   case NXP_UART_CLOCK:
     Clock = NxpPlatformGetClock (NXP_SYSTEM_CLOCK);
+    Clock = SocGetClock (Clock, ClockType, Args);
     break;
   default:
     break;
@@ -98,7 +100,7 @@ ArmPlatformInitialize (
   IN  UINTN                     MpId
   )
 {
-  //TODO: Implement me
+  SocInit ();
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
index ad6862dd81eb..391dab265ad7 100644
--- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
+++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
@@ -7,7 +7,12 @@
 **/
 
 #include <Library/ArmPlatformLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Soc.h>
+
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          6
 
 /**
   Return the Virtual Memory Map of your platform
@@ -24,5 +29,52 @@ ArmPlatformGetVirtualMemoryMap (
   IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
   )
 {
-  ASSERT(0);
+  UINTN                            Index;
+  ARM_MEMORY_REGION_DESCRIPTOR     *VirtualMemoryTable;
+
+  Index = 0;
+
+  ASSERT (VirtualMemoryMap != NULL);
+
+  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
+                                     MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
+
+  if (VirtualMemoryTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__));
+    return;
+  }
+
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM0_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM0_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_DRAM0_SIZE;
+  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM1_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM1_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_DRAM1_SIZE;
+  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM2_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM2_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_DRAM2_SIZE;
+  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  // CCSR Space
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_CCSR_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_CCSR_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_CCSR_SIZE;
+  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // FlexSPI
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_FSPI0_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_FSPI0_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_FSPI0_SIZE;
+  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // End of Table
+  ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
+
+  ASSERT (Index < MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
+
+  *VirtualMemoryMap = VirtualMemoryTable;
 }
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60328): https://edk2.groups.io/g/devel/message/60328
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Wed, May 27, 2020 at 14:21:34 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> 
> LX2160A Reference Design Board (RDB) is a high-performance development
> platform that supports the QorIQ LX2160A Layerscape Architecture SOCs.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

Minor style comment below.

> ---
> 
> Notes:
>     V2:
>     - split Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ in two parts
>       part containing gPlatformGetClockPpi is put before PL011UartClockLib
>       implementation.
> 
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec                          |  23 +++
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc                          |  46 ++++++
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf                          | 168 ++++++++++++++++++++
>  Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf  |   3 +
>  Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c    |   4 +-
>  Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c |  54 ++++++-
>  6 files changed, 296 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec
> new file mode 100644
> index 000000000000..03996b07fcb9
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec
> @@ -0,0 +1,23 @@
> +#  LX2160aRdbPkg.dec
> +#  LX2160a board package.
> +#
> +#  Copyright 2018, 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +[Defines]
> +  PACKAGE_NAME                   = LX2160aRdbPkg
> +  PACKAGE_GUID                   = 474e0c59-5f77-4060-82dd-9025ee4f4939
> +
> +################################################################################
> +#
> +# 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
> diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
> new file mode 100644
> index 000000000000..9b3e0386c13e
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
> @@ -0,0 +1,46 @@
> +#  LX2160aRdbPkg.dsc
> +#
> +#  LX2160ARDB Board package.
> +#
> +#  Copyright 2018-2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  #
> +  # Defines for default states.  These can be changed on the command line.
> +  # -D FLAG=VALUE
> +  #
> +  PLATFORM_NAME                  = LX2160aRdbPkg
> +  PLATFORM_GUID                  = be06d8bc-05eb-44d6-b39f-191e93617ebd
> +  OUTPUT_DIRECTORY               = Build/LX2160aRdbPkg
> +  FLASH_DEFINITION               = Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> +
> +!include Silicon/NXP/NxpQoriqLs.dsc.inc
> +!include Silicon/NXP/LX2160A/LX2160A.dsc.inc
> +
> +[LibraryClasses.common]
> +  ArmPlatformLib|Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +  RealTimeClockLib|EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
> +
> +################################################################################
> +#
> +# Components Section - list of all EDK II Modules needed by this Platform
> +#
> +################################################################################
> +[Components.common]
> +  #
> +  # Architectural Protocols
> +  #
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> +    <PcdsFixedAtBuild>
> +    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
> +  }
> +
> + ##
> diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> new file mode 100644
> index 000000000000..19d2ca9bbe58
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> @@ -0,0 +1,168 @@
> +#  LX2160aRdbPkg.fdf
> +#
> +#  FLASH layout file for LX2160a board.
> +#
> +#  Copyright 2018-2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +################################################################################
> +#
> +# 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.LX2160ARDB_EFI]
> +BaseAddress   = 0x82000000|gArmTokenSpaceGuid.PcdFdBaseAddress  #The base address of the FLASH Device.
> +Size          = 0x00140000|gArmTokenSpaceGuid.PcdFdSize         #The size in bytes of the FLASH Device
> +ErasePolarity = 1
> +BlockSize     = 0x10000
> +NumBlocks     = 0x14
> +
> +################################################################################
> +#
> +# 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|0x00140000
> +gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> +FV = FVMAIN_COMPACT
> +
> +!include Platform/NXP/FVRules.fdf.inc
> +################################################################################
> +#
> +# 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]
> +FvNameGuid         = 1037c42b-8452-4c41-aac7-41e6c31468da
> +BlockSize          = 0x1
> +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
> +  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.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 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
> +  INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/Metronome/Metronome.inf
> +  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +
> +  #
> +  # Multiple Console IO support
> +  #
> +  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> +  INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> +  INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.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 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.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
> +
> +  #
> +  # UEFI application (Shell Embedded Boot Loader)
> +  #
> +  INF ShellPkg/Application/Shell/Shell.inf
> +
> +  #
> +  # Bds
> +  #
> +  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> +  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
> +  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +  INF MdeModulePkg/Application/UiApp/UiApp.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/PrePeiCore/PrePeiCoreUniCore.inf
> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +
> +  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> +    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> +      SECTION FV_IMAGE = FVMAIN
> +    }
> +  }
> diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 743836e57141..692ba74ec59e 100644
> --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -19,9 +19,12 @@
>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
>    Silicon/NXP/NxpQoriqLs.dec
> +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> +  Silicon/NXP/LX2160A/LX2160A.dec

Please insert alphabetically sorted.

>  
>  [LibraryClasses]
>    ArmLib
> +  SocLib
>    DebugLib

Please insert alphabetically sorted.

No further comments on this set for v2.
For the patches I have not commented on:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

/
    Leif

>  
>  [Sources.common]
> diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index 806cfd180bee..f3f1e5b3f220 100644
> --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -8,6 +8,7 @@
>  
>  #include <Library/ArmLib.h>
>  #include <Library/ArmPlatformLib.h>
> +#include <Library/SocLib.h>
>  
>  #include <Ppi/ArmMpCoreInfo.h>
>  #include <Ppi/NxpPlatformGetClock.h>
> @@ -76,6 +77,7 @@ NxpPlatformGetClock(
>    case NXP_I2C_CLOCK:
>    case NXP_UART_CLOCK:
>      Clock = NxpPlatformGetClock (NXP_SYSTEM_CLOCK);
> +    Clock = SocGetClock (Clock, ClockType, Args);
>      break;
>    default:
>      break;
> @@ -98,7 +100,7 @@ ArmPlatformInitialize (
>    IN  UINTN                     MpId
>    )
>  {
> -  //TODO: Implement me
> +  SocInit ();
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> index ad6862dd81eb..391dab265ad7 100644
> --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> +++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> @@ -7,7 +7,12 @@
>  **/
>  
>  #include <Library/ArmPlatformLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Soc.h>
> +
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          6
>  
>  /**
>    Return the Virtual Memory Map of your platform
> @@ -24,5 +29,52 @@ ArmPlatformGetVirtualMemoryMap (
>    IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
>    )
>  {
> -  ASSERT(0);
> +  UINTN                            Index;
> +  ARM_MEMORY_REGION_DESCRIPTOR     *VirtualMemoryTable;
> +
> +  Index = 0;
> +
> +  ASSERT (VirtualMemoryMap != NULL);
> +
> +  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> +                                     MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +
> +  if (VirtualMemoryTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__));
> +    return;
> +  }
> +
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM0_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM0_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_DRAM0_SIZE;
> +  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM1_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM1_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_DRAM1_SIZE;
> +  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_DRAM2_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_DRAM2_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_DRAM2_SIZE;
> +  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // CCSR Space
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_CCSR_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_CCSR_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_CCSR_SIZE;
> +  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // FlexSPI
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_FSPI0_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_FSPI0_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_FSPI0_SIZE;
> +  VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // End of Table
> +  ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +
> +  ASSERT (Index < MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
>  }
> -- 
> 2.17.1
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60404): https://edk2.groups.io/g/devel/message/60404
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> > ---
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > +++
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > @@ -19,9 +19,12 @@
> >    ArmPkg/ArmPkg.dec
> >    ArmPlatformPkg/ArmPlatformPkg.dec
> >    Silicon/NXP/NxpQoriqLs.dec
> > +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> > +  Silicon/NXP/LX2160A/LX2160A.dec
> 
> Please insert alphabetically sorted.

I intentionally kept these in this order. because Soc depends on Chassis for any include files
Chassis depends on NxpQoriqLs.dec for any include files.

> 
> >
> >  [LibraryClasses]
> >    ArmLib
> > +  SocLib
> >    DebugLib
> 
> Please insert alphabetically sorted.

This I can take care

> 
> No further comments on this set for v2.
> For the patches I have not commented on:
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> 
> /
>     Leif
> 
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60426): https://edk2.groups.io/g/devel/message/60426
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, May 29, 2020 at 05:21:24 +0000, Pankaj Bansal (OSS) wrote:
> > > ---
> > a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > +++
> > b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > @@ -19,9 +19,12 @@
> > >    ArmPkg/ArmPkg.dec
> > >    ArmPlatformPkg/ArmPlatformPkg.dec
> > >    Silicon/NXP/NxpQoriqLs.dec
> > > +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> > > +  Silicon/NXP/LX2160A/LX2160A.dec
> > 
> > Please insert alphabetically sorted.
> 
> I intentionally kept these in this order. because Soc depends on Chassis for any include files
> Chassis depends on NxpQoriqLs.dec for any include files.

OK, I see the logic behind that, but as it has no functional meaning I
don't think it outweighs the benefit of alphabetical sorting.

I *do* tend to say "alphabetically, unless there is a better logical
order", but by that I usually mean grouping related Pcd dependency
declarations.

This is my only remaining niggle with this set - are you OK with me
folding in

--- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
+++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
@@ -18,9 +18,9 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
   ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
-  Silicon/NXP/NxpQoriqLs.dec
   Silicon/NXP/Chassis3V2/Chassis3V2.dec
   Silicon/NXP/LX2160A/LX2160A.dec
+  Silicon/NXP/NxpQoriqLs.dec

to v3 9/10 and pushing the set?

/
    Leif

> > 
> > >
> > >  [LibraryClasses]
> > >    ArmLib
> > > +  SocLib
> > >    DebugLib
> > 
> > Please insert alphabetically sorted.
> 
> This I can take care
> 
> > 
> > No further comments on this set for v2.
> > For the patches I have not commented on:
> > Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> > 
> > /
> >     Leif
> > 
> > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60449): https://edk2.groups.io/g/devel/message/60449
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> On Fri, May 29, 2020 at 05:21:24 +0000, Pankaj Bansal (OSS) wrote:
> > > > ---
> > >
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > > +++
> > >
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > > @@ -19,9 +19,12 @@
> > > >    ArmPkg/ArmPkg.dec
> > > >    ArmPlatformPkg/ArmPlatformPkg.dec
> > > >    Silicon/NXP/NxpQoriqLs.dec
> > > > +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> > > > +  Silicon/NXP/LX2160A/LX2160A.dec
> > >
> > > Please insert alphabetically sorted.
> >
> > I intentionally kept these in this order. because Soc depends on Chassis for any
> include files
> > Chassis depends on NxpQoriqLs.dec for any include files.
> 
> OK, I see the logic behind that, but as it has no functional meaning I
> don't think it outweighs the benefit of alphabetical sorting.
> 

I referred to the inf specifications for this:
https://edk2-docs.gitbook.io/edk-ii-inf-specification/3_edk_ii_inf_file_format/37_-packages-_sections

"Packages must be listed in the order that may be required for specifying include path statements for a compiler.
For example, the MdePkg/MdePkg.dec file must be listed before the MdeModulePkg/MdeModulePkg.dec file"

By design the Chassis package files can include generic NXP package files. not the other way around.
same goes for SOC package files. SOC package files can include Chassis Package file but not the other way around
and we can extend this further. any platform package files can include SOC package files

> I *do* tend to say "alphabetically, unless there is a better logical
> order", but by that I usually mean grouping related Pcd dependency
> declarations.

as I mentioned above, there *is* a logical order to packages.

> 
> This is my only remaining niggle with this set - are you OK with me
> folding in
> 
> --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -18,9 +18,9 @@ [Packages]
>    MdeModulePkg/MdeModulePkg.dec
>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> -  Silicon/NXP/NxpQoriqLs.dec
>    Silicon/NXP/Chassis3V2/Chassis3V2.dec
>    Silicon/NXP/LX2160A/LX2160A.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> 
> to v3 9/10 and pushing the set?
> 
> /
>     Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60452): https://edk2.groups.io/g/devel/message/60452
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, May 29, 2020 at 12:25:43 +0000, Pankaj Bansal (OSS) wrote:
> > > I intentionally kept these in this order. because Soc depends on Chassis for any
> > include files
> > > Chassis depends on NxpQoriqLs.dec for any include files.
> > 
> > OK, I see the logic behind that, but as it has no functional meaning I
> > don't think it outweighs the benefit of alphabetical sorting.
> > 
> 
> I referred to the inf specifications for this:
> https://edk2-docs.gitbook.io/edk-ii-inf-specification/3_edk_ii_inf_file_format/37_-packages-_sections
> 
> "Packages must be listed in the order that may be required for
> specifying include path statements for a compiler.
> For example, the MdePkg/MdePkg.dec file must be listed before the
> MdeModulePkg/MdeModulePkg.dec file"

I see that. (And sigh.) Clearly, that needs to be fixed. I have cc:d
you on the patch to do that.

I am sure that made sense when that text was originally written, which
was presumably the previous millenium. But this is akin to "optimising"
the order in which header files is included.

> By design the Chassis package files can include generic NXP package
> files. not the other way around.
> same goes for SOC package files. SOC package files can include
> Chassis Package file but not the other way around
> and we can extend this further. any platform package files can
> include SOC package files
> 
> > I *do* tend to say "alphabetically, unless there is a better logical
> > order", but by that I usually mean grouping related Pcd dependency
> > declarations.
> 
> as I mentioned above, there *is* a logical order to packages.

And as I said "I see the logic behind that".
I'm saying readability trumps that logic.

> > 
> > This is my only remaining niggle with this set - are you OK with me
> > folding in
> > 
> > --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > +++
> > b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > @@ -18,9 +18,9 @@ [Packages]
> >    MdeModulePkg/MdeModulePkg.dec
> >    ArmPkg/ArmPkg.dec
> >    ArmPlatformPkg/ArmPlatformPkg.dec
> > -  Silicon/NXP/NxpQoriqLs.dec
> >    Silicon/NXP/Chassis3V2/Chassis3V2.dec
> >    Silicon/NXP/LX2160A/LX2160A.dec
> > +  Silicon/NXP/NxpQoriqLs.dec
> > 
> > to v3 9/10 and pushing the set?
> > 
> > /
> >     Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60456): https://edk2.groups.io/g/devel/message/60456
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> On Fri, May 29, 2020 at 12:25:43 +0000, Pankaj Bansal (OSS) wrote:
> > > > I intentionally kept these in this order. because Soc depends on Chassis
> for any
> > > include files
> > > > Chassis depends on NxpQoriqLs.dec for any include files.
> > >
> > > OK, I see the logic behind that, but as it has no functional meaning I
> > > don't think it outweighs the benefit of alphabetical sorting.
> > >
> >
> > I referred to the inf specifications for this:
> > https://edk2-docs.gitbook.io/edk-ii-inf-
> specification/3_edk_ii_inf_file_format/37_-packages-_sections
> >
> > "Packages must be listed in the order that may be required for
> > specifying include path statements for a compiler.
> > For example, the MdePkg/MdePkg.dec file must be listed before the
> > MdeModulePkg/MdeModulePkg.dec file"
> 
> I see that. (And sigh.) Clearly, that needs to be fixed. I have cc:d
> you on the patch to do that.
> 
> I am sure that made sense when that text was originally written, which
> was presumably the previous millenium. But this is akin to "optimising"
> the order in which header files is included.
> 

hmm. I think it was written to maintain the dependency between include files.
But I guess if we follow edk2 c guideline we can avoid this problem.

https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-4-include-files-may-include-only-those-headers-that-it-directly-depends-upon


> > By design the Chassis package files can include generic NXP package
> > files. not the other way around.
> > same goes for SOC package files. SOC package files can include
> > Chassis Package file but not the other way around
> > and we can extend this further. any platform package files can
> > include SOC package files
> >
> > > I *do* tend to say "alphabetically, unless there is a better logical
> > > order", but by that I usually mean grouping related Pcd dependency
> > > declarations.
> >
> > as I mentioned above, there *is* a logical order to packages.
> 
> And as I said "I see the logic behind that".
> I'm saying readability trumps that logic.

Ok. it makes sense.

> 
> > >
> > > This is my only remaining niggle with this set - are you OK with me
> > > folding in
> > >
> > > ---
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > +++
> > >
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > @@ -18,9 +18,9 @@ [Packages]
> > >    MdeModulePkg/MdeModulePkg.dec
> > >    ArmPkg/ArmPkg.dec
> > >    ArmPlatformPkg/ArmPlatformPkg.dec
> > > -  Silicon/NXP/NxpQoriqLs.dec
> > >    Silicon/NXP/Chassis3V2/Chassis3V2.dec
> > >    Silicon/NXP/LX2160A/LX2160A.dec
> > > +  Silicon/NXP/NxpQoriqLs.dec
> > >
> > > to v3 9/10 and pushing the set?

Ok. can you please sort all the dec files under [Packages] based on names before pushing ?
I think this file also needs alphabetical sorting:
https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf#L16

> > >
> > > /
> > >     Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60469): https://edk2.groups.io/g/devel/message/60469
Mute This Topic: https://groups.io/mt/74496024/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-