public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/3] UefiPayloadPkg: Enhance the logic to add ConIn and ConOut
@ 2022-05-10  7:11 Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 1/3] UefiPayloadPkg: Simplify code logic Zhiguang Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Zhiguang Liu @ 2022-05-10  7:11 UTC (permalink / raw)
  To: devel; +Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You, Sean Rhodes

Fix the bug that in some platform, there is no serial output or graphics output.
Code passed open CI, and pleae check in https://github.com/tianocore/edk2/pull/2864

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>

Zhiguang Liu (3):
  UefiPayloadPkg: Simplify code logic
  UefiPayloadPkg: Add Serial IO device path according to related
    protocol
  UefiPayloadPkg: Connect all root bridge in
    PlatformBootManagerBeforeConsole

 .../PlatformBootManagerLib.inf                |   2 +
 .../PlatformBootManagerLib/PlatformConsole.c  | 282 +++++-------------
 .../PlatformBootManagerLib/PlatformConsole.h  |   1 -
 3 files changed, 82 insertions(+), 203 deletions(-)

-- 
2.32.0.windows.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] UefiPayloadPkg: Simplify code logic
  2022-05-10  7:11 [PATCH 0/3] UefiPayloadPkg: Enhance the logic to add ConIn and ConOut Zhiguang Liu
@ 2022-05-10  7:11 ` Zhiguang Liu
  2022-05-10  7:39   ` Ni, Ray
  2022-05-10  7:11 ` [PATCH 2/3] UefiPayloadPkg: Add Serial IO device path according to related protocol Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 3/3] UefiPayloadPkg: Connect all root bridge in PlatformBootManagerBeforeConsole Zhiguang Liu
  2 siblings, 1 reply; 6+ messages in thread
From: Zhiguang Liu @ 2022-05-10  7:11 UTC (permalink / raw)
  To: devel; +Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You, Sean Rhodes

A little overdesign about VisitAllPciInstances function, since there are
two call back functions. Simplify the code logic by combining the two call
back functions.

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  | 83 +++++--------------
 1 file changed, 21 insertions(+), 62 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index bfaf89e74c..9887183624 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -387,18 +387,20 @@ 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
+  @param[in]  Context   - The context of the callback
 
-  @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
@@ -424,56 +426,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 +438,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 +456,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 +469,7 @@ DetectAndPreparePlatformPciDevicePath (
   //
   // Enable all display devices
   //
-  if (IS_PCI_DISPLAY (Pci)) {
+  if (IS_PCI_DISPLAY (&Pci)) {
     //
     // Add them to ConOut.
     //
@@ -543,6 +495,8 @@ DetectAndPreparePlatformPciDevicePaths (
   BOOLEAN  DetectDisplayOnly
   )
 {
+  EFI_STATUS  Status;
+
   mDetectDisplayOnly = DetectDisplayOnly;
 
   EfiBootManagerUpdateConsoleVariable (
@@ -551,7 +505,12 @@ DetectAndPreparePlatformPciDevicePaths (
     NULL
     );
 
-  return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);
+  Status = VisitAllInstancesOfProtocol (
+             &gEfiPciIoProtocolGuid,
+             DetectAndPreparePlatformPciDevicePath,
+             NULL
+             );
+  return Status;
 }
 
 /**
-- 
2.32.0.windows.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] UefiPayloadPkg: Add Serial IO device path according to related protocol
  2022-05-10  7:11 [PATCH 0/3] UefiPayloadPkg: Enhance the logic to add ConIn and ConOut Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 1/3] UefiPayloadPkg: Simplify code logic Zhiguang Liu
@ 2022-05-10  7:11 ` Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 3/3] UefiPayloadPkg: Connect all root bridge in PlatformBootManagerBeforeConsole Zhiguang Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Zhiguang Liu @ 2022-05-10  7:11 UTC (permalink / raw)
  To: devel; +Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You, Sean Rhodes

Current code follow some rules to check if the PCI device connected to a
serial port device, but some platform or hardware doesn't follow such rule.
By locating gEfiSerialIoProtocolGuid protocol, we can find the related
device path.

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.inf                |   1 +
 .../PlatformBootManagerLib/PlatformConsole.c  | 149 +++++-------------
 .../PlatformBootManagerLib/PlatformConsole.h  |   1 -
 3 files changed, 44 insertions(+), 107 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 80390e0d98..acf2880d22 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -60,6 +60,7 @@
   gEfiDxeSmmReadyToLockProtocolGuid
   gEfiSmmAccess2ProtocolGuid
   gUniversalPayloadPlatformBootManagerOverrideProtocolGuid
+  gEfiSerialIoProtocolGuid
 
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index 9887183624..5e1c77d866 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -47,36 +47,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define gPnpPs2Keyboard \
   PNPID_DEVICE_PATH_NODE(0x0303)
 
-#define gUartVendor \
-  { \
-    { \
-      HARDWARE_DEVICE_PATH, \
-      HW_VENDOR_DP, \
-      { \
-        (UINT8) (sizeof (VENDOR_DEVICE_PATH)), \
-        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) \
-      } \
-    }, \
-    EDKII_SERIAL_PORT_LIB_VENDOR_GUID \
-  }
-
-#define gUart \
-  { \
-    { \
-      MESSAGING_DEVICE_PATH, \
-      MSG_UART_DP, \
-      { \
-        (UINT8) (sizeof (UART_DEVICE_PATH)), \
-        (UINT8) ((sizeof (UART_DEVICE_PATH)) >> 8) \
-      } \
-    }, \
-    0, \
-    115200, \
-    8, \
-    1, \
-    1 \
-  }
-
 #define gPcAnsiTerminal \
   { \
     { \
@@ -92,9 +62,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 ACPI_HID_DEVICE_PATH  gPnpPs2KeyboardDeviceNode  = gPnpPs2Keyboard;
 ACPI_HID_DEVICE_PATH  gPnp16550ComPortDeviceNode = gPnp16550ComPort;
-UART_DEVICE_PATH      gUartDeviceNode            = gUart;
 VENDOR_DEVICE_PATH    gTerminalTypeDeviceNode    = gPcAnsiTerminal;
-VENDOR_DEVICE_PATH    gUartDeviceVendorNode      = gUartVendor;
 
 //
 // Predefined platform root bridge
@@ -112,13 +80,11 @@ EFI_DEVICE_PATH_PROTOCOL  *gPlatformRootBridges[] = {
 BOOLEAN  mDetectDisplayOnly;
 
 /**
-  Add IsaKeyboard to ConIn; add IsaSerial to ConOut, ConIn, ErrOut.
+  Add IsaKeyboard to ConIn.
 
   @param[in] DeviceHandle  Handle of the LPC Bridge device.
 
-  @retval EFI_SUCCESS  Console devices on the LPC bridge have been added to
-                       ConOut, ConIn, and ErrOut.
-
+  @retval EFI_SUCCESS  IsaKeyboard on the LPC bridge have been added to ConIn.
   @return              Error codes, due to EFI_DEVICE_PATH_PROTOCOL missing
                        from DeviceHandle.
 **/
@@ -129,7 +95,6 @@ PrepareLpcBridgeDevicePath (
 {
   EFI_STATUS                Status;
   EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-  EFI_DEVICE_PATH_PROTOCOL  *TempDevicePath;
 
   DevicePath = NULL;
   Status     = gBS->HandleProtocol (
@@ -141,26 +106,11 @@ PrepareLpcBridgeDevicePath (
     return Status;
   }
 
-  TempDevicePath = DevicePath;
-
   //
   // Register Keyboard
   //
   DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gPnpPs2KeyboardDeviceNode);
   EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
-
-  //
-  // Register COM1
-  //
-  DevicePath = TempDevicePath;
-  DevicePath = AppendDevicePathNode ((EFI_DEVICE_PATH_PROTOCOL *)NULL, (EFI_DEVICE_PATH_PROTOCOL *)&gUartDeviceVendorNode);
-  DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gUartDeviceNode);
-  DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gTerminalTypeDeviceNode);
-
-  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
-  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
-  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
-
   return EFI_SUCCESS;
 }
 
@@ -291,43 +241,6 @@ PreparePciVgaDevicePath (
   return EFI_SUCCESS;
 }
 
-/**
-  Add PCI Serial to ConOut, ConIn, ErrOut.
-
-  @param[in]  DeviceHandle - Handle of PciIo protocol.
-
-  @retval EFI_SUCCESS  - PCI Serial is added to ConOut, ConIn, and ErrOut.
-  @retval EFI_STATUS   - No PCI Serial device is added.
-
-**/
-EFI_STATUS
-PreparePciSerialDevicePath (
-  IN EFI_HANDLE  DeviceHandle
-  )
-{
-  EFI_STATUS                Status;
-  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-
-  DevicePath = NULL;
-  Status     = gBS->HandleProtocol (
-                      DeviceHandle,
-                      &gEfiDevicePathProtocolGuid,
-                      (VOID *)&DevicePath
-                      );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gUartDeviceNode);
-  DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gTerminalTypeDeviceNode);
-
-  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
-  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
-  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
-
-  return EFI_SUCCESS;
-}
-
 /**
   For every PCI instance execute a callback function.
 
@@ -452,18 +365,6 @@ DetectAndPreparePlatformPciDevicePath (
       PrepareLpcBridgeDevicePath (Handle);
       return EFI_SUCCESS;
     }
-
-    //
-    // Here we decide which Serial device to enable in PCI bus
-    //
-    if (IS_PCI_16550SERIAL (&Pci)) {
-      //
-      // Add them to ConOut, ConIn, ErrOut.
-      //
-      DEBUG ((DEBUG_INFO, "Found PCI 16550 SERIAL device\n"));
-      PreparePciSerialDevicePath (Handle);
-      return EFI_SUCCESS;
-    }
   }
 
   //
@@ -481,6 +382,41 @@ DetectAndPreparePlatformPciDevicePath (
   return Status;
 }
 
+/**
+  For every Serial Io instance, add it to ConOut, ConIn, ErrOut.
+
+  @param[in]  Handle     - The Serial Io device handle
+  @param[in]  Instance   - The instance of the SerialIo protocol
+  @param[in]  Context    - The context of the callback
+
+  @retval EFI_STATUS - Callback function failed.
+
+**/
+EFI_STATUS
+EFIAPI
+AddDevicePathForOneSerialIoInstance (
+  IN EFI_HANDLE  Handle,
+  IN VOID        *Instance,
+  IN VOID        *Context
+  )
+{
+  EFI_STATUS                Status;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = NULL;
+  Status     = gBS->HandleProtocol (
+                      Handle,
+                      &gEfiDevicePathProtocolGuid,
+                      (VOID *)&DevicePath
+                      );
+  DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&gTerminalTypeDeviceNode);
+
+  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
+  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
+  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
+  return Status;
+}
+
 /**
   Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut
 
@@ -505,6 +441,12 @@ DetectAndPreparePlatformPciDevicePaths (
     NULL
     );
 
+  VisitAllInstancesOfProtocol (
+    &gEfiSerialIoProtocolGuid,
+    AddDevicePathForOneSerialIoInstance,
+    NULL
+    );
+
   Status = VisitAllInstancesOfProtocol (
              &gEfiPciIoProtocolGuid,
              DetectAndPreparePlatformPciDevicePath,
@@ -558,11 +500,6 @@ PlatformConsoleInit (
   VOID
   )
 {
-  gUartDeviceNode.BaudRate = PcdGet64 (PcdUartDefaultBaudRate);
-  gUartDeviceNode.DataBits = PcdGet8 (PcdUartDefaultDataBits);
-  gUartDeviceNode.Parity   = PcdGet8 (PcdUartDefaultParity);
-  gUartDeviceNode.StopBits = PcdGet8 (PcdUartDefaultStopBits);
-
   ConnectRootBridge ();
 
   //
diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
index a13f4b8b59..2c7d21cc84 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.h
@@ -21,7 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Protocol/PciIo.h>
 
 #define IS_PCI_ISA_PDECODE(_p)  IS_CLASS3 (_p, PCI_CLASS_BRIDGE, PCI_CLASS_BRIDGE_ISA_PDECODE, 0)
-#define IS_PCI_16550SERIAL(_p)  IS_CLASS3 (_p, PCI_CLASS_SCC, PCI_SUBCLASS_SERIAL, PCI_IF_16550)
 
 //
 // Type definitions
-- 
2.32.0.windows.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] UefiPayloadPkg: Connect all root bridge in PlatformBootManagerBeforeConsole
  2022-05-10  7:11 [PATCH 0/3] UefiPayloadPkg: Enhance the logic to add ConIn and ConOut Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 1/3] UefiPayloadPkg: Simplify code logic Zhiguang Liu
  2022-05-10  7:11 ` [PATCH 2/3] UefiPayloadPkg: Add Serial IO device path according to related protocol Zhiguang Liu
@ 2022-05-10  7:11 ` Zhiguang Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Zhiguang Liu @ 2022-05-10  7:11 UTC (permalink / raw)
  To: devel; +Cc: Guo Dong, Ray Ni, Maurice Ma, Benjamin You, Sean Rhodes

Some ConIn or ConOut device may not in the first root bridge, so connect all
root bridge  before detect ConIn and ConOut device.

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.inf                |  1 +
 .../PlatformBootManagerLib/PlatformConsole.c  | 52 ++++++-------------
 2 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index acf2880d22..9f58c460cd 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -61,6 +61,7 @@
   gEfiSmmAccess2ProtocolGuid
   gUniversalPayloadPlatformBootManagerOverrideProtocolGuid
   gEfiSerialIoProtocolGuid
+  gEfiPciRootBridgeIoProtocolGuid
 
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index 5e1c77d866..e4a9f5f0f9 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -38,9 +38,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
     0 \
   }
 
-#define gPciRootBridge \
-  PNPID_DEVICE_PATH_NODE(0x0A03)
-
 #define gPnp16550ComPort \
   PNPID_DEVICE_PATH_NODE(0x0501)
 
@@ -64,19 +61,6 @@ ACPI_HID_DEVICE_PATH  gPnpPs2KeyboardDeviceNode  = gPnpPs2Keyboard;
 ACPI_HID_DEVICE_PATH  gPnp16550ComPortDeviceNode = gPnp16550ComPort;
 VENDOR_DEVICE_PATH    gTerminalTypeDeviceNode    = gPcAnsiTerminal;
 
-//
-// Predefined platform root bridge
-//
-PLATFORM_ROOT_BRIDGE_DEVICE_PATH  gPlatformRootBridge0 = {
-  gPciRootBridge,
-  gEndEntire
-};
-
-EFI_DEVICE_PATH_PROTOCOL  *gPlatformRootBridges[] = {
-  (EFI_DEVICE_PATH_PROTOCOL *)&gPlatformRootBridge0,
-  NULL
-};
-
 BOOLEAN  mDetectDisplayOnly;
 
 /**
@@ -456,32 +440,26 @@ DetectAndPreparePlatformPciDevicePaths (
 }
 
 /**
-  The function will connect root bridge
+  The function will connect one root bridge
 
-   @return EFI_SUCCESS      Connect RootBridge successfully.
+  @param[in]  Handle     - The root bridge handle
+  @param[in]  Instance   - The instance of the root bridge
+  @param[in]  Context    - The context of the callback
+
+  @return EFI_SUCCESS      Connect RootBridge successfully.
 
 **/
 EFI_STATUS
-ConnectRootBridge (
-  VOID
+EFIAPI
+ConnectOneRootBridge (
+  IN EFI_HANDLE  Handle,
+  IN VOID        *Instance,
+  IN VOID        *Context
   )
 {
   EFI_STATUS  Status;
-  EFI_HANDLE  RootHandle;
-
-  //
-  // Make all the PCI_IO protocols on PCI Seg 0 show up
-  //
-  Status = gBS->LocateDevicePath (
-                  &gEfiDevicePathProtocolGuid,
-                  &gPlatformRootBridges[0],
-                  &RootHandle
-                  );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
 
-  Status = gBS->ConnectController (RootHandle, NULL, NULL, FALSE);
+  Status = gBS->ConnectController (Handle, NULL, NULL, FALSE);
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -500,7 +478,11 @@ PlatformConsoleInit (
   VOID
   )
 {
-  ConnectRootBridge ();
+  VisitAllInstancesOfProtocol (
+    &gEfiPciRootBridgeIoProtocolGuid,
+    ConnectOneRootBridge,
+    NULL
+    );
 
   //
   // Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut
-- 
2.32.0.windows.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] UefiPayloadPkg: Simplify code logic
  2022-05-10  7:11 ` [PATCH 1/3] UefiPayloadPkg: Simplify code logic Zhiguang Liu
@ 2022-05-10  7:39   ` Ni, Ray
  2022-05-11  1:48     ` Zhiguang Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2022-05-10  7:39 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Guo, Maurice Ma, You, Benjamin, Rhodes, Sean

> 
> +DetectAndPreparePlatformPciDevicePath (
> 
>    IN EFI_HANDLE  Handle,
> 
>    IN VOID        *Instance,
> 
>    IN VOID        *Context

Is "Context" needed? Can you please remove it?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] UefiPayloadPkg: Simplify code logic
  2022-05-10  7:39   ` Ni, Ray
@ 2022-05-11  1:48     ` Zhiguang Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Zhiguang Liu @ 2022-05-11  1:48 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Dong, Guo, Maurice Ma, You, Benjamin, Rhodes, Sean

Hi Ray,
The function DetectAndPreparePlatformPciDevicePath is the second parameter of VisitAllInstancesOfProtocol.
It follows the below type:
typedef
EFI_STATUS
(EFIAPI *PROTOCOL_INSTANCE_CALLBACK)(
  IN EFI_HANDLE            Handle,
  IN VOID                 *Instance,
  IN VOID                 *Context
  );

The same function pointer type is also defined in OvmfPkg.
I didn't change the function pointer type to avoid same type having different definition in edk2 repo.
Do I need to consider that? What's your suggestion?

Thanks
Zhiguang


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

> 
> +DetectAndPreparePlatformPciDevicePath (
> 
>    IN EFI_HANDLE  Handle,
> 
>    IN VOID        *Instance,
> 
>    IN VOID        *Context

Is "Context" needed? Can you please remove it?


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-11  1:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-10  7:11 [PATCH 0/3] UefiPayloadPkg: Enhance the logic to add ConIn and ConOut Zhiguang Liu
2022-05-10  7:11 ` [PATCH 1/3] UefiPayloadPkg: Simplify code logic Zhiguang Liu
2022-05-10  7:39   ` Ni, Ray
2022-05-11  1:48     ` Zhiguang Liu
2022-05-10  7:11 ` [PATCH 2/3] UefiPayloadPkg: Add Serial IO device path according to related protocol Zhiguang Liu
2022-05-10  7:11 ` [PATCH 3/3] UefiPayloadPkg: Connect all root bridge in PlatformBootManagerBeforeConsole Zhiguang Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox