Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752666AbbHMOW7 (ORCPT ); Thu, 13 Aug 2015 10:22:59 -0400 Received: from vps0.lunn.ch ([178.209.37.122]:59032 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751931AbbHMOW6 (ORCPT ); Thu, 13 Aug 2015 10:22:58 -0400 Date: Thu, 13 Aug 2015 16:15:47 +0200 From: Andrew Lunn To: Jacek Anaszewski Cc: linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, cooloney@gmail.com, rpurdie@rpsys.net, stsp@users.sourceforge.net, Sakari Ailus , Pavel Machek Subject: Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Message-ID: <20150813141547.GB32484@lunn.ch> References: <1439285890-27329-1-git-send-email-j.anaszewski@samsung.com> <1439285890-27329-2-git-send-email-j.anaszewski@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439285890-27329-2-git-send-email-j.anaszewski@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1912 Lines: 45 On Tue, Aug 11, 2015 at 11:37:14AM +0200, Jacek Anaszewski wrote: > This patch adds a new brightness_set_nonblocking op to the LED subsystem. > The op is intended for drivers that set brightness in a non-blocking way, > i.e. they neither sleep nor use delays while setting brightness. > > Signed-off-by: Jacek Anaszewski > Cc: Bryan Wu > Cc: Andrew Lunn > Cc: Sakari Ailus > Cc: Pavel Machek > Cc: Stas Sergeev > --- > include/linux/leds.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/leds.h b/include/linux/leds.h > index b122eea..c32f1b8 100644 > --- a/include/linux/leds.h > +++ b/include/linux/leds.h > @@ -53,6 +53,9 @@ struct led_classdev { > /* Must not sleep, use a workqueue if needed */ > void (*brightness_set)(struct led_classdev *led_cdev, > enum led_brightness brightness); > + /* Intended for drivers that set brightness in a non-blocking way */ > + void (*brightness_set_nonblocking)(struct led_classdev *led_cdev, > + enum led_brightness brightness); Hi Jacek >From an API design point of view, i'm not sure this is the best way to go. You now have two calls which do the same thing, with the plan that you want to invert the meaning of brightness_set, the old well known API call, sometime later. This inverting the meaning is going to catch people out and introduce bugs. I would rather add a brightness_set_blocking op. Then as you go thought the drivers stripping out the work queue, move the driver to use this brightness_set_blocking. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/