From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 84F9D1A1E31 for ; Mon, 10 Oct 2016 06:01:41 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D1B59C054907; Mon, 10 Oct 2016 13:01:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-14.phx2.redhat.com [10.3.116.14]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9AD1cLC004420; Mon, 10 Oct 2016 09:01:39 -0400 To: Star Zeng , edk2-devel@ml01.01.org References: <1475974734-19952-1-git-send-email-star.zeng@intel.com> Cc: Michael Kinney , Jiewen Yao , Liming Gao From: Laszlo Ersek Message-ID: <12dde0ec-7ad3-c68f-0949-50cbdebf9763@redhat.com> Date: Mon, 10 Oct 2016 15:01:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1475974734-19952-1-git-send-email-star.zeng@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 10 Oct 2016 13:01:40 +0000 (UTC) Subject: Re: [PATCH V3] PcAtChipsetPkg AcpiTimerLib: Clear bits [31:24] after reading by IoRead32() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2016 13:01:41 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 10/09/16 02:58, Star Zeng wrote: > From: "Zeng, Star" > > 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 > Cc: Jiewen Yao > Cc: Liming Gao > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng > --- > 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 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.
> +# 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.
> # 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.
> +# 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.
> # 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 >