[edk2,v3] Silicon/NXP/Pcf8563RealTimeClockLib: add Voltage-low handling

Message ID 20180620071549.2295-1-masahisa.kojima@linaro.org
State New
Headers show
Series
  • [edk2,v3] Silicon/NXP/Pcf8563RealTimeClockLib: add Voltage-low handling
Related show

Commit Message

Masahisa Kojima June 20, 2018, 7:15 a.m.
From: Masahisa Kojima <masahisa.kojima@linaro.org>


BcdToDecimal8() in LibGetTime() asserts with the
following condition.
 1) RTC device has not been initialized yet, RTC device
    returns indeterminate value with VL bit(1)
 2) DEBUG build

UEFI boot fails with assertion when it satisfies above conditions.

Aside form boot failure, UEFI shell commands "date/time" expect
that getting time from RTC should success when user sets the time.
ShellCommandRunTime() performs GetTime()->update time->SetTime(),
if the first GetTime() fails, user can not set time.
With that, simply returning EFI_DEVICE_ERROR in LibGetTime()
is not applicable to VL bit handling.

To avoid this situation, even if it only occurs in DEBUG build,
RTC driver should check the VL bit in the VL_seconds register.
This VL bit is voltage-low detector, it means integrity of the
clock information is not guaranteed if it sets to 1. In this
case, driver return dummy date/time(01/01/2000 00:00:00) to
proceed succeeding process.

linux driver also checks this bit when driver gets the time
from RTC. If VL bit is 1, linux driver discard the retreived
time data.

Note that if VL bit is 1, GetTime() returns always
01/01/2000 00:00:00 until user sets date/time.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>

---
 .../Pcf8563RealTimeClockLib.c                      | 28 +++++++++++++++-------
 1 file changed, 19 insertions(+), 9 deletions(-)

-- 
2.14.2

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

Comments

Ard Biesheuvel June 20, 2018, 10:51 a.m. | #1
On 20 June 2018 at 09:15,  <masahisa.kojima@linaro.org> wrote:
> From: Masahisa Kojima <masahisa.kojima@linaro.org>

>

> BcdToDecimal8() in LibGetTime() asserts with the

> following condition.

>  1) RTC device has not been initialized yet, RTC device

>     returns indeterminate value with VL bit(1)

>  2) DEBUG build

>

> UEFI boot fails with assertion when it satisfies above conditions.

>

> Aside form boot failure, UEFI shell commands "date/time" expect

> that getting time from RTC should success when user sets the time.

> ShellCommandRunTime() performs GetTime()->update time->SetTime(),

> if the first GetTime() fails, user can not set time.

> With that, simply returning EFI_DEVICE_ERROR in LibGetTime()

> is not applicable to VL bit handling.

>

> To avoid this situation, even if it only occurs in DEBUG build,

> RTC driver should check the VL bit in the VL_seconds register.

> This VL bit is voltage-low detector, it means integrity of the

> clock information is not guaranteed if it sets to 1. In this

> case, driver return dummy date/time(01/01/2000 00:00:00) to

> proceed succeeding process.

>

> linux driver also checks this bit when driver gets the time

> from RTC. If VL bit is 1, linux driver discard the retreived

> time data.

>

> Note that if VL bit is 1, GetTime() returns always

> 01/01/2000 00:00:00 until user sets date/time.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>


Thanks

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


Pushed as 070938aa96aa91d822de57897e39c747c56eb4de

> ---

>  .../Pcf8563RealTimeClockLib.c                      | 28 +++++++++++++++-------

>  1 file changed, 19 insertions(+), 9 deletions(-)

>

> diff --git a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c

> index fb58e1feb4..77d8998cfd 100644

> --- a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c

> +++ b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c

> @@ -24,6 +24,7 @@

>  #define SLAVE_ADDRESS             (FixedPcdGet8 (PcdI2cSlaveAddress))

>  #define PCF8563_DATA_REG_OFFSET   0x2

>

> +#define PCF8563_CLOCK_INVALID     0x80

>  #define PCF8563_SECONDS_MASK      0x7f

>  #define PCF8563_MINUTES_MASK      0x7f

>  #define PCF8563_HOURS_MASK        0x3f

> @@ -122,15 +123,24 @@ LibGetTime (

>      return EFI_DEVICE_ERROR;

>    }

>

> -  Time->Second  = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK);

> -  Time->Minute  = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK);

> -  Time->Hour    = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK);

> -  Time->Day     = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK);

> -  Time->Month   = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK);

> -  Time->Year    = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE;

> -

> -  if (DateTime.Century_months & PCF8563_CENTURY_MASK) {

> -    Time->Year += 100;

> +  if ((DateTime.VL_seconds & PCF8563_CLOCK_INVALID) != 0) {

> +      Time->Second  = 0;

> +      Time->Minute  = 0;

> +      Time->Hour    = 0;

> +      Time->Day     = 1;

> +      Time->Month   = 1;

> +      Time->Year    = EPOCH_BASE;

> +  } else {

> +      Time->Second  = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK);

> +      Time->Minute  = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK);

> +      Time->Hour    = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK);

> +      Time->Day     = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK);

> +      Time->Month   = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK);

> +      Time->Year    = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE;

> +

> +      if (DateTime.Century_months & PCF8563_CENTURY_MASK) {

> +          Time->Year += 100;

> +      }

>    }

>

>    if (Capabilities != NULL) {

> --

> 2.14.2

>

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

Patch

diff --git a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c
index fb58e1feb4..77d8998cfd 100644
--- a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c
+++ b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c
@@ -24,6 +24,7 @@ 
 #define SLAVE_ADDRESS             (FixedPcdGet8 (PcdI2cSlaveAddress))
 #define PCF8563_DATA_REG_OFFSET   0x2
 
+#define PCF8563_CLOCK_INVALID     0x80
 #define PCF8563_SECONDS_MASK      0x7f
 #define PCF8563_MINUTES_MASK      0x7f
 #define PCF8563_HOURS_MASK        0x3f
@@ -122,15 +123,24 @@  LibGetTime (
     return EFI_DEVICE_ERROR;
   }
 
-  Time->Second  = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK);
-  Time->Minute  = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK);
-  Time->Hour    = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK);
-  Time->Day     = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK);
-  Time->Month   = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK);
-  Time->Year    = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE;
-
-  if (DateTime.Century_months & PCF8563_CENTURY_MASK) {
-    Time->Year += 100;
+  if ((DateTime.VL_seconds & PCF8563_CLOCK_INVALID) != 0) {
+      Time->Second  = 0;
+      Time->Minute  = 0;
+      Time->Hour    = 0;
+      Time->Day     = 1;
+      Time->Month   = 1;
+      Time->Year    = EPOCH_BASE;
+  } else {
+      Time->Second  = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK);
+      Time->Minute  = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK);
+      Time->Hour    = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK);
+      Time->Day     = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK);
+      Time->Month   = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK);
+      Time->Year    = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE;
+
+      if (DateTime.Century_months & PCF8563_CENTURY_MASK) {
+          Time->Year += 100;
+      }
   }
 
   if (Capabilities != NULL) {