On Tue, Jun 6, 2023 at 10:10 AM Sunil V L <suni...@ventanamicro.com> wrote:
> On Tue, Jun 06, 2023 at 10:02:08AM -0700, Tuan Phan wrote:
> > On Tue, Jun 6, 2023 at 3:27 AM Sunil V L <suni...@ventanamicro.com>
> wrote:
> >
> > > On Fri, May 26, 2023 at 04:25:18PM -0700, Tuan Phan wrote:
> > > > The timer compare register is 64-bit so simplifying the delay
> > > > function.
> > > >
> > > > Signed-off-by: Tuan Phan <tp...@ventanamicro.com>
> > > > ---
> > > > MdePkg/Include/Register/RiscV64/RiscVImpl.h | 1 -
> > > > .../BaseRiscV64CpuTimerLib/CpuTimerLib.c | 62
> +++++++++----------
> > > > 2 files changed, 28 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/MdePkg/Include/Register/RiscV64/RiscVImpl.h
> > > b/MdePkg/Include/Register/RiscV64/RiscVImpl.h
> > > > index ee5c2ba60377..6997de6cc001 100644
> > > > --- a/MdePkg/Include/Register/RiscV64/RiscVImpl.h
> > > > +++ b/MdePkg/Include/Register/RiscV64/RiscVImpl.h
> > > > @@ -20,6 +20,5 @@
> > > > Name:
> > > >
> > > > #define ASM_FUNC(Name) _ASM_FUNC(ASM_PFX(Name), .text. ## Name)
> > > > -#define RISCV_TIMER_COMPARE_BITS 32
> > > >
> > > > #endif
> > > > diff --git a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
> > > b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
> > > > index 9c8efc0f3530..57800177023c 100644
> > > > --- a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
> > > > +++ b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
> > > > @@ -22,26 +22,19 @@
> > > > @param Delay A period of time to delay in ticks.
> > > >
> > > > **/
> > > > +STATIC
> > > > VOID
> > > > InternalRiscVTimerDelay (
> > > > - IN UINT32 Delay
> > > > + IN UINT64 Delay
> > > > )
> > > > {
> > > > - UINT32 Ticks;
> > > > - UINT32 Times;
> > > > -
> > > > - Times = Delay >> (RISCV_TIMER_COMPARE_BITS - 2);
> > > > - Delay &= ((1 << (RISCV_TIMER_COMPARE_BITS - 2)) - 1);
> > > > - do {
> > > > - //
> > > > - // The target timer count is calculated here
> > > > - //
> > > > - Ticks = RiscVReadTimer () + Delay;
> > > > - Delay = 1 << (RISCV_TIMER_COMPARE_BITS - 2);
> > > > - while (((Ticks - RiscVReadTimer ()) & (1 <<
> > > (RISCV_TIMER_COMPARE_BITS - 1))) == 0) {
> > > > - CpuPause ();
> > > > - }
> > > > - } while (Times-- > 0);
> > > > + UINT64 Ticks;
> > > > +
> > > > + Ticks = RiscVReadTimer () + Delay;
> > > > +
> > > > + while (RiscVReadTimer () <= Ticks) {
> > > > + CpuPause ();
> > > > + }
> > > > }
> > > >
> > > > /**
> > > > @@ -61,14 +54,14 @@ MicroSecondDelay (
> > > > )
> > > > {
> > > > InternalRiscVTimerDelay (
> > > > - (UINT32)DivU64x32 (
> > > > - MultU64x32 (
> > > > - MicroSeconds,
> > > > - PcdGet64 (PcdCpuCoreCrystalClockFrequency)
> > > > - ),
> > > > - 1000000u
> > > > - )
> > > > - );
> > > > + DivU64x32 (
> > > > + MultU64x32 (
> > > > + MicroSeconds,
> > > > + PcdGet64 (PcdCpuCoreCrystalClockFrequency)
> > > > + ),
> > > > + 1000000u
> > > > + )
> > > > + );
> > > > return MicroSeconds;
> > > > }
> > > >
> > > > @@ -89,14 +82,14 @@ NanoSecondDelay (
> > > > )
> > > > {
> > > > InternalRiscVTimerDelay (
> > > > - (UINT32)DivU64x32 (
> > > > - MultU64x32 (
> > > > - NanoSeconds,
> > > > - PcdGet64 (PcdCpuCoreCrystalClockFrequency)
> > > > - ),
> > > > - 1000000000u
> > > > - )
> > > > - );
> > > > + DivU64x32 (
> > > > + MultU64x32 (
> > > > + NanoSeconds,
> > > > + PcdGet64 (PcdCpuCoreCrystalClockFrequency)
> > > > + ),
> > > > + 1000000000u
> > > > + )
> > > > + );
> > > > return NanoSeconds;
> > > > }
> > > >
> > > > @@ -147,8 +140,9 @@ GetPerformanceCounter (
> > > > UINT64
> > > > EFIAPI
> > > > GetPerformanceCounterProperties (
> > > > - OUT UINT64 *StartValue, OPTIONAL
> > > > - OUT UINT64 *EndValue OPTIONAL
> > > > + OUT UINT64 *StartValue,
> > > > + OPTIONAL
> > > > + OUT UINT64 *EndValue OPTIONAL
> > >
> > > Hi Tuan,
> > >
> > > What is this change? The formatting doesn't look correct. Have you run
> > > CI tests?
> > >
> > => That is the result of running crutinize tool with edk2 config. Should
> I
> > leave it as before?
> >
> I have not used crutinize tool. But this change looks unnecessary. I
> would use uncrustify tool locally for any formatting fixes since the
> same tool gets used in CI tests also.
>
=> The problem is the old version of this file has not passed the
uncrustify tool. So without the "unnecessary changes", this MR will not
pass the CI.
>
> > >
> > > Otherwise LGTM. Thanks for the fix!
> > >
> > > Reviewed-by: Sunil V L <suni...@ventanamicro.com>
> > >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105821): https://edk2.groups.io/g/devel/message/105821
Mute This Topic: https://groups.io/mt/99160087/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
