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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux