LED devices

[Richard Hughes]

On Wed, 2007-05-30 at 17:19 +0100, Richard Purdie wrote:
> 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/spitz:amber/
> /sys/class/leds/spitz:green/

/sys/class/leds/spitz_amber/
/sys/class/leds/spitz_green/

makes most sense to me.

> Yes, we could easily add a colour attribute to the class but since the
> information is already available in a machine readable place, why waste
> the memory for the directory entries and code to access them?

To make it easy for userspace.

The consumer of this is _userspace_ - we shouldn't be making crazy
optimizations to save a few hundred bytes. When we add other attributes
("flash_duty_cycle" for instance) we can just add a new attribute to
devices that have this capability, rather than just tag on another
":98094832" parameter.

> The point of the class interface is to define a standardised way of
> accessing a type of hardware. The LED class has a well defined (and even
> documented) interface.

But fundamentally different from most other sysfs classes. If you are
putting stuff in /sys you need to stick to the one value per file type
mentality.

> If reading names from a string causes HAL problems, its not really
> living up to its name...

Not real problems, but every other device class can just be probed by
reading one file. The leds class needs HAL to do sscanf on the device
name to scrape key properties. This isn't very nice.

> There have  been discussions about a function attribute and to me,
> that's still a part of the LED name as it identifies the specific LED,
> particularly when a device has LEDs with the same colour. For my example
> that would become:
> 
> /sys/class/leds/spitz:amber:charging/
> /sys/class/leds/spitz:green:hdd-activity/

This is just wrong. Just declare an attribute "identifier" and then put
the description there.

> The documentation does state "The naming scheme above leaves scope for
> further attributes should they be needed" clearly implying its delimited
> by the ':' character so anyone doing sscanf("%s:%s",...) didn't read the
> documentation.

Broken.

> No decision about including the function attribute has yet been made
> although it appears to make sense and is compatible with the defined
> 'API' aka class interface.

No, in my opinion, it's fundamentally different.

Richard.