From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756233AbbJAN4J (ORCPT ); Thu, 1 Oct 2015 09:56:09 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:26764 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752709AbbJANzl (ORCPT ); Thu, 1 Oct 2015 09:55:41 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-2b-560d3b59bbec Message-id: <560D3B58.2070104@samsung.com> Date: Thu, 01 Oct 2015 15:55:36 +0200 From: Jacek Anaszewski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-version: 1.0 To: Sakari Ailus Cc: linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, andrew@lunn.ch, rpurdie@rpsys.net Subject: Re: [PATCH v2 00/12] LED core improvements References: <1443445641-9529-1-git-send-email-j.anaszewski@samsung.com> <560D338B.6010800@linux.intel.com> In-reply-to: <560D338B.6010800@linux.intel.com> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBLMWRmVeSWpSXmKPExsVy+t/xa7qR1rxhBn++GVucv3uI2eLyrjls FlvfrGO02L3rKavFpy3fmBxYPeadDPTYueMzk8ee+T9YPT5vkgtgieKySUnNySxLLdK3S+DK mLVvHnPBL5GK81uOMzUwzhTsYuTkkBAwkVjcuJIRwhaTuHBvPVsXIxeHkMBSRomf65YzgSSE BJ4xSvS3hXcxcnDwCmhJLJ4eBmKyCKhKXDzJC1LBJmAo8fPFa7BqUYEIiT+n97GC2LwCghI/ Jt9jASkXEdCXmPTADCTMLJAo8X1WP9hWYQFjieY/P1ggFuVI/Lm/AczmBCp/MmUTG0S9tcTK SdsYIWx5ic1r3jJPYBSYhWTDLCRls5CULWBkXsUomlqaXFCclJ5rqFecmFtcmpeul5yfu4kR Er5fdjAuPmZ1iFGAg1GJh/dgCk+YEGtiWXFl7iFGCQ5mJRFembdAId6UxMqq1KL8+KLSnNTi Q4zSHCxK4rxzd70PERJITyxJzU5NLUgtgskycXBKNTBy/J522vnvRMmTpZcUD9XnPH/7LNbk 8aWEqufzbwam+vSmflo6z+7U9J+qLNtqXye9y9bNWvP70bSnQXJeT12r7DWf7pvuvd9r+45n D45oRvBXfef8HhfQWvlkiqKq6Lvnqzk91GsLV/mtTV5xkXEG826BXpkk92s9tW55BiI+fnE1 9eIhp6cosRRnJBpqMRcVJwIARhEyw1sCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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