Re: [PATCH v4 1/1] PcAtChipsetPkg: RTC Runtime unable to get correct IO port by PCD

["Guo, Gua" <gua.guo@intel.com>]

Thanks for quickly review also update reviewed-by info on the PR.
https://github.com/tianocore/edk2/pull/4416

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Wednesday, May 24, 2023 5:45 PM
To: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v4 1/1] PcAtChipsetPkg: RTC Runtime unable to get correct IO port by PCD

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

> -----Original Message-----
> From: Guo, Gua <gua.guo@intel.com>
> Sent: Wednesday, May 24, 2023 4:43 PM
> To: devel@edk2.groups.io
> Cc: Guo, Gua <gua.guo@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, 
> Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v4 1/1] PcAtChipsetPkg: RTC Runtime unable to get 
> correct IO port by PCD
> 
> From: Gua Guo <gua.guo@intel.com>
> 
> RTC runtime is unable to get dynamic PCD value after booting to OS 
> using runtime services.
> 
> Resolution: Cache the dynamic PCD value in RTC driver entry point
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
>  .../PcatRealTimeClockRuntimeDxe/PcRtc.c       | 36 ++++++++++---------
>  .../PcatRealTimeClockRuntimeDxe/PcRtcEntry.c  | 14 ++++++--
>  2 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index d8b9fa8376..ff1b019ce2 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -11,9 +11,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  #include "PcRtc.h"
> 
> 
> 
> -extern UINTN  mRtcIndexRegister;
> 
> -extern UINTN  mRtcTargetRegister;
> 
> -
> 
> +extern UINTN   mRtcIndexRegister;
> 
> +extern UINTN   mRtcTargetRegister;
> 
> +extern UINT16  mRtcDefaultYear;
> 
> +extern UINT16  mMinimalValidYear;
> 
> +extern UINT16  mMaximalValidYear;
> 
>  //
> 
>  // Days of month.
> 
>  //
> 
> @@ -72,10 +74,10 @@ IoRtcRead (
>    )
> 
>  {
> 
>    IoWrite8 (
> 
> -    PcdGet8 (PcdRtcIndexRegister),
> 
> -    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) &
> 0x80))
> 
> +    mRtcIndexRegister,
> 
> +    (UINT8)(Address | (UINT8)(IoRead8 (mRtcIndexRegister) & 0x80))
> 
>      );
> 
> -  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> 
> +  return IoRead8 (mRtcTargetRegister);
> 
>  }
> 
> 
> 
>  /**
> 
> @@ -94,10 +96,10 @@ IoRtcWrite (
>    )
> 
>  {
> 
>    IoWrite8 (
> 
> -    PcdGet8 (PcdRtcIndexRegister),
> 
> -    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) &
> 0x80))
> 
> +    mRtcIndexRegister,
> 
> +    (UINT8)(Address | (UINT8)(IoRead8 (mRtcIndexRegister) & 0x80))
> 
>      );
> 
> -  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> 
> +  IoWrite8 (mRtcTargetRegister, Data);
> 
>  }
> 
> 
> 
>  /**
> 
> @@ -317,8 +319,8 @@ PcRtcInit (
>      Time.Hour       = RTC_INIT_HOUR;
> 
>      Time.Day        = RTC_INIT_DAY;
> 
>      Time.Month      = RTC_INIT_MONTH;
> 
> -    Time.Year       = MAX (PcdGet16 (PcdRtcDefaultYear), PcdGet16
> (PcdMinimalValidYear));
> 
> -    Time.Year       = MIN (Time.Year, PcdGet16 (PcdMaximalValidYear));
> 
> +    Time.Year       = MAX (mRtcDefaultYear, mMinimalValidYear);
> 
> +    Time.Year       = MIN (Time.Year, mMaximalValidYear);
> 
>      Time.Nanosecond = 0;
> 
>      Time.TimeZone   = EFI_UNSPECIFIED_TIMEZONE;
> 
>      Time.Daylight   = 0;
> 
> @@ -358,8 +360,8 @@ PcRtcInit (
>    Time.Hour       = RTC_INIT_HOUR;
> 
>    Time.Day        = RTC_INIT_DAY;
> 
>    Time.Month      = RTC_INIT_MONTH;
> 
> -  Time.Year       = MAX (PcdGet16 (PcdRtcDefaultYear), PcdGet16
> (PcdMinimalValidYear));
> 
> -  Time.Year       = MIN (Time.Year, PcdGet16 (PcdMaximalValidYear));
> 
> +  Time.Year       = MAX (mRtcDefaultYear, mMinimalValidYear);
> 
> +  Time.Year       = MIN (Time.Year, mMaximalValidYear);
> 
>    Time.Nanosecond = 0;
> 
>    Time.TimeZone   = Global->SavedTimeZone;
> 
>    Time.Daylight   = Global->Daylight;
> 
> @@ -1031,8 +1033,8 @@ ConvertRtcTimeToEfiTime (
>    //   Century is 19 if RTC year >= 70,
> 
>    //   Century is 20 otherwise.
> 
>    //
> 
> -  Century = (UINT8)(PcdGet16 (PcdMinimalValidYear) / 100);
> 
> -  if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) {
> 
> +  Century = (UINT8)(mMinimalValidYear / 100);
> 
> +  if (Time->Year < mMinimalValidYear % 100) {
> 
>      Century++;
> 
>    }
> 
> 
> 
> @@ -1114,8 +1116,8 @@ RtcTimeFieldsValid (
>    IN EFI_TIME  *Time
> 
>    )
> 
>  {
> 
> -  if ((Time->Year < PcdGet16 (PcdMinimalValidYear)) ||
> 
> -      (Time->Year > PcdGet16 (PcdMaximalValidYear)) ||
> 
> +  if ((Time->Year < mMinimalValidYear) ||
> 
> +      (Time->Year > mMaximalValidYear) ||
> 
>        (Time->Month < 1) ||
> 
>        (Time->Month > 12) ||
> 
>        (!DayValid (Time)) ||
> 
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index e6e9458d75..b600df4879 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -16,8 +16,11 @@ EFI_HANDLE  mHandle = NULL;
> 
> 
>  STATIC EFI_EVENT  mVirtualAddrChangeEvent;
> 
> 
> 
> -UINTN  mRtcIndexRegister;
> 
> -UINTN  mRtcTargetRegister;
> 
> +UINTN   mRtcIndexRegister;
> 
> +UINTN   mRtcTargetRegister;
> 
> +UINT16  mRtcDefaultYear;
> 
> +UINT16  mMinimalValidYear;
> 
> +UINT16  mMaximalValidYear;
> 
> 
> 
>  /**
> 
>    Returns the current time and date information, and the time-keeping 
> capabilities
> 
> @@ -164,8 +167,15 @@ InitializePcRtc (
>    if (FeaturePcdGet (PcdRtcUseMmio)) {
> 
>      mRtcIndexRegister  = (UINTN)PcdGet64 (PcdRtcIndexRegister64);
> 
>      mRtcTargetRegister = (UINTN)PcdGet64 (PcdRtcTargetRegister64);
> 
> +  } else {
> 
> +    mRtcIndexRegister  = (UINTN)PcdGet8 (PcdRtcIndexRegister);
> 
> +    mRtcTargetRegister = (UINTN)PcdGet8 (PcdRtcTargetRegister);
> 
>    }
> 
> 
> 
> +  mRtcDefaultYear   = PcdGet16 (PcdRtcDefaultYear);
> 
> +  mMinimalValidYear = PcdGet16 (PcdMinimalValidYear);
> 
> +  mMaximalValidYear = PcdGet16 (PcdMaximalValidYear);
> 
> +
> 
>    Status = PcRtcInit (&mModuleGlobal);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> --
> 2.39.2.windows.1