Re: [PATCH 05/17] clocksource: New RISC-V SBI timer driver

From: Palmer Dabbelt
Date: Mon Jul 31 2017 - 21:14:25 EST


Sorry, I missed this before submitting our v7. I'll respond properly in a
bit...

On Mon, 31 Jul 2017 04:00:26 PDT (-0700), daniel.lezcano@xxxxxxxxxx wrote:
> On 11/07/2017 03:39, Palmer Dabbelt wrote:
>> The RISC-V ISA defines a per-hart real-time clock and timer, which is
>> present on all systems. The clock is accessed via the 'rdtime'
>> pseudo-instruction (which reads a CSR), and the timer is set via an SBI
>> call.
>>
>> This driver attempts to split out the RISC-V ISA specific mechanisms of
>> accessing the hardware from the clocksource driver by taking a pair of
>> function pointers to issue the actual RISC-V specific instructions.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxx>
>> ---
>
> [ ... ]
>
>> +#ifndef _LINUX_TIMER_RISCV_H
>> +#define _LINUX_TIMER_RISCV_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.
>> + */
>
> Hi Palmer,
>
> The driver code must be self contained (eg. setup and irq handling).
>
> There are examples in the drivers/clocksource to stick on, for example
> the jcore-pit.c or the mips-gic-timer.c drivers.
>
> I don't see the point to setup callbacks at init time.
>
> Thanks.
>
> -- Daniel
>
>
>> +void timer_riscv_init(int cpu_id,
>> + unsigned long riscv_timebase,
>> + unsigned long long (*rdtime)(struct clocksource *),
>> + int (*next_event)(unsigned long, struct clock_event_device *));
>> +
>> +/*
>> + * Looks up the clocksource or clock_even_device that cooresponds the given
>> + * hart.
>> + */
>> +struct clocksource *timer_riscv_source(int cpuid);
>> +struct clock_event_device *timer_riscv_device(int cpu_id);
>> +
>> +#endif
>>