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

From: Thierry Reding
Date: Thu Dec 13 2012 - 02:10:56 EST


On Thu, Dec 13, 2012 at 02:06:35PM +1100, NeilBrown wrote:
>
> [Thierry: question for you near the end - thanks]
>
> On Wed, 12 Dec 2012 10:08:28 -0600 Jon Hunter <jon-hunter@xxxxxx> wrote:
>
> > Hi Neil,
> >
> > On 12/12/2012 02:24 AM, NeilBrown wrote:
[...]
> > > +{
> > > + struct omap_chip *omap = platform_get_drvdata(pdev);
> > > + int status = 0;
> > > +
> > > + status = pwmchip_remove(&omap->chip);
> > > + if (status < 0)
> > > + goto done;
> > > +
> > > + omap_dm_timer_free(omap->dm_timer);
> >
> > Is it guaranteed that the timer will be disabled at this point?
>
> Uhmm... it seems that pwm_put() doesn't call pwm_disable(), so I guess it
> might not be disabled.
> Thierry: should pwm_put do that, or do I need a 'free' function in my chip
> ops to do that?

To be honest, I haven't decided yet. =) There have been discussions that
resulted in a request to run pwm_disable() from pwmchip_remove() on all
PWM devices a chip provides.

This isn't implemented yet and I'm not sure about all the side-effects.
I think for now the best way would be to implement .free() within this
driver, or even do an explicit pwm_disable() in the driver's .remove()
function to do this. When I've come to a decision I'll refactor all of
that in one patch across the whole subsystem.

Thierry

Attachment: pgp00000.pgp
Description: PGP signature