Re: [PATCH v2 21/28] Slicon/NXP: Add PlatformPei Lib

["Leif Lindholm" <leif@nuviainc.com>]

On Fri, Mar 20, 2020 at 20:05:36 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> 
> PlatformPeiLib is going to be linked to Platform PEIM.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  .../Library/PlatformPeiLib/PlatformPeiLib.c   | 30 ++++++++++++++
>  .../Library/PlatformPeiLib/PlatformPeiLib.inf | 41 +++++++++++++++++++
>  Silicon/NXP/NxpQoriqLs.dsc.inc                |  3 +-
>  3 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> 
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> new file mode 100644
> index 000000000000..f64e564469f8
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> @@ -0,0 +1,30 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <PiPei.h>
> +
> +#include <Library/HobLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>

Although this is based on an existing library, please sort includes
alphabetically here.

> +
> +#define XPRINT(x) PRINT(x)
> +#define PRINT(x) #x

This isn't a PRINT operation, this is a Stringize operation.

> +
> +EFI_STATUS
> +EFIAPI
> +PlatformPeim (
> +  VOID
> +  )
> +{
> +  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> +  DEBUG ((DEBUG_INIT, "Edk2 version is %a\n", XPRINT (WORKSPACE_GIT_VERSION)));
> +  DEBUG ((DEBUG_INIT, "Edk2 platforms version is %a\n", XPRINT (PACKAGES_PATH_GIT_VERSION)));

The only benefit I can see from the macro as opposed to using '#'
directly is that it permits wrapping of too long lines, so please do
that.

> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> new file mode 100644
> index 000000000000..fb42693daa20
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> @@ -0,0 +1,41 @@
> +#/** @file
> +#
> +#  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

Update version.

> +  BASE_NAME                      = ArmPlatformPeiLib
> +  FILE_GUID                      = 49d37060-70b5-11e0-aa2d-0002a5d5c51b

Unless this is another magic GUID filename (like ACPI), please
generate a new GUID.

> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformPeiLib
> +
> +[BuildOptions]
> +  GCC:*_*_*_CC_FLAGS = -DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"
> +  GCC:*_*_*_CC_FLAGS = -DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"

Does this not require special magic build command line options to do
anything useful? This needs documenting.

/
    Leif

> +
> +[Sources]
> +  PlatformPeiLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  HobLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdFvSize
> +
> +[depex]
> +  TRUE
> diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> index 234a5e2707cd..5f77f47f0399 100644
> --- a/Silicon/NXP/NxpQoriqLs.dsc.inc
> +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
> @@ -101,6 +101,8 @@ [LibraryClasses.common]
>    PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>    PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
>  
> +  PlatformPeiLib|Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> +
>  [LibraryClasses.common.SEC]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> @@ -111,7 +113,6 @@ [LibraryClasses.common.SEC]
>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>    MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>    MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
>    # 1/123 faster than Stm or Vstm version
> -- 
> 2.17.1
>