Re: [PATCH] MdeModulePkg/PciBusDxe: Refine code to make it more readable

["Wu, Hao A" <hao.a.wu@intel.com>]

Some minor comments below.

With the changes:
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu
> Ni
> Sent: Monday, February 20, 2017 2:26 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [edk2] [PATCH] MdeModulePkg/PciBusDxe: Refine code to make it
> more readable
> 
> The patch doesn't impact functionality.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> ---
>  .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c       | 24 ++++++++--------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> index d9a83be..00b6622 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> @@ -1341,7 +1341,6 @@ UpdatePciInfo (
>  {
>    EFI_STATUS                        Status;
>    UINTN                             BarIndex;
> -  UINTN                             BarEndIndex;
>    BOOLEAN                           SetFlag;
>    VOID                              *Configuration;
>    EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Ptr;
> @@ -1395,24 +1394,19 @@ UpdatePciInfo (
>        break;
>      }
> 
> -    if ((Ptr->AddrTranslationOffset == MAX_UINT64) || (Ptr-
> >AddrTranslationOffset == MAX_UINT8)) {
> +    for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) {
> +      if ((Ptr->AddrTranslationOffset != MAX_UINT64) &&
> +          (Ptr->AddrTranslationOffset != MAX_UINT8) &&
> +          (Ptr->AddrTranslationOffset != BarIndex)
> +          ) {
>        //
> -      // Update all the bars in the device
> +      // Skip updating when AddrTranslationOffset is MAX_UINT64 or
> MAX_UINT8 (wide match).

According to the codes, the comment should be:
Skip updating when TranslationOffset is not MAX_UINT64 or MAX_UINT8 (wide
match).
Right?

> +      // SKip updating when current BarIndex doesn't equal to

Please fix 'SKip' as 'Skip'.

> AddrTranslationOffset.
>        // Compare against MAX_UINT8 is to keep backward compatibility.
>        //
> -      BarIndex    = 0;
> -      BarEndIndex = PCI_MAX_BAR - 1;
> -    } else {
> -      BarIndex    = (UINTN) Ptr->AddrTranslationOffset;
> -      BarEndIndex = BarIndex;
> -    }
> -
> -    if (BarIndex >= PCI_MAX_BAR) {
> -      Ptr++;
> -      continue;
> -    }
> +        continue;
> +      }
> 
> -    for (; BarIndex <= BarEndIndex; BarIndex++) {
>        SetFlag = FALSE;
>        switch (Ptr->ResType) {
>        case ACPI_ADDRESS_SPACE_TYPE_MEM:
> --
> 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel