linux-kernel - [patch] Move led attributes out of device name and into sysfs attributes, was Re: LED devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 01 Jun 2007 16:04:30 +0100
From:	Richard Hughes <hughsient@...il.com>
To:	Greg KH <greg@...ah.com>
Cc:	"kay.sievers" <kay.sievers@...y.org>,
	Richard Purdie <rpurdie@...nedhand.com>,
	"Bryn M. Reeves" <breeves@...hat.com>,
	John Lenz <lenz@...wisc.edu>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: [patch] Move led attributes out of device name and into sysfs
	attributes, was Re: LED devices

On Thu, 2007-05-31 at 00:24 -0700, Greg KH wrote: 
> 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.

Kay,

Patch attached corrects all the brokenness with the led class (encoding
some attributes in the device handle).

 Documentation/leds-class.txt    |   13 --------
 drivers/leds/led-class.c        |   62 ++++++++++++++++++++++++++++++++++++++--
 drivers/leds/leds-ams-delta.c   |   18 +++++++----
 drivers/leds/leds-corgi.c       |    8 +++--
 drivers/leds/leds-h1940.c       |   12 +++++--
 drivers/leds/leds-locomo.c      |    8 +++--
 drivers/leds/leds-net48xx.c     |    3 +
 drivers/leds/leds-spitz.c       |    8 +++--
 drivers/leds/leds-tosa.c        |    8 +++--
 drivers/leds/leds-wrap.c        |    6 ++-
 drivers/macintosh/via-pmu-led.c |    1 
 drivers/misc/asus-laptop.c      |    3 +
 include/linux/leds.h            |    2 +
 13 files changed, 115 insertions(+), 37 deletions(-)

Signed-off-by: Richard Hughes <richard@...hsie.com>

Please review attached patch, thanks.


View attachment "leds-add-colour-and-description.patch" of type "text/x-patch" (12142 bytes)