Hi Daniel,
> From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@xxxxxxxxx>
>
> [ ... ]
>
> > +static struct timer_of keembay_ce_to = {
> > + .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> > + .clkevt = {
> > + .name = "keembay_sys_clkevt",
> > + .features = CLOCK_EVT_FEAT_PERIODIC |
> > + CLOCK_EVT_FEAT_ONESHOT |
> > + CLOCK_EVT_FEAT_DYNIRQ,
> > + .rating = TIM_RATING,
> > + .set_next_event =
> keembay_timer_set_next_event,
> > + .set_state_periodic = keembay_timer_periodic,
> > + .set_state_shutdown = keembay_timer_shutdown,
> > + },
> > + .of_base = {
> > + .index = 0,
> > + },
> > + .of_irq = {
> > + .handler = keembay_timer_isr,
> > + .flags = IRQF_TIMER | IRQF_IRQPOLL,
>
> Is the IRQPOLL flag really needed here ?
>
Not really needed. I will remove this redundant Flag in my next version.
> > +static int __init keembay_timer_init(struct device_node *np)
> > +{
> > + struct keembay_init_data data;
> > + int ret;
> > +
> > + data.base = of_iomap(np, 2);
> > + if (!data.base)
> > + return -ENXIO;
> > +
> > + ret = keembay_clocksource_init(np, &data);
> > + if (ret)
> > + goto exit;
> > +
> > + ret = keembay_clockevent_init(np, &data);
>
> Is this missing ?
>
Yes. Either case it goes to the exit path. So I thought of avoiding this error handling code.
> if (ret)
> goto exit;
>
> return 0;
>
> > +
> > +exit:
> > + keembay_timer_cleanup(np, &data);
> > +
> > + return ret;
> > +}
> > +
> > +TIMER_OF_DECLARE(keembay_timer, "intel,keembay-timer",
> keembay_timer_init);
> >
>
Thanks,
Vijay
