[edk2-devel] [PATCH 08/19] Silicon/NXP: Remove unnecessary PCDs

Leif Lindholm leif at nuviainc.com
Thu Feb 20 18:56:55 UTC 2020


On Tue, Feb 11, 2020 at 08:45:13 +0000, Pankaj Bansal wrote:
> > -----Original Message-----
> > From: Leif Lindholm <leif at nuviainc.com>
> > Sent: Monday, February 10, 2020 11:02 PM
> > To: Pankaj Bansal <pankaj.bansal at nxp.com>
> > Cc: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Michael D Kinney
> > <michael.d.kinney at intel.com>; Varun Sethi <V.Sethi at nxp.com>;
> > devel at edk2.groups.io
> > Subject: Re: [PATCH 08/19] Silicon/NXP: Remove unnecessary PCDs
> > 
> > On Fri, Feb 07, 2020 at 18:13:17 +0530, Pankaj Bansal wrote:
> > > There is no need to keep SOC specific PCDs defined for each SOC.
> > 
> > That sound like the definition of why we have SoC-specific Pcds, so I don't
> > follow.
> > 
> > > we can do away with these PCDs.
> > 
> > After looking through this patchset, it looks like:
> > 1) Initial implementation defined a bunch of things as Pcds which
> >    really cannot vary between different platforms using the same SoC.
> 
> Yes.
> 
> > 2) This patch moves several things that can differ between platforms
> >    into #defines, when they should be Pcds.
> > 
> > I am OK with the 1)s, but there are some you would need to convince me are not
> > 2:s.
> 
> I have moved only those things to #define, which would be common for all platforms using the SOC.
> I will explain more below.

Thanks.

> > 
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal at nxp.com>
> > > ---
> > >  .../Drivers/PlatformDxe/PlatformDxe.c         |  15 +--
> > >  .../Drivers/PlatformDxe/PlatformDxe.inf       |   8 +-
> > >  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  |   1 -
> > >  .../Library/PlatformLib/ArmPlatformLib.inf    |  21 +---
> > >  .../Library/PlatformLib/NxpQoriqLsMem.c       | 103 +++++++++---------
> > >  Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf         |   6 -
> > >  Silicon/NXP/Include/Chassis2/NxpSoc.h         |   2 +
> > >  Silicon/NXP/LS1043A/Include/Soc.h             |  44 ++++++++
> > >  Silicon/NXP/LS1043A/LS1043A.dsc.inc           |  32 ------
> > >  Silicon/NXP/Library/SocLib/Chassis2/Soc.c     |   2 +-
> > >  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  |   4 -
> > >  Silicon/NXP/NxpQoriqLs.dec                    |  74 -------------
> > >  12 files changed, 108 insertions(+), 204 deletions(-)  create mode
> > > 100644 Silicon/NXP/LS1043A/Include/Soc.h
> > >
> > > diff --git
> > > a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> > > b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> > > index f89dcdeff3..62c400eb1a 100644
> > > --- a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> > > +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    LS1043 DXE platform driver.
> > >
> > > -  Copyright 2018-2019 NXP
> > > +  Copyright 2018-2020 NXP
> > >
> > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -14,6 +14,7 @@
> > >  #include <Library/PcdLib.h>
> > >  #include <Library/UefiBootServicesTableLib.h>
> > >  #include <Library/UefiLib.h>
> > > +#include <Soc.h>
> > >
> > >  #include <Protocol/NonDiscoverableDevice.h>
> > >
> > > @@ -22,7 +23,7 @@ typedef struct {
> > >    UINT8 EndDesc;
> > >  } ADDRESS_SPACE_DESCRIPTOR;
> > >
> > > -STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[FixedPcdGet64
> > > (PcdNumI2cController)];
> > > +STATIC ADDRESS_SPACE_DESCRIPTOR
> > > +mI2cDesc[LS1043A_I2C_NUM_CONTROLLERS];
> > 
> > Sure, this one sounds like something not configurable in software or board
> > design.
> > 
> > >
> > >  STATIC
> > >  EFI_STATUS
> > > @@ -65,19 +66,19 @@ PopulateI2cInformation (  {
> > >    UINT32 Index;
> > >
> > > -  for (Index = 0; Index < FixedPcdGet32 (PcdNumI2cController);
> > > Index++) {
> > > +  for (Index = 0; Index < ARRAY_SIZE (mI2cDesc); Index++) {
> > >      mI2cDesc[Index].StartDesc.Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> > >      mI2cDesc[Index].StartDesc.Len = sizeof
> > (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
> > >      mI2cDesc[Index].StartDesc.ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
> > >      mI2cDesc[Index].StartDesc.GenFlag = 0;
> > >      mI2cDesc[Index].StartDesc.SpecificFlag = 0;
> > >      mI2cDesc[Index].StartDesc.AddrSpaceGranularity = 32;
> > > -    mI2cDesc[Index].StartDesc.AddrRangeMin = FixedPcdGet64
> > (PcdI2c0BaseAddr) +
> > > -                                             (Index * FixedPcdGet32 (PcdI2cSize));
> > 
> > As does this.
> > 
> > > +    mI2cDesc[Index].StartDesc.AddrRangeMin =
> > LS1043A_I2C0_PHYS_ADDRESS +
> > > +                                             (Index *
> > > + LS1043A_I2C_SIZE);
> > >      mI2cDesc[Index].StartDesc.AddrRangeMax =
> > mI2cDesc[Index].StartDesc.AddrRangeMin +
> > > -                                             FixedPcdGet32 (PcdI2cSize) - 1;
> > > +                                             LS1043A_I2C_SIZE - 1;
> > >      mI2cDesc[Index].StartDesc.AddrTranslationOffset = 0;
> > > -    mI2cDesc[Index].StartDesc.AddrLen = FixedPcdGet32 (PcdI2cSize);
> > > +    mI2cDesc[Index].StartDesc.AddrLen = LS1043A_I2C_SIZE;
> > >
> > >      mI2cDesc[Index].EndDesc = ACPI_END_TAG_DESCRIPTOR;
> > >    }
> > > diff --git
> > > a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> > > b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> > > index d689cf4db5..126a1174fa 100644
> > > --- a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> > > +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> > > @@ -2,7 +2,7 @@
> > >  #
> > >  #  Component description file for LS1043 DXE platform driver.
> > >  #
> > > -#  Copyright 2018-2019 NXP
> > > +#  Copyright 2018-2020 NXP
> > >  #
> > >  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -25,6 +25,7 @@
> > >    MdeModulePkg/MdeModulePkg.dec
> > >    Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.dec
> > >    Silicon/NXP/NxpQoriqLs.dec
> > > +  Silicon/NXP/LS1043A/LS1043A.dec
> > 
> > Please insert sorted alphabetically.
> > 
> > >
> > >  [LibraryClasses]
> > >    BaseLib
> > > @@ -43,10 +44,5 @@
> > >    gEdkiiNonDiscoverableDeviceProtocolGuid        ## PRODUCES
> > >    gDs1307RealTimeClockLibI2cMasterProtocolGuid   ## PRODUCES
> > >
> > > -[FixedPcd]
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController
> > 
> > Still good.
> > 
> > > -
> > >  [Depex]
> > >    TRUE
> > > diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > > b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > > index 802cccdce6..74a1948fc6 100644
> > > --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > > +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > > @@ -42,7 +42,6 @@
> > >    #
> > >    # Board Specific Pcds
> > >    #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled|FALSE
> > 
> > But this sounds exactly like the kind of thing that would be configured differently
> > for different boards. Can you please explain why this should be removed?
> 
> This PCD was defined because LS1043A (Chassis2 based SOC) has one Serdes.
> Other Soc based on Chassis2 LS1046A, has two Serdes. So this Pcd is false for LS1043A
> and true for LS1046A. Serdes block in Layerscape SOCs control which devices are enabled
> and which devices are disabled ? e.g. (hypothetical example) with Serdes protocol 5,
> out of 8 MACs, only 3 and 6 and enabled.
> 
> Going forward the plan is to move the Serdes handling
> to SOC specific code and NOT to Chassis specific code. This is because the serdes
> protocols are SOC specific and not chassis specific. i.e. both LS1046A and LS1043A can have
> serdes protocol 5. But their meaning can be different for both
> 
> For the time being we have removed Serdes related code (in PATCH 07/19). We will introduce
> It after PEI phase changes are merged.

OK. This explanation makes sense, and I approve of the future
strategy. But I think it would be clearer if this change moved to the
same patch as the one deleting the code.

> > 
> > >    gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x1
> > >
> > >    #
> > > diff --git
> > > a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > > b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > > index f7ae74afc6..054dc4d003 100644
> > > ---
> > > a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > > +++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.in
> > > +++ f
> > > @@ -1,7 +1,7 @@
> > >  #  @file
> > >  #
> > >  #  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> > > -#  Copyright 2017, 2019 NXP
> > > +#  Copyright 2017, 2019-2020 NXP
> > >  #
> > >  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -20,6 +20,7 @@
> > >    EmbeddedPkg/EmbeddedPkg.dec
> > >    MdePkg/MdePkg.dec
> > >    Silicon/NXP/NxpQoriqLs.dec
> > > +  Silicon/NXP/LS1043A/LS1043A.dec
> > 
> > Insert sorted.
> > 
> > >
> > >  [LibraryClasses]
> > >    ArmLib
> > > @@ -35,21 +36,3 @@
> > >
> > >  [FixedPcd]
> > >    gArmTokenSpaceGuid.PcdArmPrimaryCore
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1Size
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2Size
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpBaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpBaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
> > > diff --git
> > > a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > > b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > > index c6c256da07..3a72c8bdd8 100644
> > > --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > > +++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > > @@ -6,7 +6,7 @@
> > >  *
> > >  *  Copyright (c) 2011, ARM Limited. All rights reserved.
> > >  *  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> > > -*  Copyright 2017, 2019 NXP
> > > +*  Copyright 2017, 2019-2020 NXP
> > >  *
> > >  *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >  *
> > > @@ -16,7 +16,7 @@
> > >  #include <Library/DebugLib.h>
> > >  #include <Library/PcdLib.h>
> > >  #include <Library/MemoryAllocationLib.h> -#include <DramInfo.h>
> > > +#include <Soc.h>
> > >
> > >  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          25
> > >
> > > @@ -38,7 +38,6 @@ ArmPlatformGetVirtualMemoryMap (  {
> > >    UINTN                            Index;
> > >    ARM_MEMORY_REGION_DESCRIPTOR     *VirtualMemoryTable;
> > > -  DRAM_INFO                        DramInfo;
> > >
> > >    Index = 0;
> > >
> > > @@ -51,25 +50,21 @@ ArmPlatformGetVirtualMemoryMap (
> > >      return;
> > >    }
> > >
> > > -  if (GetDramBankInfo (&DramInfo)) {
> > > -    DEBUG ((DEBUG_ERROR, "Failed to get DRAM information, exiting...\n"));
> > > -    return;
> > > -  }
> > > +  VirtualMemoryTable[Index].PhysicalBase =
> > > + LS1043A_DRAM0_PHYS_ADDRESS;  VirtualMemoryTable[Index].VirtualBase
> > = LS1043A_DRAM0_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_DRAM0_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > >
> > > -
> > > -  for (Index = 0; Index < DramInfo.NumOfDrams; Index++) {
> > > -    // DRAM1 (Must be 1st entry)
> > > -    VirtualMemoryTable[Index].PhysicalBase =
> > DramInfo.DramRegion[Index].BaseAddress;
> > > -    VirtualMemoryTable[Index].VirtualBase  =
> > DramInfo.DramRegion[Index].BaseAddress;
> > > -    VirtualMemoryTable[Index].Length       = DramInfo.DramRegion[Index].Size;
> > > -    VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > -  }
> > > +  VirtualMemoryTable[Index].PhysicalBase =
> > > + LS1043A_DRAM1_PHYS_ADDRESS;  VirtualMemoryTable[Index].VirtualBase
> > = LS1043A_DRAM1_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_DRAM1_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > 
> > No changes to this file so far appear to have anything to do with what the
> > commit message says the patch does.
> > 
> > >
> > >    // CCSR Space
> > > -  VirtualMemoryTable[Index].PhysicalBase = FixedPcdGet64
> > > (PcdCcsrBaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64 (PcdCcsrBaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdCcsrSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_CCSR_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_CCSR_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_CCSR_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > 
> > If these are not reconfigurable for different platforms, sure.
> 
> Yes. The SOC memory map is fixed in hardware. It doesn't change with Platform using SOC.

OK, cool. Then I'm OK with all of these.

> > 
> > >
> > >    // IFC region 1
> > >    //
> > > @@ -85,60 +80,60 @@ ArmPlatformGetVirtualMemoryMap (
> > >    //             For write transactions from non-core masters (like system DMA),
> > the address
> > >    //                should be 16 byte aligned and the data size should be multiple of
> > 16 bytes.
> > >    //
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdIfcRegion1BaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdIfcRegion1BaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdIfcRegion1Size);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_IFC0_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_IFC0_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_IFC0_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >
> > >    // QMAN SWP
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdQmanSwpBaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdQmanSwpBaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdQmanSwpSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > > +  VirtualMemoryTable[Index].PhysicalBase =
> > > + LS1043A_QMAN_SW_PORTAL_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].VirtualBase  =
> > LS1043A_QMAN_SW_PORTAL_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_QMAN_SW_PORTAL_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > >
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >    // BMAN SWP
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdBmanSwpBaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdBmanSwpBaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdBmanSwpSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > > +  VirtualMemoryTable[Index].PhysicalBase =
> > > + LS1043A_BMAN_SW_PORTAL_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].VirtualBase  =
> > LS1043A_BMAN_SW_PORTAL_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_QMAN_SW_PORTAL_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > >
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >    // IFC region 2
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdIfcRegion2BaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdIfcRegion2BaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdIfcRegion2Size);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_IFC1_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_IFC1_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_IFC1_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > >
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >    // PCIe1
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdPciExp1BaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdPciExp1BaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64
> > (PcdPciExp1BaseSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_PCI0_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_PCI0_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_PCI_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > >
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >    // PCIe2
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdPciExp2BaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdPciExp2BaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64
> > (PcdPciExp2BaseSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_PCI1_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_PCI1_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_PCI_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > >
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >    // PCIe3
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdPciExp3BaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdPciExp3BaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64
> > (PcdPciExp3BaseSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_PCI2_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_PCI2_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_PCI_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > 
> > If these are not reconfigurable for different platforms, sure.
> > >
> > >    // QSPI region
> > > -  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64
> > > (PcdQspiRegionBaseAddr);
> > > -  VirtualMemoryTable[Index].VirtualBase  = FixedPcdGet64
> > (PcdQspiRegionBaseAddr);
> > > -  VirtualMemoryTable[Index].Length       = FixedPcdGet64 (PcdQspiRegionSize);
> > > -  VirtualMemoryTable[Index].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > > +  VirtualMemoryTable[Index].PhysicalBase = LS1043A_QSPI_PHYS_ADDRESS;
> > > + VirtualMemoryTable[Index].VirtualBase  = LS1043A_QSPI_PHYS_ADDRESS;
> > > +  VirtualMemoryTable[Index].Length       = LS1043A_QSPI_SIZE;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> > 
> > If these are not reconfigurable for different platforms, sure.
> > 
> > >
> > >    // End of Table
> > > -  VirtualMemoryTable[++Index].PhysicalBase = 0;
> > > +  VirtualMemoryTable[Index].PhysicalBase = 0;
> > 
> > Move that ++ back where it belongs - there is no functional change here.
> 
> I will remove this change and will keep functional changes only in this patch.

Thx.

> > 
> > >    VirtualMemoryTable[Index].VirtualBase  = 0;
> > >    VirtualMemoryTable[Index].Length       = 0;
> > > -  VirtualMemoryTable[Index].Attributes   =
> > (ARM_MEMORY_REGION_ATTRIBUTES)0;
> > > +  VirtualMemoryTable[Index++].Attributes   =
> > (ARM_MEMORY_REGION_ATTRIBUTES)0;
> > 
> > Move that ++ back where it belongs - there is no functional change here.
> > 
> > >
> > > -  ASSERT ((Index + 1) <= MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> > > +  ASSERT (Index < MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> > 
> > Drop that +1 = - there is no functional change here.
> > 
> > If you want to submit separate patches for coding style changes, that's fine. But
> > given that this set looks like it's making up for not synchronising internal
> > development with the (very long) upstreaming effort so far, I am not interested
> > in those until all functional changes are in.
> 
> I will remove this change and will keep functional changes only in this patch.

Thanks.

> > 
> > >
> > >    *VirtualMemoryMap = VirtualMemoryTable;  } diff --git
> > > a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> > > b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> > > index 784139065f..fc4bb618fa 100644
> > > --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> > > +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> > > @@ -49,11 +49,5 @@
> > >    gEdkiiNonDiscoverableDeviceProtocolGuid    ## TO_START
> > >    gEfiI2cMasterProtocolGuid                  ## BY_START
> > >
> > > -[Pcd]
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSpeed
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController
> > > -
> > >  [Depex]
> > >    TRUE
> > > diff --git a/Silicon/NXP/Include/Chassis2/NxpSoc.h
> > > b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> > > index 74330b6205..6812beafe4 100644
> > > --- a/Silicon/NXP/Include/Chassis2/NxpSoc.h
> > > +++ b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> > > @@ -12,6 +12,8 @@
> > >
> > >  #define CLK_FREQ                   100000000
> > >
> > > +#define CHASSIS2_DCFG_ADDRESS      0x1EE0000
> > > +
> > >  /* SMMU Defintions */
> > >  #define SMMU_BASE_ADDR             0x09000000
> > >  #define SMMU_REG_SCR0              (SMMU_BASE_ADDR + 0x0)
> > > diff --git a/Silicon/NXP/LS1043A/Include/Soc.h
> > > b/Silicon/NXP/LS1043A/Include/Soc.h
> > > new file mode 100644
> > > index 0000000000..c1e00394af
> > > --- /dev/null
> > > +++ b/Silicon/NXP/LS1043A/Include/Soc.h
> > > @@ -0,0 +1,44 @@
> > > +/** @file
> > > +
> > > +  Copyright 2020 NXP
> > > +
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +#ifndef __SOC_H__
> > > +#define __SOC_H__
> > > +
> > > +/**
> > > +  Soc Memory Map
> > > +**/
> > > +#define LS1043A_DRAM0_PHYS_ADDRESS   0x80000000
> > > +#define LS1043A_DRAM0_SIZE           SIZE_2GB
> > > +#define LS1043A_DRAM1_PHYS_ADDRESS   0x880000000
> > > +#define LS1043A_DRAM1_SIZE           0x780000000 // 30 GB
> > > +
> > > +#define LS1043A_CCSR_PHYS_ADDRESS    0x1000000
> > > +#define LS1043A_CCSR_SIZE            0xF000000
> > > +
> > > +#define LS1043A_IFC0_PHYS_ADDRESS    0x60000000
> > > +#define LS1043A_IFC0_SIZE            SIZE_512MB
> > > +#define LS1043A_IFC1_PHYS_ADDRESS    0x620000000
> > > +#define LS1043A_IFC1_SIZE            0xE0000000 // 3.5 GB
> > > +
> > > +#define LS1043A_QSPI_PHYS_ADDRESS    0x40000000
> > > +#define LS1043A_QSPI_SIZE            SIZE_512MB
> > > +
> > > +#define LS1043A_QMAN_SW_PORTAL_PHYS_ADDRESS    0x500000000
> > > +#define LS1043A_QMAN_SW_PORTAL_SIZE            SIZE_128MB
> > > +#define LS1043A_BMAN_SW_PORTAL_PHYS_ADDRESS    0x508000000
> > > +#define LS1043A_BMAN_SW_PORTAL_SIZE            SIZE_128MB
> > > +
> > > +#define LS1043A_PCI0_PHYS_ADDRESS    0x4000000000
> > > +#define LS1043A_PCI1_PHYS_ADDRESS    0x4800000000
> > > +#define LS1043A_PCI2_PHYS_ADDRESS    0x5000000000
> > > +#define LS1043A_PCI_SIZE             SIZE_32GB
> > > +
> > > +#define LS1043A_I2C0_PHYS_ADDRESS    0x2180000
> > > +#define LS1043A_I2C_SIZE             0x10000
> > > +#define LS1043A_I2C_NUM_CONTROLLERS  4
> > > +
> > > +#endif
> > > diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > > b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > > index 754eff396a..7ebbb1a495 100644
> > > --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > > +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > > @@ -26,40 +26,8 @@
> > >  [PcdsFixedAtBuild.common]
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x021c0500
> > >
> > > -  #
> > > -  # CCSR Address Space and other attached Memories
> > > -  #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr|0x01000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize|0x0F000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdClkBaseAddr|0x01EE1000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1BaseAddr|0x60000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1Size|0x20000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2BaseAddr|0x0620000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2Size|0x00E0000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcNandReservedSize|0x2EA
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpBaseAddr|0x0500000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpSize|0x0080000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpBaseAddr|0x0508000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpSize|0x0080000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x4000000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseSize|0x800000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr|0x4800000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseSize|0x800000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr|0x5000000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseSize|0x800000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdScfgBaseAddr|0x1570000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdGutsBaseAddr|0x01EE0000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdWatchdog1BaseAddr|0x02AD0000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSdxcBaseAddr|0x01560000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr|0x02180000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize|0x10000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController|4
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr|0x40000000
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize|0x20000000
> > > -
> > >    #
> > >    # Big Endian IPs
> > >    #
> > >    gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|TRUE
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|TRUE
> > >  ##
> > > diff --git a/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> > > b/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> > > index 9baeb17ecf..a3dabc93d1 100644
> > > --- a/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> > > +++ b/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> > > @@ -34,7 +34,7 @@ GetSysInfo (
> > >    CCSR_GUR     *GurBase;
> > >    UINTN        SysClk;
> > >
> > > -  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > > +  GurBase = (CCSR_GUR *)CHASSIS2_DCFG_ADDRESS;
> > >    SysClk = CLK_FREQ;
> > >
> > >    SetMem (PtrSysInfo, sizeof (SYS_INFO), 0); diff --git
> > > a/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> > > b/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> > > index fe77717337..d8707927b7 100644
> > > --- a/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> > > +++ b/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> > > @@ -40,9 +40,5 @@
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdClkBaseAddr
> > >    gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdGutsBaseAddr
> > >    gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdScfgBaseAddr
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled
> > > diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> > > index 4a1cfb3e27..b478560450 100644
> > > --- a/Silicon/NXP/NxpQoriqLs.dec
> > > +++ b/Silicon/NXP/NxpQoriqLs.dec
> > > @@ -22,89 +22,15 @@
> > >    gNxpNonDiscoverableI2cMasterGuid = { 0x5f2c099c, 0x54a3, 0x4dd4,
> > > {0x9e, 0xc5, 0xe9, 0x12, 0x8c, 0x36, 0x81, 0x6a}}
> > >
> > >  [PcdsFixedAtBuild.common]
> > > -  #
> > > -  # Pcds for I2C Controller
> > > -  #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSpeed|0|UINT32|0x00000001
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController|0|UINT32|0x00000002
> > > -
> > > -  #
> > > -  # Pcds for base address and size
> > > -  #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdGutsBaseAddr|0x0|UINT64|0x00000100
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPiFdSize|0x0|UINT32|0x00000101
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdPiFdBaseAddress|0x0|UINT64|0x00000102
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdClkBaseAddr|0x0|UINT64|0x00000103
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdWatchdog1BaseAddr|0x0|UINT64|0x0000010
> > 4
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdDdrBaseAddr|0x0|UINT64|0x00000105
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSdxcBaseAddr|0x0|UINT64|0x00000106
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdScfgBaseAddr|0x0|UINT64|0x00000107
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr|0x0|UINT64|0x00000108
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize|0x0|UINT32|0x00000109
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdDcsrBaseAddr|0x0|UINT64|0x0000010A
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdDcsrSize|0x0|UINT64|0x0000010B
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSataBaseAddr|0x0|UINT32|0x0000010C
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSataSize|0x0|UINT32|0x0000010D
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpBaseAddr|0x0|UINT64|0x0000010E
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQmanSwpSize|0x0|UINT64|0x0000010F
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpBaseAddr|0x0|UINT64|0x00000110
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdBmanSwpSize|0x0|UINT64|0x00000111
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr|0x0|UINT64|0x00000112
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseSize|0x0|UINT64|0x00000113
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr|0x0|UINT64|0x00000114
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseSize|0x0|UINT64|0x00000115
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr|0x0|UINT64|0x00000116
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseSize|0x0|UINT64|0x00000117
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp4BaseAddr|0x0|UINT64|0x0000118
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdPciExp4BaseSize|0x0|UINT64|0x0000119
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr|0x0|UINT64|0x0000011
> > A
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize|0x0|UINT64|0x0000011B
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdQspiRegion2BaseAddr|0x0|UINT64|0x000001
> > 1C
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdQspiRegion2Size|0x0|UINT64|0x0000011D
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdSystemMemoryExBase|0x0|UINT64|0x00000
> > 11E
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdSystemMemoryExSize|0x0|UINT64|0x000001
> > 1F
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr|0x0|UINT32|0x00000120
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize|0x0|UINT32|0x00000121
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr|0x0|UINT64|0x00000122
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize|0x0|UINT64|0x00000123
> > > -
> > > -  #
> > > -  # IFC PCDs
> > > -  #
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1BaseAddr|0x0|UINT64|0x0000019
> > 0
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion1Size|0x0|UINT64|0x00000191
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2BaseAddr|0x0|UINT64|0x0000019
> > 2
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdIfcRegion2Size|0x0|UINT64|0x00000193
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdIfcNandReservedSize|0x0|UINT32|0x0000019
> > 4
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdFlashDeviceBase64|0x0|UINT64|0x00000195
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdFlashReservedRegionBase64|0x0|UINT64|0x0
> > 0
> > > 000196
> > > -
> > > -  #
> > > -  # NV Pcd
> > > -  #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNvFdBase|0x0|UINT64|0x00000210
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdNvFdSize|0x0|UINT64|0x00000211
> > 
> > Not able to be different between different platforms using the same SoC?
> 
> This is a redundant Pcd. For Non Volatile Variables we already have PCDs like
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

Could you break out deletion of any unused Pcds in a separate patch?

> > 
> > > -
> > >    #
> > >    # Platform PCDs
> > >    #
> > >    gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x0|UINT32|0x00000250
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled|FALSE|BOOLEAN|0x0000025
> > 1
> > > -
> > > -  #
> > > -  # Clock PCDs
> > > -  #
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdSysClk|0x0|UINT64|0x000002A0
> > > -  gNxpQoriqLsTokenSpaceGuid.PcdDdrClk|0x0|UINT64|0x000002A1
> > 
> > And clocks also sound very much like something that can change per platform?
> 
> Yes Clocks can change. And also clocks are not fixed in all
> platforms. i.e. in most of the platforms based on the
> switch settings, FPGA or Clock generator can be configured to
> provide different SYS clock / DDR Clock / SERDES clock etc.

OK, so these are simply more redundant Pcds?

> Also we need the clock even when we don't have the support for PCD
> framework.
> Which is why we have moved the clock functionality in ArmPlatformLib
> Refer "NxpPlatformGetClock" in patch 14/19.

OK. So, does this belong in 14/19 or in the patch that deletes already
redundant Pcds?

Best Regards,

Leif

> > 
> > /
> >     Leif
> > 
> > >
> > >    #
> > >    # Pcds to support Big Endian IPs
> > >    #
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdMmcBigEndian|FALSE|BOOLEAN|0x0000310
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|FALSE|BOOLEAN|0x0000311
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x0000031
> > 2
> > > -
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|FALSE|BOOLEAN|0x0000
> > 031
> > > 3
> > > -
> > gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|FALSE|BOOLEAN|0x00000314
> > >
> > >  [PcdsFeatureFlag]
> > >
> > >
> > gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x0000
> > 031
> > > 5
> > > --
> > > 2.17.1
> > >

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

View/Reply Online (#54683): https://edk2.groups.io/g/devel/message/54683
Mute This Topic: https://groups.io/mt/71046329/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list