Leif, 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. 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) 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) Do you think it would improve the maintenance of the code if macros like these were used consistently? Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif > Lindholm > Sent: Wednesday, September 14, 2016 6:09 AM > To: Tian, Feng <feng.t...@intel.com> > Cc: edk2-devel@lists.01.org; Zeng, Star <star.z...@intel.com> > Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: add 1ms delay before access > MMIO reg > during reset > > On Wed, Sep 14, 2016 at 09:37:13AM +0800, Feng Tian wrote: > > Some XHCI host controllers require to have extra 1ms delay before > > accessing any MMIO register during HC reset. > > Is this compliant with the XHCI specification or a bug workaround? > I am not going to argue about the delay, but I would like to see it > spelled out in the commit message. > > > Cc: Star Zeng <star.z...@intel.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Feng Tian <feng.t...@intel.com> > > --- > > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > > index d0f2205..cb822a6 100644 > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > > @@ -2,7 +2,7 @@ > > > > The XHCI register operation routines. > > > > -Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR> > > +Copyright (c) 2011 - 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 > > @@ -687,6 +687,10 @@ XhcResetHC ( > > if ((Xhc->DebugCapSupOffset == 0xFFFFFFFF) || ((XhcReadExtCapReg (Xhc, > > Xhc- > >DebugCapSupOffset) & 0xFF) != XHC_CAP_USB_DEBUG) || > > ((XhcReadExtCapReg (Xhc, Xhc->DebugCapSupOffset + XHC_DC_DCCTRL) & > > BIT0) == > 0)) { > > XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET); > > + // > > + // some XHCI host controllers require to have extra 1ms delay before > > accessing > any MMIO register during reset. > > + // > > + gBS->Stall (XHC_1_MILLISECOND); > > OK, so this is not actually a comment on this patch, but why do we > have so many separate definitions of how many microseconds go in a > milisecond or second? > > USB_BUS_1_MILLISECOND (Pei and Dxe) > XHC_1_MILLISECOND (Pei and Dxe) > EHC_1_MILLISECOND (Pei and Dxe) > UHC_1_MILLISECOND (Dxe, Pei defines STALL_1_MILLI_SECOND instead) > > And IdeBusPei/PciBusDxe do similar things. > > Could we add something common to Base.h instead, like the SIZE_ ones, > droppping a lot of duplication, gaining uniformity, and correcting > spelling? > > / > Leif > > > Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET, > > FALSE, > Timeout); > > } > > > > -- > > 2.7.1.windows.2 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel