Re: [PATCH v4 21/29] nios2: Time keeping

[Thomas Gleixner]

On Wed, 8 Oct 2014, Ley Foon Tan wrote:
> +static void nios2_timer_set_mode(enum clock_event_mode mode,
> +	struct clock_event_device *evt)
> +{
> +	unsigned long period;
> +	struct nios2_clockevent_dev *nios2_ced = to_nios2_clkevent(evt);
> +	struct nios2_timer *timer = &nios2_ced->timer;
> +
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		period = DIV_ROUND_UP(timer->freq, HZ);
> +		nios2_timer_config(timer, period, CLOCK_EVT_MODE_PERIODIC);
> +		break;
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		nios2_timer_config(timer, ULONG_MAX, CLOCK_EVT_MODE_ONESHOT);

What's the point of this? This should simply stop the timer.

> +		break;
> +	case CLOCK_EVT_MODE_UNUSED:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +		nios2_timer_stop(timer);
> +		break;
> +	case CLOCK_EVT_MODE_RESUME:
> +		nios2_timer_start(timer);
> +		break;
> +	}
> +}

> +static int num_called;
> +static void __init nios2_time_init(struct device_node *timer)
> +{
> +	switch (num_called) {
> +	case 0:
> +		nios2_clockevent_init(timer);
> +		break;
> +	case 1:
> +		nios2_clocksource_init(timer);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	num_called++;
> +}

Eew. So this depends on the DT ordering. If thats wrong, then stuff
will be initialized in the wrong oder. 

> +CLOCKSOURCE_OF_DECLARE(nios2_timer, "altr,timer-1.0", nios2_time_init);

Why can't you have separate match entries with where one calls
nios2_clockevent_init and the other nios2_clocksource_init?

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/