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

From: Robin Farine
Date: Fri Jun 08 2007 - 04:48:31 EST


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
-
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/