Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer

[Santosh Shilimkar <santosh.shilimkar@ti.com>]

On Thursday 05 April 2012 04:01 PM, Hiremath, Vaibhav wrote:
> On Thu, Apr 05, 2012 at 15:22:21, Russell King - ARM Linux wrote:
>> On Thu, Apr 05, 2012 at 09:36:00AM +0000, Hiremath, Vaibhav wrote:
>>> There seems to be limitation for ARM architecture, it is restricted by
>>> sched_clock implementation present in "arch/arm/kernel/sched_clock.c".
>>> Natively, clocksource framework does support change in rate/frequency for
>>> registered timer, using,
>>
>> Any kind of switching of timing sources introduces loss of time issues
>> by the mere fact that you can't instantly know the counter values of
>> both timing sources at precisely the same instant - because CPUs can
>> only do one thing at a time.  So any kind of repeated dynamic switching
>> _will_ result in a gradual loss of time - which will be indeterminant
>> as it depends on the frequency of the switching and the relative delta
>> between the two.
>>
>> To put it another way - it is not possible to maintain high precision
>> and accuracy while dynamically switching your timing sources.
>>
>> I'm not about to lift the restriction that there's only one source for
>> sched_clock() just for OMAP.  That'd require a lot of additional code -
>> it's not just about adjusting the multiplier and shift, you also have to
>> correct the returned ns value as well, which means synchronizing against
>> two counters.  (And as I've pointed out above, that's impossible to do
>> accurately.)
>>
> 
> Thanks a ton Russell for confirming on this,
> 
> I understand, we also have to adjust ns value and such confirmation is what exactly I was looking for.
> 
> So this means, we have to use compile time option, as existing
> implementation in "arch/arm/mach-omap2/timer.c".
> 
> Thanks again, I will repost patches shortly with the code changes (mentioned 
> in my last email)
> 
I suggest you wait for Kevin and Tony to look at it.

Am still going back to what I proposed initially.
Why not the conditional way as shown in the patch [1] I proposed or
your initial patch with some updates?  Something like this.

if(commandline.clksource == gpt)
	omap2_gptimer_clocksource_init(gptimer_id, fck_source);
else if (omap_init_clocksource_32k())
	omap2_gptimer_clocksource_init(gptimer_id, fck_source);

This won't need compile time option and gives you all everybody wants.

1. Ability to use gpt as a clock-source for perf like stuff
2. Hardware like AMXX where 32K synctimer doesn't exist which means
omap_init_clocksource_32k() will fail and use gptimer.
3. For OMAP, it will continue to use 32K sync timer.

What am I missing Vaibhav?

Regards
Santosh