Hi Daniel, On Thu, Jan 22, 2015 at 01:54:39PM +0100, Daniel Lezcano wrote: > On 01/21/2015 07:36 AM, Baruch Siach wrote: > >+ * Based on: > >+ * Allwinner SoCs hstimer driver > > If this is based on the Allwinnner driver, may be you can have a look at the > patchset sent by Maxim to make sure your implementation is complete ? > > https://lkml.org/lkml/2015/1/11/52
Thanks for the tip. I'll take the applicable parts from that series. [...] > >+static void __iomem *timer_base; > >+static u32 ticks_per_jiffy; > > Mind to encapsulate those global variables in a structure and use > container_of to access those fields like: > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/mtk_timer.c#n56 > > ? The trouble is that the sched_clock callback (digicolor_timer_sched_read) needs a static timer_base to find the timer counter. Do you think adding a structure is worth it only for ticks_per_jiffy? > >+static void digicolor_clkevt_mode(enum clock_event_mode mode, > >+ struct clock_event_device *clk) > >+{ > >+ switch (mode) { > >+ case CLOCK_EVT_MODE_PERIODIC: > >+ writeb(0, timer_base + CONTROL(TIMER_C)); > > Even if that sounds overkill, please replace '0', '1' with whatever macro > name (eg. TIMER_DISABLE/ENABLE). Will do for the entire file. [...] > >+static struct irqaction digicolor_timer_irq = { > >+ .name = "digicolor_timerC", > >+ .flags = IRQF_TIMER | IRQF_IRQPOLL, > >+ .handler = digicolor_timer_interrupt, > >+ .dev_id = &digicolor_clockevent, > >+}; > > The current trend is to use 'request_irq', so no such structure > declaration/initialization is needed. Thanks. Will do. > >+ writel(~0, timer_base + COUNT(TIMER_B)); > > s/~0/UINT_MAX/ ? Ack. Thanks for reviewing. I'll post an updated series shortly. baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - -- 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/
