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 <devel@edk2.groups.io>;
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" <pankaj.bansal@nxp.com>
To: Leif Lindholm <leif@nuviainc.com>, "Pankaj Bansal (OSS)"
<pankaj.bansal@oss.nxp.com>
CC: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>, "devel@edk2.groups.io"
<devel@edk2.groups.io>, Ard Biesheuvel <ard.biesheuvel@linaro.org>
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:
<VI1PR04MB5933793A4B94BEA3C5CE61B8F1460@VI1PR04MB5933.eurprd04.prod.outlook.com>
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:
<VI1PR04MB5440C3779ACA75FC78C7ECAEB0460@VI1PR04MB5440.eurprd04.prod.outlook.com>
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 <Library/IoAccessLib.h>
> > #include <Library/IoLib.h>
> > #include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > #include <Library/SerialPortLib.h>
> >
> > /**
> > @@ -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 <Library/IoAccessLib.h>
> > #include <Library/IoLib.h>
> > #include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > #include <Library/SerialPortLib.h>
> >
> > /**
> > @@ -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
> >