Hi Isaac,
Please see feedback inline.
Thanks,
Nate
> -----Original Message-----
> From: Oram, Isaac W <isaac.w.o...@intel.com>
> Sent: Friday, October 15, 2021 2:26 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@intel.com>; Liming Gao
> <gaolim...@byosoft.com.cn>; Dong, Eric <eric.d...@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH V1 10/11]
> MinPlatformPkg/SpiFvbService: Reduce duplicate code
>
> Consolidate all the entry point functions into SpiFvbServiceMm.c This
> reduces the number of files and makes the code easier to understand.
>
> Cc: Chasel Chiu <chasel.c...@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> Cc: Liming Gao <gaolim...@byosoft.com.cn>
> Cc: Eric Dong <eric.d...@intel.com>
> Signed-off-by: Isaac Oram <isaac.w.o...@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> | 42 ++++++++++++++++++++
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> | 1 -
>
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandalon
> eMm.c | 32 ---------------
>
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandalon
> eMm.inf | 1 -
> Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceTraditiona
> lMm.c | 32 ---------------
> 5 files changed, 42 insertions(+), 66 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> index 1d8d55b8f2..cc9310023e 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.c
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceMm.
> +++ c
> @@ -278,3 +278,45 @@ FvbInitialize (
>
> return EFI_SUCCESS;
> }
> +
> +/**
> + The driver Traditional MM entry point.
> +
> + @param[in] ImageHandle Image handle of this driver.
> + @param[in] SystemTable A pointer to the EFI system table.
> +
> + @retval EFI_SUCCESS This function always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SpiFvbTraditionalMmInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + FvbInitialize ();
Now that FvbInitialize() returns an EFI_STATUS code. It would be good to add a
DEBUG print here in the case where an error is returned. I understand the
hesitation to return it back to the core since this would not necessarily be a
fatal error.
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + The driver Standalone MM entry point.
> +
> + @param[in] ImageHandle Image handle of this driver.
> + @param[in] MmSystemTable A pointer to the MM system table.
> +
> + @retval EFI_SUCCESS This function always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SpiFvbStandaloneMmInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> + )
> +{
> + FvbInitialize ();
Now that FvbInitialize() returns an EFI_STATUS code. It would be good to add a
DEBUG print here in the case where an error is returned. I understand the
hesitation to return it back to the core since this would not necessarily be a
fatal error.
> +
> + return EFI_SUCCESS;
> +}
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> index 7b69bedcdc..600ad32d9f 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> +++
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSmm
> +++ .inf
> @@ -55,7 +55,6 @@
> SpiFvbServiceCommon.c
> SpiFvbServiceMm.h
> SpiFvbServiceMm.c
> - SpiFvbServiceTraditionalMm.c
>
> [Protocols]
> gEfiDevicePathProtocolGuid ## PRODUCES
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.c
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.c
> deleted file mode 100644
> index 252c818d65..0000000000
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/** @file
> - MM driver source for several Serial Flash devices
> - which are compliant with the Intel(R) Serial Flash Interface Compatibility
> Specification.
> -
> - Copyright (c) Microsoft Corporation.<BR>
> - SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "SpiFvbServiceCommon.h"
> -#include "SpiFvbServiceMm.h"
> -
> -/**
> - The driver Standalone MM entry point.
> -
> - @param[in] ImageHandle Image handle of this driver.
> - @param[in] MmSystemTable A pointer to the MM system table.
> -
> - @retval EFI_SUCCESS This function always returns EFI_SUCCESS.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -SpiFvbStandaloneMmInitialize (
> - IN EFI_HANDLE ImageHandle,
> - IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> - )
> -{
> - FvbInitialize ();
> -
> - return EFI_SUCCESS;
> -}
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> index 1dfb2520e7..4e056466a1 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceStandal
> oneMm.inf
> +++ b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceSta
> +++ ndaloneMm.inf
> @@ -54,7 +54,6 @@
> SpiFvbServiceCommon.c
> SpiFvbServiceMm.h
> SpiFvbServiceMm.c
> - SpiFvbServiceStandaloneMm.c
>
> [Protocols]
> gEfiDevicePathProtocolGuid ## PRODUCES
> diff --git
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceTraditio
> nalMm.c
> b/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceTraditio
> nalMm.c
> deleted file mode 100644
> index 1c2dac70e3..0000000000
> ---
> a/Platform/Intel/MinPlatformPkg/Flash/SpiFvbService/SpiFvbServiceTraditio
> nalMm.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/** @file
> - MM driver source for several Serial Flash devices
> - which are compliant with the Intel(R) Serial Flash Interface Compatibility
> Specification.
> -
> - Copyright (c) Microsoft Corporation.<BR>
> - SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "SpiFvbServiceCommon.h"
> -#include "SpiFvbServiceMm.h"
> -
> -/**
> - The driver Traditional MM entry point.
> -
> - @param[in] ImageHandle Image handle of this driver.
> - @param[in] SystemTable A pointer to the EFI system table.
> -
> - @retval EFI_SUCCESS This function always returns EFI_SUCCESS.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -SpiFvbTraditionalMmInitialize (
> - IN EFI_HANDLE ImageHandle,
> - IN EFI_SYSTEM_TABLE *SystemTable
> - )
> -{
> - FvbInitialize ();
> -
> - return EFI_SUCCESS;
> -}
> --
> 2.27.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82551): https://edk2.groups.io/g/devel/message/82551
Mute This Topic: https://groups.io/mt/86360117/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
