On Mon, Apr 17, 2023 at 08:33:31PM +0200, Jacek Anaszewski wrote:
> Hi Uwe,
>
> On 4/17/23 14:44, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Mon, Apr 17, 2023 at 02:28:52PM +0200, Pavel Machek wrote:
> > > > After commit ba8a86e4dadb ("leds: trigger/tty: Use
> > > > led_set_brightness_sync() from workqueue") this is the second try to
> > > > pick the right function to set the LED brightness from a trigger.
> > > >
> > > > led_set_brightness_sync() has the problem that it doesn't work for LEDs
> > > > without a .brightness_set_blocking() callback. This is (among others)
> > > > the case for LEDs connected to non-sleeping GPIOs.
> > > >
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > >
> > > I don't think this is right.
> > >
> > > _nosleep calls _nopm, which assmues it can't sleep, and schedules
> > > another workqueue to set the LED.
> >
> > Then which is the right variant?
> > led_set_brightness() and led_set_brightness_nosleep() set via a workqueue
> > (which is bad) and led_set_brightness_sync() doesn't work for some LEDs
> > (notably LEDs on non-sleeping GPIOs).
>
> Can you remind me the context of this patch, why using workqueue is
> bad here?
The workqueue is only needed if you have a slow LED and want to set the
brightness in atomic context. However if you are allowed to sleep that
is just needless overhead.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
