LED devices

[Greg KH]

On Wed, May 30, 2007 at 05:19:31PM +0100, Richard Purdie wrote:
> On Wed, 2007-05-30 at 16:34 +0100, Richard Hughes wrote:
> > 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.
> 
> I will put my side (as the above maintainer) across. When deciding on
> the naming for the LEDs in /sys/class/leds/ we had several choices.
> Taking my handheld as an example, some choices were:
> 
> /sys/class/leds/led0/
> /sys/class/leds/led1/

Yes.

> /sys/class/leds/spitz0/
> /sys/class/leds/spitz1/

Yes.

> /sys/class/leds/spitz:amber/
> /sys/class/leds/spitz:green/

No way!  Don't do that.

> The first contains no useful information, the second one is only
> fractionally better, the third is actually quite useful. Faced with the
> third name, a person can actually point to the right LED on the device.

So?  You need to read the attributes in the sysfs device directory to
find out exactly what type of device this is, what it does, and all
sorts of other information.

The first two examples above are correct.  They use a "bus id" type
naming scheme, like ALL OTHER DEVICES IN THE KERNEL.  The only
requirement is that it is unique.

Userspace programs, like HAL, can handle the fact that spitz0 is really
a gree LED and it means that the device is charging.  That all comes
from the individual sysfs files.

So please, don't imbend sysfs attributes into the bus id for the device,
that's just wrong on so many levels...

> As far as the class is concerned, LED colour is a static property (it
> could handle multi coloured through multiple LED devices).

Yes, and as a property, it needs to be an attribute, not the bus id.

Do you really want to see /sys/bus/usb/devices/usb:hub23 or
/sys/bus/usb/devices/usb:nerf_rocket_launcher3 ?  No, just read the
attributes to determine the type of the device, like all other devices.

Please, if this is the way that the code is currently working, change it
now.

thanks,

greg k-h