Re: [Qemu-arm] [Qemu-devel] [PATCH kvm-unit-tests v8 03/10] arm/arm64: add some delay routines

[Andrew Jones]

On Fri, Dec 09, 2016 at 11:41:06AM +0000, Andre Przywara wrote:
> Hi,
> 
> On 08/12/16 17:50, Andrew Jones wrote:
> > Allow a thread to wait some specified amount of time. Can
> > specify in cycles, usecs, and msecs.
> > 
> > Signed-off-by: Andrew Jones <address@hidden>
> > 
> > ---
> > v8: rewrote basing on new sysreg framework. Also decided delay
> >     functions warrant their own files (delay.[ch])
> > ---
> >  arm/Makefile.common       |  1 +
> >  lib/arm/asm/delay.h       | 14 ++++++++++++++
> >  lib/arm/asm/processor.h   | 15 +++++++++++++++
> >  lib/arm64/asm/delay.h     |  1 +
> >  lib/arm64/asm/processor.h | 12 ++++++++++++
> >  lib/arm/delay.c           | 29 +++++++++++++++++++++++++++++
> >  6 files changed, 72 insertions(+)
> >  create mode 100644 lib/arm/asm/delay.h
> >  create mode 100644 lib/arm64/asm/delay.h
> >  create mode 100644 lib/arm/delay.c
> > 
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index b2c0fc8a2fdc..89fe3f69eb44 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -48,6 +48,7 @@ cflatobjs += lib/arm/mmu.o
> >  cflatobjs += lib/arm/bitops.o
> >  cflatobjs += lib/arm/psci.o
> >  cflatobjs += lib/arm/smp.o
> > +cflatobjs += lib/arm/delay.o
> >  
> >  libeabi = lib/arm/libeabi.a
> >  eabiobjs = lib/arm/eabi_compat.o
> > diff --git a/lib/arm/asm/delay.h b/lib/arm/asm/delay.h
> > new file mode 100644
> > index 000000000000..2436b28c77ae
> > --- /dev/null
> > +++ b/lib/arm/asm/delay.h
> > @@ -0,0 +1,14 @@
> > +#ifndef _ASMARM_DELAY_H_
> > +#define _ASMARM_DELAY_H_
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +
> > +extern void delay(u64 cycles);
> 
> Nit: Shouldn't this parameter be called "ticks"? Cycles might be a bit
> misleading, especially since this prototype is the only documentation on
> this. You might just want to fix this when applying the patches.

Right or wrong the kernel uses 'cycles' for this function, named
__timer_delay for arm and __delay for arm64. I guess I prefer
consistency here.

> 
> That notwithstanding:
> Reviewed-by: Andre Przywara <address@hidden>

Thanks!

drew