Re: [PATCH edk2-platforms 35/39] Compilation: Update the fdf, dsc and dec files.

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

On Fri, Feb 16, 2018 at 02:20:31PM +0530, Meenakshi wrote:
> From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> 
> LS1043A PCIe compilation and update firmware device,
> description and declaration files.Defining Embedded Package
> PCD which should be at least 20 for 64K PCIe IO size required
> for CPU hob during PEI phase to Add IO space post PEI phase.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vabhav <vabhav.sharma@nxp.com>
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc             | 16 ++++++++++++++++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf             |  9 +++++++++
>  .../LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf |  2 ++
>  .../LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c    |  6 ++++++
>  Platform/NXP/NxpQoriqLs.dsc                              |  7 +++++++
>  Silicon/NXP/LS1043A/LS1043A.dsc                          |  4 ++++
>  Silicon/NXP/NxpQoriqLs.dec                               | 10 ++++++++++
>  7 files changed, 54 insertions(+)
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index b2b514e..8cbaf88 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -42,6 +42,8 @@
>    BoardLib|Platform/NXP/LS1043aRdbPkg/Library/BoardLib/BoardLib.inf
>    FpgaLib|Platform/NXP/LS1043aRdbPkg/Library/FpgaLib/FpgaLib.inf
>    NorFlashLib|Silicon/NXP/Library/NorFlashLib/NorFlashLib.inf
> +  PciSegmentLib|Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> +  PciHostBridgeLib|Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>  
>  [PcdsFixedAtBuild.common]
>  
> @@ -79,6 +81,13 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdFlashDeviceBase64|0x060000000
>    gNxpQoriqLsTokenSpaceGuid.PcdFlashReservedRegionBase64|0x60300000
>  
> +  #
> +  # PCI PCDs.
> +  #
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciDebug|FALSE
> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x10000
> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> @@ -99,4 +108,11 @@
>    Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
>    Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>  
> +  Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
> +  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
> +    <PcdsFixedAtBuild>
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8010004F
> +  }
> +  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +
>   ##
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> index 6b5b63f..7993bf1 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> @@ -130,6 +130,13 @@ READ_LOCK_STATUS   = TRUE
>    INF Silicon/NXP/Drivers/NorFlashDxe/NorFlashDxe.inf
>  
>    #
> +  # PCI
> +  #
> +  INF Silicon/NXP/Drivers/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
> +  INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +  INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +
> +  #
>    # Network modules
>    #
>    INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> @@ -154,6 +161,8 @@ READ_LOCK_STATUS   = TRUE
>    INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>  !endif
>  
> +  INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +

I'm pretty OK with most of these random updates squashed into one
file, but the TftpDynamicCommand is something I generally don't like
to see included by default.

Other platforms put this inside a conditional statement:

!ifdef $(INCLUDE_TFTP_COMMAND)
  INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
!endif

so that it can be included when -D INCLUDE_TFTP_COMMAND=1 is added to
the build command line.

But beyond that, there is no mention of this addition in the commit
message. So please add a notice, or break this specific item out as a
separate patch.

>    #
>    # FAT filesystem + GPT/MBR partitioning
>    #
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> index 7feac56..f2c8b66 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> @@ -65,3 +65,5 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdDram3Size
>    gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
>    gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomSize
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> index 64c5612..1ef3292 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> @@ -67,6 +67,12 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdCcsrSize);
>    VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>  
> +  // ROM Space
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdRomBaseAddr);
> +  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdRomBaseAddr);
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdRomSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
>    // IFC region 1
>    //
>    // A-009241   : Unaligned write transactions to IFC may result in corruption of data
> diff --git a/Platform/NXP/NxpQoriqLs.dsc b/Platform/NXP/NxpQoriqLs.dsc
> index 5987cd6..f5bb2e9 100644
> --- a/Platform/NXP/NxpQoriqLs.dsc
> +++ b/Platform/NXP/NxpQoriqLs.dsc
> @@ -244,6 +244,8 @@
>  
>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|20
> +
>    #
>    # Optional feature to help prevent EFI memory map fragments
>    # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
> @@ -409,4 +411,9 @@
>  !endif #$(NO_SHELL_PROFILES)
>    }
>  
> +  #
> +  # TFTP Shell Command
> +  #
> +  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +

Same comment, conditional?

/
    Leif

>    ##
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc b/Silicon/NXP/LS1043A/LS1043A.dsc
> index a4eb117..f3220fa 100644
> --- a/Silicon/NXP/LS1043A/LS1043A.dsc
> +++ b/Silicon/NXP/LS1043A/LS1043A.dsc
> @@ -64,6 +64,9 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr|0x40000000
>    gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize|0x20000000
>    gNxpQoriqLsTokenSpaceGuid.PcdIfcBaseAddr|0x1530000
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|3
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomBaseAddr|0x00000000
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomSize|0x00100000
>  
>    #
>    # Big Endian IPs
> @@ -71,5 +74,6 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|TRUE
>    gNxpQoriqLsTokenSpaceGuid.PcdWdogBigEndian|TRUE
>    gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|TRUE
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|TRUE
>  
>  ##
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index 3cb476d..a3508b5 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -79,6 +79,16 @@
>    gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize|0x0|UINT64|0x00000129
>    gNxpQoriqLsTokenSpaceGuid.PcdDramMemSize|0x0|UINT64|0x0000012A
>    gNxpQoriqLsTokenSpaceGuid.PcdIfcBaseAddr|0x0|UINT64|0x0000012B
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomBaseAddr|0x0|UINT64|0x0000012C
> +  gNxpQoriqLsTokenSpaceGuid.PcdRomSize|0x0|UINT64|0x0000012D
> +
> +  #
> +  # PCI PCDs
> +  #
> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x000001D0
> +  gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x0|UINT32|0x000001D1
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciDebug|FALSE|BOOLEAN|0x000001D2
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|0|UINT32|0x000001D3
>  
>    #
>    # IFC PCDs
> -- 
> 1.9.1
>