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.web11.8745.1596638685300158562 for ; Wed, 05 Aug 2020 07:44:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=xAOfFQFY; spf=pass (domain: nuviainc.com, ip: 209.85.128.65, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f65.google.com with SMTP id c80so6067250wme.0 for ; Wed, 05 Aug 2020 07:44:45 -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=sesdMwvRgh/yhgW7LHHGcPpAinU8KvfEBOHCslp/NRU=; b=xAOfFQFYlPB9zp8u2/QzacGoQpRWtRVdqCpJvR9+0qCaOyJWnV2n7xrDaxyNGp5cc7 fP6PdbgaSlsuOr/3/d8I4v7HsCN/82fDaj1fUuYydjbD/6y8zKG4pQ+rdJSK8Z0xOM5H xY6o+tevNlJ0rh5/sqav8j4IZSI7SgT/oukAmYr1sB1zdjAyYiwCrmAPTsiQSl+QGH3+ 6fX1kR3/Wq454ipvonlb3VgqJULe5eQih0KFQrFFNbzmhHXo8ByuN16C0x7deBrljLsT /f9qujxrBwY9vL32xfZbl5o2pJMHsERrWhyFjJfmLcfklk27HUvQk0VBWRhnO1jqpZ3e 1rEA== 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=sesdMwvRgh/yhgW7LHHGcPpAinU8KvfEBOHCslp/NRU=; b=TeuG4pYY4In3oCVHgf4dNN+2XlMLiyDv51ZB7PfQpGqAf1t4ilQZEMYzYGx+LOslTw 81c0A5gCO5z8VwFe+ecderR6mCGqa9Co/qboss/fVbQqOEvcOWtu82znoqwUny0dne/V 1WkBrXlBiOLS8V5qki0tk/Cvp9JU+2ld/gvqy0ZyoNW3Ammrlrg/+Kmm5HMelCr0uBlC FhfHQehHsU0PGBD1xSQra0l/7F/6cviWkPAopSnAwY8jHh53Xp75cvm4WXUdq/+4JFvn J4e9aMz+kjPHqxKq1EoKQ50AxObJbb8Obq/OLsI7J7TVHaqkQIPcH40UCEd5fESA9Yj/ XBDw== X-Gm-Message-State: AOAM532LYJO9JZTu05kPPK6l9tuA/uawiQq3TN1LSxSid32PKZnD8fxh 0yTpe1IOemeAoRkRvUZVmJ7Phw== X-Google-Smtp-Source: ABdhPJwU2kkc++32j1oZVIjDByErlrlUkKFDaPrZWtz7FKVTWUxI0fam1VPWitVqtdSfYS6ytVnNKw== X-Received: by 2002:a7b:c084:: with SMTP id r4mr3515030wmh.23.1596638683666; Wed, 05 Aug 2020 07:44:43 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id 15sm2803928wmo.33.2020.08.05.07.44.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Aug 2020 07:44:43 -0700 (PDT) Date: Wed, 5 Aug 2020 15:44:41 +0100 From: "Leif Lindholm" To: Pankaj Bansal Cc: Meenakshi Aggarwal , devel@edk2.groups.io, Ard Biesheuvel Subject: Re: [PATCH edk2-platforms 3/3] Silicon/NXP: Add Support for git commit info print Message-ID: <20200805144441.GO31778@vanye> References: <20200708051933.8123-1-pankaj.bansal@oss.nxp.com> <20200708051933.8123-4-pankaj.bansal@oss.nxp.com> MIME-Version: 1.0 In-Reply-To: <20200708051933.8123-4-pankaj.bansal@oss.nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jul 08, 2020 at 00:19:33 -0500, Pankaj Bansal wrote: > From: Pankaj Bansal > > This patch adds the Support for printing the git commit information > in linux build environment. > > Ideal place of retrieving this information should be python script in > BaseTools. > > A Feature request for the same has been created: > https://bugzilla.tianocore.org/show_bug.cgi?id=2838 > > Signed-off-by: Pankaj Bansal > --- > Silicon/NXP/NxpQoriqLs.dsc.inc | 3 ++ > Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf | 5 +++ > Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf | 5 +++ > Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c | 17 +++++++++ > Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c | 17 +++++++++ > Silicon/NXP/set_firmware_ver.sh | 36 ++++++++++++++++++++ > 6 files changed, 83 insertions(+) > > diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc > index 06ee012c227a..a0762a6ef61d 100644 > --- a/Silicon/NXP/NxpQoriqLs.dsc.inc > +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc > @@ -224,6 +224,9 @@ [PcdsDynamicHii.common.DEFAULT] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10 > > [PcdsFixedAtBuild.common] > + !ifdef $(FIRMWARE_VER) > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)" > + !endif > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf > index f5dbd1349dc5..69f884af9e34 100644 > --- a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf > +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf > @@ -16,6 +16,7 @@ [Defines] > > [Packages] > ArmPkg/ArmPkg.dec > + MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > Silicon/NXP/Chassis2/Chassis2.dec > Silicon/NXP/NxpQoriqLs.dec > @@ -24,6 +25,7 @@ [LibraryClasses] > IoAccessLib > IoLib > PcdLib > + PrintLib > SerialPortLib > > [Sources.common] > @@ -31,3 +33,6 @@ [Sources.common] > > [FeaturePcd] > gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian > + > +[FixedPcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString > diff --git a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf > index 75b68cc4ca2d..632acc52b20a 100644 > --- a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf > +++ b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf > @@ -16,6 +16,7 @@ [Defines] > > [Packages] > ArmPkg/ArmPkg.dec > + MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > Silicon/NXP/Chassis3V2/Chassis3V2.dec > Silicon/NXP/NxpQoriqLs.dec > @@ -24,6 +25,7 @@ [LibraryClasses] > IoAccessLib > IoLib > PcdLib > + PrintLib > SerialPortLib > > [Sources.common] > @@ -31,3 +33,6 @@ [Sources.common] > > [FeaturePcd] > gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian > + > +[FixedPcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString > diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c > index 91b19f832f00..bc782e7e3873 100644 > --- a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c > +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > > /** > @@ -89,10 +90,26 @@ ChassisInit ( > VOID > ) > { > + CHAR8 Buffer[100]; > + UINTN CharCount; > + > // > // Early init serial Port to get board information. > // > SerialPortInitialize (); > > + CharCount = AsciiSPrint ( > + Buffer, sizeof (Buffer), > + "UEFI firmware built at %a on %a. version:\n\r", > + __TIME__, __DATE__ > + ); > + SerialPortWrite ((UINT8 *) Buffer, CharCount); Drop space before Buffer. > + > + CharCount = AsciiSPrint ( > + Buffer, sizeof (Buffer), "%s\n\r", > + (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString) > + ); > + SerialPortWrite ((UINT8 *) Buffer, CharCount); > + > SmmuInit (); > } > diff --git a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c > index 30f8f945b233..6d546f4754f9 100644 > --- a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c > +++ b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > > /** > @@ -64,8 +65,24 @@ ChassisInit ( > VOID > ) > { > + CHAR8 Buffer[100]; > + UINTN CharCount; > + > // > // Early init serial Port to get board information. > // > SerialPortInitialize (); > + > + CharCount = AsciiSPrint ( > + Buffer, sizeof (Buffer), > + "UEFI firmware built at %a on %a. version:\n\r", > + __TIME__, __DATE__ > + ); > + SerialPortWrite ((UINT8 *) Buffer, CharCount); Drop space before Buffer. I will note here that you are adding identical content to two different ChassisLib implementations. This is a strong indicator that the abstraction level is currently incorrect and should be revisited. > + > + CharCount = AsciiSPrint ( > + Buffer, sizeof (Buffer), "%s\n\r", > + (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString) > + ); > + SerialPortWrite ((UINT8 *) Buffer, CharCount); > } > diff --git a/Silicon/NXP/set_firmware_ver.sh b/Silicon/NXP/set_firmware_ver.sh This script doesn't *set* anything (which is good, since I would have rejected that outright), so the name should reflect the actual function. > new file mode 100755 > index 000000000000..ba2336ad23dc > --- /dev/null > +++ b/Silicon/NXP/set_firmware_ver.sh > @@ -0,0 +1,36 @@ > +#!/bin/sh > +# > +# Copyright 2020 NXP > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# parse PACKAGES_PATH and set FIRMWARE_VER based on that This too should reflect actual function. > + > +get_git_version() > +{ > + command -v git>/dev/null 2>&1 > + if [ $? -eq 0 ] && [ -n "$1" ] > + then > + head_or_tag=`git -C $1 describe --tag --dirty --broken --always 2>/dev/null` > + printf $head_or_tag > + fi > +} > + > +if [ -n "$FIRMWARE_VER" ]; then > + echo "Warning FIRMWARE_VER=$FIRMWARE_VER is already set" > + echo "Please retry after 'unset FIRMWARE_VER' command" > +fi Just do FIRMWARE_VER= before the first code in the script. > + > +directories=$(echo $PACKAGES_PATH | tr ":" "\n") Could set IFS=: instead, it would have the same effect. > +for dir in $directories; do > + if [ -n "$FIRMWARE_VER" ]; then > + FIRMWARE_VER="$FIRMWARE_VER;$(basename $dir):$(get_git_version $dir)" > + else > + FIRMWARE_VER=$(basename $dir):$(get_git_version $dir) > + fi > +done > + > +echo "FIRMWARE_VER=$FIRMWARE_VER" > +export FIRMWARE_VER=$FIRMWARE_VER What is this line supposed to achieve? / Leif > + > +echo "build edk2 firmware with -D FIRMWARE_VER=\$FIRMWARE_VER" > -- > 2.17.1 >