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

[Grant Likely]

On Wed, Jun 3, 2009 at 7:22 AM, Anton Vorontsov<avorontsov@xxxxxxxxxxxxx> 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.
--
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/