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
