* [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