Re: [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 04/15] MdeModulePkg/PciBusDxe: Refactor the PCIe Bridge enable

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

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

> -----Original Message-----
> From: Javeed, Ashraf <ashraf.javeed@intel.com>
> Sent: Monday, May 11, 2020 12:14 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 04/15] MdeModulePkg/PciBusDxe: Refactor the PCIe Bridge enable
> 
> REF:
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1954
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2194
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2313
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2499
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2500
> 
> Refactor the PCIe Bridge enabling code.
> 
> Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 70 ++++++++++++++++++++--------------------------------------------------
>  1 file changed, 20 insertions(+), 50 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> index 5724fd6..62ef184 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> @@ -597,52 +597,36 @@ DeRegisterPciDevice (
>  }
> 
>  /**
> -  Start the PCI root Ports or PCI-PCI Bridge only.
> +  Enable all the PCI bridges under the specified root bridge or PCI-PCI Bridge.
> 
> -  @param Controller          The root bridge handle.
> -  @param RootBridge          A pointer to the PCI_IO_DEVICE.
> -  @param RemainingDevicePath A pointer to the EFI_DEVICE_PATH_PROTOCOL.
> -  @param NumberOfChildren    Children number.
> -  @param ChildHandleBuffer   A pointer to the child handle buffer.
> -
> -  @retval EFI_NOT_READY   Device is not allocated.
> -  @retval EFI_UNSUPPORTED Device only support PCI-PCI bridge.
> -  @retval EFI_NOT_FOUND   Can not find the specific device.
> -  @retval EFI_SUCCESS     Success to start Pci devices on bridge.
> +  @param Bridge          A pointer to the PCI_IO_DEVICE.
> 
>  **/
> -EFI_STATUS
> +VOID
>  EnablePciBridges (
> -  IN EFI_HANDLE                          Controller,
> -  IN PCI_IO_DEVICE                       *RootBridge
> +  IN PCI_IO_DEVICE          *Bridge
>    )
> 
>  {
>    PCI_IO_DEVICE             *PciIoDevice;
> -  EFI_STATUS                Status;
> -  LIST_ENTRY                *CurrentLink;
> +  LIST_ENTRY                *Link;
>    UINT64                    Supports;
> 
> -  PciIoDevice = NULL;
> -  CurrentLink = RootBridge->ChildList.ForwardLink;
> -
> -  while (CurrentLink != NULL && CurrentLink != &RootBridge->ChildList) {
> -
> -    PciIoDevice = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
> -
> +  for ( Link = GetFirstNode (&Bridge->ChildList)
> +      ; !IsNull (&Bridge->ChildList, Link)
> +      ; Link = GetNextNode (&Bridge->ChildList, Link)
> +      ) {
> +    PciIoDevice = PCI_IO_DEVICE_FROM_LINK (Link);
>      //
> -    // check if the device has been assigned with required resource
> -    // and registered
> +    // Skip the device hasn't been assigned with required resource
> +    // or registered.
>      //
> -    if (!PciIoDevice->Registered && !PciIoDevice->Allocated) {
> -      return EFI_NOT_READY;
> +    if (!PciIoDevice->Registered || !PciIoDevice->Allocated) {
> +      continue;
>      }
> 
>      if (IS_PCI_BRIDGE (&PciIoDevice->Pci)) {
> -      Status = EnablePciBridges (
> -                 Controller,
> -                 PciIoDevice
> -                 );
> +      EnablePciBridges (PciIoDevice);
> 
>        PciIoDevice->PciIo.Attributes (
>                             &(PciIoDevice->PciIo),
> @@ -650,27 +634,17 @@ EnablePciBridges (
>                             0,
>                             &Supports
>                           );
> -      Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
> +      Supports &= (UINT64) EFI_PCI_DEVICE_ENABLE;
>        PciIoDevice->PciIo.Attributes (
>                             &(PciIoDevice->PciIo),
>                             EfiPciIoAttributeOperationEnable,
>                             Supports,
>                             NULL
>                           );
> -
>      }
> -
> -    CurrentLink = CurrentLink->ForwardLink;
> -  }
> -
> -  if (PciIoDevice == NULL) {
> -    return EFI_NOT_FOUND;
> -  } else {
> -    return EFI_SUCCESS;
>    }
>  }
> 
> -
>  /**
>    Register to manage the PCI device on the specified root bridge or PCI-PCI Bridge.
> 
> @@ -851,9 +825,7 @@ StartPciDevicesOnBridge (
>               ChildHandleBuffer
>               );
> 
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  } else {
> +  if (!EFI_ERROR (Status)) {
>      //
>      // the late configuration of PCI Express features
>      // the platform is required to indicate its requirement for the initialization
> @@ -861,13 +833,11 @@ StartPciDevicesOnBridge (
>      //
> 
>      //
> -    // finally start those PCI bridge port devices only
> +    // finally enable those PCI bridges
>      //
> -    return EnablePciBridges (
> -             Controller,
> -             RootBridge
> -             );
> +    EnablePciBridges (RootBridge);
>    }
> +  return Status;
>  }
> 
>  /**
> --
> 2.21.0.windows.1