Re: [PATCH v3] PcAtChipsetPkg: Change the flow of PcRtcInit()

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Chao, Zhuoran <zhuoran.chao@intel.com>
> Sent: Saturday, May 7, 2022 5:40 PM
> To: devel@edk2.groups.io
> Cc: Chao, Zhuoran <zhuoran.chao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v3] PcAtChipsetPkg: Change the flow of PcRtcInit()
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3913
> 
> The original flow of PcRtcInit() is like:
> 1. Guarantee atomic accesses to the RTC time registers and
>    read out the value.
> 2. Program RTC register B. (adopt 12h mode or 24h mode. Current
>    bios code sets RTC to 24h mode by default).
> 3. Then function ConvertRtcTimeToEfiTime converts the RTC time
>    value to their 24h mode by checking the hour format bit
>    (1:24h mode,0:12h mode).
> And here lies the problem: Step3 will fail to adjust the value
> if Step2 already sets RTC to 24h mode. The hour value in 12h mode
> will not be converted to its 24h mode.
> The solution is to program RTC register B a little later when all
> the original RTC registers' value is retrieved, adjusted
> and validated.
> 
> ConvertRtcTimeToEfiTime is modified to be more robust.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> 
> Signed-off-by: Zhuoran Chao <zhuoran.chao@intel.com>
> ---
>  .../PcatRealTimeClockRuntimeDxe/PcRtc.c       | 29 ++++++++++---------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index 0fbfa4bcee..9242a2e826 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -269,13 +269,6 @@ PcRtcInit (
>    Time.Month  = RtcRead (RTC_ADDRESS_MONTH);
>    Time.Year   = RtcRead (RTC_ADDRESS_YEAR);
> 
> -  //
> -  // Set RTC configuration after get original time
> -  // The value of bit AIE should be reserved.
> -  //
> -  RegisterB.Data = FixedPcdGet8 (PcdInitialValueRtcRegisterB) | (RegisterB.Data & BIT5);
> -  RtcWrite (RTC_ADDRESS_REGISTER_B, RegisterB.Data);
> -
>    //
>    // Release RTC Lock.
>    //
> @@ -330,6 +323,13 @@ PcRtcInit (
>      Time.Daylight   = 0;
>    }
> 
> +  //
> +  // Set RTC configuration after get original time
> +  // The value of bit AIE should be reserved.
> +  //
> +  RegisterB.Data = FixedPcdGet8 (PcdInitialValueRtcRegisterB) | (RegisterB.Data & BIT5);
> +  RtcWrite (RTC_ADDRESS_REGISTER_B, RegisterB.Data);
> +
>    //
>    // Reset time value according to new RTC configuration
>    //
> @@ -995,13 +995,16 @@ ConvertRtcTimeToEfiTime (
>    BOOLEAN  IsPM;
>    UINT8    Century;
> 
> -  if ((Time->Hour & 0x80) != 0) {
> -    IsPM = TRUE;
> -  } else {
> -    IsPM = FALSE;
> -  }
> +  // IsPM only makes sense for 12-hour format.
> +  if (RegisterB.Bits.Mil == 0) {
> +    if ((Time->Hour & 0x80) != 0) {
> +      IsPM = TRUE;
> +    } else {
> +      IsPM = FALSE;
> +    }
> 
> -  Time->Hour = (UINT8)(Time->Hour & 0x7f);
> +    Time->Hour = (UINT8)(Time->Hour & 0x7f);
> +  }
> 
>    if (RegisterB.Bits.Dm == 0) {
>      Time->Year   = CheckAndConvertBcd8ToDecimal8 ((UINT8)Time->Year);
> --
> 2.31.1.windows.1