On 1/18/19 2:51 PM, Linus Walleij wrote:
On Thu, Jan 17, 2019 at 10:21 PM Jacek Anaszewski
<jacek.anaszewski@xxxxxxxxx> wrote:
Could you share why can't it be fixed by moving blink_set()
to a workqueue in the driver?
It can be done that way, but that will lead to that:
- The existing work struct in struct led_classdev
provided by the LED core is not used, albeit being used
by led_set_brightness()
Generic support for deferring brightness setting is justified because
it is called from led_timer_function() in the LED core (software
blinking fallback) and from timers in triggers.
- led_blink_set() will have different semantic requirements
than led_set_brightness() which can be called from
any context as it will defer to led_set_brightness_nosleep()
-> led_set_brightness_nopm() -> kicks workqueue.
To me that appears arbitrarily inconsistent which is IMO not
a good thing in generic APIs, but maybe there are other
ways to see it?
As stated above software blinking implementation in the core
enforces non-blocking semantics of brightness setting operations.
Currently we don't have anything similar required for blink
setting operations.
LED core maintainability is now hindered sufficiently with that
and also with bad design decision that allowed led_set_brightness()
to be called from hard irq context
(d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink"),
for which set_brightness_work was introduced initially, and its scope of
responsibilities was only extended when adding generic support for non
blocking brightness setting).
Please use in-driver workqueue for calling blink_set().
--
Best regards,
Jacek Anaszewski
