cleanup LED documentation and make it match reality

PATCH In review Pavel Machek 3 participants 6 messages Oct 3, 2016 – Oct 19, 2016 LED

lore

Share & tools

lore

Pavel Machek

author

2016-10-03 08:10 UTC #0 lore raw

PATCH 1/1 cleanup LED documentation and make it match reality

sysfs-class-led fails to mention some important details. Also fix led vs LED and english.

Signed-off-by Pavel Machek <[email protected]>

Documentation/ABI/testing/sysfs-class-led +12 -3

@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:
 		Set the brightness of the LED. Most LEDs don't
-		have hardware brightness support so will just be turned on for
+		have hardware brightness support, so will just be turned on for
 		non-zero brightness settings. The value is between 0 and
 		/sys/class/leds/<led>/max_brightness.
 
+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.
+		
+
 What:		/sys/class/leds/<led>/max_brightness
 Date:		March 2006
 KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:
-		Maximum brightness level for this led, default is 255 (LED_FULL).
+		Maximum brightness level for this LED, default is 255 (LED_FULL).
+
+		If the LED does not support different brightness levels, this
+		should be 1.
 
 What:		/sys/class/leds/<led>/trigger
 Date:		March 2006

@@ -21,7 +30,7 @@ KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:
 		Set the trigger for this LED. A trigger is a kernel based source
-		of led events.
+		of LED events.
 		You can change triggers in a similar manner to the way an IO
 		scheduler is chosen. Trigger specific parameters can appear in
 		/sys/class/leds/<led> once a given trigger is selected.

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Jacek Anaszewski

2016-10-03 09:28 UTC #2 lore raw

In reply to Pavel Machek

Hi Pavel,

Thanks for the patch.

Quoted 11 lines · Pavel Machek

sysfs-class-led fails to mention some important details. Also fix led vs LED and english.
Signed-off-by Pavel Machek <[email protected]>
Documentation/ABI/testing/sysfs-class-led +0 -0
@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:

Documentation/ABI/testing/sysfs-class-led KernelVersion: 2.6.17 · line 14

Quoted 10 lines

Documentation/ABI/testing/sysfs-class-led +5 -1

 		Set the brightness of the LED. Most LEDs don't
-		have hardware brightness support so will just be turned on for
+		have hardware brightness support, so will just be turned on for
 		non-zero brightness settings. The value is between 0 and
 		/sys/class/leds/<led>/max_brightness.
 
+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.

This is true only in case of timer trigger, as it uses blink_brightness property from struct led_classdev to cache current brightness, when the the LED is in the off cycle. This is part of software blink fallback functionality.

In case of heartbeat trigger max_brightness is always used for top level brightness. We'd need to refactor the trigger a bit to allow for different top brightness levels.

Quoted 25 lines

Documentation/ABI/testing/sysfs-class-led +7 -2

+		
+
 What:		/sys/class/leds/<led>/max_brightness
 Date:		March 2006
 KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:
-		Maximum brightness level for this led, default is 255 (LED_FULL).
+		Maximum brightness level for this LED, default is 255 (LED_FULL).
+
+		If the LED does not support different brightness levels, this
+		should be 1.
 
 What:		/sys/class/leds/<led>/trigger
 Date:		March 2006

@@ -21,7 +30,7 @@ KernelVersion:	2.6.17
 Contact:	Richard Purdie <[email protected]>
 Description:
 		Set the trigger for this LED. A trigger is a kernel based source
-		of led events.
+		of LED events.
 		You can change triggers in a similar manner to the way an IO
 		scheduler is chosen. Trigger specific parameters can appear in
 		/sys/class/leds/<led> once a given trigger is selected.

Signature · 2 lines

Best regards,
Jacek Anaszewski

Pavel Machek

author

2016-10-03 09:38 UTC #3 lore raw

Hi!

Quoted 30 lines

Thanks for the patch.
Quoted 17 lines · Pavel Machek
sysfs-class-led fails to mention some important details. Also fix led vs LED and english.
Signed-off-by Pavel Machek <[email protected]>
Documentation/ABI/testing/sysfs-class-led +0 -0
@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
Contact: Richard Purdie <[email protected]>
Description:
Set the brightness of the LED. Most LEDs don't
Documentation/ABI/testing/sysfs-class-led +1 -1
-		have hardware brightness support so will just be turned on for
+		have hardware brightness support, so will just be turned on for
non-zero brightness settings. The value is between 0 and /sys/class/leds/<led>/max_brightness.
Documentation/ABI/testing/sysfs-class-led KernelVersion: 2.6.17 · line 14
Quoted 4 lines
Documentation/ABI/testing/sysfs-class-led +4 -0
+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.
This is true only in case of timer trigger, as it uses blink_brightness property from struct led_classdev to cache current brightness, when the the LED is in the off cycle. This is part of software blink fallback functionality.

Quoted 3 lines

In case of heartbeat trigger max_brightness is always used for top level brightness. We'd need to refactor the trigger a bit to allow for different top brightness levels.

Ok, do you think you could update the documenation to match the reality? It is quite important to know what is the intended behaviour and what are the bugs.

Thanks,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Jacek Anaszewski

2016-10-03 09:54 UTC #4 lore raw

Quoted 32 lines · Pavel Machek

Hi!
Quoted 30 lines
Thanks for the patch.
Quoted 17 lines · Pavel Machek
sysfs-class-led fails to mention some important details. Also fix led vs LED and english.
Signed-off-by Pavel Machek <[email protected]>
Documentation/ABI/testing/sysfs-class-led +0 -0
@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
Contact: Richard Purdie <[email protected]>
Description:
Set the brightness of the LED. Most LEDs don't
Documentation/ABI/testing/sysfs-class-led +1 -1
-		have hardware brightness support so will just be turned on for
+		have hardware brightness support, so will just be turned on for
non-zero brightness settings. The value is between 0 and /sys/class/leds/<led>/max_brightness.
Documentation/ABI/testing/sysfs-class-led KernelVersion: 2.6.17 · line 14
Quoted 4 lines
Documentation/ABI/testing/sysfs-class-led +4 -0
+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.
This is true only in case of timer trigger, as it uses blink_brightness property from struct led_classdev to cache current brightness, when the the LED is in the off cycle. This is part of software blink fallback functionality.

Quoted 7 lines

Quoted 3 lines
In case of heartbeat trigger max_brightness is always used for top level brightness. We'd need to refactor the trigger a bit to allow for different top brightness levels.
Ok, do you think you could update the documenation to match the reality? It is quite important to know what is the intended behaviour and what are the bugs.

I'd prefer to improve the trigger. I'll try to do that in the coming days, and apply your patch afterwards.

Signature · 2 lines

Best regards,
Jacek Anaszewski

Jacek Anaszewski

2016-10-19 08:46 UTC #5 lore raw

In reply to Pavel Machek

Quoted 32 lines · Pavel Machek

Hi!
Quoted 30 lines
Thanks for the patch.
Quoted 17 lines · Pavel Machek
sysfs-class-led fails to mention some important details. Also fix led vs LED and english.
Signed-off-by Pavel Machek <[email protected]>
Documentation/ABI/testing/sysfs-class-led +0 -0
@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
Contact: Richard Purdie <[email protected]>
Description:
Set the brightness of the LED. Most LEDs don't
Documentation/ABI/testing/sysfs-class-led +1 -1
-		have hardware brightness support so will just be turned on for
+		have hardware brightness support, so will just be turned on for
non-zero brightness settings. The value is between 0 and /sys/class/leds/<led>/max_brightness.
Documentation/ABI/testing/sysfs-class-led KernelVersion: 2.6.17 · line 14
Quoted 4 lines
Documentation/ABI/testing/sysfs-class-led +4 -0
+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.
This is true only in case of timer trigger, as it uses blink_brightness property from struct led_classdev to cache current brightness, when the the LED is in the off cycle. This is part of software blink fallback functionality.

Quoted 7 lines

Quoted 3 lines
In case of heartbeat trigger max_brightness is always used for top level brightness. We'd need to refactor the trigger a bit to allow for different top brightness levels.
Ok, do you think you could update the documenation to match the reality? It is quite important to know what is the intended behaviour and what are the bugs.

Applied the patch (removed extra empty line) after improving heartbeat trigger [0] to support brightness levels other then max_brightness.

[0] https://git.kernel.org/cgit/linux/kernel/git/j.anaszewski/linux-leds.git/commit/?h=for-next&id=9a1d1e6f926a4d1f4a31a7a37abece9e44455b43

Signature · 2 lines

Best regards,
Jacek Anaszewski