Re: [PATCH 1/3] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

[Laszlo Ersek <lersek@redhat.com>]

On 03/09/17 17:03, Ard Biesheuvel wrote:
> Disable the PL031 RTC DT node unconditionally rather than only when
> the DT will be exposed to the OS. This allows us to defer the decision
> whether to expose it to the OS to a later time without creating an
> additional dependency on the FDT client code by the RTC driver.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 +++++++++-----------
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 ---
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> index 82de7a51b32e..d168424a52f5 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> @@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
>  
>    DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -    //
> -    // UEFI takes ownership of the RTC hardware, and exposes its functionality
> -    // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> -    // need to disable it in the device tree to prevent the OS from attaching
> -    // its device driver as well.
> -    //
> -    Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> -                          "disabled", sizeof ("disabled"));
> -    if (EFI_ERROR (Status)) {
> -        DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
> -    }
> +  //
> +  // UEFI takes ownership of the RTC hardware, and exposes its functionality
> +  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> +  // need to disable it in the device tree to prevent the OS from attaching
> +  // its device driver as well.
> +  //
> +  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> +                        "disabled", sizeof ("disabled"));
> +  if (EFI_ERROR (Status)) {
> +      DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
>    }
>  
>    return EFI_SUCCESS;
> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> index 32dbff6f0852..342193651a86 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> @@ -42,8 +42,5 @@ [Protocols]
>  [Pcd]
>    gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>    gFdtClientProtocolGuid
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>