On Tue, Aug 18, 2015 at 05:09:16PM +0200, Andrew Lunn wrote: > > On the other hand, there's pwm_bl.c which give us backlight device > > with PWM, > > Lets look at this. A backlight device seems to do most of its work in > the update_status callback. It is given a brightness in > bl->props.brightness, which takes a value between 0 and > props.max_brightness. > > What pwm_bl.c does it then turn this brightness value into an > artificial PWM configuration. Your proposed PWM driver then turns this > back into a brightness, since you don't actually implement the period > part of the PWM interface. > > From an architecture point of view, doesn't an LED class device, which > takes a brightness value, seem much more naturally? > > It seems like implementing a generic led_bl.c driver would make sense. > It would also allow some of the code in drivers/video/backlight/ to be > eliminated. There seems to be both an LED class driver for lp55xx and > a blacklight driver lp855x_bl.c. There are duplicate lp8788, 88pm860x, > adp5520, da903x, da9052, hp6xx, and lm3533 drivers which might all be > removed if a led_bl.c generic driver existed. > > > and a GPIO over PWM sounds more sane to me than GPIO over LED. > > Currently two LED class drivers are calling gpiochip_add: > > ~/linux/drivers/leds$ grep gpiochip_add *.c > leds-pca9532.c: err = gpiochip_add(&data->gpio); > leds-tca6507.c: err = gpiochip_add(&tca->gpio); > > The pca9532 has full GPIO capabilities, in as well as out. But it > seems like tca6507 is output only. The TLC59108/TLC59116 is also > output only. So a generic GPO driver on top of LED would make sense > for these two, and save some code/bugs. > > From a stand back, lets take a look at the architecture point of view, > generic led_bl and gpio-led drivers seem to make sense. I agree it's sensible for there to be a driver that can drive a backlight from an LED. PWM is somewhat of a lower level API than the LED API, so there are cases where LED fits better than PWM. This particular case seems to be one of them. Thierry
Attachment:
signature.asc
Description: PGP signature
