[edk2-devel] [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance

[Leif Lindholm]

Apart from the request to break out Ac01PcieConfigRW and
Ac01PcieCfgIn/Out# I noticed a further thing.

On Wed, Sep 15, 2021 at 22:55:11 +0700, Nhi Pham wrote:
> +/**
> +  Get RootBridge disable status.
> +
> +  @param[in]  HBIndex               Index to identify of PCIE Host bridge.
> +  @param[in]  RBIndex               Index to identify of underneath PCIE Root bridge.
> +
> +  @retval BOOLEAN                   Return RootBridge disable status.
> +**/
> +BOOLEAN
> +Ac01PcieCheckRootBridgeDisabled (
> +  IN UINTN HBIndex,
> +  IN UINTN RBIndex
> +  )
> +{
> +  UINTN RCIndex;
> +  INT8  Ret;
> +
> +  RCIndex = HBIndex;
> +  Ret = !RCList[RCIndex].Active;
> +  if (Ret) {
> +    PciList[HBIndex] = -1;
> +  } else {
> +    PciList[HBIndex] = HBIndex;
> +  }
> +  if (HBIndex == (AC01_MAX_PCIE_ROOT_COMPLEX -1)) {
> +    SortPciList (PciList);
> +    if (!IsSlaveSocketPresent ()) {
> +      AcpiPatchPciMem32 (PciList);
> +    }
> +    AcpiInstallMcfg (PciList);
> +    AcpiInstallIort (PciList);

Should a function named "Check if RootBridge is Disabled" really have
the undocumented side effect of going off and installing ACPI tables?

Please move that logic over to PciHostBridgeReadyToBootEvent.

With that, I think this revision is fully reviewed.

/
    Leif

> +  }
> +  return Ret;
> +}


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81219): https://edk2.groups.io/g/devel/message/81219
Mute This Topic: https://groups.io/mt/85631150/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-