public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
@ 2016-09-28 12:48 Star Zeng
  2016-09-28 12:55 ` Laszlo Ersek
  2016-09-28 14:58 ` Kinney, Michael D
  0 siblings, 2 replies; 6+ messages in thread
From: Star Zeng @ 2016-09-28 12:48 UTC (permalink / raw)
  To: edk2-devel; +Cc: Zeng, Star, Jiewen Yao, Liming Gao, Laszlo Ersek

From: "Zeng, Star" <star.zeng@intel.com>

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>
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 | 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..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..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.8.1.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
  2016-09-28 12:48 [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
@ 2016-09-28 12:55 ` Laszlo Ersek
  2016-09-28 14:58 ` Kinney, Michael D
  1 sibling, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2016-09-28 12:55 UTC (permalink / raw)
  To: Star Zeng, edk2-devel; +Cc: Jiewen Yao, Liming Gao

On 09/28/16 14:48, Star Zeng wrote:
> From: "Zeng, Star" <star.zeng@intel.com>
> 
> 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>
> 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 | 6 ++++--
>  PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf  | 8 +++++---
>  3 files changed, 13 insertions(+), 9 deletions(-)

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

Thanks!
Laszlo



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
  2016-09-28 12:48 [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
  2016-09-28 12:55 ` Laszlo Ersek
@ 2016-09-28 14:58 ` Kinney, Michael D
  2016-10-08  6:39   ` Zeng, Star
  1 sibling, 1 reply; 6+ messages in thread
From: Kinney, Michael D @ 2016-09-28 14:58 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: Laszlo Ersek, Yao, Jiewen, Gao, Liming, Zeng, Star

Star,

I think the comment should be updated.  This updated algorithm only uses
the lower 24-bits of the ACPI timer.  It is compatible with both 24-bit 
and 32-bit ACPI timers.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star Zeng
> Sent: Wednesday, September 28, 2016 5:49 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after
> reading by IoRead32()
> 
> From: "Zeng, Star" <star.zeng@intel.com>
> 
> 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>
> 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 | 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..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..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.8.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
  2016-09-28 14:58 ` Kinney, Michael D
@ 2016-10-08  6:39   ` Zeng, Star
  2016-10-08 19:49     ` Kinney, Michael D
  0 siblings, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2016-10-08  6:39 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org
  Cc: Laszlo Ersek, Yao, Jiewen, Gao, Liming, Zeng, Star

Mike,

Just get your point, that means the [31:24] will be always ignored regardless of the ACPI timer is 24Bits or 32Bits.

How about updating the comments like below?

"Note: The implementation only uses the lower 24-bits of the ACPI timer and can be compatible with both 24-bits and 32-bits ACPI timers."


Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Wednesday, September 28, 2016 10:58 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

Star,

I think the comment should be updated.  This updated algorithm only uses the lower 24-bits of the ACPI timer.  It is compatible with both 24-bit and 32-bit ACPI timers.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Star Zeng
> Sent: Wednesday, September 28, 2016 5:49 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits 
> [31:24] after reading by IoRead32()
> 
> From: "Zeng, Star" <star.zeng@intel.com>
> 
> 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>
> 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 | 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..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..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.8.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
  2016-10-08  6:39   ` Zeng, Star
@ 2016-10-08 19:49     ` Kinney, Michael D
  2016-10-09  1:07       ` Zeng, Star
  0 siblings, 1 reply; 6+ messages in thread
From: Kinney, Michael D @ 2016-10-08 19:49 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: Laszlo Ersek, Yao, Jiewen, Gao, Liming

Star,

Minor change:

"Note: The implementation uses the lower 24-bits of the ACPI timer and is compatible with both 
24-bit and 32-bit ACPI timers."

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, October 7, 2016 11:40 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after
> reading by IoRead32()
> 
> Mike,
> 
> Just get your point, that means the [31:24] will be always ignored regardless of the
> ACPI timer is 24Bits or 32Bits.
> 
> How about updating the comments like below?
> 
> "Note: The implementation only uses the lower 24-bits of the ACPI timer and can be
> compatible with both 24-bits and 32-bits ACPI timers."
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, September 28, 2016 10:58 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after
> reading by IoRead32()
> 
> Star,
> 
> I think the comment should be updated.  This updated algorithm only uses the lower
> 24-bits of the ACPI timer.  It is compatible with both 24-bit and 32-bit ACPI timers.
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Star Zeng
> > Sent: Wednesday, September 28, 2016 5:49 AM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star
> > <star.zeng@intel.com>
> > Subject: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits
> > [31:24] after reading by IoRead32()
> >
> > From: "Zeng, Star" <star.zeng@intel.com>
> >
> > 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>
> > 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 | 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..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..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.8.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
  2016-10-08 19:49     ` Kinney, Michael D
@ 2016-10-09  1:07       ` Zeng, Star
  0 siblings, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2016-10-09  1:07 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org
  Cc: Laszlo Ersek, Yao, Jiewen, Gao, Liming, Zeng, Star

Follow the suggestion, the V3 patch has been just sent out, please help review that.

Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Sunday, October 9, 2016 3:49 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()

Star,

Minor change:

"Note: The implementation uses the lower 24-bits of the ACPI timer and is compatible with both 24-bit and 32-bit ACPI timers."

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, October 7, 2016 11:40 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits 
> [31:24] after reading by IoRead32()
> 
> Mike,
> 
> Just get your point, that means the [31:24] will be always ignored 
> regardless of the ACPI timer is 24Bits or 32Bits.
> 
> How about updating the comments like below?
> 
> "Note: The implementation only uses the lower 24-bits of the ACPI 
> timer and can be compatible with both 24-bits and 32-bits ACPI timers."
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, September 28, 2016 10:58 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, 
> Michael D <michael.d.kinney@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits 
> [31:24] after reading by IoRead32()
> 
> Star,
> 
> I think the comment should be updated.  This updated algorithm only 
> uses the lower 24-bits of the ACPI timer.  It is compatible with both 24-bit and 32-bit ACPI timers.
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf 
> > Of Star Zeng
> > Sent: Wednesday, September 28, 2016 5:49 AM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen 
> > <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, 
> > Star <star.zeng@intel.com>
> > Subject: [edk2] [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits 
> > [31:24] after reading by IoRead32()
> >
> > From: "Zeng, Star" <star.zeng@intel.com>
> >
> > 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>
> > 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 | 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..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..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.8.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-09  1:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-28 12:48 [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() Star Zeng
2016-09-28 12:55 ` Laszlo Ersek
2016-09-28 14:58 ` Kinney, Michael D
2016-10-08  6:39   ` Zeng, Star
2016-10-08 19:49     ` Kinney, Michael D
2016-10-09  1:07       ` Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox