From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web10.392.1582225019207152069 for ; Thu, 20 Feb 2020 10:56:59 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=Z8JMinbU; spf=pass (domain: nuviainc.com, ip: 209.85.128.65, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f65.google.com with SMTP id p9so3157975wmc.2 for ; Thu, 20 Feb 2020 10:56:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Jit6nR61n7Yo4ayIKsZ78l3zxus6ZUBknjByRIz4y5E=; b=Z8JMinbUZWGfVKYT0Sn71DI/Xa7S2MzPdvi5giCN6qMoAyErBJHa9Vg8K4ErmVPfwW 16hmcDYsGHUlug80vF7NK3aINNW33KC5PKDRO+0NUcvsmue8tfg4RAEm9tx1D6OdXKui NGuimHMR9z3n5IX9lfn0BVUlWx3XY2myXPFI0+lGLLYmFZEHDs6wV9ofVGsFXsuICVCC XLj5p37gofE5MNwM3jlVvhH7qRZToJBPudOYByIETfh8Kqhqs60wS8irwATdgVjRG3Gk bZ22Kd48Vk/3xI5Bii8onKI35DP/350cBRcC3/mnDhkjI+IIDHGXmxA4NiMS4F9MLFBq GW6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Jit6nR61n7Yo4ayIKsZ78l3zxus6ZUBknjByRIz4y5E=; b=rauPwfg9cn/QlJoCYaSsSPblGOZxPIzGlWUgh/QzrvidfnJSNbJP4ffi/lwOFZVGuV TUMOSPykDrcHWXJDaF3WWCSZ66WRJB5MlfPmp/xVPL+y3sQlT9/P6jaQQXhBSnwpekU2 DC1fSyFjUBcYW3Lfdn9MzVcgEGGX3BcfMu8CH35/owj55iXtEbAkH3SiVJiNGPih8axy z5P+ppDlYn0YfvZakRkux7rZIO715s5BZLNNa13QTTwllj+TAyRjIGi5QqeaUDFCR6iQ iEQkhFCybw0bAsxcpwyAJxCgMkn2wxneG4kD13BRrm68kGsG/fDy/CYVM41FvAaP60UI MNkg== X-Gm-Message-State: APjAAAW2IBEVxQb8g8uuxI+TU2qOkvlfzTG9in4iUtOWOkfxTVf7Yn3i ikmy73Ao8ZD53oI1Ly8LMNdOdw== X-Google-Smtp-Source: APXvYqw60TKMHQe2N49DDjHlnBaEnKcmRc6w4MbpgyLfzZ7hwmXDXfV0A81PUsMuBFxdthIxUgmMdw== X-Received: by 2002:a7b:c14e:: with SMTP id z14mr5819482wmi.58.1582225017385; Thu, 20 Feb 2020 10:56:57 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 5sm567391wrc.75.2020.02.20.10.56.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Feb 2020 10:56:56 -0800 (PST) Date: Thu, 20 Feb 2020 18:56:55 +0000 From: "Leif Lindholm" To: Pankaj Bansal Cc: Meenakshi Aggarwal , Michael D Kinney , Varun Sethi , "devel@edk2.groups.io" Subject: Re: [PATCH 08/19] Silicon/NXP: Remove unnecessary PCDs Message-ID: <20200220185655.GH23627@bivouac.eciton.net> References: <20200207124328.8723-1-pankaj.bansal@nxp.com> <20200207124328.8723-9-pankaj.bansal@nxp.com> <20200210173200.GM23627@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Feb 11, 2020 at 08:45:13 +0000, Pankaj Bansal wrote: > > -----Original Message----- > > From: Leif Lindholm > > Sent: Monday, February 10, 2020 11:02 PM > > To: Pankaj Bansal > > Cc: Meenakshi Aggarwal ; Michael D Kinney > > ; Varun Sethi ; > > devel@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 > > > --- > > > .../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 > > > #include > > > #include > > > +#include > > > > > > #include > > > > > > @@ -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 > > > #include > > > #include -#include > > > +#include > > > > > > #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 > > >