Messages in this thread | | | Date | Thu, 30 May 2013 17:13:45 +0200 | From | Daniel Lezcano <> | Subject | Re: [PATCH] Add TI-Nspire timer support |
| |
On 05/30/2013 01:11 PM, Daniel Tang wrote: > Hi, > > This patch adds a clocksource/clockevent driver for the TI-Nspire calculator series.
Please, can you write a better changelog ? What does this chip provide ? What are the specificity of this chip ? Is there any trick in the code related to this chip ? etc ...
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Daniel Tang <dt.tangr@gmail.com>
Seconding Thomas feedbacks + some comments below.
[ ... ]
> +struct zevio_timer { > + void __iomem *base; > + void __iomem *timer1, *timer2; > + void __iomem *interrupt_regs; > + > + int irqnr;
Why do you need to add this field for a value you need to store at init time ? You can declare this variable in the function, no ?
> + > + struct clk *clk; > + struct clock_event_device clkevt; > + struct irqaction clkevt_irq; > + > + char clocksource_name[64]; > + char clockevent_name[64]; > +}; > +
[ ... ]
> +static int __init zevio_timer_add(struct device_node *node) > +{ > + struct zevio_timer *timer; > + struct resource res; > + int ret; > + > + timer = kzalloc(sizeof(*timer), GFP_ATOMIC); > + if (!timer) > + return -ENOMEM; > + > + timer->base = of_iomap(node, 0); > + if (!timer->base) { > + ret = -EINVAL; > + goto error_free; > + } > + timer->timer1 = timer->base + IO_TIMER1; > + timer->timer2 = timer->base + IO_TIMER2; > + > + timer->clk = of_clk_get(node, 0); > + if (IS_ERR(timer->clk)) { > + ret = PTR_ERR(timer->clk); > + pr_err("Timer clock not found! (error %d)\n", ret); > + goto error_unmap; > + } > + > + timer->interrupt_regs = of_iomap(node, 1); > + timer->irqnr = irq_of_parse_and_map(node, 0); > + > + of_address_to_resource(node, 0, &res); > + scnprintf(timer->clocksource_name, sizeof(timer->clocksource_name), > + "%llx.%s_clocksource", > + (unsigned long long)res.start, node->name); > + > + scnprintf(timer->clockevent_name, sizeof(timer->clockevent_name), > + "%llx.%s_clockevent", > + (unsigned long long)res.start, node->name); > + > + if (timer->interrupt_regs && timer->irqnr) { > + timer->clkevt.name = timer->clockevent_name; > + timer->clkevt.set_next_event = zevio_timer_set_event; > + timer->clkevt.set_mode = zevio_timer_set_mode; > + timer->clkevt.rating = 200; > + timer->clkevt.cpumask = cpu_all_mask; > + timer->clkevt.features = > + CLOCK_EVT_FEAT_ONESHOT;
timer->clkevt.irq = timer->irqnr;
> + > + writel(CNTL_STOP_TIMER, timer->timer1 + IO_CONTROL); > + writel(0, timer->timer1 + IO_DIVIDER); > +
[ ... ]
Thanks -- Daniel
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |