-----Original Message-----
From: Laszlo Ersek [mailto:lersek@...]
Sent: Wednesday, September 28, 2016 6:51 PM
To: Zeng, Star <star.zeng@...>
Cc: edk2-devel@...; Yao, Jiewen <jiewen.yao@...>; Gao, Liming <liming.gao@...>
Subject: Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

Star,

On 09/28/16 12:17, Star Zeng wrote:

Clear bits [31:24] when reading ACPI timer count by IoRead32(), also
add comments "Note: The library only supports 24Bits ACPI timer" in INF.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Liming Gao <liming.gao@...>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@...>
---
PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 8 ++++----
PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf | 6 ++++--
PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf | 8 +++++---
3 files changed, 13 insertions(+), 9 deletions(-)

Just a question: would the IoBitFieldRead32() function apply in this case? If it is applicable, then I think it could make the code easier to
read:

IoBitFieldRead32 (Port, 0, 23)

Just an idea.

Thanks
Laszlo

diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
index 020031e3f4a5..67f165ed20ac 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
@@ -162,14 +162,14 @@ InternalAcpiDelay (
//
// The target timer count is calculated here
//
- Ticks = IoRead32 (Port) + Delay;
+ Ticks = (IoRead32 (Port) & (BIT24 - 1)) + Delay;
Delay = BIT22;
//
// Wait until time out
// Delay >= 2^23 could not be handled by this function
// Timer wrap-arounds are handled correctly by this function
//
- while (((Ticks - IoRead32 (Port)) & BIT23) == 0) {
+ while (((Ticks - (IoRead32 (Port) & (BIT24 - 1))) & BIT23) == 0)
+ {
CpuPause ();
}
} while (Times-- > 0);
@@ -371,7 +371,7 @@ InternalCalculateTscFrequency (
// Use 363 * 9861 = 3579543 Hz which is within 2 Hz of ACPI_TIMER_FREQUENCY.
// 363 counts is a calibration time of 101.4 uS.
//
- Ticks = IoRead32 (TimerAddr) + 363;
+ Ticks = (IoRead32 (TimerAddr) & (BIT24 - 1)) + 363;

StartTSC = AsmReadTsc (); // Get base value for the TSC
//
@@ -380,7 +380,7 @@ InternalCalculateTscFrequency (
// When the current ACPI timer value is greater than 'Ticks',
// the while loop will exit.
//
- while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
+ while (((Ticks - (IoRead32 (TimerAddr) & (BIT24 - 1))) & BIT23) ==
+ 0) {
CpuPause();
}
EndTSC = AsmReadTsc (); // TSC value 101.4 us later
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
index 48caebff1354..913fbfe7d30e 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
@@ -2,9 +2,11 @@
# Base ACPI Timer Library
#
# Provides basic timer support using the ACPI timer hardware. The
performance -# counter features are provided by the processors time stamp counter.
+# counter features are provided by the processors time stamp counter.
#
-# Copyright (c) 2013 - 2015, Intel Corporation. All rights
reserved.<BR>
+# Note: The library only supports 24Bits ACPI timer.
+#
+# Copyright (c) 2013 - 2016, Intel Corporation. All rights
+reserved.<BR>
# This program and the accompanying materials # are licensed and
made available under the terms and conditions of the BSD License #
which accompanies this distribution. The full text of the license may
be found at diff --git
a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
index 3446c03eda21..33d4970afc57 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
@@ -2,9 +2,11 @@
# DXE ACPI Timer Library
#
# Provides basic timer support using the ACPI timer hardware. The
performance -# counter features are provided by the processors time stamp counter.
+# counter features are provided by the processors time stamp counter.
#
-# Copyright (c) 2013 - 2015, Intel Corporation. All rights
reserved.<BR>
+# Note: The library only supports 24Bits ACPI timer.
+#
+# Copyright (c) 2013 - 2016, Intel Corporation. All rights
+reserved.<BR>
# This program and the accompanying materials # are licensed and
made available under the terms and conditions of the BSD License #
which accompanies this distribution. The full text of the license may
be found at @@ -49,4 +51,4 @@ [Pcd]
gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBarRegisterOffset ## CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddress ## CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiPm1TmrOffset ## CONSUMES
- gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask ## CONSUMES
\ No newline at end of file
+ gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask ## CONSUMES