diff mbox series

[edk2] ArmPlatformPkg/PL031RealTimeClockLib: drop ArmPlatformSysConfigLib reference

Message ID 20171115130645.31738-1-ard.biesheuvel@linaro.org
State Superseded
Headers show
Series [edk2] ArmPlatformPkg/PL031RealTimeClockLib: drop ArmPlatformSysConfigLib reference | expand

Commit Message

Ard Biesheuvel Nov. 15, 2017, 1:06 p.m. UTC
The PL031 driver implements a VExpress/Juno specific hack to set the
battery backed clock in addition to the PL031. However, none of the
remaining VExpress based hardware we support in EDK2 actuall implements
this feature so we can just remove it, and get rid of the cumbersome
dependency on ArmPlatform.h.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 43 +++-----------------
 1 file changed, 6 insertions(+), 37 deletions(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Nov. 16, 2017, 4:19 p.m. UTC | #1
On Wed, Nov 15, 2017 at 01:06:45PM +0000, Ard Biesheuvel wrote:
> The PL031 driver implements a VExpress/Juno specific hack to set the

> battery backed clock in addition to the PL031. However, none of the

> remaining VExpress based hardware we support in EDK2 actuall implements

> this feature so we can just remove it, and get rid of the cumbersome

> dependency on ArmPlatform.h.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Yes, this is an improvement.

Can you also delete the ArmPlatformSysConfigLib dependency in .inf as
part of this patch? If you can, and fold that in:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


/
    Leif

> ---

>  ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 43 +++-----------------

>  1 file changed, 6 insertions(+), 37 deletions(-)

> 

> diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c

> index 459dcc0a0519..1334ad446cd9 100644

> --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c

> +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c

> @@ -23,7 +23,6 @@

>  #include <Library/RealTimeClockLib.h>

>  #include <Library/MemoryAllocationLib.h>

>  #include <Library/PcdLib.h>

> -#include <Library/ArmPlatformSysConfigLib.h>

>  #include <Library/DxeServicesTableLib.h>

>  #include <Library/UefiBootServicesTableLib.h>

>  #include <Library/UefiRuntimeServicesTableLib.h>

> @@ -38,8 +37,6 @@

>  

>  #include <Library/TimeBaseLib.h>

>  

> -#include <ArmPlatform.h>

> -

>  STATIC BOOLEAN                mPL031Initialized = FALSE;

>  STATIC EFI_EVENT              mRtcVirtualAddrChangeEvent;

>  STATIC UINTN                  mPL031RtcBase;

> @@ -133,6 +130,11 @@ LibGetTime (

>    EFI_STATUS  Status = EFI_SUCCESS;

>    UINT32      EpochSeconds;

>  

> +  // Ensure Time is a valid pointer

> +  if (Time == NULL) {

> +    return EFI_INVALID_PARAMETER;

> +  }

> +

>    // Initialize the hardware if not already done

>    if (!mPL031Initialized) {

>      Status = InitializePL031 ();

> @@ -141,27 +143,7 @@ LibGetTime (

>      }

>    }

>  

> -  // Snapshot the time as early in the function call as possible

> -  // On some platforms we may have access to a battery backed up hardware clock.

> -  // If such RTC exists try to use it first.

> -  Status = ArmPlatformSysConfigGet (SYS_CFG_RTC, &EpochSeconds);

> -  if (Status == EFI_UNSUPPORTED) {

> -    // Battery backed up hardware RTC does not exist, revert to PL031

> -    EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER);

> -    Status = EFI_SUCCESS;

> -  } else if (EFI_ERROR (Status)) {

> -    // Battery backed up hardware RTC exists but could not be read due to error. Abort.

> -    return Status;

> -  } else {

> -    // Battery backed up hardware RTC exists and we read the time correctly from it.

> -    // Now sync the PL031 to the new time.

> -    MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);

> -  }

> -

> -  // Ensure Time is a valid pointer

> -  if (Time == NULL) {

> -    return EFI_INVALID_PARAMETER;

> -  }

> +  EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER);

>  

>    // Adjust for the correct time zone

>    // The timezone setting also reflects the DST setting of the clock

> @@ -235,19 +217,6 @@ LibSetTime (

>      EpochSeconds -= SEC_PER_HOUR;

>    }

>  

> -  // On some platforms we may have access to a battery backed up hardware clock.

> -  //

> -  // If such RTC exists then it must be updated first, before the PL031,

> -  // to minimise any time drift. This is important because the battery backed-up

> -  // RTC maintains the master time for the platform across reboots.

> -  //

> -  // If such RTC does not exist then the following function returns UNSUPPORTED.

> -  Status = ArmPlatformSysConfigSet (SYS_CFG_RTC, EpochSeconds);

> -  if ((EFI_ERROR (Status)) && (Status != EFI_UNSUPPORTED)){

> -    // Any status message except SUCCESS and UNSUPPORTED indicates a hardware failure.

> -    return Status;

> -  }

> -

>    // Set the PL031

>    MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);

>  

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
index 459dcc0a0519..1334ad446cd9 100644
--- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
+++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
@@ -23,7 +23,6 @@ 
 #include <Library/RealTimeClockLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
-#include <Library/ArmPlatformSysConfigLib.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
@@ -38,8 +37,6 @@ 
 
 #include <Library/TimeBaseLib.h>
 
-#include <ArmPlatform.h>
-
 STATIC BOOLEAN                mPL031Initialized = FALSE;
 STATIC EFI_EVENT              mRtcVirtualAddrChangeEvent;
 STATIC UINTN                  mPL031RtcBase;
@@ -133,6 +130,11 @@  LibGetTime (
   EFI_STATUS  Status = EFI_SUCCESS;
   UINT32      EpochSeconds;
 
+  // Ensure Time is a valid pointer
+  if (Time == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   // Initialize the hardware if not already done
   if (!mPL031Initialized) {
     Status = InitializePL031 ();
@@ -141,27 +143,7 @@  LibGetTime (
     }
   }
 
-  // Snapshot the time as early in the function call as possible
-  // On some platforms we may have access to a battery backed up hardware clock.
-  // If such RTC exists try to use it first.
-  Status = ArmPlatformSysConfigGet (SYS_CFG_RTC, &EpochSeconds);
-  if (Status == EFI_UNSUPPORTED) {
-    // Battery backed up hardware RTC does not exist, revert to PL031
-    EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER);
-    Status = EFI_SUCCESS;
-  } else if (EFI_ERROR (Status)) {
-    // Battery backed up hardware RTC exists but could not be read due to error. Abort.
-    return Status;
-  } else {
-    // Battery backed up hardware RTC exists and we read the time correctly from it.
-    // Now sync the PL031 to the new time.
-    MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);
-  }
-
-  // Ensure Time is a valid pointer
-  if (Time == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
+  EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER);
 
   // Adjust for the correct time zone
   // The timezone setting also reflects the DST setting of the clock
@@ -235,19 +217,6 @@  LibSetTime (
     EpochSeconds -= SEC_PER_HOUR;
   }
 
-  // On some platforms we may have access to a battery backed up hardware clock.
-  //
-  // If such RTC exists then it must be updated first, before the PL031,
-  // to minimise any time drift. This is important because the battery backed-up
-  // RTC maintains the master time for the platform across reboots.
-  //
-  // If such RTC does not exist then the following function returns UNSUPPORTED.
-  Status = ArmPlatformSysConfigSet (SYS_CFG_RTC, EpochSeconds);
-  if ((EFI_ERROR (Status)) && (Status != EFI_UNSUPPORTED)){
-    // Any status message except SUCCESS and UNSUPPORTED indicates a hardware failure.
-    return Status;
-  }
-
   // Set the PL031
   MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);