On Mon, Apr 02, 2012 at 12:26:11, Shilimkar, Santosh wrote:
> On Friday 30 March 2012 07:24 PM, Vaibhav Hiremath wrote:
> > Current OMAP code supports couple of clocksource options based
> > on compilation flag (CONFIG_OMAP_32K_TIMER). The 32KHz sync-timer
> > and a gptimer which can run on 32KHz or system clock (e.g 38.4 MHz).
> > So there can be 3 options -
> >
> > 1. 32KHz sync-timer
> > 2. Sys_clock based (e.g 13/19.2/26/38.4 MHz) gptimer
> > 3. 32KHz based gptimer.
> >
> > The optional gptimer based clocksource was added so that it can
> > give the high precision than sync-timer, so expected usage was 2
> > and not 3.
> > Unfortunately option 2, clocksource doesn't meet the requirement of
> > free-running clock as per clocksource need. It stops in low power states
> > when sys_clock is cut. That makes gptimer based clocksource option
> > useless for OMAP2/3/4 devices with sys_clock as a clock input.
> > Option 3 will still work but it is no better than 32K sync-timer
> > based clocksource.
> >
> > So ideally we can kill the gptimer based clocksource option but there
> > are some OMAP based derivative SoCs like AM33XX which doesn't have
> > 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer
> > clocksource.
> > Considering above, make sync-timer and gptimer clocksource runtime
> > selectable so that both OMAP and AMXXXX continue to use the same code.
> >
> > Use hwmod database lookup mechanism, through which at run-time
> > we can identify availability of 32k-sync timer on the device,
> > else fall back to gptimer.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx>
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > CC: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
> > Cc: Benoit Cousson <b-cousson@xxxxxx>
> > Cc: Tony Lindgren <tony@xxxxxxxxxxx>
> > Cc: Paul Walmsley <paul@xxxxxxxxx>
> > Cc: Tarun Kanti DebBarma <tarun.kanti@xxxxxx>
> > Cc: Ming Lei <tom.leiming@xxxxxxxxx>
> > ---
>
> Thanks for the change log update Vaibhav.
> Some more comments.
>
>
> > arch/arm/mach-omap1/timer32k.c | 6 ++-
> > arch/arm/mach-omap2/timer.c | 45 +++++++++-------
> > arch/arm/plat-omap/counter_32k.c | 86 ++++++++++++-----------------
> > arch/arm/plat-omap/include/plat/common.h | 2 +-
> > 4 files changed, 68 insertions(+), 71 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
> > index a2e6d07..3f96a00 100644
> > --- a/arch/arm/mach-omap1/timer32k.c
> > +++ b/arch/arm/mach-omap1/timer32k.c
> > @@ -72,6 +72,7 @@
> >
> > /* 16xx specific defines */
> > #define OMAP1_32K_TIMER_BASE 0xfffb9000
> > +#define OMAP1_32KSYNC_TIMER_BASE 0xfffbc410
> Shouldn't you just update OMAP1_32K_TIMER_BASE ?
>
No, they are different. First one is normal timer base address
and next one is 32k-sync timer base address.
> > #define OMAP1_32K_TIMER_CR 0x08
> > #define OMAP1_32K_TIMER_TVR 0x00
> > #define OMAP1_32K_TIMER_TCR 0x04
> > @@ -185,7 +186,10 @@ static __init void omap_init_32k_timer(void)
> > */
> > bool __init omap_32k_timer_init(void)
> > {
> > - omap_init_clocksource_32k();
> > + u32 pbase;
> > +
> > + pbase = cpu_is_omap16xx() ? OMAP1_32KSYNC_TIMER_BASE : NULL;
> > + omap_init_clocksource_32k(pbase, SZ_1K);
> If pbase is NULL, you might not want to call the init.
>
First check we do in _init function is, check for NULL, so you
can ignore it.
> > omap_init_32k_timer();
> >
> > return true;
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index 5c9acea..f35db1a 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -234,21 +234,6 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
> > }
> >
> > /* Clocksource code */
> > -
> > -#ifdef CONFIG_OMAP_32K_TIMER
> > -/*
> > - * When 32k-timer is enabled, don't use GPTimer for clocksource
> > - * instead, just leave default clocksource which uses the 32k
> > - * sync counter. See clocksource setup in plat-omap/counter_32k.c
> > - */
> > -
> > -static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
> > -{
> > - omap_init_clocksource_32k();
> > -}
> > -
> > -#else
> > -
> > static struct omap_dm_timer clksrc;
> >
> > /*
> > @@ -280,13 +265,33 @@ static void __init omap2_gp_clocksource_init(int gptimer_id,
> > const char *fck_source)
> > {
> > int res;
> > + struct omap_hwmod *oh;
> > + const char *oh_name = "counter_32k";
> > +
> > + /*
> > + * First check for availability for 32k-sync timer.
> > + *
> > + * In case of failure in finding 32k_counter module or
> > + * registering it as clocksource, execution will fallback to
> > + * gp-timer.
> > + */
> > + oh = omap_hwmod_lookup(oh_name);
> > + if (oh && oh->slaves_cnt != 0) {
> > + u32 pbase;
> > + unsigned long size;
> > +
> > + pbase = oh->slaves[0]->addr->pa_start + 0x10;
> Don't hardcode the offset here. This is error prone when the IP
> changes so use the register define.
>
Ok, I will define macro for this and use it.
Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
