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

[Vabhav Sharma <vabhav.sharma@nxp.com>]

>-----Original Message-----
>From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
>Sent: Friday, April 20, 2018 8:53 PM
>To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>Cc: ard.biesheuvel@linaro.org; edk2-devel@lists.01.org; Udit Kumar
><udit.kumar@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Vabhav Sharma
><vabhav.sharma@nxp.com>
>Subject: Re: [PATCH edk2-platforms 35/39] Compilation: Update the fdf, dsc and
>dec files.
>
>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/PciHostBridgeL
>> + ib.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.
Alright agree, I will update for conditional inclusion and submit as 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.in
>> +++ f
>> @@ -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
>
Ok, Sure.
>>    ##
>> 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
>>