Re: [edk2-platforms: PATCH v4 8/9] Marvell/Drivers: SmbiosPlatformDxe: Load SMBIOS strings from PCD

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

On Fri, Oct 11, 2019 at 05:20:30PM +0200, Marcin Wojtas wrote:
> From: Patryk Duda <pdk@semihalf.com>
> 
> This patch implements convenient way of changing strings included
> in SMBIOS Table1, Table2, Table3.
> 
> Strings can be altered by defining following PCDs:
>   gMarvellTokenSpaceGuid.PcdProductManufacturer
>   gMarvellTokenSpaceGuid.PcdProductPlatformName
>   gMarvellTokenSpaceGuid.PcdProductSerial
>   gMarvellTokenSpaceGuid.PcdProductVersion
> 
> Signed-off-by: Patryk Duda <pdk@semihalf.com>
> ---
>  Silicon/Marvell/Marvell.dec                                     |  6 ++++++
>  Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf |  4 ++++
>  Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c   | 22 ++++++++++----------
>  3 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
> index d337d3e..cdf8154 100644
> --- a/Silicon/Marvell/Marvell.dec
> +++ b/Silicon/Marvell/Marvell.dec
> @@ -169,6 +169,12 @@
>    gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x0 }|VOID*|0x3000034
>    gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x0 }|VOID*|0x3000035
>  
> +#Platform description
> +  gMarvellTokenSpaceGuid.PcdProductManufacturer|"Marvell"|VOID*|0x50000100
> +  gMarvellTokenSpaceGuid.PcdProductPlatformName|"Marvell Development Board"|VOID*|0x50000101
> +  gMarvellTokenSpaceGuid.PcdProductSerial|"Serial Not Set"|VOID*|0x50000103
> +  gMarvellTokenSpaceGuid.PcdProductVersion|"Revision unknown"|VOID*|0x50000102
> +
>  #RTC
>    gMarvellTokenSpaceGuid.PcdRtcBaseAddress|0x0|UINT64|0x40000052
>  
> diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> index 8b4586c..7722146 100644
> --- a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> +++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> @@ -36,6 +36,10 @@
>  
>  [FixedPcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
> +  gMarvellTokenSpaceGuid.PcdProductManufacturer
> +  gMarvellTokenSpaceGuid.PcdProductPlatformName
> +  gMarvellTokenSpaceGuid.PcdProductSerial
> +  gMarvellTokenSpaceGuid.PcdProductVersion
>  
>  [Protocols]
>    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> index 08f4fa7..d29478c 100644
> --- a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> +++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> @@ -101,10 +101,10 @@ STATIC SMBIOS_TABLE_TYPE1 mArmadaDefaultType1 = {
>  };
>  
>  STATIC CHAR8 CONST *mArmadaDefaultType1Strings[] = {
> -  "Marvell                        \0",/* Manufacturer */
> -  "Armada 7k/8k Family Board      \0",/* Product Name placeholder*/
> -  "Revision unknown               \0",/* Version placeholder */
> -  "                               \0",/* 32 character buffer */
> +  (CHAR8 *)PcdGetPtr (PcdProductManufacturer),
> +  (CHAR8 *)PcdGetPtr (PcdProductPlatformName),
> +  (CHAR8 *)PcdGetPtr (PcdProductVersion),
> +  (CHAR8 *)PcdGetPtr (PcdProductSerial),

These really ought to be (CONST CHAR8 *), as the compiler informs you
if dropping the cast altogether:
pointer targets in initialization of ‘const CHAR8 *’ {aka ‘const char
*’} from ‘const UINT8 *’ {aka ‘const unsigned char *’} differ in signedness

The build error without the cast is related to the type the pointer is
pointing to (UINT8), not the const attribute.

This would fail if we started building with clang -Weverything, or
ecplicitly with -Wcast-qual:
error: cast from 'const unsigned char *' to 'char *' drops const
qualifier [-Werror,-Wcast-qual]

Could you resubmit a version of this patch addressing only this
concern, throughout?

Best Regards,

Leif


>    NULL
>  };
>  
> @@ -129,10 +129,10 @@ STATIC SMBIOS_TABLE_TYPE2 mArmadaDefaultType2 = {
>  };
>  
>  STATIC CHAR8 CONST *mArmadaDefaultType2Strings[] = {
> -  "Marvell                        \0",/* Manufacturer */
> -  "Armada 7k/8k Family Board      \0",/* Product Name placeholder*/
> -  "Revision unknown               \0",/* Version placeholder */
> -  "Serial Not Set                 \0",/* Serial */
> +  (CHAR8 *)PcdGetPtr (PcdProductManufacturer),
> +  (CHAR8 *)PcdGetPtr (PcdProductPlatformName),
> +  (CHAR8 *)PcdGetPtr (PcdProductVersion),
> +  (CHAR8 *)PcdGetPtr (PcdProductSerial),
>    "Base of Chassis                \0",/* Board location */
>    NULL
>  };
> @@ -160,9 +160,9 @@ STATIC SMBIOS_TABLE_TYPE3 mArmadaDefaultType3 = {
>  };
>  
>  STATIC CHAR8 CONST *mArmadaDefaultType3Strings[] = {
> -  "Marvell                        \0",/* Manufacturer placeholder */
> -  "Revision unknown               \0",/* Version placeholder */
> -  "Serial Not Set                 \0",/* Serial placeholder */
> +  (CHAR8 *)PcdGetPtr (PcdProductManufacturer),
> +  (CHAR8 *)PcdGetPtr (PcdProductVersion),
> +  (CHAR8 *)PcdGetPtr (PcdProductSerial),
>    NULL
>  };
>  
> -- 
> 2.7.4
>