On 09/06/2012 09:20 AM, Jon Hunter wrote:
>
> On 09/06/2012 07:57 AM, Vaibhav Hiremath wrote:
>>
>>
>> On 9/6/2012 12:34 AM, Jon Hunter wrote:
>>> Currently the dmtimer posted mode is being enabled when the function
>>> __omap_dm_timer_reset() is called. This function is only being called for
>>> OMAP1 timers and OMAP2+ timers that are being used as system timers. Hence,
>>> for OMAP2+ timers that are NOT being used as a system timer, posted mode is
>>> not enabled but the "timer->posted" variable is still set (incorrectly) in
>>> the omap_dm_timer_prepare() function.
>>>
>>> This is a regression introduced by commit 3392cdd3 (ARM: OMAP: dmtimer:
>>> switch-over to platform device driver) which changed the code to only call
>>> omap_dm_timer_reset() for OMAP1 devices. Although this is a regression from
>>> the original code it only impacts performance and so is not needed for stable.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@xxxxxx>
>>> ---
>>> arch/arm/mach-omap2/timer.c | 3 +--
>>> arch/arm/plat-omap/dmtimer.c | 14 +++++---------
>>> arch/arm/plat-omap/include/plat/dmtimer.h | 9 ++++++++-
>>> 3 files changed, 14 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index 5471706..e24ee0f 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -194,10 +194,9 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>>> }
>>> __omap_dm_timer_init_regs(timer);
>>> __omap_dm_timer_reset(timer, 1, 1);
>>> - timer->posted = 1;
>>> + __omap_dm_timer_enable_posted(timer);
>>>
>>> timer->rate = clk_get_rate(timer->fclk);
>>> -
>>> timer->reserved = 1;
>>>
>>> return res;
>>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>>> index c34f55b..22790ea 100644
>>> --- a/arch/arm/plat-omap/dmtimer.c
>>> +++ b/arch/arm/plat-omap/dmtimer.c
>>> @@ -122,21 +122,15 @@ static void omap_dm_timer_wait_for_reset(struct omap_dm_timer *timer)
>>>
>>> static void omap_dm_timer_reset(struct omap_dm_timer *timer)
>>> {
>>> - omap_dm_timer_enable(timer);
>>> if (timer->pdev->id != 1) {
>>> omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG, 0x06);
>>> omap_dm_timer_wait_for_reset(timer);
>>> }
>>> -
>>> __omap_dm_timer_reset(timer, 0, 0);
>>> - omap_dm_timer_disable(timer);
>>> - timer->posted = 1;
>>> }
>>>
>>> int omap_dm_timer_prepare(struct omap_dm_timer *timer)
>>> {
>>> - int ret;
>>> -
>>> /*
>>> * FIXME: OMAP1 devices do not use the clock framework for dmtimers so
>>> * do not call clk_get() for these devices.
>>> @@ -150,13 +144,15 @@ int omap_dm_timer_prepare(struct omap_dm_timer *timer)
>>> }
>>> }
>>>
>>> + omap_dm_timer_enable(timer);
>>> +
>>> if (timer->capability & OMAP_TIMER_NEEDS_RESET)
>>> omap_dm_timer_reset(timer);
>>>
>>> - ret = omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
>>> + __omap_dm_timer_enable_posted(timer);
>>> + omap_dm_timer_disable(timer);
>>>
>>> - timer->posted = 1;
>>> - return ret;
>>> + return omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
>>
>> May be I am speculating here and I know this is tested and supposed to
>> work, but Isn't it safe to set parent keeping module enables.
>
> So you would rather change the functional clock while the timer is
> enabled/active? So although that we are doing this today, that does not
> sound like a good idea to me :-)
Actually, we are not doing this today. If you look at the current code
we are only enabling the timer while doing the soft-reset for omap1
devices. Hence, even in the current code we set the parent while the
timer is not enabled. So there is no actual change here in the sequence.
Cheers
Jon
--
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
