On 6/1/19 11:47 PM, Linus Walleij wrote:
On Sat, Jun 1, 2019 at 10:37 PM Pavel Machek <pavel@xxxxxx> wrote:On Tue 2018-11-27 16:01:06, Linus Walleij wrote:I ran into this when working on a keyboard driver for the Razer family: the .blink_set() callback for setting hardware blinking on a LED only exist in a non-blocking (fastpath) variant, such as when blinking can be enabled by simply writing a memory-mapped register and protected by spinlocks. On USB keyboards with blinkable LEDs controlled with USB out-of-band commands this will of course not work: these calls need to come from process context. To support this: add a new .blink_set_blocking() callback in the same vein as .brightness_set_blocking() and add a flag and some code to the delayed work so that this will be able to fire the .blink_set_blocking() call. ALl of this will be handled transparently from the led_blink_set() call so all current users can keep using that. Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>Fun. I just realized thinkpad x60 needs something similar...Hm yeah. The discussion with Jacek came to the conclusion that he thinks (if I understand correctly!) that the LED core is too helpful and client drivers should create process contexts instead (like workers I suppose) and use the opaque interfaces from there, whether they are blocking or not, and that it was a mistake from the beginning to create a helper thread inside the LED core. I like APIs that are narrow and deep so I would prefer to do it my way (i.e. this patch) but arguably it is a matter of taste. I hope to get back to this patch set at some point.
Well, yes, I missed the fact that we already had the use case
in mainline where blink_set is called from atomic context,
which is led_trigger_blink{_oneshot}().
So, Pavel, you seem to have good setup for testing this Linus'
patch.
--
Best regards,
Jacek Anaszewski
