Re: [PATCHv5 2/2] leds: tlc591xx: Driver for the TI 8/16 Channel i2c LED driver

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

Hi,

On 26/01/15 19:10, Andrew Lunn wrote:
>> So... To me it's still slightly unclear when should one write a PWM
>> driver and when a LED driver. But I would say that as the TLC591xx
>> outputs a PWM signal, it should be a PWM driver. Then the different
>> users of this PWM signal could be made on top of that (LED, backlight, GPO).
>>
>> What would be the technical drawbacks with having the TLC591xx driver as
>> a PWM, instead of LED?
>  
> Hi Tomi
> 
> We have been through this once, but the big technical drawback is that
> this hardware cannot do what the Linux Kernel defines as PWM.
> 
> It cannot correctly implement the PMW call:
> 
> int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> 
> This hardware has a fixed period, since it is clocked at 97-kHz.  So
> you cannot set the period. The duty is also somewhat restrictive, in
> that it only allows 1/256 increments of the 97Khz.

Surely other PWM devices also have restrictions in the period or duty cycle?

> This hardware does however perfectly fit the LED API:
> 
> enum led_brightness {
>         LED_OFF         = 0,
>         LED_HALF        = 127,
>         LED_FULL        = 255,
> };
> 
>         void            (*brightness_set)(struct led_classdev *led_cdev,
>                                           enum led_brightness brightness);
> 
> So we can model it perfectly as an LED driver, or badly as a PWM
> driver.

Maybe so, but what does it mean in practice? If it's implemented as a
PWM driver, and the existing leds-pwm driver is used for the led
functionality, shall we miss some brightness values? Is the
configuration more difficult? Or what?

So my point here is that it outputs a PWM signal, so it makes sense to
have it as a PWM driver. It has restricted configurability compared to
more versatile PWM hardware, but I so far don't see why that would be an
issue.

If it is a PWM driver, we get backlight support for free via the
existing pwm_bl driver, and LED support via leds-pwm. And there has been
a clear acceptance on GPO functionality with PWM outputs (in the Peter's
mail thread), whereas I would bet that a LED based GPO functionality
would encounter resistance, because that doesn't quite make sense.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]