From: "Kinney, Michael D" <michael.d.kinney@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
"edk2-devel@lists.01.org" <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: [PATCH V2] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32()
Date: Sat, 8 Oct 2016 19:49:04 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F564820830@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103956B159@shsmsx102.ccr.corp.intel.com>
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
next prev parent reply other threads:[~2016-10-08 19:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2016-10-09 1:07 ` Zeng, Star
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E92EE9817A31E24EB0585FDF735412F564820830@ORSMSX113.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox