> > 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 = AsciiSPrint (
> > + Buffer, sizeof (Buffer),
> > + "UEFI firmware built at %a on %a. version:\n\r",
> > + __TIME__, __DATE__
> > + );
> > + SerialPortWrite ((UINT8 *) Buffer, CharCount);
>
> Drop space before Buffer.
OK.
>
> > +
> > + 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 <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 = AsciiSPrint (
> > + Buffer, sizeof (Buffer),
> > + "UEFI firmware built at %a on %a. version:\n\r",
> > + __TIME__, __DATE__
> > + );
> > + SerialPortWrite ((UINT8 *) Buffer, CharCount);
>
> Drop space before Buffer.
OK.
>
> 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.
>
I am preparing the patches to get the core/cluster info in NXP SOCs.
I will move the version print in that code.
> > +
> > + 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.
it *does* set the FIRMWARE_VER environment variable.
FIRMWARE_VER is just like WORKSPACE and PACKAGES_PATH.
>
> > 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.
see explanation above.
>
> > +
> > +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.
OK.
>
> > +
> > +directories=$(echo $PACKAGES_PATH | tr ":" "\n")
>
> Could set IFS=: instead, it would have the same effect.
Good suggestion. will modify.
>
> > +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?
It would set the FIRMWARE_VER environment variable when "set_firmware_ver.sh"
script is executed.
After that user can build platform firmware with -D FIRMWARE_VER=$ FIRMWARE_VER
flag.
I have put this suggestion for users in "set_firmware_ver.sh" script.
>
> /
> Leif
>
> > +
> > +echo "build edk2 firmware with -D FIRMWARE_VER=\$FIRMWARE_VER"
> > --
> > 2.17.1
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63892): https://edk2.groups.io/g/devel/message/63892
Mute This Topic: https://groups.io/mt/75371035/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
