Hi Daniel,

> From: Daniel Lezcano <[email protected]>
> > From: Vijayakannan Ayyathurai <[email protected]>
> 
> [ ... ]
> 
> > +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

Reply via email to