Re: [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST#

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Dec 12, 2017 at 10:38:02AM +0000, Ard Biesheuvel wrote:
> Attempt to adhere more closely to the PCIe spec by ensuring that PERST#
> remains asserted for at least 100 ms. Give it a good margin, and delay
> for 150 ms; the additional boot time delay is not going to be noticeable
> by anyone anyway.
> 
> So split the init routine in a pre and post part, and put the delay in
> the middle so we only need to do it once.
> 

This patch also adds two missing memory barriers.
Please add this to commit message. If you do:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf          |  1 +
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 46 +++++++++++++++-----
>  2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> index 08484f4f8b1a..5d87727c73ba 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> @@ -45,6 +45,7 @@ [LibraryClasses]
>    DebugLib
>    DevicePathLib
>    MemoryAllocationLib
> +  UefiBootServicesTableLib
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdPciIoTranslation
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> index e63b3a4bb23b..3da94945f96a 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> @@ -19,6 +19,7 @@
>  #include <Library/DebugLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/PciHostBridgeLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
>  #include <Platform/Pcie.h>
>  #include <Protocol/PciHostBridgeResourceAllocation.h>
>  
> @@ -176,6 +177,8 @@ SnPcieSetData (
>    }
>  
>    MmioWrite32 (Base + Offset, Data);
> +
> +  ArmDataMemoryBarrier ();
>  }
>  
>  STATIC
> @@ -194,6 +197,8 @@ SnPcieReadData (
>      Shift++;
>    }
>  
> +  ArmDataMemoryBarrier ();
> +
>    return (MmioRead32 (Base + Offset) >> Shift) & Mask;
>  }
>  
> @@ -219,12 +224,8 @@ SnDbiRoWrEn (
>  
>  STATIC
>  VOID
> -PciInitController (
> -  IN  EFI_PHYSICAL_ADDRESS    ExsBase,
> -  IN  EFI_PHYSICAL_ADDRESS    DbiBase,
> -  IN  EFI_PHYSICAL_ADDRESS    ConfigBase,
> -  IN  EFI_PHYSICAL_ADDRESS    IoMemBase,
> -  IN  CONST PCI_ROOT_BRIDGE   *RootBridge
> +PciInitControllerPre (
> +  IN  EFI_PHYSICAL_ADDRESS    ExsBase
>    )
>  {
>    SnPcieSetData (ExsBase, EM_SELECT, PRE_DET_STT_SEL, 0);
> @@ -256,7 +257,18 @@ PciInitController (
>  
>    // 3: Set device_type (RC)
>    SnPcieSetData (ExsBase, CORE_CONTROL, DEVICE_TYPE, 4);
> +}
>  
> +STATIC
> +VOID
> +PciInitControllerPost (
> +  IN  EFI_PHYSICAL_ADDRESS    ExsBase,
> +  IN  EFI_PHYSICAL_ADDRESS    DbiBase,
> +  IN  EFI_PHYSICAL_ADDRESS    ConfigBase,
> +  IN  EFI_PHYSICAL_ADDRESS    IoMemBase,
> +  IN  CONST PCI_ROOT_BRIDGE   *RootBridge
> +  )
> +{
>    // 4: Set Bifurcation  1=disable  4=able
>    // 5: Supply Reference (It has executed)
>    // 6: Wait for 10usec (Reference Clocks is stable)
> @@ -389,11 +401,23 @@ SynQuacerPciHostBridgeLibConstructor (
>    }
>  
>    for (Idx = 0; Idx < Count; Idx++) {
> -    PciInitController (mBaseAddresses[Idx].ExsBase,
> -                       mBaseAddresses[Idx].DbiBase,
> -                       mBaseAddresses[Idx].ConfigBase,
> -                       mBaseAddresses[Idx].IoMemBase,
> -                       &RootBridges[Idx]);
> +    PciInitControllerPre (mBaseAddresses[Idx].ExsBase);
> +  }
> +
> +  //
> +  // The PCIe spec requires that PERST# is asserted for at least 100 ms after
> +  // the power and clocks have become stable. So let's give a bit or margin,
> +  // and stall for 150 ms between asserting PERST# on both controllers and
> +  // de-asserting it again.
> +  //
> +  gBS->Stall (150 * 1000);
> +
> +  for (Idx = 0; Idx < Count; Idx++) {
> +    PciInitControllerPost (mBaseAddresses[Idx].ExsBase,
> +                           mBaseAddresses[Idx].DbiBase,
> +                           mBaseAddresses[Idx].ConfigBase,
> +                           mBaseAddresses[Idx].IoMemBase,
> +                           &RootBridges[Idx]);
>    }
>  
>    return EFI_SUCCESS;
> -- 
> 2.11.0
>