LED devices

[Richard Hughes]

On Wed, 2007-05-30 at 14:31 +0100, Richard Purdie wrote:
> On Wed, 2007-05-30 at 14:16 +0100, Richard Hughes wrote:
> > On Wed, 2007-05-30 at 13:55 +0100, Richard Purdie wrote:
> > > 
> > > > What's the big problem having names such as "thinklight" and then
> > > > another attribute of "color"?
> > > 
> > > Each attribute uses memory for the sysfs entry and code for the access
> > > functions. The value will never change. Exporting the information as
> > > part of the device name seemed like a neat solution.
> > 
> > NO! This is not what sysfs was designed for. In that logic:
> > 
> > /sys/battery/batt1:lion:rechargeable:sanyo:14400
> > /sys/battery/batt1::rechargeable::11000
> > 
> > Would be a valid devices. This means _nothing_ to userspace.
> 
> It depends whether the device names have a defined meaning or not. The
> LED device names were defined in a particular way such that the colour
> was extractable. Certainly you could never do this with device
> parameters that could change (and for the LED class, colour is defined
> to be fixed, there are comments on bi/tri/multi coloured LEDs in the
> docs).

Sure, I understand that.

> To be honest, the LED device names are a lot more meaningful than
> "batt1", "batt2" and "batt3" which really do tell you absolutely
> nothing.

That's not the point. We shouldn't be scanning for magic tokens in the
device name in sysfs.

> > If we have to parse the values in sysfs device names then we might as
> > well go back to the horrors that are /proc. This is bad. One value per
> > file, simple as that.
> 
> This doesn't break that rule. We just encode some information into the
> device name. You don't need that information to control the LED and it
> is machine extractable.

Yes, but so is all the stuff in /proc - but this broke everytime the
interface changed. Scanning for tokens is wrong.

If you have the led device with extra parameters you can add parameters
without breaking API, for instance, adding a color attribute would only
be added to devices that have a colour.

Similarly, if the device (like the thinkpad leds) supports hardware
flashing, then an attribute could be added to define what duty cycle the
flash exhibits. Maybe a silly idea, but helps explain my point.

If the LED has a meaningful descriptor, then this too can be exposed,
without breaking API, adding yet another ":" entry.

Then the device would look like the other /sys/class entries and be easy
to interface with in userspace/hal.

/sys/class/leds/power
 brightness
 description
 color
 subsystem/
 trigger
 uevent

> The whole idea behind /sys/class/ is you define a standard interface
> which is exactly what the LED class does.

Using:delimited:names:in:the:title is not standard, and I've only seen
it used in limited numeric form with SCSI devices and PCI devices, not
freeform text descriptors.

> >  I'm surprised this hasn't been ripped to pieces on LKML already.
> 
> It was discussed, no objections were raised.

But what actual userspace code actually uses the led class at the
moment?

Richard.