PWM class? (was: Re: MPC52xx simple GPIO support)

PWM class? (was: Re: MPC52xx simple GPIO support)

[Anton Vorontsov]

On Wed, Jun 03, 2009 at 02:42:26PM +0200, Stefan Strobl wrote:
[...]
> The led class provides support for setting the brightness, which
> obviously the gpio driver doesn't support. The hardware (mpc52xx_gpt)
> would support it in PWM mode though. I'm now wandering how this could be
> best implemented.
> 
> 1) - Create some PWM class similar to the GPIO class
>    - Add support for PWM mode in mpc52xx_gpt.c that uses that PWM class
>    - And add an interface for the LED to use the PWM class
> 
> 2) - Create an LED driver that accesses the mpc52xx_gpt directly.
> 
> I think I would be overwhelmed trying to implement (1) but am confident
> to do (2). What do you think is the right approach?

I'd suggest creating a generic PWM class, i.e. PWMLIB, alike to
GPIOLIB. (2) can be an acceptable approach for now, but for the
long-term solution (1) is the way to go.

The non-lib PWM API is already there, see include/linux/pwm.h,
and arch/arm/mach-pxa/pwm.c as an implementation example.

Note that PXA implementation is SOC-specific, which is not very
good.

So I'd suggest creating drivers/pwm/pwmlib.c, borrowing
ideas from gpiolib. And then we can reuse drivers/leds/leds-pwm.c
driver (of course, after adding appropriate OF code into it).

Sure, as you've said, it could be quite boringly to implement,
could take quite some time to pass all review cycles etc.
But someday someone will have to do this. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: PWM class? (was: Re: MPC52xx simple GPIO support)

[Jon Smirl]

On Wed, Jun 3, 2009 at 9:22 AM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Wed, Jun 03, 2009 at 02:42:26PM +0200, Stefan Strobl wrote:
> [...]
>> The led class provides support for setting the brightness, which
>> obviously the gpio driver doesn't support. The hardware (mpc52xx_gpt)
>> would support it in PWM mode though. I'm now wandering how this could be
>> best implemented.
>>
>> 1) - Create some PWM class similar to the GPIO class
>>    - Add support for PWM mode in mpc52xx_gpt.c that uses that PWM class
>>    - And add an interface for the LED to use the PWM class
>>
>> 2) - Create an LED driver that accesses the mpc52xx_gpt directly.
>>
>> I think I would be overwhelmed trying to implement (1) but am confident
>> to do (2). What do you think is the right approach?
>
> I'd suggest creating a generic PWM class, i.e. PWMLIB, alike to
> GPIOLIB. (2) can be an acceptable approach for now, but for the
> long-term solution (1) is the way to go.

What happened to this one?

http://ozlabs.org/pipermail/linuxppc-dev/2008-October/063562.html


>
> The non-lib PWM API is already there, see include/linux/pwm.h,
> and arch/arm/mach-pxa/pwm.c as an implementation example.
>
> Note that PXA implementation is SOC-specific, which is not very
> good.
>
> So I'd suggest creating drivers/pwm/pwmlib.c, borrowing
> ideas from gpiolib. And then we can reuse drivers/leds/leds-pwm.c
> driver (of course, after adding appropriate OF code into it).
>
> Sure, as you've said, it could be quite boringly to implement,
> could take quite some time to pass all review cycles etc.
> But someday someone will have to do this. :-)
>
> --
> Anton Vorontsov
> email: cbouatmailru@gmail.com
> irc://irc.freenode.net/bd2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Jon Smirl
jonsmirl@gmail.com

Re: PWM class? (was: Re: MPC52xx simple GPIO support)

[Trilok Soni]

Hi Jon,

On Wed, Jun 3, 2009 at 9:08 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On Wed, Jun 3, 2009 at 9:22 AM, Anton Vorontsov
> <avorontsov@ru.mvista.com> wrote:
>> On Wed, Jun 03, 2009 at 02:42:26PM +0200, Stefan Strobl wrote:
>> [...]
>>> The led class provides support for setting the brightness, which
>>> obviously the gpio driver doesn't support. The hardware (mpc52xx_gpt)
>>> would support it in PWM mode though. I'm now wandering how this could be
>>> best implemented.
>>>
>>> 1) - Create some PWM class similar to the GPIO class
>>>    - Add support for PWM mode in mpc52xx_gpt.c that uses that PWM class
>>>    - And add an interface for the LED to use the PWM class
>>>
>>> 2) - Create an LED driver that accesses the mpc52xx_gpt directly.
>>>
>>> I think I would be overwhelmed trying to implement (1) but am confident
>>> to do (2). What do you think is the right approach?
>>
>> I'd suggest creating a generic PWM class, i.e. PWMLIB, alike to
>> GPIOLIB. (2) can be an acceptable approach for now, but for the
>> long-term solution (1) is the way to go.
>
> What happened to this one?
>
> http://ozlabs.org/pipermail/linuxppc-dev/2008-October/063562.html
>

Adding Bill to see if he has any updates.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: PWM class? (was: Re: MPC52xx simple GPIO support)

[Grant Likely]

On Wed, Jun 3, 2009 at 7:22 AM, Anton Vorontsov<avorontsov@ru.mvista.com> wrote:
> On Wed, Jun 03, 2009 at 02:42:26PM +0200, Stefan Strobl wrote:
> [...]
>> The led class provides support for setting the brightness, which
>> obviously the gpio driver doesn't support. The hardware (mpc52xx_gpt)
>> would support it in PWM mode though. I'm now wandering how this could be
>> best implemented.
>>
>> 1) - Create some PWM class similar to the GPIO class
>>    - Add support for PWM mode in mpc52xx_gpt.c that uses that PWM class
>>    - And add an interface for the LED to use the PWM class
>>
>> 2) - Create an LED driver that accesses the mpc52xx_gpt directly.
>>
>> I think I would be overwhelmed trying to implement (1) but am confident
>> to do (2). What do you think is the right approach?
>
> I'd suggest creating a generic PWM class, i.e. PWMLIB, alike to
> GPIOLIB. (2) can be an acceptable approach for now, but for the
> long-term solution (1) is the way to go.
>
> The non-lib PWM API is already there, see include/linux/pwm.h,
> and arch/arm/mach-pxa/pwm.c as an implementation example.
>
> Note that PXA implementation is SOC-specific, which is not very
> good.
>
> So I'd suggest creating drivers/pwm/pwmlib.c, borrowing
> ideas from gpiolib. And then we can reuse drivers/leds/leds-pwm.c
> driver (of course, after adding appropriate OF code into it).

Ugh.  The referenced pwm api is about as trivial as it gets; it is an
anonymous context pointer (anonymous struct pwm_device *) with a set
of accessor functions.  PWMs are also not nearly as common as GPIO
pins, and I am not interested in gpiolib being duplicated for PWMs, at
least not until there are more that just two examples of use to draw
from.

If anything, I'd rather struct pwm_device be non-anonymous and contain
a set of ops which call directly into the driver.  That way is at
least multiplatform friendly.  I don't think the gpio API is the
example to follow here.  But even then I think it is premature to try
and define a PWM api.  Personally, I'd modify mpc52xx_gpt to export
its own PWM interface for the time being using the existing GPIO
infrastructure to find the appropriate pin.

If you do decide to do a generic PWM api, then I think the way to go
is to build it as an extension to gpiolib.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: PWM class? (was: Re: MPC52xx simple GPIO support)

[Anton Vorontsov]

On Thu, Jun 11, 2009 at 04:00:51PM -0600, Grant Likely wrote:
> On Wed, Jun 3, 2009 at 7:22 AM, Anton Vorontsov<avorontsov@ru.mvista.com> wrote:
> > On Wed, Jun 03, 2009 at 02:42:26PM +0200, Stefan Strobl wrote:
> > [...]
> >> The led class provides support for setting the brightness, which
> >> obviously the gpio driver doesn't support. The hardware (mpc52xx_gpt)
> >> would support it in PWM mode though. I'm now wandering how this could be
> >> best implemented.
> >>
> >> 1) - Create some PWM class similar to the GPIO class
> >>    - Add support for PWM mode in mpc52xx_gpt.c that uses that PWM class
> >>    - And add an interface for the LED to use the PWM class
> >>
> >> 2) - Create an LED driver that accesses the mpc52xx_gpt directly.
> >>
> >> I think I would be overwhelmed trying to implement (1) but am confident
> >> to do (2). What do you think is the right approach?
> >
> > I'd suggest creating a generic PWM class, i.e. PWMLIB, alike to
> > GPIOLIB. (2) can be an acceptable approach for now, but for the
> > long-term solution (1) is the way to go.
> >
> > The non-lib PWM API is already there, see include/linux/pwm.h,
> > and arch/arm/mach-pxa/pwm.c as an implementation example.
> >
> > Note that PXA implementation is SOC-specific, which is not very
> > good.
> >
> > So I'd suggest creating drivers/pwm/pwmlib.c, borrowing
> > ideas from gpiolib. And then we can reuse drivers/leds/leds-pwm.c
> > driver (of course, after adding appropriate OF code into it).
> 
> Ugh.  The referenced pwm api is about as trivial as it gets; it is an
> anonymous context pointer (anonymous struct pwm_device *) with a set
> of accessor functions.  PWMs are also not nearly as common as GPIO
> pins, and I am not interested in gpiolib being duplicated for PWMs, at
> least not until there are more that just two examples of use to draw
> from.

I didn't say that we should duplicate gpiolib. I said that we
might borrow some ideas. ;-)

> If anything, I'd rather struct pwm_device be non-anonymous and contain
> a set of ops which call directly into the driver.  That way is at
> least multiplatform friendly.  I don't think the gpio API is the
> example to follow here.  But even then I think it is premature to try
> and define a PWM api.  Personally, I'd modify mpc52xx_gpt to export
> its own PWM interface for the time being using the existing GPIO
> infrastructure to find the appropriate pin.

Jon Smirl found that there were already some efforts put
into making generic PWM class:

http://thread.gmane.org/gmane.linux.kernel.embedded/1160

Briefly looking, the class is pretty cool. Somebody should just
continue this work.

> If you do decide to do a generic PWM api, then I think the way to go
> is to build it as an extension to gpiolib.

I don't think David Brownell will like this idea. But
who knows... maybe.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2