[PATCH v2 12/13] ARM: move sp804 and integrator timers to drivers/clocksource

[john.stultz@linaro.org (John Stultz)]

On 04/01/2013 03:21 PM, Rob Herring wrote:
> diff --git a/drivers/clocksource/integrator_ap_timer.c b/drivers/clocksource/integrator_ap_timer.c
> new file mode 100644
> index 0000000..05e6204
> --- /dev/null
> +++ b/drivers/clocksource/integrator_ap_timer.c
[snip]
> +static void __iomem * clkevt_base;
> +
> +/*
> + * IRQ handler for the timer
> + */
> +static irqreturn_t integrator_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = dev_id;
> +
> +	/* clear the interrupt */
> +	writel(1, clkevt_base + TIMER_INTCLR);
> +
> +	evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void clkevt_set_mode(enum clock_event_mode mode, struct clock_event_device *evt)
> +{
> +	u32 ctrl = readl(clkevt_base + TIMER_CTRL) & ~TIMER_CTRL_ENABLE;
> +
> +	/* Disable timer */
> +	writel(ctrl, clkevt_base + TIMER_CTRL);
> +
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		/* Enable the timer and start the periodic tick */
> +		writel(timer_reload, clkevt_base + TIMER_LOAD);
> +		ctrl |= TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
> +		writel(ctrl, clkevt_base + TIMER_CTRL);
> +		break;
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		/* Leave the timer disabled, .set_next_event will enable it */
> +		ctrl &= ~TIMER_CTRL_PERIODIC;
> +		writel(ctrl, clkevt_base + TIMER_CTRL);
> +		break;
> +	case CLOCK_EVT_MODE_UNUSED:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +	case CLOCK_EVT_MODE_RESUME:
> +	default:
> +		/* Just leave in disabled state */
> +		break;
> +	}
> +
> +}
> +
> +static int clkevt_set_next_event(unsigned long next, struct clock_event_device *evt)
> +{
> +	unsigned long ctrl = readl(clkevt_base + TIMER_CTRL);
> +
> +	writel(ctrl & ~TIMER_CTRL_ENABLE, clkevt_base + TIMER_CTRL);
> +	writel(next, clkevt_base + TIMER_LOAD);
> +	writel(ctrl | TIMER_CTRL_ENABLE, clkevt_base + TIMER_CTRL);
> +
> +	return 0;
> +}
> +
> +static struct clock_event_device integrator_clockevent = {
> +	.name		= "timer1",
> +	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> +	.set_mode	= clkevt_set_mode,
> +	.set_next_event	= clkevt_set_next_event,
> +	.rating		= 300,
> +};
> +
> +static struct irqaction integrator_timer_irq = {
> +	.name		= "timer",
> +	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
> +	.handler	= integrator_timer_interrupt,
> +	.dev_id		= &integrator_clockevent,
> +};
> +
> +void __init integrator_clockevent_init(unsigned long inrate,
> +				       void __iomem *base, int irq)
> +{
> +	unsigned long rate = inrate;
> +	unsigned int ctrl = 0;
> +
> +	clkevt_base = base;
> +	/* Calculate and program a divisor */
> +	if (rate > 0x100000 * HZ) {
> +		rate /= 256;
> +		ctrl |= TIMER_CTRL_DIV256;
> +	} else if (rate > 0x10000 * HZ) {
> +		rate /= 16;
> +		ctrl |= TIMER_CTRL_DIV16;
> +	}
> +	timer_reload = rate / HZ;
> +	writel(ctrl, clkevt_base + TIMER_CTRL);
> +
> +	setup_irq(irq, &integrator_timer_irq);
> +	clockevents_config_and_register(&integrator_clockevent,
> +					rate,
> +					1,
> +					0xffffU);
> +}
> +
> +static void __init ap_of_timer_init(struct device_node *node)
> +{
> +	void __iomem *base;
> +	int irq;
> +	struct clk *clk;
> +	unsigned long rate;
> +
> +	clk = clk_get_sys("ap_timer", NULL);
> +	BUG_ON(IS_ERR(clk));
> +	clk_prepare_enable(clk);
> +	rate = clk_get_rate(clk);
> +
> +	base = of_iomap(node, 0);
> +	if (WARN_ON(!base))
> +		return;
> +
> +	writel(0, base + TIMER_CTRL);
> +
> +	if ((clksrc_base && clkevt_base) || !of_device_is_available(node)) {
> +		iounmap(base);
> +		return;
> +	}
> +
> +	irq = irq_of_parse_and_map(node, 0);
> +	if (!clkevt_base && (irq > 0))
> +		integrator_clockevent_init(rate, base, irq);
> +	else
> +		integrator_clocksource_init(rate, base);
> +}
> +CLOCKSOURCE_OF_DECLARE(integrator_ap, "integrator-timer", ap_of_timer_init);

So, most of the code here is really clockevent code and not clocksource 
code. I realize they were combined in the mach directory you're copying 
them from, but if we're going to move all this code out of the arch 
directory and into drivers/, I'd like to propose we not dump it all into 
drivers/clocksource.

For more context here see:
http://www.spinics.net/lists/arm-kernel/msg234074.html

So unless Thomas objects, would you be willing to break this change up 
adding the relevant irq related bits to a new drivers/clockevents 
directory? You can leave the sched_clock bits to clocksource for now, 
since I think that logic should eventually be integrated with the 
clocksource core.

When you resubmit, please also add Daniel Lezcano 
<daniel.lezcano@linaro.org> to the cc.

thanks
-john