Re: [PATCH 5/6] riscv: don't use the rdtime(h) pseudo-instructions

[Atish Patra <Atish.Patra@wdc.com>]

On Wed, 2019-08-21 at 23:58 +0900, Christoph Hellwig wrote:
> If we just use the CSRs that these map to directly the code is
> simpler
> and doesn't require extra inline assembly code.  Also fix up the top-
> level
> comment in timer-riscv.c to not talk about the cycle count or mention
> details of the clocksource interface, of which this file is just a
> consumer.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/include/asm/timex.h    | 44 +++++++++++++++------------
> ----
>  drivers/clocksource/timer-riscv.c | 17 +++---------
>  2 files changed, 25 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/timex.h
> b/arch/riscv/include/asm/timex.h
> index 6a703ec9d796..c7ef131b9e4c 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -6,43 +6,41 @@
>  #ifndef _ASM_RISCV_TIMEX_H
>  #define _ASM_RISCV_TIMEX_H
>  
> -#include <asm/param.h>
> +#include <asm/csr.h>
>  
>  typedef unsigned long cycles_t;
>  
> -static inline cycles_t get_cycles_inline(void)
> +static inline cycles_t get_cycles(void)
>  {
> -	cycles_t n;
> -
> -	__asm__ __volatile__ (
> -		"rdtime %0"
> -		: "=r" (n));
> -	return n;
> +	return csr_read(CSR_TIME);

Does this work correctly in QEMU ? I was looking at the qemu code and
it looks like it returns cpu_get_host_ticks which seems wrong to me.

https://github.com/qemu/qemu/blob/master/target/riscv/csr.c#L213


>  }
> -#define get_cycles get_cycles_inline
> +#define get_cycles get_cycles
>  
>  #ifdef CONFIG_64BIT
> -static inline uint64_t get_cycles64(void)
> +static inline u64 get_cycles64(void)
> +{
> +	return get_cycles();
> +}
> +#else /* CONFIG_64BIT */
> +static inline u32 get_cycles_hi(void)
>  {
> -        return get_cycles();
> +	return csr_read(CSR_TIMEH);
>  }
> -#else
> -static inline uint64_t get_cycles64(void)
> +
> +static inline u64 get_cycles64(void)
>  {
> -	u32 lo, hi, tmp;
> -	__asm__ __volatile__ (
> -		"1:\n"
> -		"rdtimeh %0\n"
> -		"rdtime %1\n"
> -		"rdtimeh %2\n"
> -		"bne %0, %2, 1b"
> -		: "=&r" (hi), "=&r" (lo), "=&r" (tmp));
> +	u32 hi, lo;
> +
> +	do {
> +		hi = get_cycles_hi();
> +		lo = get_cycles();
> +	} while (hi != get_cycles_hi());
> +
>  	return ((u64)hi << 32) | lo;
>  }
> -#endif
> +#endif /* CONFIG_64BIT */
>  
>  #define ARCH_HAS_READ_CURRENT_TIMER
> -
>  static inline int read_current_timer(unsigned long *timer_val)
>  {
>  	*timer_val = get_cycles();
> diff --git a/drivers/clocksource/timer-riscv.c
> b/drivers/clocksource/timer-riscv.c
> index 09e031176bc6..470c7ef02ea4 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -2,6 +2,10 @@
>  /*
>   * Copyright (C) 2012 Regents of the University of California
>   * Copyright (C) 2017 SiFive
> + *
> + * All RISC-V systems have a timer attached to every hart.  These
> timers can be
> + * read from the "time" and "timeh" CSRs, and can use the SBI to
> setup
> + * events.
>   */
>  #include <linux/clocksource.h>
>  #include <linux/clockchips.h>
> @@ -12,19 +16,6 @@
>  #include <asm/smp.h>
>  #include <asm/sbi.h>
>  
> -/*
> - * All RISC-V systems have a timer attached to every hart.  These
> timers can be
> - * read by the 'rdcycle' pseudo instruction, and can use the SBI to
> setup
> - * events.  In order to abstract the architecture-specific timer
> reading and
> - * setting functions away from the clock event insertion code, we
> provide
> - * function pointers to the clockevent subsystem that perform two
> basic
> - * operations: rdtime() reads the timer on the current CPU, and
> - * next_event(delta) sets the next timer event to 'delta' cycles in
> the future.
> - * As the timers are inherently a per-cpu resource, these callbacks
> perform
> - * operations on the current hart.  There is guaranteed to be
> exactly one timer
> - * per hart on all RISC-V systems.
> - */
> -
>  static int riscv_clock_next_event(unsigned long delta,
>  		struct clock_event_device *ce)
>  {

-- 
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv