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

[Laszlo Ersek <lersek@redhat.com>]

On 10/09/16 02:58, Star Zeng wrote:
> From: "Zeng, Star" <star.zeng@intel.com>
> 
> Clear bits [31:24] after reading ACPI timer count by IoRead32(), and also add
> comments "Note: The implementation uses the lower 24-bits of the ACPI timer
> and is compatible with both 24-bit and 32-bit ACPI timers." in INF.
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.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 | 7 +++++--
>  PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf  | 9 ++++++---
>  3 files changed, 15 insertions(+), 9 deletions(-)

If I understand correctly, from v2 to v3, only the comment changed, in
the INF file and in the commit message. If that's the case,

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> index 020031e3f4a5..792781a33f3f 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 = IoBitFieldRead32 (Port, 0, 23) + 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 - IoBitFieldRead32 (Port, 0, 23)) & 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 = IoBitFieldRead32 (TimerAddr, 0, 23) + 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 - IoBitFieldRead32 (TimerAddr, 0, 23)) & 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..0113b26d366d 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> @@ -2,9 +2,12 @@
>  #  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 implementation uses the lower 24-bits of the ACPI timer and
> +#  is compatible with both 24-bit and 32-bit ACPI timers.
> +#
> +#  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..2c1cc7d33cdb 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> @@ -2,9 +2,12 @@
>  #  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 implementation uses the lower 24-bits of the ACPI timer and
> +#  is compatible with both 24-bit and 32-bit ACPI timers.
> +#
> +#  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 +52,4 @@ [Pcd]
>    gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBarRegisterOffset     ## CONSUMES
>    gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddress          ## CONSUMES
>    gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiPm1TmrOffset               ## CONSUMES
> -  gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask      ## CONSUMES
> \ No newline at end of file
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask      ## CONSUMES
>