Re: [PATCH v2 00/12] LED core improvements

[Jacek Anaszewski]

Hi Sakari,

On 10/01/2015 03:22 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> Jacek Anaszewski wrote:
> > This is a second version of the patch set preparing the
> > ground for removing work queues from LED class drivers.
> > Andrew and Sakari - thanks for a review.
> >
> > ================
> > Changes from v1:
> > ================
> > - removed useless use of else in led_set_brightness()
> > - switched to removing error code instead of emitting
> >    warning from led_set_brightness_sync() in case software
> >    blink fallback is enabled
> > - moved __led_set_brightness() contents to set_brightness_delayed()
> > - removed description of internal led_set_brightness_nosleep() from
> >    led-class.txt documentation, and modified description of
> >    led_set_brightness() and led_set_brightness_sync()
> > - renamed LED_BLINK_CHANGE flag to more meaningful
> >    LED_BLINK_BRIGHTNESS_CHANGE
> > - Fixed stylistic issues in a few comments
> > - Adopted old brightness_set_sync() op description to
> >    a new brightness_set_blocking() op
> > - Fixed led_set_brightness_nosleep to avoid scheduling
> >    spurious set_brightness_work
> > - made LED core using EXPORT_SYMBOL_GPL consistently
> > - switched v4l2-flash-led-class wrapper to using
> >    led_set_brightness_sync API for setting torch brightness
> > - merged with patch set
> >    "LED flash: Set brightness in a sync way on demand"
>
> Thanks for the update.
>
> A few comments on no particular patch ---
>
> - Synchronous brightness setting is only possible with drivers that
> implement blocking brigtness setting op. Should the work queue be
> removed from the rest of the drivers, this would be easy to fix.

Work queues are removed from drivers in the patch set [1], and
I plan on picking them after merging this series. Those changes
replace brightness_set op initialization with brightness_set_blocking.
What needs modifications to support led_set_brightness_sync API are
drivers with non-blocking brightness_set op. The question is whether
it should be accomplished by simply adding brightness_set_blocking
op initialization, with the same function as in case of brightness_set
op. Alternatively we could avoid removing SET_BRIGHTNESS_SYNC flag,
and mark non-blocking drivers with it to inform the core that they
set brightness synchronously.

> - led_classdev_suspend() and led_classdev_resume() still unconditionally
> call ->brightness_set() which may now be NULL. This needs to be fixed.

Right, I noticed that at some point but finally forgot to address.
Will fix in the next version.

[1] https://lkml.org/lkml/2015/8/20/426

--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/