Re: [PATCH v3 4/6] ArmPlatformPkg/ArmJunoDxe: don't register OnEndOfDxe event on rev R0

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

On Tue, Apr 04, 2017 at 01:30:08PM +0100, Ard Biesheuvel wrote:
> The ArmJunoDxe driver code registers a callback for the EndOfDxe event,
> at which time it does some manipulation of the PCI peripherals on the
> board. Given that R0 has no working PCIe, we can omit the registration
> of the callback altogether.

This is a nice simplification.
However, could the final sentence of the commit message be changed to

"
Given that R0 has no working PCIe, instead of conditionally
performing these operations, omit the registration of the
callback altogether on that platform.
"

With that:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 43 +++++++++-----------
>  1 file changed, 19 insertions(+), 24 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index 14ff189a3078..f7e33961b4e7 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -379,7 +379,6 @@ OnEndOfDxe (
>    EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
>    EFI_HANDLE                Handle;
>    EFI_STATUS                Status;
> -  UINT32                    JunoRevision;
>  
>    //
>    // PCI Root Complex initialization
> @@ -395,13 +394,9 @@ OnEndOfDxe (
>    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>    ASSERT_EFI_ERROR (Status);
>  
> -  GetJunoRevision (JunoRevision);
> -
> -  if (JunoRevision != JUNO_REVISION_R0) {
> -    Status = ArmJunoSetNicMacAddress ();
> -    if (EFI_ERROR (Status)) {
> -      DEBUG ((DEBUG_ERROR, "ArmJunoDxe: Failed to set Marvell Yukon NIC MAC address\n"));
> -    }
> +  Status = ArmJunoSetNicMacAddress ();
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "ArmJunoDxe: Failed to set Marvell Yukon NIC MAC address\n"));
>    }
>  }
>  
> @@ -511,22 +506,6 @@ ArmJunoEntryPoint (
>      }
>    }
>  
> -  //
> -  // Create an event belonging to the "gEfiEndOfDxeEventGroupGuid" group.
> -  // The "OnEndOfDxe()" function is declared as the call back function.
> -  // It will be called at the end of the DXE phase when an event of the
> -  // same group is signalled to inform about the end of the DXE phase.
> -  // Install the INSTALL_FDT_PROTOCOL protocol.
> -  //
> -  Status = gBS->CreateEventEx (
> -                  EVT_NOTIFY_SIGNAL,
> -                  TPL_CALLBACK,
> -                  OnEndOfDxe,
> -                  NULL,
> -                  &gEfiEndOfDxeEventGroupGuid,
> -                  &EndOfDxeEvent
> -                  );
> -
>    // Install dynamic Shell command to run baremetal binaries.
>    Status = ShellDynCmdRunAxfInstall (ImageHandle);
>    if (EFI_ERROR (Status)) {
> @@ -555,6 +534,22 @@ ArmJunoEntryPoint (
>      // Enable PCI enumeration
>      PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
>  
> +    //
> +    // Create an event belonging to the "gEfiEndOfDxeEventGroupGuid" group.
> +    // The "OnEndOfDxe()" function is declared as the call back function.
> +    // It will be called at the end of the DXE phase when an event of the
> +    // same group is signalled to inform about the end of the DXE phase.
> +    // Install the INSTALL_FDT_PROTOCOL protocol.
> +    //
> +    Status = gBS->CreateEventEx (
> +                    EVT_NOTIFY_SIGNAL,
> +                    TPL_CALLBACK,
> +                    OnEndOfDxe,
> +                    NULL,
> +                    &gEfiEndOfDxeEventGroupGuid,
> +                    &EndOfDxeEvent
> +                    );
> +
>      // Declare the related ACPI Tables
>      EfiCreateProtocolNotifyEvent (
>          &gEfiAcpiTableProtocolGuid,
> -- 
> 2.9.3
>