From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web10.8224.1586264043264560283 for ; Tue, 07 Apr 2020 05:54:03 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=l9DB5YUO; spf=pass (domain: nuviainc.com, ip: 209.85.221.68, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f68.google.com with SMTP id h15so3746307wrx.9 for ; Tue, 07 Apr 2020 05:54:03 -0700 (PDT) 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=0CZf4ad4QtBViiiaLETWm6gu+RqYVMFM42DTguARJqI=; b=l9DB5YUOTZWJRoo9/o2vW246T9ocpxIOTG2oAVxSsBFPGvIPbxvAv+ujx+vaql5145 GHgaZGL3Rtzsm94rEskmGY0RsgftqneQrprpQA7CXsE5ouo70fI/jnuOcqclpoYGj5dh neJLHOe1fhMTbaqH4bPwpnlsFApxTvOGkf5EOhfLUuA69GaUTun6BzeLt0BN69g36EuY XbXsKF8qTzVDR+jRhg/zBazbSWvodbyX5oSUY6Yf1l/OtYfRu5twQhYnleEeqIcB/iBN w5DOQ9D/6KUemheb+bThrF+nq1eme75I2u4bDZbBB1xde+EqsuDn1Z7/wz8zlKLuM2AV dFcA== 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=0CZf4ad4QtBViiiaLETWm6gu+RqYVMFM42DTguARJqI=; b=qrN94rzQwD1uWWnR8ian95HRIh4/tw/qY5M2JzgRm+nxIraJMveCDb8QJInBRA3Fo/ B75+uF/GCOSHOIG/4jKGFjhK3dJlra3HUqW6IfnCsUD5HUxvcrW5GayIBLZndFV4Wo6P //01nM3pELmTvbkx6zH+GtfVcuo3ENUHpNiXNUkfyV1ugUxX6AyLVe6PxQ5Ain8licVV klBb4H3Ii6ZnJ8cCW9xiBoHSc8urVVkGiTY8RzDi1Z1iMnjHXHKYSpetqZe/v3PDhUyf oexxah2dR0gy5vbP9FGUck4AevDuot7QdzzwtLnVOSI8zA9OaEOGOofB5q8wGXRVkLgD 8Igg== X-Gm-Message-State: AGi0PuZ1UD6mzhmxI8Olb5OFvwaSQEe44qb0rCWHapLu4qtpAWvT7ROI wfsnfZSFaIwDCabmAoZ3L+AGJQ== X-Google-Smtp-Source: APiQypI9Ranozq1TdvcRw9d0iC2Z6lsbADjmxhBTnYgtV0sygkwPwUay/PfxMF2F79hMxS8p7OrNmg== X-Received: by 2002:a5d:4488:: with SMTP id j8mr2800770wrq.170.1586264041773; Tue, 07 Apr 2020 05:54:01 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id b82sm2402517wmh.1.2020.04.07.05.54.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Apr 2020 05:54:01 -0700 (PDT) Date: Tue, 7 Apr 2020 13:53:59 +0100 From: "Leif Lindholm" To: "Pankaj Bansal (OSS)" Cc: Meenakshi Aggarwal , Michael D Kinney , "devel@edk2.groups.io" , Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton Subject: Re: [PATCH v2 21/28] Slicon/NXP: Add PlatformPei Lib Message-ID: <20200407125359.GO14075@vanye> References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> <20200320143543.18615-22-pankaj.bansal@oss.nxp.com> <20200401145324.GD7468@vanye> 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 OK, taking another look at this patch, this simply needs to be deleted. Here is the sum total relevant difference compared to the ArmPlatformPkg one. 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))); If all you want to do is to print that sort of thing, please don't fork a core library to do so. First of all, please do like most other platforms and override gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString if -D FIRMWARE_VER is specified on your build command line. You can then extract current top commits of your respective repositories and not worry about getting this. I would suggest iterating across all locations in PACKAGES_PATH and then doing something similar to https://git.linaro.org/uefi/uefi-tools.git/tree/edk2-build.sh#n400 appending together. If something like this should be integrated into the build system (which might not be a bad idea), then it needs to be so properly, rather than shoehorned in for each platform. (In the past, this was difficult because we supported both git and svn, but I would say we have given up pretending that is possible.) For now, you could add the printout in a standalone Depex TRUE PEIM added to your [FV.FVMAIN_COMPACT]. / Leif On Mon, Apr 06, 2020 at 14:53:02 +0000, Pankaj Bansal (OSS) wrote: > > > > -----Original Message----- > > From: Leif Lindholm > > Sent: Wednesday, April 1, 2020 8:23 PM > > To: Pankaj Bansal (OSS) > > Cc: Meenakshi Aggarwal ; Michael D Kinney > > ; devel@edk2.groups.io; Varun Sethi > > ; Samer El-Haj-Mahmoud > Mahmoud@arm.com>; Jon Nettleton > > Subject: Re: [PATCH v2 21/28] Slicon/NXP: Add PlatformPei Lib > > > > On Fri, Mar 20, 2020 at 20:05:36 +0530, Pankaj Bansal wrote: > > > From: Pankaj Bansal > > > > > > PlatformPeiLib is going to be linked to Platform PEIM. > > > > > > Signed-off-by: Pankaj Bansal > > > --- > > > .../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 > > > + > > > +#include > > > +#include > > > +#include > > > > 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. > > OK, I can rename these to > #define PRINTSTR(x) STR(x) > #define STR(x) #x > > > > > > + > > > +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. > > OK. > > > > > > + > > > + 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. > > Since this Library is replacement implementation of > ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf, > Couldn't we use the same 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. > > Actually I had submitted a patch is BaseTools for this: > https://edk2.groups.io/g/devel/message/53146 > > This patch makes use of that BaseTools patch. > But the BaseTools patch was not accepted because that is Linux specific. > Still these changes don't cause any negative affect. > Without BaseTools patch, empty string would be printed. > > > > > / > > 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/BaseUefiDecomp > > ressLib.inf > > > @@ -111,7 +113,6 @@ [LibraryClasses.common.SEC] > > > > > PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiH > > obListPointerLib.inf > > > > > MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiM > > emoryAllocationLib.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 > > >