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 <[email protected]>
Please review attached patch, thanks.
On Fri, 2007-06-01 at 16:04 +0100, Richard Hughes wrote:
> Patch attached corrects all the brokenness with the led class (encoding
> some attributes in the device handle).
For the benefit of LKML, there has been some discussion of this
elsewhere. My arguments do not particularly come across in Richard's
choice of quoting and I'm a little annoyed he's chosen to do things this
way, particularly before any further feedback from Greg/Kay but so be
it.
Greg said that the LEDs class should export any property data as a class
attribute rather than this just being available in the device name. I
added the patch in
http://git.o-hand.com/?p=linux-rpurdie-leds;a=shortlog;h=for-mm to deal
with this, without breaking the existing LED naming and documented API.
Greg said that the only requirement on bus id naming was that it needed
to be unique so this should therefore be compatible. HAL could then go
ahead and ignore the device name, all the attributes are there in the
fashion it needs which was the original complaint.
I accept this is basically out of my hands now. Greg/Kay have the
appropriate emails and if they'll let me know which approach they want
to take, I'll apply an appropriate patch. I'd suggest not applying this
patch directly as it introduces a race in the error path it alters.
Richard
On Fri, 2007-06-01 at 16:43 +0100, Richard Purdie wrote:
> On Fri, 2007-06-01 at 16:04 +0100, Richard Hughes wrote:
> > Patch attached corrects all the brokenness with the led class (encoding
> > some attributes in the device handle).
>
> For the benefit of LKML, there has been some discussion of this
> elsewhere. My arguments do not particularly come across in Richard's
> choice of quoting and I'm a little annoyed he's chosen to do things this
> way, particularly before any further feedback from Greg/Kay but so be
> it.
To be honest, I didn't want to do this, but you would not accept my
argument - and you wanted to add _more_ properties into the device
handle.
> Greg said that the LEDs class should export any property data as a class
> attribute rather than this just being available in the device name. I
> added the patch in
> http://git.o-hand.com/?p=linux-rpurdie-leds;a=shortlog;h=for-mm to deal
> with this, without breaking the existing LED naming and documented API.
Yes, but as you'll notice with my patch, lots of the users that use the
led class do not use the handle:colour name, and so stripping off
second : parameter for the color attribute was just wrong. And ugly.
> Greg said that the only requirement on bus id naming was that it needed
> to be unique so this should therefore be compatible. HAL could then go
> ahead and ignore the device name, all the attributes are there in the
> fashion it needs which was the original complaint.
But when I was investigating adding led class support to thinkpad_acpi I
couldn't use the existing LED class, as some of the LED's did not have a
pre-defined colour, but did have a description. Again, adding support
into HAL was made more difficult until you proposed screenscaping the
led colour from the device name. This isn't very nice.
> I accept this is basically out of my hands now. Greg/Kay have the
> appropriate emails and if they'll let me know which approach they want
> to take, I'll apply an appropriate patch. I'd suggest not applying this
> patch directly as it introduces a race in the error path it alters.
Sure, I really wanted to convert the class to struct device (which I'm
more familiar with) but just tried to do minimal changes. What changes
need to be made to avoid a race? Thanks.
Richard.
On Fri, 2007-06-01 at 16:59 +0100, Richard Hughes wrote:
> On Fri, 2007-06-01 at 16:43 +0100, Richard Purdie wrote:
> > I accept this is basically out of my hands now. Greg/Kay have the
> > appropriate emails and if they'll let me know which approach they want
> > to take, I'll apply an appropriate patch. I'd suggest not applying this
> > patch directly as it introduces a race in the error path it alters.
>
> Sure, I really wanted to convert the class to struct device (which I'm
> more familiar with) but just tried to do minimal changes. What changes
> need to be made to avoid a race? Thanks.
It has nothing to do with the struct device vs. class devices, you need
to think more carefully about the placement of the list_del().
My other concerns with this patch are that it exports incorrect
information on spitz (which I had warned about and I will get bug
reports about) and that "description" is not really a suitable name for
a sysfs attribute, "function" might give a better idea of what it
represents to developers.
I still question whether either colour or function properties are
actually particularly useful to userspace other than for naming purposes
which is why they were part of the device name.
Anyhow, I'm trying not to say too much more as it will just go in
circles. I'll await a reply from Greg.
Regards,
Richard
On Fri, Jun 01, 2007 at 05:23:24PM +0100, Richard Purdie wrote:
> On Fri, 2007-06-01 at 16:59 +0100, Richard Hughes wrote:
> > On Fri, 2007-06-01 at 16:43 +0100, Richard Purdie wrote:
> > > I accept this is basically out of my hands now. Greg/Kay have the
> > > appropriate emails and if they'll let me know which approach they want
> > > to take, I'll apply an appropriate patch. I'd suggest not applying this
> > > patch directly as it introduces a race in the error path it alters.
> >
> > Sure, I really wanted to convert the class to struct device (which I'm
> > more familiar with) but just tried to do minimal changes. What changes
> > need to be made to avoid a race? Thanks.
>
> It has nothing to do with the struct device vs. class devices, you need
> to think more carefully about the placement of the list_del().
>
> My other concerns with this patch are that it exports incorrect
> information on spitz (which I had warned about and I will get bug
> reports about) and that "description" is not really a suitable name for
> a sysfs attribute, "function" might give a better idea of what it
> represents to developers.
>
> I still question whether either colour or function properties are
> actually particularly useful to userspace other than for naming purposes
> which is why they were part of the device name.
>
> Anyhow, I'm trying not to say too much more as it will just go in
> circles. I'll await a reply from Greg.
Hm, I thought I made my opinion pretty clear in the beginning.
Why not just do a simple:
led01
led02
led03
...
and so on?
And use the 'name' field to put the name of your device (disk,
bluetooth, etc.) This is the way all other busses and devices work, and
I don't think that LEDs are anything more "special" than anything else
in the kernel, right?
So the original patch in this thread is close, but not quite there.
thanks,
greg k-h
On Fri, 2007-06-08 at 11:57 -0700, Greg KH wrote:
> On Fri, Jun 01, 2007 at 05:23:24PM +0100, Richard Purdie wrote:
> > I still question whether either colour or function properties are
> > actually particularly useful to userspace other than for naming purposes
> > which is why they were part of the device name.
> >
> > Anyhow, I'm trying not to say too much more as it will just go in
> > circles. I'll await a reply from Greg.
>
> Hm, I thought I made my opinion pretty clear in the beginning.
>
> Why not just do a simple:
> led01
> led02
> led03
> ...
>
> and so on?
If we do this, my point is we're exporting something to userspace which
is totally devoid of information.
sysfs is for userspace and "led01" means absolutely nothing to it. The
most you can summarise from it is it happens to have registered first
and its an LED. Since its in class/leds/ we can summarise the latter
without help from the prefix. I'd hate to think userspace cares about
the former.
> And use the 'name' field to put the name of your device (disk,
> bluetooth, etc.) This is the way all other busses and devices work, and
> I don't think that LEDs are anything more "special" than anything else
> in the kernel, right?
Since the "busid" field means absolutely nothing, why not give it a
sensible id and save the overhead of a separate name?
If kernel policy is that we should have useless data in sysfs, so be it,
I just make sure that is on record and then break the defined LED class
API.
<serious mode>
Ok, so I make the point above with a sledge hammer. There are more
subtle issues here too. People are asking for a ton of extra attributes
for the LED class. We can have a name, a colour, an LED "function", a
location on the device and probably 101 other things.
As I understand it, sysfs was put there to pass information *the kernel
knows* to userspace. The kernel doesn't actually care about the location
of an LED or its colour. All the kernel cares is we have an LED and it
has a certain brightness.
Yes, we can teach the kernel all this extra info but it is simply to
pass to userspace. Why should my kernel be bloated with all that extra
information which it doesn't need itself?
My conclusion is that there should be something in userspace
supplementing this information from the kernel. Going back the the
problem at hand, HAL is already equipped to do this, all it needs is
some kind of unique ID so it can identify the LED. This would be the
busid.
So I do stand by what the LED class does as being sane. Yes, the class
defined a way of writing its busids which isn't like any other but there
is a good reason for it. We could make it machine readable to provide
hints to userspace and I never saw any reason why we shouldn't have
done. It was discussed on LKML and no objections were raised (it was in
fact requested). The choice of field separator is unfortunate since its
developed other meanings after it was used in the LED class, such is
life.
So there are various options:
1. We keep 'useful' busids and the LED class continues as is. We accept
classes can provide a busid naming policy if it makes sense.
2. We adopt free form busids and export a ton of attributes just useful
to userspace (bloating the kernel IMO).
3. As 2. but use 'useless' busids that mean nothing.
4. We adopt free form busids but let userspace fill in its own
information based on the busid (no policy on naming).
5. Other combinations thereof.
I'm very aware I'm isolated here and ultimately this is probably Greg's
decision which I will end up abiding by. If anyone else does have a
view, speak now ;-). I think there are some important issues here and
they do need clarification, one way or another.
Cheers,
Richard
On Sat, Jun 09, 2007 at 12:02:28AM +0100, Richard Purdie wrote:
> On Fri, 2007-06-08 at 11:57 -0700, Greg KH wrote:
> > On Fri, Jun 01, 2007 at 05:23:24PM +0100, Richard Purdie wrote:
> > > I still question whether either colour or function properties are
> > > actually particularly useful to userspace other than for naming purposes
> > > which is why they were part of the device name.
> > >
> > > Anyhow, I'm trying not to say too much more as it will just go in
> > > circles. I'll await a reply from Greg.
> >
> > Hm, I thought I made my opinion pretty clear in the beginning.
> >
> > Why not just do a simple:
> > led01
> > led02
> > led03
> > ...
> >
> > and so on?
>
> If we do this, my point is we're exporting something to userspace which
> is totally devoid of information.
Just like all other subsystems?
No, the only information the device name is that it shows a UNIQUE name
at that point in time in the kernel. Heck, we could just use random
numbers that are unique like some other people have suggested, it would
mean the exact same thing.
> sysfs is for userspace and "led01" means absolutely nothing to it.
Wrong, the attributes in that directory mean everything. The name means
nothing.
> The most you can summarise from it is it happens to have registered
> first and its an LED.
Exactly.
> Since its in class/leds/ we can summarise the latter without help from
> the prefix. I'd hate to think userspace cares about the former.
Ok, then use a random number please. Start with 00000000000001 and just
increment from there, or use the idr subsystem.
I was trying to be nice and at least give you a prefix that looked kind
of sane to people, but if you want to be difficult... :)
>
> > And use the 'name' field to put the name of your device (disk,
> > bluetooth, etc.) This is the way all other busses and devices work, and
> > I don't think that LEDs are anything more "special" than anything else
> > in the kernel, right?
>
> Since the "busid" field means absolutely nothing, why not give it a
> sensible id and save the overhead of a separate name?
Because that is not how things work, sorry.
> If kernel policy is that we should have useless data in sysfs, so be it,
> I just make sure that is on record and then break the defined LED class
> API.
Again, the bus id needs to be unique, for that specific class/bus that's
it. The attributes in the directory let you figure out what specifics
are for the device.
Look at the PCI and USB devices. There is some information you can
glean from those names, but they are primary a unique identifier, you
need to look at the attributes to get the real information about your
device.
LED devices are no different.
> <serious mode>
> Ok, so I make the point above with a sledge hammer. There are more
> subtle issues here too. People are asking for a ton of extra attributes
> for the LED class. We can have a name, a colour, an LED "function", a
> location on the device and probably 101 other things.
Great, the more the merrier. Seriously, I have no problem with that.
> As I understand it, sysfs was put there to pass information *the kernel
> knows* to userspace. The kernel doesn't actually care about the location
> of an LED or its colour. All the kernel cares is we have an LED and it
> has a certain brightness.
If you know the color and location already, why not export that
information? Unless you can determine it from userspace some other way
already?
> Yes, we can teach the kernel all this extra info but it is simply to
> pass to userspace. Why should my kernel be bloated with all that extra
> information which it doesn't need itself?
If there's no other way for userspace to determine it, then put it in
the kernel. Otherwise leave it for userspace to handle.
> My conclusion is that there should be something in userspace
> supplementing this information from the kernel. Going back the the
> problem at hand, HAL is already equipped to do this, all it needs is
> some kind of unique ID so it can identify the LED. This would be the
> busid.
Well, if that is possible. Just put it in an attribute.
> So I do stand by what the LED class does as being sane. Yes, the class
> defined a way of writing its busids which isn't like any other but there
> is a good reason for it. We could make it machine readable to provide
> hints to userspace and I never saw any reason why we shouldn't have
> done. It was discussed on LKML and no objections were raised (it was in
> fact requested). The choice of field separator is unfortunate since its
> developed other meanings after it was used in the LED class, such is
> life.
Sorry, I missed it with the other stuff on lkml, but now I'm trying to
get this fixed. Just use an attribute like the whole rest of the
kernel and a number for a bus id.
> So there are various options:
>
> 1. We keep 'useful' busids and the LED class continues as is. We accept
> classes can provide a busid naming policy if it makes sense.
Nope.
> 2. We adopt free form busids and export a ton of attributes just useful
> to userspace (bloating the kernel IMO).
Yup, and there's no real bloat at all.
> 3. As 2. but use 'useless' busids that mean nothing.
bus ids mean almost nothing today, see above.
> 4. We adopt free form busids but let userspace fill in its own
> information based on the busid (no policy on naming).
No, use an attribute please.
> I'm very aware I'm isolated here and ultimately this is probably Greg's
> decision which I will end up abiding by. If anyone else does have a
> view, speak now ;-). I think there are some important issues here and
> they do need clarification, one way or another.
I know that LEDs are special and unique, just like everyone else, so
please work like everyone else does :)
thanks,
greg k-h
On Fri, 2007-06-08 at 16:46 -0700, Greg KH wrote:
> On Sat, Jun 09, 2007 at 12:02:28AM +0100, Richard Purdie wrote:
> > On Fri, 2007-06-08 at 11:57 -0700, Greg KH wrote:
> > > On Fri, Jun 01, 2007 at 05:23:24PM +0100, Richard Purdie wrote:
> > > Why not just do a simple:
> > > led01
> > > led02
> > > led03
> > > ...
> > >
> > > and so on?
> >
> > If we do this, my point is we're exporting something to userspace which
> > is totally devoid of information.
>
> Just like all other subsystems?
>
> No, the only information the device name is that it shows a UNIQUE name
> at that point in time in the kernel. Heck, we could just use random
> numbers that are unique like some other people have suggested, it would
> mean the exact same thing.
>
> > Since its in class/leds/ we can summarise the latter without help from
> > the prefix. I'd hate to think userspace cares about the former.
>
> Ok, then use a random number please. Start with 00000000000001 and just
> increment from there, or use the idr subsystem.
>
> I was trying to be nice and at least give you a prefix that looked kind
> of sane to people, but if you want to be difficult... :)
This whole mess is because the LED class tried to be nice with sane
looking ids :/.
> > > And use the 'name' field to put the name of your device (disk,
> > > bluetooth, etc.) This is the way all other busses and devices work, and
> > > I don't think that LEDs are anything more "special" than anything else
> > > in the kernel, right?
> >
> > Since the "busid" field means absolutely nothing, why not give it a
> > sensible id and save the overhead of a separate name?
>
> Because that is not how things work, sorry.
Most of your argument seems to read that this shouldn't be done because
nobody else does it which isn't a particularly good one. Everyone else
is jumping off a bridge, I should as well? You then point out that PCI
and USB busids do have some meaning and a quick glance at sysfs shows
others too.
> > If kernel policy is that we should have useless data in sysfs, so be it,
> > I just make sure that is on record and then break the defined LED class
> > API.
>
> Again, the bus id needs to be unique, for that specific class/bus that's
> it.
and as I've said several times, the LED scheme *is* unique meeting your
criteria ;-).
> The attributes in the directory let you figure out what specifics
> are for the device.
>
> Look at the PCI and USB devices. There is some information you can
> glean from those names, but they are primary a unique identifier, you
> need to look at the attributes to get the real information about your
> device.
So the PCI and USB subsystems need adjusting to use random numbers since
they're also breaking the rules and might have meaningful information in
their busids? Please fix them too ;-).
> > <serious mode>
> > Ok, so I make the point above with a sledge hammer. There are more
> > subtle issues here too. People are asking for a ton of extra attributes
> > for the LED class. We can have a name, a colour, an LED "function", a
> > location on the device and probably 101 other things.
>
> Great, the more the merrier. Seriously, I have no problem with that.
Where do you draw the line though?
> > As I understand it, sysfs was put there to pass information *the kernel
> > knows* to userspace. The kernel doesn't actually care about the location
> > of an LED or its colour. All the kernel cares is we have an LED and it
> > has a certain brightness.
>
> If you know the color and location already, why not export that
> information? Unless you can determine it from userspace some other way
> already?
The kernel doesn't know this and it doesn't care about this.
We could teach the kernel. Alternatively we could teach userspace.
> > Yes, we can teach the kernel all this extra info but it is simply to
> > pass to userspace. Why should my kernel be bloated with all that extra
> > information which it doesn't need itself?
>
> If there's no other way for userspace to determine it, then put it in
> the kernel. Otherwise leave it for userspace to handle.
Ok. The LED class can provide a name attribute which uniquely identifies
each LED. Userspace can then store and look up all the information it
wants against that "name".
So by this argument, colour, function and all these other attributes
should be left for userspace/HAL to deal with.
> > I'm very aware I'm isolated here and ultimately this is probably Greg's
> > decision which I will end up abiding by. If anyone else does have a
> > view, speak now ;-). I think there are some important issues here and
> > they do need clarification, one way or another.
>
> I know that LEDs are special and unique, just like everyone else, so
> please work like everyone else does :)
I've never claimed that it was special and unique. Just because it does
something differently, that doesn't mean its wrong either.
One last try to demonstrate this is grossly inefficient (and this kind
of inefficiency is one reason I'm beginning to dislike sysfs). On my
handheld, you can currently script something to light a specific LED
with something like:
echo "1" > /sys/class/leds/spitz\:green/brightness
With the busid changes, this becomes:
for busid in `ls /sys/class/leds`
do
name=`cat /sys/class/leds/$busid/name`
if [ "$name" = "spitz_green" ]; then
break
fi
done
echo "1" > /sys/class/leds/$busid/brightness
Even after accounting for my lack of l33t shell skills, the latter will
be much more complex. For what gain? It doesn't matter whether you do
this through shell or any other language. Even if you know the name of
the device you want to talk to you have to convert to some meaningless
busid first which is inefficient. Things like HAL are going to be forced
to read things after every boot. This has wider implications than the
LEDs which are just a tiny consideration in the grand sysfs scheme. If
we can make sysfs more efficient, wouldn't that be a good idea?
Anyhow, time to stop pretending I have any choice in this ;-). I will
have the LED class use random numbers for busids and add a name
attribute unless anyone else voices an opinion.
My current view is that notions of colour, function, location etc.
shouldn't be in the class since userspace can handle it in userspace and
these don't mean anything to the kernel.
Cheers,
Richard
Hi!
> > My other concerns with this patch are that it exports incorrect
> > information on spitz (which I had warned about and I will get bug
> > reports about) and that "description" is not really a suitable name for
> > a sysfs attribute, "function" might give a better idea of what it
> > represents to developers.
> >
> > I still question whether either colour or function properties are
> > actually particularly useful to userspace other than for naming purposes
> > which is why they were part of the device name.
> >
> > Anyhow, I'm trying not to say too much more as it will just go in
> > circles. I'll await a reply from Greg.
>
> Hm, I thought I made my opinion pretty clear in the beginning.
>
> Why not just do a simple:
> led01
> led02
> led03
> ...
>
> and so on?
>
> And use the 'name' field to put the name of your device (disk,
> bluetooth, etc.) This is the way all other busses and devices work, and
> I don't think that LEDs are anything more "special" than anything else
> in the kernel, right?
Can we keep the original naming? spitz:disk is as unique as led02, and
it is _way_ easier to use.
Come on, I want to use the led subsystem from the scripts...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, 2007-06-10 at 12:11 +0200, Pavel Machek wrote:
> Can we keep the original naming? spitz:disk is as unique as led02, and
> it is _way_ easier to use.
> Come on, I want to use the led subsystem from the scripts...
I don't see a problem with spitz_disk, which is just as easy to use in
scripts.
Richard.
Hi Richard!
On Sun, 10 Jun 2007, Richard Hughes wrote:
> On Sun, 2007-06-10 at 12:11 +0200, Pavel Machek wrote:
> > Can we keep the original naming? spitz:disk is as unique as led02, and
> > it is _way_ easier to use.
> > Come on, I want to use the led subsystem from the scripts...
>
> I don't see a problem with spitz_disk, which is just as easy to use in
> scripts.
It is the old problem with mV in battery_level_mV. How do you know for sure
what is the designator, and what is the unit (or in this case, color)? You
could take the last _[^_]+, but what if there is no unit/color? etc.
There is a BIIIIG thread about it (or more than one, actually) in the
archives, and I don't recall a consensus being reached.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Sat, Jun 09, 2007 at 10:25:16AM +0100, Richard Purdie wrote:
> Anyhow, time to stop pretending I have any choice in this ;-). I will
> have the LED class use random numbers for busids and add a name
> attribute unless anyone else voices an opinion.
Thank you, I really appreciate it, and so does the people who write the
tools that parse sysfs (like HAL.)
greg k-h
On Sun, 2007-06-24 at 22:06 -0700, Greg KH wrote:
> On Sat, Jun 09, 2007 at 10:25:16AM +0100, Richard Purdie wrote:
> > Anyhow, time to stop pretending I have any choice in this ;-). I will
> > have the LED class use random numbers for busids and add a name
> > attribute unless anyone else voices an opinion.
>
> Thank you, I really appreciate it, and so does the people who write the
> tools that parse sysfs (like HAL.)
There were some other opinions voiced including one from the person who
started this discussion.
So no, the people who write the tools that parse sysfs (like HAL.) don't
appreciate this.
People who write tools that parse sysfs like shell scripts don't
appreciate it either, as I illustrated.
You've yet to give any technical reason why we can't have meaningful
busids rather than random numbers. Your entire argument seems to be that
its wrong because its a bit different and nobody else does it...
Richard
On Tue, 2007-06-26 at 11:02 +0100, Richard Purdie wrote:
> There were some other opinions voiced including one from the person
> who started this discussion.
>
> So no, the people who write the tools that parse sysfs (like HAL.)
> don't appreciate this.
>
> People who write tools that parse sysfs like shell scripts don't
> appreciate it either, as I illustrated.
>From a hal point of view, we don't care if the device name is 'led01' or
'light_to_dance_the_fandango' and from a shell point of view it's
probably best for the latter. I think the point Greg tried to make is
that it shouldn't matter, and HAL shouldn't export (nor parse) the
device name as anything sensible.
> You've yet to give any technical reason why we can't have meaningful
> busids rather than random numbers. Your entire argument seems to be
> that its wrong because its a bit different and nobody else does it...
If it's a trivial name then I think led_thinklight0 is perfectly okay, I
think Kay was talking more about the attribute vs. name-in-device
encoding.
Richard.
On Tue, 2007-06-26 at 11:46 +0100, Richard Hughes wrote:
> On Tue, 2007-06-26 at 11:02 +0100, Richard Purdie wrote:
> > There were some other opinions voiced including one from the person
> > who started this discussion.
> >
> > So no, the people who write the tools that parse sysfs (like HAL.)
> > don't appreciate this.
> >
> > People who write tools that parse sysfs like shell scripts don't
> > appreciate it either, as I illustrated.
>
> >From a hal point of view, we don't care if the device name is 'led01' or
> 'light_to_dance_the_fandango' and from a shell point of view it's
> probably best for the latter. I think the point Greg tried to make is
> that it shouldn't matter, and HAL shouldn't export (nor parse) the
> device name as anything sensible.
>
> > You've yet to give any technical reason why we can't have meaningful
> > busids rather than random numbers. Your entire argument seems to be
> > that its wrong because its a bit different and nobody else does it...
>
> If it's a trivial name then I think led_thinklight0 is perfectly okay, I
> think Kay was talking more about the attribute vs. name-in-device
> encoding.
I see no problem to use the function name and add an enumeration number
to that name to be able to handle multiple instances with the same
function name. Like pointed out in earlier mail:
http://lists.freedesktop.org/archives/hal/2007-May/008552.html
Thanks,
Kay