LED devices

[Richard Hughes]

I've talked with other kernel guys here at red hat and they don't think
putting properties in the handle is a good idea.

I've cc'd Kay, Greg KH and a few other developers for comments.

To summarize, the LED device class creates a device with a ":" delimited
handle, where properties that are not going to changed get included in
the handle name for userspace to parse with sscanf. For
example, /sys/class/leds/thinklight:blue:keyboard_backlight/brightness

I think this breaks the one-value-per sysfs file rule and sure makes it
more difficult to extract information in HAL. The backlight class should
have an attribute "color" and "function" so new properties can be added
(or ones that make no sense removed) without breaking API, but the
maintainer doesn't like that idea.

Comments please,

Richard.

On Wed, 2007-05-30 at 15:46 +0100, Richard Purdie wrote:
> On Wed, 2007-05-30 at 14:59 +0100, Richard Hughes wrote:
> > On Wed, 2007-05-30 at 14:31 +0100, Richard Purdie wrote:
> > > 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.
> 
> Why not? I'm sure HAL can cope with this.
> 
> > > > 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.
> 
> Entries under /sys/class/ get to define their interface. I don't see why
> that interface can't include a naming convention for the devices which
> is all this is.
> 
> > 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.
> 
> Adding another : does not break the API. Its certainly implied if not
> explicitly specified that the name field is ':' delimited and you should
> be ignoring any 3rd field or greater.
> 
> > 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.
> 
> If the interface defines it to work like that, it is a standard.
> 
> > > >  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?
> 
> Why does that matter?