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

Ruiyu Ni posted 1 patch 130 weeks ago
Failed in applying to current master (apply log)
.../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c       | 24 ++++++++--------------
1 file changed, 9 insertions(+), 15 deletions(-)

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

Posted by Ruiyu Ni 130 weeks ago
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).
+      // SKip updating when current BarIndex doesn't equal to 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

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

Posted by Wu, Hao A 130 weeks ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel