lkml.org 
[lkml]   [2017]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 03/15] clocksource: New RISC-V SBI timer driver
On Mon, 31 Jul 2017, Palmer Dabbelt wrote:

> +void timer_riscv_init(int cpu_id,
> + unsigned long riscv_timebase,
> + unsigned long long (*rdtime)(struct clocksource *),
> + int (*next)(unsigned long, struct clock_event_device*))
> +{
> + struct clocksource *cs = &per_cpu(clock_source, cpu_id);
> + struct clock_event_device *ce = &per_cpu(clock_event, cpu_id);

per_cpu_ptr() please

> + *cs = (struct clocksource) {
> + .name = "riscv_clocksource",
> + .rating = 300,
> + .read = rdtime,
> + .mask = CLOCKSOURCE_MASK(BITS_PER_LONG),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + };

Hmm, why do you try to register clocksources per cpu. The core code will
only use the first one registered and the others are just ballast. You
should just have that once at boot time and not on a per cpu basis.

> + clocksource_register_hz(cs, riscv_timebase);

Clock events are per CPU though.

> + *ce = (struct clock_event_device){
> + .name = "riscv_timer_clockevent",
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> + .rating = 300,
> + .cpumask = cpumask_of(cpu_id),
> + .set_next_event = next,
> + .set_state_oneshot = NULL,
> + .set_state_shutdown = NULL,
> + };

I'm not a big fan of that. What's wrong with just making it:

static DEFINE_PER_CPU(struct clock_event_device, clock_event) = {
.name = "riscv_timer_clockevent",
.features = CLOCK_EVT_FEAT_ONESHOT,
.rating = 300,
};

and here just do:

ce->cpumask = cpumask_of(cpu_id);
ce->set_next_event = next;

> + clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff);

Hmm?

> +/*
> + * 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);

What for? You register and forget them ....

Thanks,

tglx

\
 
 \ /
  Last update: 2017-08-16 17:25    [W:0.164 / U:1.284 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site