Re: [GIT PULL] LED updates

[Richard Purdie <rpurdie@rpsys.net>]

On Thu, 2008-02-07 at 19:38 -0200, Henrique de Moraes Holschuh wrote:
> On Thu, 07 Feb 2008, Richard Purdie wrote:
> > Márton Németh:
> >       leds: Add support for hardware accelerated LED flashing
> >       leds: hw acceleration for Clevo mail LED driver
> 
> This one has a loose end: when you call brightness_set on a led with
> hardware flash acceleration, you will leave the trigger armed, BUT the led
> won't blink anymore.  That's just wrong.

Agreed.

> Either we should always remove *any* (hardware accelerated or not!) active
> trigger when a write to brightness_set is done, or the stuff about "calling
> brightness_set will disable the hardware accelerated blink" has to go.
> 
> I personally prefer that we would always remove any active trigger if
> brightness_set is to be called.  IMHO, it is neater, and it is also the
> least-surprise-behaviour from an user perspective with the LED_OFF:LED_FULL
> triggers we have right now.

Even without the hardware acceleration, a user write to set_brightness
leaves any active trigger active and isn't really intuitive or right
either. 

> Which one will be?  If it is "remove any active trigger", I'd not mind
> writing the patch.

I'll accept a patch for that :).

> > Richard Purdie:
> >       leds: Standardise LED naming scheme
> 
> This one causes trouble (at least on 2.6.23 -- I backported the patch) due
> to the 20-byte length limit on sysfs names.  I had to use "tp::<somecrap>"
> instead of "thinkpad::<somecrap>" to name LEDs, and still had to reduce
> ultrabase_battery to ultrabase_batt :-)
> 
> Anyway, IMHO, the LED function should come first, and we should not even
> need the led driver name anywhere.  In case of clashes in the class sysfs
> dir, just tack a .# to the end or somesuch.  The device the LED is tied to
> already differentiates them.  That would save a lot of chars for something
> much more useful (the function).

Ouch, I'm looking into this. I wish I'd known about it earlier. I agree
function is more important but didn't want to break the existing
convention. I guess this limitation comes from the kobjects involved...

Richard