From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753259AbbIWIgg (ORCPT ); Wed, 23 Sep 2015 04:36:36 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:63439 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752632AbbIWIge (ORCPT ); Wed, 23 Sep 2015 04:36:34 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-3e-5602648f51d6 Message-id: <5602648D.9040609@samsung.com> Date: Wed, 23 Sep 2015 10:36:29 +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: Andrew Lunn Cc: linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, sakari.ailus@linux.intel.com, stsp@users.sourceforge.net, pavel@ucw.cz, ospite@studenti.unina.it, davem@davemloft.net, linus.walleij@linaro.org, ricardo.ribalda@gmail.com, p.meerwald@bct-electronic.com Subject: Re: [PATCH 3/5] leds: Rename brightness_set_sync op to brightness_set_blocking References: <1442400464-27367-1-git-send-email-j.anaszewski@samsung.com> <1442400464-27367-4-git-send-email-j.anaszewski@samsung.com> <20150922185426.GC20029@lunn.ch> In-reply-to: <20150922185426.GC20029@lunn.ch> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupkkeLIzCtJLcpLzFFi42I5/e/4Zd3+FKYwg/lnFSzO3z3EbDHnfAuL xZQ/y5ksLu+aw2ax9c06Rovf3btZLPoOSVrcPXWUzaKrex6Txact35gsOvumsThwe6y4d5fJ Y8vKm0weO2fdZfe4c20Pm8e8k4EeO3d8ZvI4dGEdi8eK1d/ZPZpOtbN6fN4kF8AVxWWTkpqT WZZapG+XwJXx6NtjtoJmgYrHfe0sDYz3eboYOTkkBEwkDu+9yAZhi0lcuLceyObiEBJYyijx 48dVZgjnGaPE3IPHWUGqeAW0JDZ8nM8MYrMIqEr8a1/LDmKzCRhK/HzxmgnEFhWIkPhzeh9U vaDEj8n3WEBsEQEFiSkn/7CCDGUW6GWS+P5tM9hqYYFIieXtK6G2rWKUuL/zI1gHp4CuREd3 ByOIzSxgLbFy0jYoW15i85q3zBMYBWYhWTILSdksJGULGJlXMYqmliYXFCel5xrqFSfmFpfm pesl5+duYoRE0JcdjIuPWR1iFOBgVOLhtfjOGCbEmlhWXJl7iFGCg1lJhPdpAFOYEG9KYmVV alF+fFFpTmrxIUZpDhYlcd65u96HCAmkJ5akZqemFqQWwWSZODilGhgLe+/7xiy4yZEecIL9 i9QkOW31t9fY/sz6ueeSgVTWg6KjgVI9fxN5HsyyXxf3f/N/1bTiS+6h8tacvQ90Jxczscwy fO5kz3bMfuvRTVvmMl75VB782ths8fcWZXczTs43n67obEw6eHK/xoOLWh8+dUzUty5VS/o7 fysbg8IS1ldl2ae5WOcpsRRnJBpqMRcVJwIAq9YWopwCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/22/2015 08:54 PM, Andrew Lunn wrote: > On Wed, Sep 16, 2015 at 12:47:42PM +0200, Jacek Anaszewski wrote: >> The initial purpose of brightness_set_sync op, introduced along with >> the LED flash class extension, was to add a means for setting torch LED >> brightness as soon as possible, which couldn't have been guaranteed by >> brightness_set op. This patch renames the op to brightness_set_blocking, >> which describes its purpose in a more generic way, and is beneficial >> in view of the prospective changes in the core related to using >> LED core's set_brightness_work for setting brightness for LED class >> drivers that can sleep or use delays while setting brightness. > > ... > >> - /* >> - * Set LED brightness level immediately - it can block the caller for >> - * the time required for accessing a LED device register. >> - */ >> - int (*brightness_set_sync)(struct led_classdev *led_cdev, >> - enum led_brightness brightness); >> + /* Can sleep or use delays */ >> + int (*brightness_set_blocking)(struct led_classdev *led_cdev, > > I'm no expert when it comes to flash photography with digital > cameras. This op is now not specific to flash LEDs. The last sentence in the commit message explains this, but now I see that it is too long. Let's change it to: "This patch renames the op to brightness_set_blocking, which describes its purpose in a more generic way. It is beneficial in view of the prospective changes in the LED core, aiming at removing the need for using work queues in LED class drivers that can sleep or use delays while setting brightness." > But to me the old comment seems better. I changed it to highlight the essence of how it differs from brightness_set. > Doesn't the caller > want to know the flash is now giving out light? We have strobe_get op for this, but it is in led-class-flash extension. This is irrelevant here. > The new comment gives > no indication of this, where as the old one does? -- Best Regards, Jacek Anaszewski