Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

2016-09-28 Thread Zeng, Star 
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 ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after 
reading by IoRead32()

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, September 28, 2016 6:18 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen 
> ; Gao, Liming 
> 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 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: 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(-)
> 
> 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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights 
> +reserved.
>  #  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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights 
> +reserved.
>  #  This program and the

Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

2016-09-28 Thread Zeng, Star 
Laszlo,

I agree that is a better idea, please check the V2 patch.

Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, September 28, 2016 6:51 PM
To: Zeng, Star <star.z...@intel.com>
Cc: edk2-de...@ml01.01.org; Yao, Jiewen <jiewen@intel.com>; Gao, Liming 
<liming@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@intel.com>
> Cc: Liming Gao <liming@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.z...@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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights 
> +reserved.
>  #  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.
> +#  counte

Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

2016-09-28 Thread Yao, Jiewen 
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, September 28, 2016 6:18 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Gao, Liming 
> 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 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: 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(-)
> 
> 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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>  #  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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>  #  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
> -

Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

2016-09-28 Thread Laszlo Ersek 
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 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: 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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>  #  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.
> +#  Note: The library only supports 24Bits ACPI timer.
> +#
> +#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>  #  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
> -

[edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

2016-09-28 Thread Star Zeng 
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 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: 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(-)

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.
+#  Note: The library only supports 24Bits ACPI timer.
+#
+#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
 #  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.
+#  Note: The library only supports 24Bits ACPI timer.
+#
+#  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
 #  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

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel