Re: [edk2-platforms][PATCH v2 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population

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

On Tue, Oct 08, 2019 at 01:38:38PM +0100, Pete Batard wrote:
> Improve RpiFirmwareGetSerial() to derive a serial number from the
> MAC address, in case the platform returns 0 for the serial number.
> 
> Also fix a typo where "%s" was used instead of "%a".

I did not see a reply to my feedback on the previous round, so I'm
unsure if this is a mistake or a disagreement?

> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index 378c99bcba05..c2344252d2c0 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -219,7 +219,7 @@ RpiFirmwareSetPowerState (
>  
>    if (!EFI_ERROR (Status) &&
>        PowerState ^ (Cmd->TagBody.PowerState & RPI_MBOX_POWER_STATE_ENABLE)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to %sable power for device %d\n",
> +    DEBUG ((DEBUG_ERROR, "%a: failed to %aable power for device %d\n",

This is clearly a bugfix, but I don't see it as being part of "Improve
serial number population", which is what it claims to be if I view it
wih git blame.

/
    Leif

>        __FUNCTION__, PowerState ? "en" : "dis", DeviceId));
>      Status = EFI_DEVICE_ERROR;
>    }
> @@ -393,7 +393,14 @@ RpiFirmwareGetSerial (
>    }
>  
>    *Serial = Cmd->TagBody.Serial;
> -  return EFI_SUCCESS;
> +  // Some platforms return 0 for serial. For those, try to use the MAC address.
> +  if (*Serial == 0) {
> +    Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
> +    // Convert to a more user-friendly value
> +    *Serial = SwapBytes64 (*Serial << 16);
> +  }
> +
> +  return Status;
>  }
>  
>  #pragma pack()
> -- 
> 2.21.0.windows.1
>