lkml.org 
[lkml]   [2015]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH/RFC RESEND] leds: Use set_brightness_work for brightness_set ops that can sleep
30.06.2015 11:27, Jacek Anaszewski пишет:
> On 06/29/2015 05:17 PM, Stas Sergeev wrote:> 29.06.2015 17:05, Jacek Anaszewski пишет:
>>> + * If need to disable soft blinking delegate this to the
>>> + * work queue task to avoid problems in case we are
>>> + * called from hard irq context.
>>> + */
>>> + led_cdev->flags |= LED_BLINK_DISABLE;
>> Wouldn't it be better to just enforce the callers
>> to explicitly disable software blink, so that it to
>> never happen from irq context? Something like in this
>> patch:
>> https://lkml.org/lkml/2015/5/13/491
>>
>
> Blinking can be disabled not only by removing trigger explicitly,
> but also by setting brightness to 0 and led_set_brightness
> can be called from hard irq context. set_brightness_work
> was originally introduced exactly for this use case.
Could you please describe where does this happen?
I can see LED_OFF happening only in led_heartbeat_function(), but
it doesn't use soft-blink, so it will not activate the delayed
timer cancel.

I would suggest the patch below to make it explicit that
led_heartbeat_function() doesn't want to cancel soft blink.
Note that led_heartbeat_function() already uses led_set_brightness_async()
in a normal case.


diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c
index fea6871..0f89d12 100644
--- a/drivers/leds/trigger/ledtrig-heartbeat.c
+++ b/drivers/leds/trigger/ledtrig-heartbeat.c
@@ -37,7 +37,7 @@ static void led_heartbeat_function(unsigned long data)
unsigned long delay = 0;

if (unlikely(panic_heartbeats)) {
- led_set_brightness(led_cdev, LED_OFF);
+ led_set_brightness_async(led_cdev, LED_OFF);
return;
}


\
 
 \ /
  Last update: 2015-06-30 14:01    [W:0.082 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site