From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web12.6052.1585286122013976126 for ; Thu, 26 Mar 2020 22:15:22 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: guomin.jiang@intel.com) IronPort-SDR: IHoE4DLbtC3AAZofsU5EvWFc1IXrjXC+tNsbLuniEKm1NFS2N4wULaxSN8GMZS0Bt3n4hTUjLs n1BAbsh/aY8Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 22:15:21 -0700 IronPort-SDR: VkNxBwfLoduvvsWqWUPA00BgLYkofHW+Rtak86WZyH7JrNgXVMeNgjTfLFBjP8kzEj+mHx9pil 7lf9ZmN1qwNA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,311,1580803200"; d="scan'208";a="420974964" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 26 Mar 2020 22:15:21 -0700 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 26 Mar 2020 22:15:20 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 26 Mar 2020 22:15:20 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.43]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.155]) with mapi id 14.03.0439.000; Fri, 27 Mar 2020 13:15:16 +0800 From: "Guomin Jiang" To: "devel@edk2.groups.io" , "kaiyau@gmail.com" CC: "Yau, KaiX" Subject: Re: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP Thread-Topic: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR displayed in SETUP Thread-Index: AQHWAMIOvWjS1stLsk+xYc4JXA8RqKhb5M5g Date: Fri, 27 Mar 2020 05:15:15 +0000 Message-ID: References: <50102ef8b38c0e5287ab4d1cb8fc636213f67299.1584672719.git.kaix.yau@intel.com> In-Reply-To: <50102ef8b38c0e5287ab4d1cb8fc636213f67299.1584672719.git.kaix.yau@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: guomin.jiang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Add commets in your original message > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Yau > Sent: Friday, March 20, 2020 11:42 AM > To: devel@edk2.groups.io > Cc: Yau, KaiX > Subject: [edk2-devel] [PATCH] PcAtChipsetPkg: Wrong System YEAR > displayed in SETUP >=20 > Function ConvertRtcTimeToEfiTime() will reset to Century 19XX when the > year is set larger than the value of PcdMaximalValidYear outside SETUP. >=20 > Signed-off-by: Yau > --- > PcAtChipsetPkg/PcAtChipsetPkg.dec | 7 +++--- > .../PcatRealTimeClockRuntimeDxe/PcRtc.c | 25 +++++++++++-------- > .../PcatRealTimeClockRuntimeDxe/PcRtc.h | 17 +++++++------ > 3 files changed, 27 insertions(+), 22 deletions(-) >=20 > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec > b/PcAtChipsetPkg/PcAtChipsetPkg.dec > index 88de5cceea..660cb5d52e 100644 > --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec > @@ -4,7 +4,7 @@ > # This package is designed to public interfaces and implementation whic= h > follows # PcAt defacto standard. > # > -# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. > +# Copyright (c) 2009 - 2020, Intel Corporation. All rights > +reserved.
> # Copyright (c) 2017, AMD Inc. All rights reserved.
# # SPDX-Lice= nse- > Identifier: BSD-2-Clause-Patent @@ -61,12 +61,13 @@ >=20 > ## This PCD specifies the minimal valid year in RTC. > # @Prompt Minimal valid year in RTC. > - > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|1998|UINT16|0x000 > 0000D > + # @Expression 0x80000001 | > + gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear >=3D 2000 Why add above comment in herein? I can get the information from below code= , it is redundant information. > + > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear|2000|UINT16|0x000 > 000 > + 0D >=20 > ## This PCD specifies the maximal valid year in RTC. > # @Prompt Maximal valid year in RTC. > # @Expression 0x80000001 | > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 > - > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000 > 0000E > + > + > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x000 > 000 > + 0E >=20 > [PcdsFixedAtBuild, PcdsPatchableInModule] > ## Defines the ACPI register set base address. > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > index 52af179417..38a3521dfc 100644 > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > @@ -1,7 +1,7 @@ > /** @file > RTC Architectural Protocol GUID as defined in DxeCis 0.96. >=20 > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
> Copyright (c) 2017, AMD Inc. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -441,8 +441,8 @@ > PcRtcGetTime ( **/ EFI_STATUS PcRtcSetTime ( > - IN EFI_TIME *Time, > - IN PC_RTC_MODULE_GLOBALS *Global > + IN EFI_TIME *Time, > + IN OUT PC_RTC_MODULE_GLOBALS *Global > ) > { > EFI_STATUS Status; > @@ -525,6 +525,8 @@ PcRtcSetTime ( > // > if (Global->CenturyRtcAddress !=3D 0) { > RtcWrite (Global->CenturyRtcAddress, DecimalToBcd8 ((UINT8) > (RtcTime.Year / 100))); > + } else { > + DEBUG ((EFI_D_INFO, "PcRtc: Century RTC Address is not found\n")); It is simple so the message is unnecessary, please add debug message for c= ritical situation. > } >=20 > ConvertEfiTimeToRtcTime (&RtcTime, RegisterB); @@ -868,6 +870,9 @@ > ConvertRtcTimeToEfiTime ( { > BOOLEAN IsPM; > UINT8 Century; > + UINT8 CenturyRtcAddress; > + > + CenturyRtcAddress =3D GetCenturyRtcAddress (); >=20 > if ((Time->Hour & 0x80) !=3D 0) { > IsPM =3D TRUE; > @@ -891,14 +896,12 @@ ConvertRtcTimeToEfiTime ( > return EFI_INVALID_PARAMETER; > } >=20 > - // > - // For minimal/maximum year range [1970, 2069], > - // Century is 19 if RTC year >=3D 70, > - // Century is 20 otherwise. > - // > - Century =3D (UINT8) (PcdGet16 (PcdMinimalValidYear) / 100); > - if (Time->Year < PcdGet16 (PcdMinimalValidYear) % 100) { > - Century++; > + if (CenturyRtcAddress !=3D 0) { > + Century =3D CheckAndConvertBcd8ToDecimal8 ((UINT8) (RtcRead > + (CenturyRtcAddress))); } else if ((PcdGet16 (PcdMinimalValidYear) / 1= 00) > =3D=3D (PcdGet16 (PcdMaximalValidYear) / 100)) { In the practice, the PcdMinimalValidYear and PcdMaximalValidYear is consta= nt, so (PcdGet16(PcdMinimalValidYear))/100) =3D=3D (PcdGet16(PcdMaximalVali= dYear)/100) equal 2000/100 =3D=3D 2099/100 equal 20 =3D=3D 20, it is always= true and so this code is meaningless. > + Century =3D (UINT8)(PcdGet16 (PcdMinimalValidYear) / 100); } else = { Arrived here at no time according to the above comment. > + Century =3D RTC_INIT_CENTURY; > } > Time->Year =3D (UINT16) (Century * 100 + Time->Year); >=20 > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > index 47293ce44c..94926fe73e 100644 > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > @@ -1,7 +1,7 @@ > /** @file > Header file for real time clock driver. >=20 > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
> Copyright (c) 2017, AMD Inc. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent @@ -62,11 +62,12 @@ extern > PC_RTC_MODULE_GLOBALS mModuleGlobal; // Date and time initial > values. > // They are used if the RTC values are invalid during driver initializa= tion // - > #define RTC_INIT_SECOND 0 -#define RTC_INIT_MINUTE 0 > -#define RTC_INIT_HOUR 0 > -#define RTC_INIT_DAY 1 > -#define RTC_INIT_MONTH 1 > +#define RTC_INIT_SECOND 0 > +#define RTC_INIT_MINUTE 0 > +#define RTC_INIT_HOUR 0 > +#define RTC_INIT_DAY 1 > +#define RTC_INIT_MONTH 1 > +#define RTC_INIT_CENTURY 20 Please keep the original style, I see you add a space in original code, it= is unnecessary. >=20 > #pragma pack(1) > // > @@ -160,8 +161,8 @@ PcRtcInit ( > **/ > EFI_STATUS > PcRtcSetTime ( > - IN EFI_TIME *Time, > - IN PC_RTC_MODULE_GLOBALS *Global > + IN EFI_TIME *Time, > + IN OUT PC_RTC_MODULE_GLOBALS *Global > ); >=20 > /** > -- > 2.17.1.windows.2 >=20 >=20 >=20