[edk2-devel] [PATCH V1 1/4] Platform/NXP: Add generic log in CM to print SoC version
Leif Lindholm
leif at nuviainc.com
Mon Jun 14 20:58:02 UTC 2021
On Fri, Jun 11, 2021 at 21:21:57 +0530, Vikas Singh wrote:
> Summary -
> 1.Configuration Manager(CM) is a common implementation
> and should not evaluate the SoC version using macro's
> However CM must directly consume SoC ver string from
> platfrom who is extending CM services for ACPI table
> generation.
This tells me nothing about what this patch does.
> 2.Platforms who extends CM services for themselves must
> notify their SoC details to CM.
Neither does this.
> 3.This patch will update the lx2160ardb platform header
> also with PLAT_SOC_NAME, this will be consumed by CM.
And this sound like it should be a separate patch.
*However* when I look at the code, this does look like a single
change. And what is descibed as point 3 is the actual change in the patch.
Moreover, this patch addresses a historic horror, in that SVR_LX2160A
was defined both in the platform header and in the SoC header (which
I'm not saying you had necessarily noticed, but I am suggesting it is
added to the message).
If I wrote this commit message, I would start with a slightly tweaked
subject line:
Platform/NXP: Make SoC version log in ConfigurationManager generic
(CM is not a recognised abbreviation, so should be printed expanded)
As for the body, I would say something like:
This patch replaces the logic in ConfigurationManager to print
platform name based on platform ID with a simple #define.
This also removes a duplication of the SVR_LX2160A definition between
SoC and platform headers.
No comments on the code itself.
/
Leif
> Signed-off-by: Vikas Singh <vikas.singh at puresoftware.com>
> ---
> Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c | 10 +++-------
> Platform/NXP/LX2160aRdbPkg/Include/Platform.h | 5 ++---
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
> index 80ce8412c4..dc1a7f5f85 100644
> --- a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -2,7 +2,7 @@
> Configuration Manager Dxe
>
> Copyright 2020 NXP
> - Copyright 2020 Puresoftware Ltd
> + Copyright 2020-2021 Puresoftware Ltd
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -170,12 +170,8 @@ InitializePlatformRepository (
> PlatformRepo = This->PlatRepoInfo;
>
> Svr = SocGetSvr ();
> - if (SVR_SOC_VER(Svr) == SVR_LX2160A) {
> - PlatformRepo->FslBoardRevision = SVR_MAJOR(Svr);
> - DEBUG ((DEBUG_INFO, "Fsl : SoC LX2160A Rev = 0x%x\n", PlatformRepo->FslBoardRevision));
> - } else {
> - DEBUG ((DEBUG_INFO, "Fsl : SoC Unknown Rev = 0x%x\n", PlatformRepo->FslBoardRevision));
> - }
> + PlatformRepo->FslBoardRevision = SVR_MAJOR(Svr);
> + DEBUG ((DEBUG_INFO, "Fsl : SoC = %s Rev = 0x%x\n", PLAT_SOC_NAME, PlatformRepo->FslBoardRevision));
>
> return EFI_SUCCESS;
> }
> diff --git a/Platform/NXP/LX2160aRdbPkg/Include/Platform.h b/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
> index 76a41d4369..c18faf28cd 100644
> --- a/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
> +++ b/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
> @@ -2,7 +2,7 @@
> * Platform headers
> *
> * Copyright 2020 NXP
> - * Copyright 2020 Puresoftware Ltd
> + * Copyright 2020-2021 Puresoftware Ltd
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> *
> @@ -15,12 +15,11 @@
> #define EFI_ACPI_ARM_OEM_REVISION 0x00000000
>
> // Soc defines
> +#define PLAT_SOC_NAME "LX2160ARDB"
> #define SVR_SOC_VER(svr) (((svr) >> 8) & 0xFFFFFE)
> #define SVR_MAJOR(svr) (((svr) >> 4) & 0xf)
> #define SVR_MINOR(svr) (((svr) >> 0) & 0xf)
>
> -#define SVR_LX2160A 0x873600
> -
> // PCLK
> #define DCFG_BASE 0x1E00000
> #define DCFG_LEN 0x1FFFF
> --
> 2.25.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76486): https://edk2.groups.io/g/devel/message/76486
Mute This Topic: https://groups.io/mt/83471705/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