Re: [PATCH 2/2] LEDS: generic driver

[Robin Farine <robin.farine@terminus.org>]

On Fri June 8 2007 01:27, Richard Purdie wrote:

> I'm not sure about this to be honest. I can see a case for
> perhaps having a couple of standard suspend/resume functions for
> platform device based LED drivers as those functions are often
> identical. I'm not sure whether there is going to be much need
> for more than that.

I am not persuaded either. To give a better idea of the context, my 
platform has to sets of LEDs, a first attached to a write-only 
register on the extension bus and the second on an optional 
daughter board and I2C controlled. The value of the register on the 
extension bus is shadowed and needs to be manipulated through an 
API that takes care of the locking.

Thus, instead of adding new board specific drivers to drivers/leds, 
my idea was that given a generic LED driver:

- the module that implements accesses to the write-only register 
defines one platform device for each LED attached to the register 
and implements the LED toggling internally;

- the I2C driver for the daughter board defines one platform device 
for each LED on the daughter board and implement the LED toggling 
internally as well.

> Having looked through a few of the LED drivers, even the
> suspend/resume functions are often different...

I looked at this too and it seemed to me that in all but one case 
the suspend function ends up by calling led_classdev_suspend for 
each LED the driver controls, and the resume function calls 
led_classdev_resume for each LED as well. In the generic LED 
driver, it is equivalent since there is a different platform device 
for each LED so only one call to led_classdev_xyz is needed.

> >From a style point of view, why not
> > s/pdev_to_led/platform_get_drvdata/?

Yes, of course :-).

> What are your thoughts on multiple LEDs on a single device?

Physically that is what I have on my platform, but the system sees 
each LED as a separate device. The platform device data and 
function for a set of LEDS attached to the same physical device 
take care of the multiplexing.

> Given the LED class is going to get a conversion to struct device
> soon, I'd prefer to put this on hold until after I've made that
> conversion at which point I'll reconsider this.

Yes, sure. I am well aware that it applies to very specific 
situations and that there may be a cleaner solution in which case I 
would be happy to drop this idea. I am not pushing for this to be 
included in mainline.

Thanks for looking into this and for the feed-back,

Robin