Re: [PATCH v2 33/35] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

[Linus Walleij]

On Mon, Nov 27, 2017 at 1:28 PM, Greentime Hu <green.hu@xxxxxxxxx> wrote:

> From: Rick Chen <rickchen36@xxxxxxxxx>
>
> ATCPIT100 is often used on the Andes architecture,
> This timer provide 4 PIT channels. Each PIT channel is a
> multi-function timer, can be configured as 32,16,8 bit timers
> or PWM as well.
>
> For system timer it will set channel 1 32-bit timer0 as clock
> source and count downwards until underflow and restart again.
>
> It also set channel 0 32-bit timer0 as clock event and count
> downwards until condition match. It will generate an interrupt
> for handling periodically.
>
> Signed-off-by: Rick Chen <rickchen36@xxxxxxxxx>
> Signed-off-by: Greentime Hu <green.hu@xxxxxxxxx>

The driver looks nice overall.

> +static struct timer_of to = {
> +       .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
> +
> +       .clkevt = {
> +               .name = "atcpit100_tick",
> +               .rating = 300,
> +               .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> +               .set_state_shutdown = atcpit100_clkevt_shutdown,
> +               .set_state_periodic = atcpit100_clkevt_set_periodic,
> +               .set_state_oneshot = atcpit100_clkevt_set_oneshot,
> +               .tick_resume = atcpit100_clkevt_shutdown,
> +               .set_next_event = atcpit100_clkevt_next_event,
> +               .cpumask = cpu_all_mask,
> +       },
> +
> +       .of_irq = {
> +               .handler = atcpit100_timer_interrupt,
> +               .flags = IRQF_TIMER | IRQF_IRQPOLL,
> +       },

I would add:

.of_clk = {
    .name = "PCLK",
};

To be explicit on what we use.

(I hope I understand this OF timer right.)

Otherwise it looks good!
Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Yours,
Linus Walleij