The following sequence:
echo timer >/sys/class/leds/<name>/trigger
echo 1 >/sys/class/leds/<name>/brightness
should change the ON brightness for blinking.
The function led_set_brightness() was mistakenly initiating the
delayed blink stop procedure, which resulted in no blinking with
the timer trigger still active.
This patch fixes the problem by changing led_set_brightness()
to not initiate the delayed blink stop when brightness is not 0.
CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: Kyungmin Park <[email protected]>
CC: [email protected]
CC: [email protected]
Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/led-class.c | 5 +++++
drivers/leds/led-core.c | 5 +++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 795ec99..65c2c80 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -121,6 +121,11 @@ static void led_timer_function(unsigned long data)
brightness = led_get_brightness(led_cdev);
if (!brightness) {
/* Time to switch the LED on. */
+ if (led_cdev->delayed_set_value) {
+ led_cdev->blink_brightness =
+ led_cdev->delayed_set_value;
+ led_cdev->delayed_set_value = 0;
+ }
brightness = led_cdev->blink_brightness;
delay = led_cdev->blink_delay_on;
} else {
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 356e851..a90dd26 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -119,10 +119,11 @@ void led_set_brightness(struct led_classdev *led_cdev,
{
int ret = 0;
- /* delay brightness setting if need to stop soft-blink timer */
+ /* delay brightness if soft-blink is active */
if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
led_cdev->delayed_set_value = brightness;
- schedule_work(&led_cdev->set_brightness_work);
+ if (brightness == LED_OFF)
+ schedule_work(&led_cdev->set_brightness_work);
return;
}
--
1.7.9.5
Hi Stas,
On 05/14/2015 05:24 PM, Stas Sergeev wrote:
>
> The following sequence:
> echo timer >/sys/class/leds/<name>/trigger
> echo 1 >/sys/class/leds/<name>/brightness
> should change the ON brightness for blinking.
> The function led_set_brightness() was mistakenly initiating the
> delayed blink stop procedure, which resulted in no blinking with
> the timer trigger still active.
>
> This patch fixes the problem by changing led_set_brightness()
> to not initiate the delayed blink stop when brightness is not 0.
>
> CC: Bryan Wu <[email protected]>
> CC: Richard Purdie <[email protected]>
> CC: Jacek Anaszewski <[email protected]>
> CC: Kyungmin Park <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> Signed-off-by: Stas Sergeev <[email protected]>
> ---
> drivers/leds/led-class.c | 5 +++++
> drivers/leds/led-core.c | 5 +++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 795ec99..65c2c80 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -121,6 +121,11 @@ static void led_timer_function(unsigned long data)
> brightness = led_get_brightness(led_cdev);
> if (!brightness) {
> /* Time to switch the LED on. */
> + if (led_cdev->delayed_set_value) {
> + led_cdev->blink_brightness =
> + led_cdev->delayed_set_value;
> + led_cdev->delayed_set_value = 0;
> + }
> brightness = led_cdev->blink_brightness;
> delay = led_cdev->blink_delay_on;
> } else {
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 356e851..a90dd26 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -119,10 +119,11 @@ void led_set_brightness(struct led_classdev *led_cdev,
> {
> int ret = 0;
>
> - /* delay brightness setting if need to stop soft-blink timer */
> + /* delay brightness if soft-blink is active */
> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
> led_cdev->delayed_set_value = brightness;
> - schedule_work(&led_cdev->set_brightness_work);
> + if (brightness == LED_OFF)
> + schedule_work(&led_cdev->set_brightness_work);
> return;
> }
>
Acked-by: Jacek Anaszewski <[email protected]>
--
Best Regards,
Jacek Anaszewski
On 05/15/2015 10:12 AM, Jacek Anaszewski wrote:
> Hi Stas,
>
> On 05/14/2015 05:24 PM, Stas Sergeev wrote:
>>
>> The following sequence:
>> echo timer >/sys/class/leds/<name>/trigger
>> echo 1 >/sys/class/leds/<name>/brightness
>> should change the ON brightness for blinking.
>> The function led_set_brightness() was mistakenly initiating the
>> delayed blink stop procedure, which resulted in no blinking with
>> the timer trigger still active.
>>
>> This patch fixes the problem by changing led_set_brightness()
>> to not initiate the delayed blink stop when brightness is not 0.
This commit message is not valid for this version of the patch.
Current patch just allows to change the LED brightness while
timer trigger is active. Please fix this description.
>>
>> CC: Bryan Wu <[email protected]>
>> CC: Richard Purdie <[email protected]>
>> CC: Jacek Anaszewski <[email protected]>
>> CC: Kyungmin Park <[email protected]>
>> CC: [email protected]
>> CC: [email protected]
>>
>> Signed-off-by: Stas Sergeev <[email protected]>
>> ---
>> drivers/leds/led-class.c | 5 +++++
>> drivers/leds/led-core.c | 5 +++--
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 795ec99..65c2c80 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -121,6 +121,11 @@ static void led_timer_function(unsigned long data)
>> brightness = led_get_brightness(led_cdev);
>> if (!brightness) {
>> /* Time to switch the LED on. */
>> + if (led_cdev->delayed_set_value) {
>> + led_cdev->blink_brightness =
>> + led_cdev->delayed_set_value;
>> + led_cdev->delayed_set_value = 0;
>> + }
>> brightness = led_cdev->blink_brightness;
>> delay = led_cdev->blink_delay_on;
>> } else {
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index 356e851..a90dd26 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -119,10 +119,11 @@ void led_set_brightness(struct led_classdev
>> *led_cdev,
>> {
>> int ret = 0;
>>
>> - /* delay brightness setting if need to stop soft-blink timer */
>> + /* delay brightness if soft-blink is active */
>> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
>> led_cdev->delayed_set_value = brightness;
>> - schedule_work(&led_cdev->set_brightness_work);
>> + if (brightness == LED_OFF)
>> + schedule_work(&led_cdev->set_brightness_work);
>> return;
>> }
>>
>
> Acked-by: Jacek Anaszewski <[email protected]>
>
--
Best Regards,
Jacek Anaszewski
15.05.2015 17:09, Jacek Anaszewski пишет:
> On 05/15/2015 10:12 AM, Jacek Anaszewski wrote:
>> Hi Stas,
>>
>> On 05/14/2015 05:24 PM, Stas Sergeev wrote:
>>>
>>> The following sequence:
>>> echo timer >/sys/class/leds/<name>/trigger
>>> echo 1 >/sys/class/leds/<name>/brightness
>>> should change the ON brightness for blinking.
>>> The function led_set_brightness() was mistakenly initiating the
>>> delayed blink stop procedure, which resulted in no blinking with
>>> the timer trigger still active.
>>>
>>> This patch fixes the problem by changing led_set_brightness()
>>> to not initiate the delayed blink stop when brightness is not 0.
>
> This commit message is not valid for this version of the patch.
Why do you think so?
---
- schedule_work(&led_cdev->set_brightness_work);
+ if (brightness == LED_OFF)
+ schedule_work(&led_cdev->set_brightness_work);
---
LED_OFF is a 0 define.
On 05/15/2015 04:11 PM, Stas Sergeev wrote:
> 15.05.2015 17:09, Jacek Anaszewski пишет:
>> On 05/15/2015 10:12 AM, Jacek Anaszewski wrote:
>>> Hi Stas,
>>>
>>> On 05/14/2015 05:24 PM, Stas Sergeev wrote:
>>>>
>>>> The following sequence:
>>>> echo timer >/sys/class/leds/<name>/trigger
>>>> echo 1 >/sys/class/leds/<name>/brightness
>>>> should change the ON brightness for blinking.
>>>> The function led_set_brightness() was mistakenly initiating the
>>>> delayed blink stop procedure, which resulted in no blinking with
>>>> the timer trigger still active.
>>>>
>>>> This patch fixes the problem by changing led_set_brightness()
>>>> to not initiate the delayed blink stop when brightness is not 0.
>>
>> This commit message is not valid for this version of the patch.
> Why do you think so?
> ---
> - schedule_work(&led_cdev->set_brightness_work);
> + if (brightness == LED_OFF)
> + schedule_work(&led_cdev->set_brightness_work);
> ---
>
> LED_OFF is a 0 define.
>
You're right, I was just looking at the issue from different
perspective.
--
Best Regards,
Jacek Anaszewski
On Fri, May 15, 2015 at 7:41 AM, Jacek Anaszewski
<[email protected]> wrote:
> On 05/15/2015 04:11 PM, Stas Sergeev wrote:
>>
>> 15.05.2015 17:09, Jacek Anaszewski пишет:
>>>
>>> On 05/15/2015 10:12 AM, Jacek Anaszewski wrote:
>>>>
>>>> Hi Stas,
>>>>
>>>> On 05/14/2015 05:24 PM, Stas Sergeev wrote:
>>>>>
>>>>>
>>>>> The following sequence:
>>>>> echo timer >/sys/class/leds/<name>/trigger
>>>>> echo 1 >/sys/class/leds/<name>/brightness
>>>>> should change the ON brightness for blinking.
>>>>> The function led_set_brightness() was mistakenly initiating the
>>>>> delayed blink stop procedure, which resulted in no blinking with
>>>>> the timer trigger still active.
>>>>>
>>>>> This patch fixes the problem by changing led_set_brightness()
>>>>> to not initiate the delayed blink stop when brightness is not 0.
>>>
>>>
>>> This commit message is not valid for this version of the patch.
>>
>> Why do you think so?
>> ---
>> - schedule_work(&led_cdev->set_brightness_work);
>> + if (brightness == LED_OFF)
>> + schedule_work(&led_cdev->set_brightness_work);
>> ---
>>
>> LED_OFF is a 0 define.
>>
>
> You're right, I was just looking at the issue from different
> perspective.
>
Applied into my tree. Thanks,
-Bryan
On Thu 2015-05-14 18:24:02, Stas Sergeev wrote:
>
> The following sequence:
> echo timer >/sys/class/leds/<name>/trigger
> echo 1 >/sys/class/leds/<name>/brightness
> should change the ON brightness for blinking.
> The function led_set_brightness() was mistakenly initiating the
> delayed blink stop procedure, which resulted in no blinking with
> the timer trigger still active.
>
> This patch fixes the problem by changing led_set_brightness()
> to not initiate the delayed blink stop when brightness is not 0.
Could we get this part of API documented? It is quite non-obvious... 0 clears the trigger,
other values do not, I thought it is a bug when I saw it...
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
02.05.2015 17:59, Pavel Machek пишет:
> On Thu 2015-05-14 18:24:02, Stas Sergeev wrote:
>>
>> The following sequence:
>> echo timer >/sys/class/leds/<name>/trigger
>> echo 1 >/sys/class/leds/<name>/brightness
>> should change the ON brightness for blinking.
>> The function led_set_brightness() was mistakenly initiating the
>> delayed blink stop procedure, which resulted in no blinking with
>> the timer trigger still active.
>>
>> This patch fixes the problem by changing led_set_brightness()
>> to not initiate the delayed blink stop when brightness is not 0.
>
> Could we get this part of API documented? It is quite non-obvious... 0 clears the trigger,
> other values do not, I thought it is a bug when I saw it...
I guess the thinking was that if ON brightness differs
from OFF brightness, you should not clear the trigger.
But if you put ON brightness similar to OFF brightness,
then you can as well stop blinking at all.
Not that I am amused by that kind of logic though.
On Tue 2015-06-23 19:59:27, Stas Sergeev wrote:
> 02.05.2015 17:59, Pavel Machek пишет:
> > On Thu 2015-05-14 18:24:02, Stas Sergeev wrote:
> >>
> >> The following sequence:
> >> echo timer >/sys/class/leds/<name>/trigger
> >> echo 1 >/sys/class/leds/<name>/brightness
> >> should change the ON brightness for blinking.
> >> The function led_set_brightness() was mistakenly initiating the
> >> delayed blink stop procedure, which resulted in no blinking with
> >> the timer trigger still active.
> >>
> >> This patch fixes the problem by changing led_set_brightness()
> >> to not initiate the delayed blink stop when brightness is not 0.
> >
> > Could we get this part of API documented? It is quite non-obvious... 0 clears the trigger,
> > other values do not, I thought it is a bug when I saw it...
> I guess the thinking was that if ON brightness differs
> from OFF brightness, you should not clear the trigger.
> But if you put ON brightness similar to OFF brightness,
> then you can as well stop blinking at all.
And none of this is documented. So what about this?
diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index 3646ec8..b734d7f 100644
--- 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 <[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.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On 06/24/2015 10:44 AM, Pavel Machek wrote:
> On Tue 2015-06-23 19:59:27, Stas Sergeev wrote:
>> 02.05.2015 17:59, Pavel Machek пишет:
>>> On Thu 2015-05-14 18:24:02, Stas Sergeev wrote:
>>>>
>>>> The following sequence:
>>>> echo timer >/sys/class/leds/<name>/trigger
>>>> echo 1 >/sys/class/leds/<name>/brightness
>>>> should change the ON brightness for blinking.
>>>> The function led_set_brightness() was mistakenly initiating the
>>>> delayed blink stop procedure, which resulted in no blinking with
>>>> the timer trigger still active.
>>>>
>>>> This patch fixes the problem by changing led_set_brightness()
>>>> to not initiate the delayed blink stop when brightness is not 0.
>>>
>>> Could we get this part of API documented? It is quite non-obvious... 0 clears the trigger,
>>> other values do not, I thought it is a bug when I saw it...
>> I guess the thinking was that if ON brightness differs
>> from OFF brightness, you should not clear the trigger.
>> But if you put ON brightness similar to OFF brightness,
>> then you can as well stop blinking at all.
>
> And none of this is documented. So what about this?
It is documented in Documentation/leds/leds-class.txt:
"However, if you set the brightness value to LED_OFF it will
also disable the timer trigger".
Nonetheless, obviously it should be also covered in the
ABI documentation.
>
> diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
> index 3646ec8..b734d7f 100644
> --- 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 <[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.
This currently will not be true e.g. for heartbeat trigger which
uses max_brightness.
> +
> +
> 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.
>
--
Best Regards,
Jacek Anaszewski