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

["Zeng, Star" <star.zeng@intel.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