From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BE6691A1EEA for ; Thu, 15 Sep 2016 03:23:33 -0700 (PDT) Received: by mail-wm0-x22f.google.com with SMTP id b187so89005615wme.1 for ; Thu, 15 Sep 2016 03:23:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=p2bGcjIKPZO+C40KIVGziiI4xiw4OB17aY0EQUIjPIM=; b=RUTL1ntNb9QK7bavMqpKLfj156yNfC1pNCljM6uahhByhXgHSU9FJxzZieBOWdWtTm 8FLXfOg9qwYeON30jXLUKXZnXQqFYSOEf1o5qYS8cG0pHKefb7Vtpl/+S/GJPvT3o6Jv wb50e1PdVbRvsIK34IsyuceGhx1oBkFfuszs4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=p2bGcjIKPZO+C40KIVGziiI4xiw4OB17aY0EQUIjPIM=; b=Eh36vXREPyfrh5SZzxp0XmnyBgOgzpe+FWqx0KjSGYzDJ9LrOMQz52fPw75jewu8hU M1mPcfRAxzXV4T9XnfqVT+fzkO1f8q8hc5Kwdk66poOIoad6dRshWj+r/HKzgAqHXgst PMmuPx0TALSX8Y/HDGb5GLPB9/jvKSEb8S1Vcn8uL8phudXjrFIf+4EI68dIIg7aOukt igBbcNx+s/ZSiUD49lSlsSTwPxAbaxRYhsNDurnXzto4kbOmiIi+XC/7TX53LI3kvObU LVVwIEBoUuOhTiTz+ktFIDaJUq8Jzq9aTrnA/h9EOYIusOLVR9zczOPZUpt0cuBIbC+r uAug== X-Gm-Message-State: AE9vXwOdcUQtkqsIVsdk7d5B9toA3jCxGs4WFhscJEicDH3r/VU7iAhknZDtjpTHrUMyTqP+ X-Received: by 10.194.140.131 with SMTP id rg3mr6977311wjb.180.1473934801161; Thu, 15 Sep 2016 03:20:01 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id n131sm1825510wmd.3.2016.09.15.03.20.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Sep 2016 03:20:00 -0700 (PDT) Date: Thu, 15 Sep 2016 11:19:58 +0100 From: Leif Lindholm To: "Kinney, Michael D" Cc: "Tian, Feng" , "edk2-devel@lists.01.org" , "Zeng, Star" Message-ID: <20160915101958.GG16080@bivouac.eciton.net> References: <5a0c21d0fe889950fda9e59aaf2272b66d429b8f.1473816993.git.feng.tian@intel.com> <20160914130833.GB16080@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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: Thu, 15 Sep 2016 10:23:34 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > setting timer events periods that are in 100 nS units: > > #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) > > I believe the examples you show are for use with the gBS->Stall() > service which is in 1 uS units. Correct. > Maybe we should consider some additional macros in UefiLib.h > > #define EFI_STALL_PERIOD_MICROSECONDS(Microseconds) (Microseconds) > #define EFI_STALL_PERIOD_MILLISECONDS(Milliseconds) ((Milliseconds) * 1000) > #define EFI_STALL_PERIOD_SECONDS(Seconds) ((Seconds) * 1000000) > > Or maybe some macros that actually do the call to gBS->Stall() too. > > #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) * 1000000) > Either (or both) of those two look good to me. The latter has the benefit of a smaller call site, at the cost of perhaps obscuring the dependency on UefiRuntimeServicesTableLib. > The other method of generating timed delays for PEI/DXE/SMM modules > is using the Services in MdePkg/Include/Library/TimerLib.h: > > UINTN > EFIAPI > NanoSecondDelay ( > IN UINTN NanoSeconds > ); > > UINTN > EFIAPI > MicroSecondDelay ( > IN UINTN MicroSeconds > ); > > If we wanted macros helper to use these services to match UEFI ones, maybe > add the following to TimerLib.h: > > #define DELAY_NANOSECONDS(Nanoseconds) NanoSecondDelay (Nanoseconds) > #define DELAY_MICROSECONDS(Microseconds) MicroSecondDelay (Microseconds) > #define DELAY_MILLISECONDS(Milliseconds) MicroSecondDelay ((Microseconds) * 1000) > #define DELAY_SECONDS(Seconds) MicroSecondDelay ((Microseconds) * 1000000) 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 > like these were used consistently? It would certainly be good to reduce duplication, and consistency would help with the readability of the code. (Which is good for reviewing.) Regards, Leif