[PATCH edk2-platforms 0/7] NXP: Add PCIe Support for LX2160aRdbPkg

[PATCH edk2-platforms 0/7] NXP: Add PCIe Support for LX2160aRdbPkg

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

This patch series adds PCIe support for LX2160aRdbPkg.
LX2160-Rev1 and LX2160-Rev2 has different PCIe controller. Platform
driver checks the SoC version and enable corresponding PCIe controller
and its feature using dynamic PCDs. 

PciHostBridgeLib and PciSegmentLib already has support for both
PCIe controllers.


This patch series dependes on below patch series:
1 - Silicon/NXP: LX2160A: Add SerDes Support
2 - Silicon/NXP: Add SVR and DEVDISRn config configuration

Wasim Khan (7):
  Platform/NXP: LX2160aRdbPkg: Add PCIe space in VirtualMemoryMap
  Silicon/NXP: LX2160A: Define PCIe related PCDs
  Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver
  Platform/NXP: LX2160aRdbPkg: Enable PlatformDxe driver
  Platform/NXP: LX2160aRdbPkg: Hide Root Port for LX2160A-Rev2
  Platform/NXP: LX2160aRdbPkg: Enable NetworkPkg
  Platform/NXP: LX2160aRdbPkg: Enable PCIE support

 Silicon/NXP/NxpQoriqLs.dec                                            |  1 +
 Silicon/NXP/LX2160A/LX2160A.dsc.inc                                   |  5 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc                          | 22 +++++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf                          | 15 +++
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf        | 36 ++++++++
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf             |  1 +
 Silicon/NXP/Include/Pcie.h                                            |  1 +
 Silicon/NXP/LX2160A/Include/Soc.h                                     |  8 ++
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c          | 96 ++++++++++++++++++++
 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 38 +++++++-
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c               |  6 +-
 11 files changed, 227 insertions(+), 2 deletions(-)
 create mode 100644 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
 create mode 100644 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c

-- 
2.7.4

[PATCH edk2-platforms 1/7] Platform/NXP: LX2160aRdbPkg: Add PCIe space in VirtualMemoryMap

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Add PCIe space in VirtualMemoryMap

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/LX2160A/Include/Soc.h                                     |  8 +++++
 Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 38 +++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Silicon/NXP/LX2160A/Include/Soc.h b/Silicon/NXP/LX2160A/Include/Soc.h
index 6c745d580a6d..7e6359485aaf 100644
--- a/Silicon/NXP/LX2160A/Include/Soc.h
+++ b/Silicon/NXP/LX2160A/Include/Soc.h
@@ -26,6 +26,14 @@
 #define LX2160A_FSPI0_PHYS_ADDRESS   (BASE_512MB)
 #define LX2160A_FSPI0_SIZE           (SIZE_256MB)
 
+#define LX2160A_PCI1_PHYS_ADDRESS    0x8000000000
+#define LX2160A_PCI2_PHYS_ADDRESS    0x8800000000
+#define LX2160A_PCI3_PHYS_ADDRESS    0x9000000000
+#define LX2160A_PCI4_PHYS_ADDRESS    0x9800000000
+#define LX2160A_PCI5_PHYS_ADDRESS    0xa000000000
+#define LX2160A_PCI6_PHYS_ADDRESS    0xa800000000
+#define LX2160A_PCI_SIZE             SIZE_32GB
+
 #define LX2160A_DCFG_ADDRESS         NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS
 
 // SVR
diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
index 391dab265ad7..c03bc23f4a1d 100644
--- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
+++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
@@ -12,7 +12,7 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Soc.h>
 
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          6
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          12
 
 /**
   Return the Virtual Memory Map of your platform
@@ -71,6 +71,42 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Length       = LX2160A_FSPI0_SIZE;
   VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
+  // PCIe1
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI1_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI1_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // PCIe2
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI2_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI2_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // PCIe3
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI3_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI3_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // PCIe4
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI4_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI4_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // PCIe5
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI5_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI5_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+  // PCIe6
+  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI6_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI6_PHYS_ADDRESS;
+  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
+  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
   // End of Table
   ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
 
-- 
2.7.4

[PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe related PCDs

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Define PCIe related PCDs for LX2160A.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/LX2160A/LX2160A.dsc.inc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
index fe8ed402fc4e..43e361464c8e 100644
--- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
+++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
@@ -38,6 +38,11 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C0000
 
+  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x8000000000
+  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|6
+  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x80000
+  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x407FC
+
 [PcdsFeatureFlag]
   gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|TRUE
 
-- 
2.7.4

[PATCH edk2-platforms 3/7] Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Add PlatformDxe to do platform specific work.
At present it perform platform specific Pci initialization.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf | 35 +++++++++
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c   | 78 ++++++++++++++++++++
 2 files changed, 113 insertions(+)

diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
new file mode 100644
index 000000000000..2514adf1d69d
--- /dev/null
+++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -0,0 +1,35 @@
+## @file
+#
+#  Copyright 2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION               = 0x00010019
+  BASE_NAME                 = PlatformDxe
+  FILE_GUID                 = C4063EBA-7729-11EA-BC55-0232AC130003
+  MODULE_TYPE               = DXE_DRIVER
+  VERSION_STRING            = 1.0
+  ENTRY_POINT               = PlatformDxeEntryPoint
+
+[Sources]
+  PlatformDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  Silicon/NXP/Chassis3V2/Chassis3V2.dec
+  Silicon/NXP/LX2160A/LX2160A.dec
+  Silicon/NXP/NxpQoriqLs.dec
+
+[LibraryClasses]
+  PcdLib
+  UefiDriverEntryPoint
+
+[Pcd]
+  gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable
+  gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl
+
+[Depex]
+  TRUE
diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
new file mode 100644
index 000000000000..73599aaeb7bf
--- /dev/null
+++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -0,0 +1,78 @@
+/** @file
+*
+*  Copyright 2020 NXP
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <Library/PcdLib.h>
+#include <Library/SocLib.h>
+#include <Soc.h>
+
+/**
+  Enable PciCfgShift feature for LX2160-Rev2
+
+**/
+VOID
+EnableCfgShift (
+  VOID
+  )
+{
+  UINT32 Svr;
+
+  Svr = SocGetSvr ();
+  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x2)) {
+    PcdSetBoolS (PcdPciCfgShiftEnable, TRUE);
+  }
+}
+
+/**
+  Enable Layerscape Gen4 controller for LX2160A-Rev1
+
+**/
+VOID
+EnablePciController (
+  VOID
+  )
+{
+  UINT32 Svr;
+
+  Svr = SocGetSvr ();
+  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x1)) {
+    PcdSetBoolS (PcdPciLsGen4Ctrl, TRUE);
+  }
+}
+
+/**
+  Platfrom Specific PCI Initialization
+
+**/
+VOID
+PlatformPciInit (
+  VOID
+  )
+{
+  EnableCfgShift ();
+  EnablePciController ();
+}
+
+/**
+  The entry point for PlatformDxe driver. This driver
+  intends to perform platform specific initialization.
+
+  @param[in] ImageHandle     The image handle of the driver.
+  @param[in] SystemTable     The system table.
+
+  @retval EFI_SUCCESS         Driver initialization success.
+
+**/
+EFI_STATUS
+EFIAPI
+PlatformDxeEntryPoint (
+  IN EFI_HANDLE                   ImageHandle,
+  IN EFI_SYSTEM_TABLE             *SystemTable
+  )
+{
+  // Platfrom Specific PCI Initialization
+  PlatformPciInit ();
+  return EFI_SUCCESS;
+}
-- 
2.7.4

[PATCH edk2-platforms 4/7] Platform/NXP: LX2160aRdbPkg: Enable PlatformDxe driver

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Enable PlatformDxe driver for LX2160aRdbPkg

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 2 ++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
index 9b3e0386c13e..d0ec7b5a8906 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
@@ -43,4 +43,6 @@ [Components.common]
     gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
   }
 
+  Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
+
  ##
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
index eec1c0774a86..41b8f1ececa4 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
@@ -89,6 +89,9 @@ [FV.FvMain]
   #
   INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
 
+  # Platform DXE Driver
+  INF Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
+
   INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
   INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
-- 
2.7.4

[PATCH edk2-platforms 5/7] Platform/NXP: LX2160aRdbPkg: Hide Root Port for LX2160A-Rev2

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

PCIe Layerscape controller in LX2160A-Rev2 is not completely
ECAM-compliant. It is non-ECAM only for the root bus (bus 0)
and for any other bus underneath the root bus it does support
ECAM access.
One approach can be to setup the controller in firmware and
expose bus[0x1-0xff] to OS via MCFG and DSDT table.
Introduce PcdPciHideRootPort PCD and set it for LX2160A-Rev2 for this.
Limit the size of CFG0 iATU window (for type0 config transactions)
to 32KB to avoid enumuration of unwanted devices in OS.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/NxpQoriqLs.dec                                     |  1 +
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf |  1 +
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf      |  1 +
 Silicon/NXP/Include/Pcie.h                                     |  1 +
 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c   | 18 ++++++++++++++++++
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c        |  6 +++++-
 6 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
index d09a1ae194be..188a9fe1f382 100644
--- a/Silicon/NXP/NxpQoriqLs.dec
+++ b/Silicon/NXP/NxpQoriqLs.dec
@@ -40,3 +40,4 @@ [PcdsFixedAtBuild.common]
 [PcdsDynamic.common]
   gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x00000600
   gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl|FALSE|BOOLEAN|0x00000601
+  gNxpQoriqLsTokenSpaceGuid.PcdPciHideRootPort|FALSE|BOOLEAN|0x00000602
diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
index 2514adf1d69d..674ba3b298f3 100644
--- a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -30,6 +30,7 @@ [LibraryClasses]
 [Pcd]
   gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable
   gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl
+  gNxpQoriqLsTokenSpaceGuid.PcdPciHideRootPort
 
 [Depex]
   TRUE
diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
index 6003da708698..3726ec15317f 100644
--- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
+++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
@@ -42,3 +42,4 @@ [FixedPcd]
 [Pcd]
   gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable
   gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl
+  gNxpQoriqLsTokenSpaceGuid.PcdPciHideRootPort
diff --git a/Silicon/NXP/Include/Pcie.h b/Silicon/NXP/Include/Pcie.h
index b5bf0ff5d1d4..4bc99bb53025 100755
--- a/Silicon/NXP/Include/Pcie.h
+++ b/Silicon/NXP/Include/Pcie.h
@@ -31,6 +31,7 @@
 #define PCI_SEG_PORTIO_MIN        0x0
 #define PCI_SEG_PORTIO_MAX        0xffff
 #define SEG_CFG_SIZE              0x00001000
+#define ECAM_DEVICE_SIZE          SIZE_32KB
 #define ECAM_BUS_SIZE             SIZE_1MB
 #define ECAM_CFG_REGION_SIZE      SIZE_256MB
 #define SEG_MEM_BASE              0x40000000
diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
index 73599aaeb7bf..4733309da47c 100644
--- a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -9,6 +9,23 @@
 #include <Soc.h>
 
 /**
+  Hide PCI Root Port for LX2160A-Rev2
+
+**/
+VOID
+HidePciRootPort (
+  VOID
+  )
+{
+  UINT32 Svr;
+
+  Svr = SocGetSvr ();
+  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x2)) {
+    PcdSetBoolS (PcdPciHideRootPort, TRUE);
+  }
+}
+
+/**
   Enable PciCfgShift feature for LX2160-Rev2
 
 **/
@@ -53,6 +70,7 @@ PlatformPciInit (
 {
   EnableCfgShift ();
   EnablePciController ();
+  HidePciRootPort ();
 }
 
 /**
diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
index 8bbbaaa6e24d..7f11d7a60256 100644
--- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -432,7 +432,11 @@ PcieLsSetupAtu (
     Cfg0BusAddress = SIZE_1MB;
     Cfg1BusAddress = SIZE_2MB;
     // Region for type0 CFG transactions (only for bus1)
-    Cfg0Size = ECAM_BUS_SIZE;
+    if (PcdGetBool (PcdPciHideRootPort)) {
+      Cfg0Size = ECAM_DEVICE_SIZE;
+    } else {
+      Cfg0Size = ECAM_BUS_SIZE;
+    }
     // Region for type1 CFG transactions (for bus > 1)
     Cfg1Size = (ECAM_CFG_REGION_SIZE - ECAM_BUS_SIZE); // 255MB
   } else {
-- 
2.7.4

[PATCH edk2-platforms 6/7] Platform/NXP: LX2160aRdbPkg: Enable NetworkPkg

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Enable NetworkPkg for LX2160aRdbPkg Platform.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 11 +++++++++++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
index d0ec7b5a8906..c73f9fd4d393 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
@@ -22,6 +22,13 @@ [Defines]
   OUTPUT_DIRECTORY               = Build/LX2160aRdbPkg
   FLASH_DEFINITION               = Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
 
+  #
+  # Network definition
+  #
+  DEFINE NETWORK_TLS_ENABLE             = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
+  DEFINE NETWORK_ISCSI_ENABLE           = FALSE
+
 !include Silicon/NXP/NxpQoriqLs.dsc.inc
 !include Silicon/NXP/LX2160A/LX2160A.dsc.inc
 
@@ -45,4 +52,8 @@ [Components.common]
 
   Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
 
+  #
+  # Networking stack
+  #
+!include NetworkPkg/Network.dsc.inc
  ##
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
index 41b8f1ececa4..4c9cab5d0938 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
@@ -116,6 +116,11 @@ [FV.FvMain]
   INF ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
 
   #
+  # Networking stack
+  #
+!include NetworkPkg/Network.fdf.inc
+
+  #
   # FAT filesystem + GPT/MBR partitioning
   #
   INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
-- 
2.7.4

[PATCH edk2-platforms 7/7] Platform/NXP: LX2160aRdbPkg: Enable PCIE support

[Wasim Khan]

From: Wasim Khan <wasim.khan@nxp.com>

Enable generic PCIe drivers and Wire up PciHostBridgeLib,
PciSegmentLib and PciCpuIo2Dxe.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 9 +++++++++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
index c73f9fd4d393..579a72d363a4 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
@@ -35,6 +35,8 @@ [Defines]
 [LibraryClasses.common]
   ArmPlatformLib|Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
   RealTimeClockLib|EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
+  PciSegmentLib|Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
+  PciHostBridgeLib|Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
 
 ################################################################################
 #
@@ -53,6 +55,13 @@ [Components.common]
   Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
 
   #
+  # PCI
+  #
+  Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
+  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+
+  #
   # Networking stack
   #
 !include NetworkPkg/Network.dsc.inc
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
index 4c9cab5d0938..b803da124844 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
@@ -121,6 +121,13 @@ [FV.FvMain]
 !include NetworkPkg/Network.fdf.inc
 
   #
+  # PCI
+  #
+  INF Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
+  INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+
+  #
   # FAT filesystem + GPT/MBR partitioning
   #
   INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
-- 
2.7.4

Re: [PATCH edk2-platforms 1/7] Platform/NXP: LX2160aRdbPkg: Add PCIe space in VirtualMemoryMap

[Leif Lindholm]

On Wed, Jun 10, 2020 at 03:47:24 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Add PCIe space in VirtualMemoryMap

You appear to be adding six spaces to the memory map, not one.

I may know why, but most people do not. Please describe in the commit
message what the spaces are and why there's six of them.

/
    Leif

> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Silicon/NXP/LX2160A/Include/Soc.h                                     |  8 +++++
>  Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 38 +++++++++++++++++++-
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/NXP/LX2160A/Include/Soc.h b/Silicon/NXP/LX2160A/Include/Soc.h
> index 6c745d580a6d..7e6359485aaf 100644
> --- a/Silicon/NXP/LX2160A/Include/Soc.h
> +++ b/Silicon/NXP/LX2160A/Include/Soc.h
> @@ -26,6 +26,14 @@
>  #define LX2160A_FSPI0_PHYS_ADDRESS   (BASE_512MB)
>  #define LX2160A_FSPI0_SIZE           (SIZE_256MB)
>  
> +#define LX2160A_PCI1_PHYS_ADDRESS    0x8000000000
> +#define LX2160A_PCI2_PHYS_ADDRESS    0x8800000000
> +#define LX2160A_PCI3_PHYS_ADDRESS    0x9000000000
> +#define LX2160A_PCI4_PHYS_ADDRESS    0x9800000000
> +#define LX2160A_PCI5_PHYS_ADDRESS    0xa000000000
> +#define LX2160A_PCI6_PHYS_ADDRESS    0xa800000000
> +#define LX2160A_PCI_SIZE             SIZE_32GB
> +
>  #define LX2160A_DCFG_ADDRESS         NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS
>  
>  // SVR
> diff --git a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> index 391dab265ad7..c03bc23f4a1d 100644
> --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> +++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> @@ -12,7 +12,7 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Soc.h>
>  
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          6
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          12
>  
>  /**
>    Return the Virtual Memory Map of your platform
> @@ -71,6 +71,42 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[Index].Length       = LX2160A_FSPI0_SIZE;
>    VirtualMemoryTable[Index++].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>  
> +  // PCIe1
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI1_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI1_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe2
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI2_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI2_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe3
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI3_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI3_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe4
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI4_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI4_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe5
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI5_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI5_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // PCIe6
> +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI6_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI6_PHYS_ADDRESS;
> +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> +  VirtualMemoryTable[Index++].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
>    // End of Table
>    ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
>  
> -- 
> 2.7.4
> 

Re: [PATCH edk2-platforms 1/7] Platform/NXP: LX2160aRdbPkg: Add PCIe space in VirtualMemoryMap

[Wasim Khan (OSS)]

> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Thursday, June 18, 2020 3:22 PM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> ard.biesheuvel@arm.com; Wasim Khan <wasim.khan@nxp.com>
> Subject: Re: [PATCH edk2-platforms 1/7] Platform/NXP: LX2160aRdbPkg: Add
> PCIe space in VirtualMemoryMap
> 
> On Wed, Jun 10, 2020 at 03:47:24 +0530, Wasim Khan wrote:
> > From: Wasim Khan <wasim.khan@nxp.com>
> >
> > Add PCIe space in VirtualMemoryMap
> 
> You appear to be adding six spaces to the memory map, not one.
> 
> I may know why, but most people do not. Please describe in the commit
> message what the spaces are and why there's six of them.

Thank for the review.
We have six PCIe controller on LX2160 SoC. Multiple PCIe controller may get enabled based on RCW.
OK, I will update the commit message.

> 
> /
>     Leif
> 
> > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > ---
> >  Silicon/NXP/LX2160A/Include/Soc.h                                     |  8 +++++
> >
> Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> > | 38 +++++++++++++++++++-
> >  2 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/Silicon/NXP/LX2160A/Include/Soc.h
> > b/Silicon/NXP/LX2160A/Include/Soc.h
> > index 6c745d580a6d..7e6359485aaf 100644
> > --- a/Silicon/NXP/LX2160A/Include/Soc.h
> > +++ b/Silicon/NXP/LX2160A/Include/Soc.h
> > @@ -26,6 +26,14 @@
> >  #define LX2160A_FSPI0_PHYS_ADDRESS   (BASE_512MB)
> >  #define LX2160A_FSPI0_SIZE           (SIZE_256MB)
> >
> > +#define LX2160A_PCI1_PHYS_ADDRESS    0x8000000000
> > +#define LX2160A_PCI2_PHYS_ADDRESS    0x8800000000
> > +#define LX2160A_PCI3_PHYS_ADDRESS    0x9000000000
> > +#define LX2160A_PCI4_PHYS_ADDRESS    0x9800000000
> > +#define LX2160A_PCI5_PHYS_ADDRESS    0xa000000000
> > +#define LX2160A_PCI6_PHYS_ADDRESS    0xa800000000
> > +#define LX2160A_PCI_SIZE             SIZE_32GB
> > +
> >  #define LX2160A_DCFG_ADDRESS
> NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS
> >
> >  // SVR
> > diff --git
> >
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.
> > c
> >
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.
> > c index 391dab265ad7..c03bc23f4a1d 100644
> > ---
> >
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.
> > c
> > +++ b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib
> > +++ Mem.c
> > @@ -12,7 +12,7 @@
> >  #include <Library/MemoryAllocationLib.h>  #include <Soc.h>
> >
> > -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          6
> > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          12
> >
> >  /**
> >    Return the Virtual Memory Map of your platform @@ -71,6 +71,42 @@
> > ArmPlatformGetVirtualMemoryMap (
> >    VirtualMemoryTable[Index].Length       = LX2160A_FSPI0_SIZE;
> >    VirtualMemoryTable[Index++].Attributes =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> >
> > +  // PCIe1
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI1_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI1_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> > +  // PCIe2
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI2_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI2_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> > +  // PCIe3
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI3_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI3_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> > +  // PCIe4
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI4_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI4_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> > +  // PCIe5
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI5_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI5_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> > +  // PCIe6
> > +  VirtualMemoryTable[Index].PhysicalBase = LX2160A_PCI6_PHYS_ADDRESS;
> > + VirtualMemoryTable[Index].VirtualBase  = LX2160A_PCI6_PHYS_ADDRESS;
> > +  VirtualMemoryTable[Index].Length       = LX2160A_PCI_SIZE;
> > +  VirtualMemoryTable[Index++].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +
> >    // End of Table
> >    ZeroMem (&VirtualMemoryTable[Index], sizeof
> > (ARM_MEMORY_REGION_DESCRIPTOR));
> >
> > --
> > 2.7.4
> >

Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe related PCDs

[Leif Lindholm]

On Wed, Jun 10, 2020 at 03:47:25 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Define PCIe related PCDs for LX2160A.
> 
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Silicon/NXP/LX2160A/LX2160A.dsc.inc | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> index fe8ed402fc4e..43e361464c8e 100644
> --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> @@ -38,6 +38,11 @@ [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C0000
>  
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x8000000000

This is already provided by LX2160A_PCI1_PHYS_ADDRESS in
Silicon/NXP/LX2160A/Include/Soc.h, and PCI_SEG0_MMIO_MEMBASE would be
better described as an alias of that. Unless the NXP
PciHostBridgeLib/PciSegmentLib is intended to be shared with SoCs
where these base addresses can be different in different platforms.

If so, the PHYS_ADDRESSES would be better defined as derivatives of
PcdPciExp1BaseAddr.

> +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|6

Is this number possible to be different for different platforms based
on the same SoC?

/
    Leif

> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x80000
> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x407FC
> +
>  [PcdsFeatureFlag]
>    gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|TRUE
>  
> -- 
> 2.7.4
> 

Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe related PCDs

[Wasim Khan (OSS)]

> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Thursday, June 18, 2020 3:38 PM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> ard.biesheuvel@arm.com; Wasim Khan <wasim.khan@nxp.com>
> Subject: Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe
> related PCDs
> 
> On Wed, Jun 10, 2020 at 03:47:25 +0530, Wasim Khan wrote:
> > From: Wasim Khan <wasim.khan@nxp.com>
> >
> > Define PCIe related PCDs for LX2160A.
> >
> > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > ---
> >  Silicon/NXP/LX2160A/LX2160A.dsc.inc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > index fe8ed402fc4e..43e361464c8e 100644
> > --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > @@ -38,6 +38,11 @@ [PcdsFixedAtBuild.common]
> >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C0000
> >
> > +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x8000000000
> 
> This is already provided by LX2160A_PCI1_PHYS_ADDRESS in
> Silicon/NXP/LX2160A/Include/Soc.h, and PCI_SEG0_MMIO_MEMBASE would be
> better described as an alias of that. Unless the NXP
> PciHostBridgeLib/PciSegmentLib is intended to be shared with SoCs where these
> base addresses can be different in different platforms.

Yes, PciHostBridgeLib/PciSegmentLib are shared with different SoC with different base address and number of PCIe controllers.

> 
> If so, the PHYS_ADDRESSES would be better defined as derivatives of
> PcdPciExp1BaseAddr.

OK, I will use PcdPciExp1BaseAddr and PcdNumPciController for PHYS_ADDRESSES

> 
> > +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|6
> 
> Is this number possible to be different for different platforms based on the same
> SoC?

This is the total number of PCIe controller on a SoC (for LS1043 its 3, for LX2160 its 6)
A platform may use few of them based on the RCW (For Ex: LX2160aRdbPkg has only PEX3 and PEX5 . LX2160aQdsPkg may have all 6 controller enabled).

> 
> /
>     Leif
> 
> > +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x80000
> > +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x407FC
> > +
> >  [PcdsFeatureFlag]
> >    gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|TRUE
> >
> > --
> > 2.7.4
> >

Re: [PATCH edk2-platforms 3/7] Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver

[Leif Lindholm]

On Wed, Jun 10, 2020 at 03:47:26 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Add PlatformDxe to do platform specific work.
> At present it perform platform specific Pci initialization.
 
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf | 35 +++++++++
>  Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c   | 78 ++++++++++++++++++++
>  2 files changed, 113 insertions(+)
> 
> diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 000000000000..2514adf1d69d
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,35 @@
> +## @file
> +#
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION               = 0x00010019
> +  BASE_NAME                 = PlatformDxe
> +  FILE_GUID                 = C4063EBA-7729-11EA-BC55-0232AC130003
> +  MODULE_TYPE               = DXE_DRIVER
> +  VERSION_STRING            = 1.0
> +  ENTRY_POINT               = PlatformDxeEntryPoint
> +
> +[Sources]
> +  PlatformDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> +  Silicon/NXP/LX2160A/LX2160A.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  PcdLib
> +  UefiDriverEntryPoint
> +
> +[Pcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 000000000000..73599aaeb7bf
> --- /dev/null
> +++ b/Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,78 @@
> +/** @file
> +*
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +#include <Library/PcdLib.h>
> +#include <Library/SocLib.h>
> +#include <Soc.h>
> +
> +/**
> +  Enable PciCfgShift feature for LX2160-Rev2

... which means what?

> +
> +**/
> +VOID
> +EnableCfgShift (

But even more importantly, that isn't what this function does:

> +  VOID
> +  )
> +{
> +  UINT32 Svr;
> +
> +  Svr = SocGetSvr ();
> +  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x2)) {

... it performs variant/version detection and sets feature flags based
on it. The fact that it currently happens to be setting only a single
flag shouldn't mean that in order to understand what PlatformPciInit()
does even at a high level, I need to know what an "EnableCfgShift" is.

The function should be called something like DetectSocVersion,
EnableSocRevisionWorkarounds, or something other descriptive.

*Then* there should be a comment here describing what
PcdPciCfgShiftEnable is and what it does.

> +    PcdSetBoolS (PcdPciCfgShiftEnable, TRUE);
> +  }
> +}
> +
> +/**
> +  Enable Layerscape Gen4 controller for LX2160A-Rev1
> +
> +**/
> +VOID
> +EnablePciController (
> +  VOID
> +  )
> +{
> +  UINT32 Svr;
> +
> +  Svr = SocGetSvr ();
> +  if ((SVR_SOC_VER(Svr) == SVR_LX2160A) && (SVR_MAJOR(Svr) == 0x1)) {
> +    PcdSetBoolS (PcdPciLsGen4Ctrl, TRUE);

Again, this doesn't *enable* a controller, which is what the function
name says it does. It sets a Pcd, which is then used by subsequent
code. I don't know if this function should be smashed together with
the previous one under an even more generic name, but this too
motivates a comment on what a PcdPciLsGen4Ctrl is.

> +  }
> +}
> +
> +/**
> +  Platfrom Specific PCI Initialization

Platform

/
    Leif

> +
> +**/
> +VOID
> +PlatformPciInit (
> +  VOID
> +  )
> +{
> +  EnableCfgShift ();
> +  EnablePciController ();
> +}
> +
> +/**
> +  The entry point for PlatformDxe driver. This driver
> +  intends to perform platform specific initialization.
> +
> +  @param[in] ImageHandle     The image handle of the driver.
> +  @param[in] SystemTable     The system table.
> +
> +  @retval EFI_SUCCESS         Driver initialization success.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformDxeEntryPoint (
> +  IN EFI_HANDLE                   ImageHandle,
> +  IN EFI_SYSTEM_TABLE             *SystemTable
> +  )
> +{
> +  // Platfrom Specific PCI Initialization
> +  PlatformPciInit ();
> +  return EFI_SUCCESS;
> +}
> -- 
> 2.7.4
> 

Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe related PCDs

[Leif Lindholm]

On Thu, Jun 18, 2020 at 10:33:05 +0000, Wasim Khan (OSS) wrote:
> 
> 
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: Thursday, June 18, 2020 3:38 PM
> > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> > <meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> > ard.biesheuvel@arm.com; Wasim Khan <wasim.khan@nxp.com>
> > Subject: Re: [PATCH edk2-platforms 2/7] Silicon/NXP: LX2160A: Define PCIe
> > related PCDs
> > 
> > On Wed, Jun 10, 2020 at 03:47:25 +0530, Wasim Khan wrote:
> > > From: Wasim Khan <wasim.khan@nxp.com>
> > >
> > > Define PCIe related PCDs for LX2160A.
> > >
> > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > ---
> > >  Silicon/NXP/LX2160A/LX2160A.dsc.inc | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > index fe8ed402fc4e..43e361464c8e 100644
> > > --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > > @@ -38,6 +38,11 @@ [PcdsFixedAtBuild.common]
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C0000
> > >
> > > +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x8000000000
> > 
> > This is already provided by LX2160A_PCI1_PHYS_ADDRESS in
> > Silicon/NXP/LX2160A/Include/Soc.h, and PCI_SEG0_MMIO_MEMBASE would be
> > better described as an alias of that. Unless the NXP
> > PciHostBridgeLib/PciSegmentLib is intended to be shared with SoCs where these
> > base addresses can be different in different platforms.
> 
> Yes, PciHostBridgeLib/PciSegmentLib are shared with different SoC
> with different base address and number of PCIe controllers.

Yes. What I meant was whether the base address could change for the
SoC depending on which platform it is integrated in?

> > If so, the PHYS_ADDRESSES would be better defined as derivatives of
> > PcdPciExp1BaseAddr.
> 
> OK, I will use PcdPciExp1BaseAddr and PcdNumPciController for PHYS_ADDRESSES
> 
> > 
> > > +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|6
> > 
> > Is this number possible to be different for different platforms based on the same
> > SoC?
> 
> This is the total number of PCIe controller on a SoC (for LS1043 its
> 3, for LX2160 its 6)
> A platform may use few of them based on the RCW (For Ex:
> LX2160aRdbPkg has only PEX3 and PEX5 . LX2160aQdsPkg may have all 6
> controller enabled).

OK, then the use of the Pcd is definitely the right way to go, but
yes, please do as you suggest above to avoid multiple (and potentially
conflicting) definitions for the same address.

Regards,

Leif

Re: [PATCH edk2-platforms 6/7] Platform/NXP: LX2160aRdbPkg: Enable NetworkPkg

[Leif Lindholm]

On Wed, Jun 10, 2020 at 03:47:29 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Enable NetworkPkg for LX2160aRdbPkg Platform.

This is done as part of a pacthset called "enable PCIe support
for...".
Again, I understand why you're doing this, but it is far from obvious.
Please add a note to the commit message explaining *why*.
(Also, this deserves explicit pointing out in cover letter.)

> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc | 11 +++++++++++
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf |  5 +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
> index d0ec7b5a8906..c73f9fd4d393 100644
> --- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
> +++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
> @@ -22,6 +22,13 @@ [Defines]
>    OUTPUT_DIRECTORY               = Build/LX2160aRdbPkg
>    FLASH_DEFINITION               = Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
>  
> +  #
> +  # Network definition
> +  #
> +  DEFINE NETWORK_TLS_ENABLE             = FALSE
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
> +  DEFINE NETWORK_ISCSI_ENABLE           = FALSE
> +
>  !include Silicon/NXP/NxpQoriqLs.dsc.inc
>  !include Silicon/NXP/LX2160A/LX2160A.dsc.inc
>  
> @@ -45,4 +52,8 @@ [Components.common]
>  
>    Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
>  
> +  #
> +  # Networking stack
> +  #
> +!include NetworkPkg/Network.dsc.inc
>   ##
> diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> index 41b8f1ececa4..4c9cab5d0938 100644
> --- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> +++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
> @@ -116,6 +116,11 @@ [FV.FvMain]
>    INF ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
>  
>    #
> +  # Networking stack
> +  #
> +!include NetworkPkg/Network.fdf.inc
> +
> +  #
>    # FAT filesystem + GPT/MBR partitioning
>    #
>    INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
> -- 
> 2.7.4
> 

Re: [PATCH edk2-platforms 0/7] NXP: Add PCIe Support for LX2160aRdbPkg

[Leif Lindholm]

On Wed, Jun 10, 2020 at 03:47:23 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> This patch series adds PCIe support for LX2160aRdbPkg.
> LX2160-Rev1 and LX2160-Rev2 has different PCIe controller. Platform
> driver checks the SoC version and enable corresponding PCIe controller
> and its feature using dynamic PCDs. 
> 
> PciHostBridgeLib and PciSegmentLib already has support for both
> PCIe controllers.
> 
> 
> This patch series dependes on below patch series:
> 1 - Silicon/NXP: LX2160A: Add SerDes Support
> 2 - Silicon/NXP: Add SVR and DEVDISRn config configuration

No further comments on this set.
You can consider any patch without a comment as:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

Now, I'm going to take some time off - I'll be back 29/6 and will look
at any new revisions then if this hasn't been merged yet..

> Wasim Khan (7):
>   Platform/NXP: LX2160aRdbPkg: Add PCIe space in VirtualMemoryMap
>   Silicon/NXP: LX2160A: Define PCIe related PCDs
>   Platform/NXP: LX2160aRdbPkg: Add PlatformDxe driver
>   Platform/NXP: LX2160aRdbPkg: Enable PlatformDxe driver
>   Platform/NXP: LX2160aRdbPkg: Hide Root Port for LX2160A-Rev2
>   Platform/NXP: LX2160aRdbPkg: Enable NetworkPkg
>   Platform/NXP: LX2160aRdbPkg: Enable PCIE support
> 
>  Silicon/NXP/NxpQoriqLs.dec                                            |  1 +
>  Silicon/NXP/LX2160A/LX2160A.dsc.inc                                   |  5 +
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc                          | 22 +++++
>  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf                          | 15 +++
>  Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf        | 36 ++++++++
>  Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf             |  1 +
>  Silicon/NXP/Include/Pcie.h                                            |  1 +
>  Silicon/NXP/LX2160A/Include/Soc.h                                     |  8 ++
>  Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c          | 96 ++++++++++++++++++++
>  Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 38 +++++++-
>  Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c               |  6 +-
>  11 files changed, 227 insertions(+), 2 deletions(-)
>  create mode 100644 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
>  create mode 100644 Platform/NXP/LX2160aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> 
> -- 
> 2.7.4
>