Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Dec 12, 2017 at 06:28:31PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 18:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 06:20:00PM +0000, Ard Biesheuvel wrote:
> >> On 12 December 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
> >> >> Expose the contents of the .DSC macro BUILD_NUMBER via the
> >> >> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
> >> >> and as the FMP system firmware version (for capsule update).
> >> >>
> >> >> Also, set the firmware vendor to 'Linaro Enterprise Group', to
> >> >> distinguish our builds from builds by other parties.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >> ---
> >> >>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc                                                  | 8 +++++++-
> >> >>  Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf             | 1 +
> >> >>  Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc       | 6 ++++--
> >> >>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc                                      | 8 +++++++-
> >> >>  Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf       | 1 +
> >> >>  Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> >> >>  6 files changed, 24 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> index 8fbd7b2d908f..5ec26f9cdd34 100644
> >> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> @@ -26,6 +26,7 @@ [Defines]
> >> >>    BUILD_TARGETS                  = DEBUG|RELEASE
> >> >>    SKUID_IDENTIFIER               = DEFAULT
> >> >>    FLASH_DEFINITION               = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
> >> >> +  BUILD_NUMBER                   = 1
> >> >>
> >> >>  [BuildOptions]
> >> >>    RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
> >> >> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
> >> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> >> >>
> >> >>  [PcdsFixedAtBuild.common]
> >> >> -  gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> >> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
> >> >
> >> > Actually, could you just delete this Pcd and let it fall back to the
> >> > default value of "EDK II"?
> >> >
> >>
> >> Yes. But perhaps it makes sense to put $(FW_VENDOR) in there if it is
> >> defined, so we can set it at build time? I would like to have a way to
> >> put something in the FirmwareVendor field in the UEFI system table
> >> that can help us identify firmware builds done by Linaro.
> >
> > Sure, that makes sense. Although we already have FIRMWARE_VER for
> > similar purposes, so something to match that name?
> > I.e. FIRMWARE_VENDOR.
> >
> 
> Yes, so replace the hunk above with
> 
> @@ -222,7 +227,9 @@ [PcdsFeatureFlag]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> 
>  [PcdsFixedAtBuild.common]
> -  gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> +!ifdef $(FIRMWARE_VENDOR)
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"$(FIRMWARE_VENDOR)"
> +!endif
> 
>    # non-secure SRAM
>    gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> 
> ?

Works for me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>