LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Richard Purdie <rpurdie@rpsys.net>
To: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: LED naming standard for LED class
Date: Mon, 17 Mar 2008 09:51:55 +0000	[thread overview]
Message-ID: <1205747515.4552.15.camel@dax.rpnet.com> (raw)
In-Reply-To: <20080317033458.GA20486@khazad-dum.debian.net>

On Mon, 2008-03-17 at 00:34 -0300, Henrique de Moraes Holschuh wrote:
> Richard, isn't there *any* way to change this new ABI?  It has not been in
> any stable mainline release yet, so it is not too late if we do it now.

It is not an new ABI, its an extension as per the way it was documented
to work.

> Even if the lenght limit for sysfs entries (19 chars + NULL) is lifted in
> 2.6.26, the current proposal that would become stable in 2.6.25 is still
> ugly, and not nearly close to the best we could do, IMHO.
> 
> The only technical reason I can see for the "devicename" part of the LED
> name is to avoid clashes (or as a placeholder when you have no function).
> However, it is the least important information for any sort of generic LED
> tool (which is the only reason to even bother with a LED naming standard to
> begin with), so it certainly should not come first in the name.  And there
> are other ways to avoid clashes that waste far less real state than the
> device name.
>
> Can't we have "function[:color].<number>" instead?   And allow for a choice
> of "devicename" or simply "led" instead of "function" for leds with no
> defined or implied function?
> 
> It places fields in order of importance (thus it is far more user-friendly),
> it wastes less real state (typically just two chars) to avoid clashes, and
> it also follows what we already have on other sysfs subsystems (except for
> the :color part, and I agree with you that it is a desireable thing to have
> in there).
> 
> The clash-avoiding ".%d" postfix would be taken care of by the class.  If
> two devices are registered with a "battery:green" name, you'd end up with
> "battery:green.0" and "battery:green.1".  If just one device tries to
> register "battery:yellow", you'd get "battery:yellow.0".
> 
> Later (for 2.6.26) I'd also suggest adding the color notion to the *class*
> itself, including the notion of "undefined" color.  The class would take
> care to add the ":color" part of the node name (when it is not "undefined")
> and *also* export the color as an attribute. I can write this patch too, if
> you want.
> 
> Would you take patches to implement the "function[:color].<ordinal>" naming
> scheme, and try to merge them into 2.6.25?  Avoiding a bad ABI becoming
> stable *is* an acceptable reason for a late merge.

The problem is the "bad" ABI has existed in that form since somewhere
around 2.6.15 and it is therefore too late to drop things from it or
rearrange it. The ABI described additions being allowed so we're taking
advantage of that to gain something which should have really been there
originally. Short term there is some complexity with some limits but
they're going away so there is no problem.

The whole point of the naming was so at a glance you can tell which LEDs
are which and whilst the device name isn't important in the things
you're considering, in general it does make sense, particularly if we
see LEDs attached to removable hardware for example.

The alternative is we throw the whole thing away and go to random
numbers for the different LEDs and attributes. To work out which LED is
which you then have to read up to 3 files per LED before you can even
decide whether its the one you want. I don't like that idea...

Cheers,

Richard



  reply	other threads:[~2008-03-17  9:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-07 10:35 [GIT PULL] LED updates Richard Purdie
2008-02-07 21:38 ` Henrique de Moraes Holschuh
2008-02-07 22:13   ` Richard Purdie
2008-02-07 23:23     ` [PATCH] leds: disable triggers on brightness set Henrique de Moraes Holschuh
2008-02-10 11:52       ` Németh Márton
2008-02-17 23:30         ` Richard Purdie
2008-02-18  1:59           ` Henrique de Moraes Holschuh
2008-02-08  7:03     ` [GIT PULL] LED updates Németh Márton
2008-02-08 11:20       ` Henrique de Moraes Holschuh
2008-02-10 11:52         ` Németh Márton
2008-03-16 18:18     ` Henrique de Moraes Holschuh
2008-03-16 19:29       ` Richard Purdie
2008-03-16 19:48         ` Henrique de Moraes Holschuh
2008-03-17  3:34           ` LED naming standard for LED class Henrique de Moraes Holschuh
2008-03-17  9:51             ` Richard Purdie [this message]
2008-03-18  3:35               ` Henrique de Moraes Holschuh
2008-03-18  4:55                 ` Henrique de Moraes Holschuh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1205747515.4552.15.camel@dax.rpnet.com \
    --to=rpurdie@rpsys.net \
    --cc=hmh@hmh.eng.br \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: LED naming standard for LED class' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).