From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 A8C151A1E31 for ; Mon, 19 Sep 2016 19:02:22 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 19 Sep 2016 19:02:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,365,1470726000"; d="scan'208";a="1053603883" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 19 Sep 2016 19:02:22 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Sep 2016 19:02:21 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Sep 2016 19:02:21 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.118]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.234]) with mapi id 14.03.0248.002; Tue, 20 Sep 2016 10:01:42 +0800 From: "Tian, Feng" To: Leif Lindholm , "Kinney, Michael D" CC: "edk2-devel@lists.01.org" , "Zeng, Star" , "Tian, Feng" Thread-Topic: [edk2] [patch] MdeModulePkg/Xhci: add 1ms delay before access MMIO reg during reset Thread-Index: AQHSDiijBkxkvbxojUeAsAoIvhne5KB4b8OAgABEqoCAAR6RAIAH1BNA Date: Tue, 20 Sep 2016 02:01:41 +0000 Message-ID: <7F1BAD85ADEA444D97065A60D2E97EE566E1C20B@SHSMSX101.ccr.corp.intel.com> References: <5a0c21d0fe889950fda9e59aaf2272b66d429b8f.1473816993.git.feng.tian@intel.com> <20160914130833.GB16080@bivouac.eciton.net> <20160915101958.GG16080@bivouac.eciton.net> In-Reply-To: <20160915101958.GG16080@bivouac.eciton.net> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [patch] MdeModulePkg/Xhci: add 1ms delay before access MMIO reg during reset 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: Tue, 20 Sep 2016 02:02:22 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leif & Mike I will help to submit another tracker to Bugzilla to track this request. 1. create below new clear time definitions 2. remove those separate definitions in modules to get code clean. Thanks Feng -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif= Lindholm Sent: Thursday, September 15, 2016 6:20 PM To: Kinney, Michael D Cc: Tian, Feng ; edk2-devel@lists.01.org; Zeng, Star <= star.zeng@intel.com> Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: add 1ms delay before access = MMIO reg during reset On Wed, Sep 14, 2016 at 05:14:19PM +0000, Kinney, Michael D wrote: > MdePkg/Include/Library/UefiLib.h does have some helper macros for=20 > setting timer events periods that are in 100 nS units: >=20 > #define EFI_TIMER_PERIOD_MICROSECONDS(Microseconds) MultU64x32((UINT64)= (Microseconds), 10) > #define EFI_TIMER_PERIOD_MILLISECONDS(Milliseconds) MultU64x32((UINT64)= (Milliseconds), 10000) > #define EFI_TIMER_PERIOD_SECONDS(Seconds) MultU64x32((UINT64)= (Seconds), 10000000) >=20 > I believe the examples you show are for use with the gBS->Stall()=20 > service which is in 1 uS units. Correct. > Maybe we should consider some additional macros in UefiLib.h >=20 > #define EFI_STALL_PERIOD_MICROSECONDS(Microseconds) (Microseconds) > #define EFI_STALL_PERIOD_MILLISECONDS(Milliseconds) ((Milliseconds) * 1= 000) > #define EFI_STALL_PERIOD_SECONDS(Seconds) ((Seconds) * 100000= 0) >=20 > Or maybe some macros that actually do the call to gBS->Stall() too. >=20 > #define EFI_STALL_MICROSECONDS(Microseconds) gBS->Stall (Microseconds) > #define EFI_STALL_MILLISECONDS(Milliseconds) gBS->Stall ((Milliseconds)= * 1000) > #define EFI_STALL_SECONDS(Seconds) gBS->Stall ((Seconds) * 10= 00000) >=20 Either (or both) of those two look good to me. The latter has the benefit o= f a smaller call site, at the cost of perhaps obscuring the dependency on U= efiRuntimeServicesTableLib. > The other method of generating timed delays for PEI/DXE/SMM modules is=20 > using the Services in MdePkg/Include/Library/TimerLib.h: >=20 > UINTN > EFIAPI > NanoSecondDelay ( > IN UINTN NanoSeconds > ); >=20 > UINTN > EFIAPI > MicroSecondDelay ( > IN UINTN MicroSeconds > ); >=20 > If we wanted macros helper to use these services to match UEFI ones,=20 > maybe add the following to TimerLib.h: >=20 > #define DELAY_NANOSECONDS(Nanoseconds) NanoSecondDelay (Nanoseconds) > #define DELAY_MICROSECONDS(Microseconds) MicroSecondDelay (Microseconds= ) > #define DELAY_MILLISECONDS(Milliseconds) MicroSecondDelay ((Microsecond= s) * 1000) > #define DELAY_SECONDS(Seconds) MicroSecondDelay ((Microsecond= s) * 1000000)=20 I'm less concerned about those, but it could make sense for completeness. > Do you think it would improve the maintenance of the code if macros=20 > like these were used consistently? It would certainly be good to reduce duplication, and consistency would hel= p with the readability of the code. (Which is good for reviewing.) Regards, Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel