On Sun 2019-06-02 13:20:20, Jacek Anaszewski wrote:
> 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}().
Yep.
> So, Pavel, you seem to have good setup for testing this Linus'
> patch.
OTOH that is _not_ passed down to drivers, as they can blink, but not
do oneshot, and blinking on X60 is limited to single frequency so
likely not useful for triggers.
But we probably want to annotate can/can not sleep and use lockdep to
catch the bugs.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
