Re: [PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Jon Hunter <jon-hunter@ti.com>]

On 02/01/2013 02:59 AM, Jon Hunter wrote:
> 
> On 02/01/2013 02:41 AM, Bedia, Vaibhav wrote:
>> Hi Jon,
>>
>> On Wed, Jan 30, 2013 at 22:34:31, Hunter, Jon wrote:
>>> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
>>> is used as the clocksource (which is always the case for AM335x), a
>>> gptimer located in a power domain that is not always-on is selected.
>>> Ideally we should use a gptimer located in a power domain that is always
>>> on (such as the wake-up domain) so that time can be maintained during a
>>> kernel suspend without keeping on additional power domains unnecessarily.
>>>
>>> In order to fix this so that we can select a gptimer located in a power
>>> domain that is always-on, the following changes were made ...
>>> 1. Currently, only when selecting a gptimer to use for a clockevent
>>>    timer, do we pass a timer property that can be used to select a
>>>    specific gptimer. Change this so that we can pass a property when
>>>    selecting a gptimer to use for a clocksource timer too.
>>> 2. Currently, when selecting either a gptimer to use for a clockevent
>>>    timer or a clocksource timer and no timer property is passed, then
>>>    the first available timer is selected regardless of the properties
>>>    it has. Change this so that if no properties are passed, then a timer
>>>    that does not have additional features (such as always-on, dsp-irq,
>>>    pwm, and secure) is selected.
>>>
>>> Please note that using a gptimer for both clocksource and clockevents
>>> can have a system power impact during idle. The reason being is that
>>> OMAP and AMxxx devices typically only have one gptimer in a power domain
>>> that is always-on. Therefore when the kernel is idle both the clocksource
>>> and clockevent timers will be active and this will keep additional power
>>> domains on. During kernel suspend, only the clocksource timer is active
>>> and therefore, it is better to use a gptimer in a power domain that is
>>> always-on for clocksource.
>>>
>>
>> It's always a pleasure reading the changelog in your patches :)
> 
> Thanks.
> 
>> [...]
>>>  
>>>  #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
>>> -OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
>>> -		       2, "timer_sys_ck");
>>> +OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
>>> +		       1, "timer_sys_ck", "ti,timer-alwon");
>>>  #endif
>>>
>>
>> Minor point... was the intention of changing of clkev_nr and clksrc_nr to make
>> it consistent with what happens on AM33xx which is DT-only?
> 
> I don't see this as being DT specific. It is more of a policy change to
> ensure a wake-up domain timer is used for clocksource when we are using
> gptimers for both clocksource and clockevents. It was your patch for
> AM335x that made me see the need for this, if that makes sense. May be I
> should have referenced that in the changelog.

Sorry to be clear, I don't see the policy change as DT specific.

In answer to your question, yes clkev_nr and clksrc_nr are changed so
the policy it is consistent regardless of whether you use DT or not. In
other words, an OMAP3 board using a gptimer for clocksource  (such as
cm-t3517) and does not use DT, would work the same as AM335x with DT.

Cheers
Jon