All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacek Anaszewski <j.anaszewski@samsung.com>
To: Stas Sergeev <stsp@list.ru>
Cc: linux-leds@vger.kernel.org,
	Linux kernel <linux-kernel@vger.kernel.org>,
	Bryan Wu <cooloney@gmail.com>, Richard Purdie <rpurdie@rpsys.net>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Stas Sergeev <stsp@users.sourceforge.net>
Subject: Re: [PATCH] leds: fix brightness changing when software blinking is active
Date: Thu, 14 May 2015 14:09:16 +0200	[thread overview]
Message-ID: <5554906C.9060405@samsung.com> (raw)
In-Reply-To: <555480FD.3020301@list.ru>

On 05/14/2015 01:03 PM, Stas Sergeev wrote:
> 14.05.2015 13:33, Jacek Anaszewski пишет:
>>> Indeed, but with these changes there should be no requirement
>>> for disabling a soft-blink from hard-irq context, which is what
>>> I really wanted to have. What am I missing?
>> Please look at this [1]. Author mentions setting brightness
>> from sound-card irq handler.
>>
>> [1] http://www.spinics.net/lists/linux-leds/msg00006.html
> He points to the following (out-of-tree??) code:
> ---
>          if ((jiffies / HZ / 2) & 1)
>                  led_trigger_blink_oneshot(ledtrig_ide,
>                                  &ide_blink_delay, &ide_blink_delay, 0);
>          if ((jiffies / HZ / 4) & 1)
>                  led_trigger_event(ledtrig_ide, 100);
>          if ((jiffies / HZ / 8) & 1)
>                  led_trigger_event(ledtrig_ide, 0);
> ---
> I think the problem was that oneshot_trig_deactivate() was
> not doing led_stop_software_blink(led_cdev), and so he needed
> a work-queue for switching out from oneshot trigger.
> My patch fixes exactly that: now oneshot trigger does the
> proper cleanup itself.
> Do you think my patch is not enough to handle this case?
>

This problem is related to the issue of locking between hard IRQ
and Softirqs [1]. I've just discovered also that there had been already
an attempt [2] of using work queue internally in led_set_brightness,
but it wouldn't have found its way to the mainline. Probably for a
reason.

[1] 
https://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/c188.html
[2] http://www.serverphorums.com/read.php?12,563432

-- 
Best Regards,
Jacek Anaszewski

      parent reply	other threads:[~2015-05-14 12:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 14:20 [PATCH] leds: fix brightness changing when software blinking is active Stas Sergeev
2015-05-14  8:44 ` Jacek Anaszewski
2015-05-14  9:45   ` Stas Sergeev
2015-05-14 10:33     ` Jacek Anaszewski
2015-05-14 11:03       ` Stas Sergeev
2015-05-14 11:52         ` Stas Sergeev
2015-05-14 12:09         ` Jacek Anaszewski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5554906C.9060405@samsung.com \
    --to=j.anaszewski@samsung.com \
    --cc=cooloney@gmail.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=rpurdie@rpsys.net \
    --cc=stsp@list.ru \
    --cc=stsp@users.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.