Re: [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Mike,
Thanks for reducing the API.

For the other comments I raised (single library instance usable for PEI and DXE), do you have any comments?


> -----Original Message-----
> From: Kubacki, Michael A <michael.a.kubacki@intel.com>
> Sent: Tuesday, October 15, 2019 5:26 AM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib:
> Reduce library API
> 
> Removes the following functions from FirmwareBootMediaLib.h:
>  * FirmwareBootMediaIsSpi ()
>  * FirmwareBootMediaIsUfs ()
>  * FirmwareBootMediaIsEmmc ()
>  * FirmwareBootMediaIsNvme ()
> 
> It is preferred to have a single method to retrieve the firmware boot media.
> To reduce overall maintenance effort over time, the
> FirmwareBootMediaIsXxx () pattern is removed in favor of returning the
> firmware boot media type via GetFirmwareBootMediaType ().
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> wareBootMediaLib.inf |   1 -
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> ootMediaLib.inf    |   1 -
>  Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> |  48 ---------
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareBoo
> tMediaLib.c         | 109 --------------------
>  4 files changed, 159 deletions(-)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFir
> mwareBootMediaLib.inf
> b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFir
> mwareBootMediaLib.inf
> index 83ed5f04af..7e10b5f7a7 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFir
> mwareBootMediaLib.inf
> +++
> b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmm
> +++ FirmwareBootMediaLib.inf
> @@ -27,7 +27,6 @@
>  #
> 
>  [Sources]
> -  FirmwareBootMediaLib.c
>    DxeSmmFirmwareBootMediaLib.c
> 
>  [Packages]
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwar
> eBootMediaLib.inf
> b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwar
> eBootMediaLib.inf
> index 063c4027d3..ff1da31387 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwar
> eBootMediaLib.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFir
> +++ mwareBootMediaLib.inf
> @@ -22,7 +22,6 @@
>    LIBRARY_CLASS        = FirmwareBootMediaLib
> 
>  [Sources]
> -  FirmwareBootMediaLib.c
>    PeiFirmwareBootMediaLib.c
> 
>  [Packages]
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> index aca9593a84..b36ebacf30 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib
> +++ .h
> @@ -55,52 +55,4 @@ FirmwareBootMediaIsKnown (
>    VOID
>    );
> 
> -/**
> -  Determines if the platform firmware is booting from SPI.
> -
> -  @retval TRUE        Platform firmware is booting from SPI
> -  @retval FALSE       Platform firmware is booting from a non-SPI device or the
> boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsSpi (
> -  VOID
> -  );
> -
> -/**
> -  Determines if the platform firmware is booting from UFS.
> -
> -  @retval TRUE        Platform firmware is booting from UFS
> -  @retval FALSE       Platform firmware is booting from a non-UFS device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsUfs (
> -  VOID
> -  );
> -
> -/**
> -  Determines if the platform firmware is booting from eMMC.
> -
> -  @retval TRUE        Platform firmware is booting from eMMC
> -  @retval FALSE       Platform firmware is booting from a non-eMMC device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsEmmc (
> -  VOID
> -  );
> -
> -/**
> -  Determines if the platform firmware is booting from NVMe.
> -
> -  @retval TRUE        Platform firmware is booting from NVMe.
> -  @retval FALSE       Platform firmware is booting from a non-NVMe device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsNvme (
> -  VOID
> -  );
> -
>  #endif
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareB
> ootMediaLib.c
> b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareB
> ootMediaLib.c
> deleted file mode 100644
> index 11a14d172d..0000000000
> ---
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareB
> ootMediaLib.c
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -/** @file
> -  This library identifies the firmware boot media device.
> -
> -  The firmware boot media device is used to make system initialization
> decisions in the boot flow dependent
> -  upon firmware boot media. Note that the firmware boot media is the
> storage media that the boot firmware is stored on.
> -  It is not the OS storage media which may be stored upon a different non-
> volatile storage device.
> -
> -  This file contains library implementation common to all boot phases.
> -
> -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/FirmwareBootMediaLib.h>
> -
> -/**
> -  Determines if the platform firmware is booting from SPI.
> -
> -  @retval TRUE        Platform firmware is booting from SPI
> -  @retval FALSE       Platform firmware is booting from a non-SPI device or the
> boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsSpi (
> -  VOID
> -  )
> -{
> -  EFI_STATUS          Status;
> -  FW_BOOT_MEDIA_TYPE  BootMedia;
> -
> -  Status = GetFirmwareBootMediaType (&BootMedia);
> -  if (EFI_ERROR (Status) || BootMedia != FwBootMediaSpi) {
> -    return FALSE;
> -  } else {
> -    return TRUE;
> -  }
> -}
> -
> -/**
> -  Determines if the platform firmware is booting from UFS.
> -
> -  @retval TRUE        Platform firmware is booting from UFS
> -  @retval FALSE       Platform firmware is booting from a non-UFS device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsUfs (
> -  VOID
> -  )
> -{
> -  EFI_STATUS          Status;
> -  FW_BOOT_MEDIA_TYPE  BootMedia;
> -
> -  Status = GetFirmwareBootMediaType (&BootMedia);
> -  if (EFI_ERROR (Status) || BootMedia != FwBootMediaUfs) {
> -    return FALSE;
> -  } else {
> -    return TRUE;
> -  }
> -}
> -
> -/**
> -  Determines if the platform firmware is booting from eMMC.
> -
> -  @retval TRUE        Platform firmware is booting from eMMC
> -  @retval FALSE       Platform firmware is booting from a non-eMMC device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsEmmc (
> -  VOID
> -  )
> -{
> -  EFI_STATUS          Status;
> -  FW_BOOT_MEDIA_TYPE  BootMedia;
> -
> -  Status = GetFirmwareBootMediaType (&BootMedia);
> -  if (EFI_ERROR (Status) || BootMedia != FwBootMediaEmmc) {
> -    return FALSE;
> -  } else {
> -    return TRUE;
> -  }
> -}
> -
> -/**
> -  Determines if the platform firmware is booting from NVMe.
> -
> -  @retval TRUE        Platform firmware is booting from NVMe.
> -  @retval FALSE       Platform firmware is booting from a non-NVMe device or
> the boot media is unknown
> -**/
> -BOOLEAN
> -EFIAPI
> -FirmwareBootMediaIsNvme (
> -  VOID
> -  )
> -{
> -  EFI_STATUS          Status;
> -  FW_BOOT_MEDIA_TYPE  BootMedia;
> -
> -  Status = GetFirmwareBootMediaType (&BootMedia);
> -  if (EFI_ERROR (Status) || BootMedia != FwBootMediaNvme) {
> -    return FALSE;
> -  } else {
> -    return TRUE;
> -  }
> -}
> --
> 2.16.2.windows.1