Re: [PATCH 1/2] MdeModulePkg/PciBus: Refine ActiveVGADeviceOnTheRootBridge

["Zeng, Star" <star.zeng@intel.com>]

On 2018/8/23 10:53, Ruiyu Ni wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1109
> The patch doesn't change any behavior of this function.
> It just renames the function to LocateVgaDevice() and renames
> some parameters and local variables.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
>   MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 35 +++++++++++------------
>   MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h | 10 +++----
>   2 files changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> index f7039da992..fdec0bcd53 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
> @@ -1002,7 +1002,7 @@ ActiveVGADeviceOnTheSameSegment (
>   
>       if (Temp->PciRootBridgeIo->SegmentNumber == VgaDevice->PciRootBridgeIo->SegmentNumber) {
>   
> -      Temp = ActiveVGADeviceOnTheRootBridge (Temp);
> +      Temp = LocateVgaDevice (Temp);
>   
>         if (Temp != NULL) {
>           return Temp;
> @@ -1016,41 +1016,41 @@ ActiveVGADeviceOnTheSameSegment (
>   }
>   
>   /**
> -  Get the active VGA device on the root bridge.
> +  Locate the active VGA device under the bridge.
>   
> -  @param RootBridge  PCI IO instance for the root bridge.
> +  @param Bridge  PCI IO instance for the bridge.
>   
>     @return The active VGA device.
>   
>   **/
>   PCI_IO_DEVICE *
> -ActiveVGADeviceOnTheRootBridge (
> -  IN PCI_IO_DEVICE        *RootBridge
> +LocateVgaDevice (
> +  IN PCI_IO_DEVICE        *Bridge
>     )
>   {
>     LIST_ENTRY      *CurrentLink;
> -  PCI_IO_DEVICE   *Temp;
> +  PCI_IO_DEVICE   *PciIo;

How about using PciIoDev as the local variable name? PciIo makes me 
regard it for PciIo protocol at the first glance. :)

With the new name,
Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks,
Star

>   
> -  CurrentLink = RootBridge->ChildList.ForwardLink;
> +  CurrentLink = Bridge->ChildList.ForwardLink;
>   
> -  while (CurrentLink != NULL && CurrentLink != &RootBridge->ChildList) {
> +  while (CurrentLink != NULL && CurrentLink != &Bridge->ChildList) {
>   
> -    Temp = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
> +    PciIo = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
>   
> -    if (IS_PCI_VGA(&Temp->Pci) &&
> -        (Temp->Attributes &
> +    if (IS_PCI_VGA(&PciIo->Pci) &&
> +        (PciIo->Attributes &
>            (EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY |
>             EFI_PCI_IO_ATTRIBUTE_VGA_IO     |
>             EFI_PCI_IO_ATTRIBUTE_VGA_IO_16)) != 0) {
> -      return Temp;
> +      return PciIo;
>       }
>   
> -    if (IS_PCI_BRIDGE (&Temp->Pci)) {
> +    if (IS_PCI_BRIDGE (&PciIo->Pci)) {
>   
> -      Temp = ActiveVGADeviceOnTheRootBridge (Temp);
> +      PciIo = LocateVgaDevice (PciIo);
>   
> -      if (Temp != NULL) {
> -        return Temp;
> +      if (PciIo != NULL) {
> +        return PciIo;
>         }
>       }
>   
> @@ -1060,6 +1060,3 @@ ActiveVGADeviceOnTheRootBridge (
>     return NULL;
>   }
>   
> -
> -
> -
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
> index c282381f85..1ec2178a21 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
> @@ -1,7 +1,7 @@
>   /** @file
>     Supporting functions declaration for PCI devices management.
>   
> -Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD License
>   which accompanies this distribution.  The full text of the license may be found at
> @@ -243,16 +243,16 @@ ActiveVGADeviceOnTheSameSegment (
>     );
>   
>   /**
> -  Get the active VGA device on the root bridge.
> +  Locate the active VGA device under the bridge.
>   
> -  @param RootBridge  PCI IO instance for the root bridge.
> +  @param Bridge  PCI IO instance for the bridge.
>   
>     @return The active VGA device.
>   
>   **/
>   PCI_IO_DEVICE *
> -ActiveVGADeviceOnTheRootBridge (
> -  IN PCI_IO_DEVICE        *RootBridge
> +LocateVgaDevice (
> +  IN PCI_IO_DEVICE        *Bridge
>     );
>   
>   
>