Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1206857

Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op

From Andrew Lunn <andrew@lunn.ch>
Newsgroups linux.kernel
Subject Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op
Date 2015-08-13 16:30 +0200
Message-ID <pX1G2-5TR-13@gated-at.bofh.it> (permalink)
References <pWeci-17Z-9@gated-at.bofh.it> <pWevF-1vq-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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 <j.anaszewski@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Stas Sergeev <stsp@users.sourceforge.net>
> ---
>  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/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-11 12:00 +0200
  Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Pavel Machek <pavel@ucw.cz> - 2015-08-13 16:30 +0200
  Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Andrew Lunn <andrew@lunn.ch> - 2015-08-13 16:30 +0200
    Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-14 10:00 +0200
      Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-19 16:10 +0200
        Re: [PATCH/RFC v5 01/57] leds: Add brightness_set_nonblocking op Andrew Lunn <andrew@lunn.ch> - 2015-08-19 16:20 +0200

csiph-web