Re: [PATCH] clocksource: clocksource/clockevent driver for Synopsysdw_apb_timer

[jacob pan]

On Thu, 31 Mar 2011 18:42:47 +0100
Jamie Iles <jamie@xxxxxxxxxxxxx> wrote:

> On Thu, Mar 31, 2011 at 09:50:51AM -0700, jacob pan wrote:
> > On Thu, 31 Mar 2011 17:33:12 +0200 (CEST)
> > Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > 
> > > On Thu, 31 Mar 2011, Jamie Iles wrote:
> > > 
> > > > This patch adds a driver for the Synopsys DesignWare APB timer
> > > > block found in some ARM systems.  This uses the timers with an
> > > > IRQ as a clockevents device and a timer without an IRQ as a
> > > > clocksource device.
> > > 
> > > Interesting. That's probably the same thing as:
> > > 
> > >      arch/x86/kernel/apb_timer.c
> > > 
> > > So if we merge that thing, then we should make sure, that we can
> > > replace the x86 one with it.
> > > 
> > It seems we have room to consolidate, here is my 2c:
> > 1. need to support multiple timer channels
> > 2. support percpu clockevent, need to deal with cpu online/offline
> > 3. early boot needs. I don't know if abp timer is needed for
> > booting on ARM platforms. But for Moorestown, we need timer before
> > platform bus running. So I guess we cannot enumerate the timer as
> > platform device.
> 
> ARM does need it quite early to calibrate the delay loop but I've
> been registering it early by creating an early_platform_device.  The
> other difference is that x86 doesn't support the clk API.
> 
> How about factoring out the clockevent and clocksource operations 
> (.set_next_event, .set_mode, .read etc) and the bits that actually
> drive the hardware into an apb_timer.c then have stuff that does
> platform device registration or langwell specific stuff in a higher
> layer?
> 
I agree to extract out hardware access bits. how about also include a
common allocation/reservation mechanism for apbt timer channels. i
guess we have to stay with device specific bits. e.g.
struct apbt_dev {
	int irq;
	__iomem *base;
	int channel_id;
	unsigned int flags; /* for possible platform quirks */
};
The allocation should also allow hints or specific request a specific
timer. Our platform has hardcoded usage of timer channels.

Can you be more specific about "higher layer"?, my guess is that you
meant platform code or drivers that uses apb timer. So arch specific
timer code can alloc apb timer and combine with things that is
appropriate for their platform. e.g. clk api.


> It looks like arch/x86/kernel/apb_timer.c would still need some lower 
> level access to the timers for doing things like the calibration in 
> apbt_quick_calibrate() before registering the clocksource.
yeah, we still need some early access. perhaps by-pass the clocksource
as an exception. I don't have a better idea at the moment.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/