Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1494733 > unrolled thread

[PATCH] cleanup LED documentation and make it match reality

Started byPavel Machek <pavel@ucw.cz>
First post2016-10-03 10:20 +0200
Last post2016-10-03 12:00 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] cleanup LED documentation and make it match reality Pavel Machek <pavel@ucw.cz> - 2016-10-03 10:20 +0200
    Re: [PATCH] cleanup LED documentation and make it match reality Greg KH <greg@kroah.com> - 2016-10-03 10:30 +0200
    Re: [PATCH] cleanup LED documentation and make it match reality Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-10-03 11:30 +0200
      Re: [PATCH] cleanup LED documentation and make it match reality Pavel Machek <pavel@ucw.cz> - 2016-10-03 11:40 +0200
        Re: [PATCH] cleanup LED documentation and make it match reality Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-10-03 12:00 +0200

#1494733 — [PATCH] cleanup LED documentation and make it match reality

FromPavel Machek <pavel@ucw.cz>
Date2016-10-03 10:20 +0200
Subject[PATCH] cleanup LED documentation and make it match reality
Message-ID<so6DD-22l-9@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

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

Signed-off-by: Pavel Machek <pavel@ucw.cz>

--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
 Contact:	Richard Purdie <rpurdie@rpsys.net>
 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 <rpurdie@rpsys.net>
 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 <rpurdie@rpsys.net>
 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.

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

[toc] | [next] | [standalone]


#1494742

FromGreg KH <greg@kroah.com>
Date2016-10-03 10:30 +0200
Message-ID<so6Nj-2be-1@gated-at.bofh.it>
In reply to#1494733
On Mon, Oct 03, 2016 at 10:10:50AM +0200, Pavel Machek wrote:
> 
> sysfs-class-led fails to mention some important details. Also fix led
> vs LED and english.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> --- a/Documentation/ABI/testing/sysfs-class-led
> +++ b/Documentation/ABI/testing/sysfs-class-led
> @@ -4,16 +4,25 @@ KernelVersion:	2.6.17
>  Contact:	Richard Purdie <rpurdie@rpsys.net>
>  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.
> +		

No trailing whitespace please...

[toc] | [prev] | [next] | [standalone]


#1494782

FromJacek Anaszewski <j.anaszewski@samsung.com>
Date2016-10-03 11:30 +0200
Message-ID<so7Jo-32K-21@gated-at.bofh.it>
In reply to#1494733
Hi Pavel,

Thanks for the patch.

On 10/03/2016 10:10 AM, Pavel Machek wrote:
>
> sysfs-class-led fails to mention some important details. Also fix led
> vs LED and english.
>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>
> --- a/Documentation/ABI/testing/sysfs-class-led
> +++ b/Documentation/ABI/testing/sysfs-class-led
> @@ -4,16 +4,25 @@ KernelVersion:	2.6.17
>  Contact:	Richard Purdie <rpurdie@rpsys.net>
>  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.

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.

> +		
> +
>  What:		/sys/class/leds/<led>/max_brightness
>  Date:		March 2006
>  KernelVersion:	2.6.17
>  Contact:	Richard Purdie <rpurdie@rpsys.net>
>  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 <rpurdie@rpsys.net>
>  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.
>


-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]


#1494790

FromPavel Machek <pavel@ucw.cz>
Date2016-10-03 11:40 +0200
Message-ID<so7T3-35V-31@gated-at.bofh.it>
In reply to#1494782

[Multipart message — attachments visible in raw view] — view raw

Hi!

> Thanks for the patch.
> 
> On 10/03/2016 10:10 AM, Pavel Machek wrote:
> >
> >sysfs-class-led fails to mention some important details. Also fix led
> >vs LED and english.
> >
> >Signed-off-by: Pavel Machek <pavel@ucw.cz>
> >
> >--- a/Documentation/ABI/testing/sysfs-class-led
> >+++ b/Documentation/ABI/testing/sysfs-class-led
> >@@ -4,16 +4,25 @@ KernelVersion:	2.6.17
> > Contact:	Richard Purdie <rpurdie@rpsys.net>
> > 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.
> 
> 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.

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]


#1494798

FromJacek Anaszewski <j.anaszewski@samsung.com>
Date2016-10-03 12:00 +0200
Message-ID<so8cr-3cD-55@gated-at.bofh.it>
In reply to#1494790
On 10/03/2016 11:38 AM, Pavel Machek wrote:
> Hi!
>
>> Thanks for the patch.
>>
>> On 10/03/2016 10:10 AM, Pavel Machek wrote:
>>>
>>> sysfs-class-led fails to mention some important details. Also fix led
>>> vs LED and english.
>>>
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>>
>>> --- a/Documentation/ABI/testing/sysfs-class-led
>>> +++ b/Documentation/ABI/testing/sysfs-class-led
>>> @@ -4,16 +4,25 @@ KernelVersion:	2.6.17
>>> Contact:	Richard Purdie <rpurdie@rpsys.net>
>>> 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.
>>
>> 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.
>
> 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.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web