Re: [PATCH] OMAP: add pwm driver using dmtimers.

[Jon Hunter]

On 12/12/2012 10:33 PM, NeilBrown wrote:
> On Thu, 13 Dec 2012 14:06:35 +1100 NeilBrown <neilb@xxxxxxx> wrote:
> 
>>>> +	omap_dm_timer_enable(omap->dm_timer);
>>>
>>> Do you need to call omap_dm_timer_enable here? _set_load and _set_match
>>> will enable the timer. So this should not be necessary.
>>
>> True.  That is what you get for copying someone else's code and not
>> understanding it fully.
> 
> However .... omap_dm_timer_write_counter *doesn't* enable the timer, and
> explicitly checks that it is already runtime-enabled.
> 
> Does that mean I don't need to call omap_dm_timer_write_counter here?  Or
> does it mean that I do need the enable/disable pair?

Typically, omap_dm_timer_write_counter() is used to update the counter
value while the counter is running and hence is enabled.

Looking at the code, some more I now see what they are trying to do. It
seems that they are trying to force an overflow to occur as soon as they
enable the timer. This will cause the timer to load the count value from
the timer load register into the timer counter register. So that does
make sense to me. However, this should not be necessary as
omap_dm_timer_set_load should do this for you. Therefore, I think that
you could accomplish the same thing by doing ...

omap_pwm_config
	--> omap_dm_timer_set_load()
	--> omap_dm_timer_set_match()
	--> omap_dm_timer_set_pwm()

omap_pwm_enable
	--> omap_dm_timer_start()

If we call _set_load in config then we don't need to call _load_start in
the enable, we can just call _start.

Can you try this and see if this is working ok?

Cheers
Jon
--
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/