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

[NeilBrown]

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?

> 
> > 
> > > +	omap_dm_timer_set_load(omap->dm_timer, autoreload, load_value);
> > > +	omap_dm_timer_set_match(omap->dm_timer, enable, match_value);
> > > +
> > > +	dev_dbg(chip->dev,
> > > +			"load value: %#08x (%d), "
> > > +			"match value: %#08x (%d)\n",
> > > +			load_value, load_value,
> > > +			match_value, match_value);
> > > +
> > > +	omap_dm_timer_set_pwm(omap->dm_timer,
> > > +			      !omap->polarity,
> > > +			      toggle,
> > > +			      trigger);
> > > +
> > > +	/* Set the counter to generate an overflow event immediately. */
> > > +
> > > +	omap_dm_timer_write_counter(omap->dm_timer, DM_TIMER_LOAD_MIN);
> > > +
> > > +	/* Now that we're done configuring the dual-mode timer, disable it
> > > +	 * again. We'll enable and start it later, when requested.
> > > +	 */
> > > +
> > > +	omap_dm_timer_disable(omap->dm_timer);
> > 
> > Similarly the disable should not be needed here either.
> > 


Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature