From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-VI1-obe.outbound.protection.outlook.com (EUR05-VI1-obe.outbound.protection.outlook.com [40.107.21.75]) by mx.groups.io with SMTP id smtpd.web12.3569.1596854375571351328 for ; Fri, 07 Aug 2020 19:39:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp1.onmicrosoft.com header.s=selector2-nxp1-onmicrosoft-com header.b=aA96TWQC; spf=pass (domain: oss.nxp.com, ip: 40.107.21.75, mailfrom: pankaj.bansal@oss.nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=a9hk9daA9Y0eZscuNnUjbrFY0UejrNR7SsQjTaT27WHmeCIWpSU2k4BwgvhDQu57Cxj+3zRlQigkp/6BIKrNyzbteP8g7wLkj2YSJIFt3zlMb0Qt0RUKOfI8BBtRMDC5NilS3d+DfqSrt0iMiwG7JU+qJ41Vx/FgOwrw674yfutDWgSf/OnFNS82SFO1p9au7xZOCVSCbBfeXd6OBpyhpTv8W1GByylwsF3z4VXNPoehhI2HxH+XeD0C8KigmMx5Wh0TuVfmQdzympV6UpFytfFmOtosp2to7Qa4asLsA8JESBfyJp6TuCrVF2lBkPanRcHa985cdqVHPXA9WFRc+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GkQRNY/7ojchAGbie1t8BnwfVepGVfdwWt7MzCqdpaE=; b=GCv8Tbtkwcp+BDM8Oy5N+QWXCIsovQ+7Koa++4aYEK7VyJ4Kp6oDzGh26zZYqTfMX4bVQlmPLOX/ZvCqvCbAtWnBTS/0Hxhjbw6ig0QcKzkI8hHestRE2MJYqD+QJD6L55CIdn5whKw3dKVOXhjB9erh/MA7jEriVBV79CjmfeSoHAsXRqtc2/9Gh75Q+CYc422FcZWJbPctiLTu2pAYG3ETWQmxz2+TM0Mc3MjrWiIxlCiBEM5lZbMom0geD8KU2steq4SJOGDeFNGwCkv0RoyROXIBVNheEslcAg7To5/BXct4zchHOjQDzf4JMizVhEZImk+W4wa9QnuTZJxO5Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oss.nxp.com; dmarc=pass action=none header.from=oss.nxp.com; dkim=pass header.d=oss.nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=NXP1.onmicrosoft.com; s=selector2-NXP1-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GkQRNY/7ojchAGbie1t8BnwfVepGVfdwWt7MzCqdpaE=; b=aA96TWQCuxwEQaycNMpb53dlxJG5iaedihPwVHsd8BcHXV3douKL48XAcIye9Zt0kwsYexT7AZ3aKeGjxutP2YxBsEYT8rC290wCjGsaKfgFIS30xskIhQwn5LEX6IMog6QjUvIvB+VPCUiYgRQJUWd/pYVtO0yUpBnyrWLG2MY= Received: from VI1PR04MB5933.eurprd04.prod.outlook.com (2603:10a6:803:ec::16) by VI1PR04MB5440.eurprd04.prod.outlook.com (2603:10a6:803:ce::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3261.16; Sat, 8 Aug 2020 02:39:32 +0000 Received: from VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::4521:b667:cf06:b79b]) by VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::4521:b667:cf06:b79b%7]) with mapi id 15.20.3261.022; Sat, 8 Aug 2020 02:39:32 +0000 From: "Pankaj Bansal" To: Leif Lindholm , "Pankaj Bansal (OSS)" 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 Thread-Topic: [PATCH edk2-platforms 3/3] Silicon/NXP: Add Support for git commit info print Thread-Index: AQHWbS0lxsXT5/emZkSNM9JS6rJ0JA== Date: Sat, 8 Aug 2020 02:39:32 +0000 Message-ID: References: <20200708051933.8123-1-pankaj.bansal@oss.nxp.com> <20200708051933.8123-4-pankaj.bansal@oss.nxp.com> <20200805144441.GO31778@vanye> In-Reply-To: <20200805144441.GO31778@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: nuviainc.com; dkim=none (message not signed) header.d=none;nuviainc.com; dmarc=none action=none header.from=oss.nxp.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [103.217.116.83] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: c32bcc2b-48cc-4ad4-b235-08d83b4447e0 x-ms-traffictypediagnostic: VI1PR04MB5440: x-ms-exchange-sharedmailbox-routingagent-processed: True x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:785; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: RSceT80fo7YDZPllcEEVmJwVoaUTalRvks7kbvEvM294XYsTZ0racFTpbHpGmez3TGbsGdexCujgt0Z8fqYIL8BB7dPwhTClJM5jJ+SEy02qinWzDAZsV0eTYv9MVk5TN28M5EbFtD4/R1Cthh9yxkbqXU2SRPciyv6aMYP4cp4bDk89pat0vZNIQzGXFcN9QOQ1viV6ZbEWiSy/FyXcxxb4ApPcz/y4uy7YaP134kX5sv9q59N92zL7f+kUVFUB2nBqllc970GEauEbai61rmVPhePsEqBXKfn8E+8T3/XZhqwQnVgqJfLFMhM7t6yzj/je7UEGM4Tn784WswkS1Q== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR04MB5933.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(396003)(39860400002)(136003)(346002)(376002)(366004)(55016002)(66946007)(64756008)(8936002)(26005)(186003)(316002)(110136005)(33656002)(8676002)(6506007)(54906003)(5660300002)(52536014)(71200400001)(2906002)(66446008)(76116006)(66556008)(66476007)(86362001)(9686003)(4326008)(83380400001)(7696005)(478600001);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: Un/55nrug3osraaOSbKjgNgClQy8fRX5tGHXTUpQAgPTLqOPPqy0ZI97c0U6NwcnOlF3hPr1AQHHgL49AKC5wShXzr5x5xF+31+qr2yrixY302/YijJG9Lk/ZII+Afh7rbpuqwI+QjOoBNmFa2blkSmOJ8P/FlpfWpXM7IDexcaaB7ckHgPHqftXGFX1AJm5cs68NbHBlfiR7udbT++qM7n8kru3FbnA5Xe7gxz6kDwdnkxl4/PDX7qXVQCOq0CjFjaLK21hyFOFm7F7rVdnNFl/F1dv7BaTb5S8yAODAI2qGgran34gyieFJyOSFdKluiSRh5quyM9CT/vzb/ca242hQcFf8zFyVNPS40uUVmDKMnvs41S+NNYxIGRw5uUovbMAc5h73GFAp5HS+QOAkdiThVJ4ncb/5HE6JabX60W9DNE91Loilff+XybJWd5jQPc78HSi3nAjnGV224+o4uh09KN9HT5v8wjeAoWMPy/myIiq9SMpvCPdsBRSQlgrxMqu7w7FyBy+zRCZr1aBOrsm/nFKTFCjYaZH0uCv+VdYJKgf3RGozZUeL/FoK9XT9wLmSiSX0giQXuj3/5owIyLat5cD9EJyKElRl1XwPForeYHUnVWISgaHIX+lo6GZVRSL0nuWR/wTjoEaZm1d2g== MIME-Version: 1.0 X-OriginatorOrg: oss.nxp.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5933.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c32bcc2b-48cc-4ad4-b235-08d83b4447e0 X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Aug 2020 02:39:32.1458 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Tm2cKDKh2kUVi3rYtLSTgXznxMStAKrNlSqBkgkg1cPkRnNVZS5jGk/MpfVOxagabJ6JJEakgTIIPdHDM2j6Yw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5440 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > > 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 =3D AsciiSPrint ( > > + Buffer, sizeof (Buffer), > > + "UEFI firmware built at %a on %a. version:\n\r", > > + __TIME__, __DATE__ > > + ); > > + SerialPortWrite ((UINT8 *) Buffer, CharCount); >=20 > Drop space before Buffer. OK. >=20 > > + > > + CharCount =3D 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 =3D AsciiSPrint ( > > + Buffer, sizeof (Buffer), > > + "UEFI firmware built at %a on %a. version:\n\r", > > + __TIME__, __DATE__ > > + ); > > + SerialPortWrite ((UINT8 *) Buffer, CharCount); >=20 > Drop space before Buffer. OK. >=20 > I will note here that you are adding identical content to two > different ChassisLib implementations. >=20 > This is a strong indicator that the abstraction level is currently > incorrect and should be revisited. >=20 I am preparing the patches to get the core/cluster info in NXP SOCs. I will move the version print in that code. > > + > > + CharCount =3D 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 >=20 > This script doesn't *set* anything (which is good, since I would have > rejected that outright), so the name should reflect the actual function. it *does* set the FIRMWARE_VER environment variable. FIRMWARE_VER is just like WORKSPACE and PACKAGES_PATH. >=20 > > 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 >=20 > This too should reflect actual function. see explanation above. >=20 > > + > > +get_git_version() > > +{ > > + command -v git>/dev/null 2>&1 > > + if [ $? -eq 0 ] && [ -n "$1" ] > > + then > > + head_or_tag=3D`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=3D$FIRMWARE_VER is already set" > > + echo "Please retry after 'unset FIRMWARE_VER' command" > > +fi >=20 > Just do > FIRMWARE_VER=3D > before the first code in the script. OK. >=20 > > + > > +directories=3D$(echo $PACKAGES_PATH | tr ":" "\n") >=20 > Could set IFS=3D: instead, it would have the same effect. Good suggestion. will modify. >=20 > > +for dir in $directories; do > > + if [ -n "$FIRMWARE_VER" ]; then > > + FIRMWARE_VER=3D"$FIRMWARE_VER;$(basename $dir):$(get_git_version > $dir)" > > + else > > + FIRMWARE_VER=3D$(basename $dir):$(get_git_version $dir) > > + fi > > +done > > + > > +echo "FIRMWARE_VER=3D$FIRMWARE_VER" > > +export FIRMWARE_VER=3D$FIRMWARE_VER >=20 > What is this line supposed to achieve? It would set the FIRMWARE_VER environment variable when "set_firmware_ver.s= h" script is executed. After that user can build platform firmware with -D FIRMWARE_VER=3D$ FIRMWA= RE_VER flag. I have put this suggestion for users in "set_firmware_ver.sh" script. >=20 > / > Leif >=20 > > + > > +echo "build edk2 firmware with -D FIRMWARE_VER=3D\$FIRMWARE_VER" > > -- > > 2.17.1 > >