* [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
@ 2016-09-28 10:17 Star Zeng
2016-09-28 10:51 ` Laszlo Ersek
2016-09-28 11:58 ` Yao, Jiewen
0 siblings, 2 replies; 5+ messages in thread
From: Star Zeng @ 2016-09-28 10:17 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Liming Gao
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@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
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(-)
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
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
2016-09-28 10:17 [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
@ 2016-09-28 10:51 ` Laszlo Ersek
2016-09-28 12:50 ` Zeng, Star
2016-09-28 11:58 ` Yao, Jiewen
1 sibling, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2016-09-28 10:51 UTC (permalink / raw)
To: Star Zeng; +Cc: edk2-devel, Jiewen Yao, Liming Gao
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@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
2016-09-28 10:17 [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
2016-09-28 10:51 ` Laszlo Ersek
@ 2016-09-28 11:58 ` Yao, Jiewen
2016-09-28 12:52 ` Zeng, Star
1 sibling, 1 reply; 5+ messages in thread
From: Yao, Jiewen @ 2016-09-28 11:58 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Gao, Liming
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 28, 2016 6:18 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after
> reading by IoRead32()
>
> 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@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> 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(-)
>
> 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
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
2016-09-28 10:51 ` Laszlo Ersek
@ 2016-09-28 12:50 ` Zeng, Star
0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2016-09-28 12:50 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@ml01.01.org, Yao, Jiewen, Gao, Liming
Laszlo,
I agree that is a better idea, please check the V2 patch.
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, September 28, 2016 6:51 PM
To: Zeng, Star <star.zeng@intel.com>
Cc: edk2-devel@ml01.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
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@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
2016-09-28 11:58 ` Yao, Jiewen
@ 2016-09-28 12:52 ` Zeng, Star
0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2016-09-28 12:52 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org
Cc: Gao, Liming, Laszlo Ersek (lersek@redhat.com)
Jiewen,
Thanks for the Rb, I just send the V2 patch that follows Laszlo's suggestion to use IoBitFieldRead32 (Port, 0, 23), could you help review the patch again?
Star
-----Original Message-----
From: Yao, Jiewen
Sent: Wednesday, September 28, 2016 7:59 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 28, 2016 6:18 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after
> reading by IoRead32()
>
> 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@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> 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(-)
>
> 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
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-28 12:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-28 10:17 [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
2016-09-28 10:51 ` Laszlo Ersek
2016-09-28 12:50 ` Zeng, Star
2016-09-28 11:58 ` Yao, Jiewen
2016-09-28 12:52 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox