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.wall...@linaro.org>
> Signed-off-by: Daniel Tang <dt.ta...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
