Re: [PATCH v2 1/3] UefiPayloadPkg: Simplify code logic

["Guo Dong" <guo.dong@intel.com>]

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com> 
Sent: Thursday, May 12, 2022 3:55 AM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Maurice Ma <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>
Subject: [PATCH v2 1/3] UefiPayloadPkg: Simplify code logic

A little overdesign about VisitAllPciInstances function, since there are two call back functions. Simplify the code logic by combining the two call back functions, and unused parameters.
Change the PROTOCOL_INSTANCE_CALLBACK to SIMPLE_PROTOCOL_INSTANCE_CALLBACK because the former is also defined in OvmfPkg. Rename it to avoid confusion.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../PlatformBootManagerLib/PlatformConsole.c  | 93 +++++--------------  .../PlatformBootManagerLib/PlatformConsole.h  |  5 +-
 2 files changed, 25 insertions(+), 73 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index bfaf89e74c..75aafebccd 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -333,7 +333,6 @@ PreparePciSerialDevicePath (
    @param[in]  Id                 - The protocol GUID for callback   @param[in]  CallBackFunction   - The callback function-  @param[in]  Context    - The context of the callback    @retval EFI_STATUS - Callback function failed. @@ -341,9 +340,8 @@ PreparePciSerialDevicePath (
 EFI_STATUS EFIAPI VisitAllInstancesOfProtocol (-  IN EFI_GUID                    *Id,-  IN PROTOCOL_INSTANCE_CALLBACK  CallBackFunction,-  IN VOID                        *Context+  IN EFI_GUID                           *Id,+  IN SIMPLE_PROTOCOL_INSTANCE_CALLBACK  CallBackFunction   ) {   EFI_STATUS  Status;@@ -376,8 +374,7 @@ VisitAllInstancesOfProtocol (
      Status = (*CallBackFunction)(   HandleBuffer[Index],-  Instance,-  Context+  Instance   );   } @@ -387,21 +384,21 @@ VisitAllInstancesOfProtocol (
 }  /**-  For every PCI instance execute a callback function.+  Do platform specific PCI Device check and add them to+  ConOut, ConIn, ErrOut. -  @param[in]  Handle     - The PCI device handle-  @param[in]  Instance   - The instance of the PciIo protocol-  @param[in]  Context    - The context of the callback+  @param[in]  Handle    - Handle of PCI device instance+  @param[in]  Instance  - The instance of PCI device -  @retval EFI_STATUS - Callback function failed.+  @retval EFI_SUCCESS - PCI Device check and Console variable update successfully.+  @retval EFI_STATUS - PCI Device check or Console variable update fail.  **/ EFI_STATUS EFIAPI-VisitingAPciInstance (+DetectAndPreparePlatformPciDevicePath (   IN EFI_HANDLE  Handle,-  IN VOID        *Instance,-  IN VOID        *Context+  IN VOID        *Instance   ) {   EFI_STATUS           Status;@@ -424,56 +421,6 @@ VisitingAPciInstance (
     return Status;   } -  return (*(VISIT_PCI_INSTANCE_CALLBACK)(UINTN)Context)(-  Handle,-  PciIo,-  &Pci-  );-}--/**-  For every PCI instance execute a callback function.--  @param[in]  CallBackFunction - Callback function pointer--  @retval EFI_STATUS - Callback function failed.--**/-EFI_STATUS-EFIAPI-VisitAllPciInstances (-  IN VISIT_PCI_INSTANCE_CALLBACK  CallBackFunction-  )-{-  return VisitAllInstancesOfProtocol (-           &gEfiPciIoProtocolGuid,-           VisitingAPciInstance,-           (VOID *)(UINTN)CallBackFunction-           );-}--/**-  Do platform specific PCI Device check and add them to-  ConOut, ConIn, ErrOut.--  @param[in]  Handle - Handle of PCI device instance-  @param[in]  PciIo - PCI IO protocol instance-  @param[in]  Pci - PCI Header register block--  @retval EFI_SUCCESS - PCI Device check and Console variable update successfully.-  @retval EFI_STATUS - PCI Device check or Console variable update fail.--**/-EFI_STATUS-EFIAPI-DetectAndPreparePlatformPciDevicePath (-  IN EFI_HANDLE           Handle,-  IN EFI_PCI_IO_PROTOCOL  *PciIo,-  IN PCI_TYPE00           *Pci-  )-{-  EFI_STATUS  Status;-   Status = PciIo->Attributes (                     PciIo,                     EfiPciIoAttributeOperationEnable,@@ -486,9 +433,9 @@ DetectAndPreparePlatformPciDevicePath (
     //     // Here we decide whether it is LPC Bridge     //-    if ((IS_PCI_LPC (Pci)) ||-        ((IS_PCI_ISA_PDECODE (Pci)) &&-         (Pci->Hdr.VendorId == 0x8086)+    if ((IS_PCI_LPC (&Pci)) ||+        ((IS_PCI_ISA_PDECODE (&Pci)) &&+         (Pci.Hdr.VendorId == 0x8086)         )         )     {@@ -504,7 +451,7 @@ DetectAndPreparePlatformPciDevicePath (
     //     // Here we decide which Serial device to enable in PCI bus     //-    if (IS_PCI_16550SERIAL (Pci)) {+    if (IS_PCI_16550SERIAL (&Pci)) {       //       // Add them to ConOut, ConIn, ErrOut.       //@@ -517,7 +464,7 @@ DetectAndPreparePlatformPciDevicePath (
   //   // Enable all display devices   //-  if (IS_PCI_DISPLAY (Pci)) {+  if (IS_PCI_DISPLAY (&Pci)) {     //     // Add them to ConOut.     //@@ -543,6 +490,8 @@ DetectAndPreparePlatformPciDevicePaths (
   BOOLEAN  DetectDisplayOnly   ) {+  EFI_STATUS  Status;+   mDetectDisplayOnly = DetectDisplayOnly;    EfiBootManagerUpdateConsoleVariable (@@ -551,7 +500,11 @@ DetectAndPreparePlatformPciDevicePaths (
     NULL     ); -  return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);+  Status = VisitAllInstancesOfProtocol (+             &gEfiPciIoProtocolGuid,+             DetectAndPreparePlatformPciDevicePath+             );+  return Status; }  /**diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
index a13f4b8b59..3713c21c45 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
@@ -37,10 +37,9 @@ typedef struct {
  typedef EFI_STATUS-(EFIAPI *PROTOCOL_INSTANCE_CALLBACK)(+(EFIAPI *SIMPLE_PROTOCOL_INSTANCE_CALLBACK)(   IN EFI_HANDLE            Handle,-  IN VOID                 *Instance,-  IN VOID                 *Context+  IN VOID                 *Instance   );  /**-- 
2.32.0.windows.2