linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: jacek.anaszewski@gmail.com, pavel@ucw.cz, robh+dt@kernel.org,
	mark.rutland@arm.com, lee.jones@linaro.org, dmurphy@ti.com,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	dridevel@lists.freedesktop.org, tomi.valkeinen@ti.com
Subject: Re: [PATCH v5 4/4] backlight: add led-backlight driver
Date: Tue, 10 Sep 2019 12:26:22 +0100	[thread overview]
Message-ID: <20190910112622.iflmknh5qplbfoyu@holly.lan> (raw)
In-Reply-To: <20190910105946.23057-5-jjhiblot@ti.com>

On Tue, Sep 10, 2019 at 12:59:46PM +0200, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> This patch adds a led-backlight driver (led_bl), which is similar to
> pwm_bl except the driver uses a LED class driver to adjust the
> brightness in the HW. Multiple LEDs can be used for a single backlight.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  drivers/video/backlight/Kconfig  |   7 +
>  drivers/video/backlight/Makefile |   1 +
>  drivers/video/backlight/led_bl.c | 264 +++++++++++++++++++++++++++++++
>  3 files changed, 272 insertions(+)
>  create mode 100644 drivers/video/backlight/led_bl.c
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 8b081d61773e..585a1787618c 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -458,6 +458,13 @@ config BACKLIGHT_RAVE_SP
>  	help
>  	  Support for backlight control on RAVE SP device.
>  
> +config BACKLIGHT_LED
> +	tristate "Generic LED based Backlight Driver"
> +	depends on LEDS_CLASS && OF
> +	help
> +	  If you have a LCD backlight adjustable by LED class driver, say Y
> +	  to enable this driver.
> +
>  endif # BACKLIGHT_CLASS_DEVICE
>  
>  endmenu
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 63c507c07437..2a67642966a5 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
>  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
>  obj-$(CONFIG_BACKLIGHT_ARCXCNN) 	+= arcxcnn_bl.o
>  obj-$(CONFIG_BACKLIGHT_RAVE_SP)		+= rave-sp-backlight.o
> +obj-$(CONFIG_BACKLIGHT_LED)		+= led_bl.o
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> new file mode 100644
> index 000000000000..a72456e11fb9
> --- /dev/null
> +++ b/drivers/video/backlight/led_bl.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2015-2019 Texas Instruments Incorporated -  http://www.ti.com/
> + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> + *
> + * Based on pwm_bl.c
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/gpio/consumer.h>

Maybe this is a nitpick but it is one I have now raised three times and
I don't recall any response, what symbols from this header are used in
this file?

AFAICT everything defined in this header includes the string "gpio" and
that string doesn't appear anywhere in the file (except this line).


> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>

Come to think of it, are you sure you need this include? devm_kzalloc()
doesn't comes from this file.


> +#define BKL_FULL_BRIGHTNESS 255

This is unused. Please remove.


Other than that, looks good!


Daniel.

  reply	other threads:[~2019-09-10 11:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10 10:59 [PATCH v5 0/4] Add a generic driver for LED-based backlight Jean-Jacques Hiblot
2019-09-10 10:59 ` [PATCH v5 1/4] leds: Add of_led_get() and led_put() Jean-Jacques Hiblot
2019-09-10 10:59 ` [PATCH v5 2/4] leds: Add managed API to get a LED from a device driver Jean-Jacques Hiblot
2019-09-10 10:59 ` [PATCH v5 3/4] dt-bindings: backlight: Add led-backlight binding Jean-Jacques Hiblot
2019-09-10 11:26   ` Daniel Thompson
2019-09-10 10:59 ` [PATCH v5 4/4] backlight: add led-backlight driver Jean-Jacques Hiblot
2019-09-10 11:26   ` Daniel Thompson [this message]
2019-09-10 11:38     ` Jean-Jacques Hiblot
2019-09-10 11:39       ` Daniel Thompson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190910112622.iflmknh5qplbfoyu@holly.lan \
    --to=daniel.thompson@linaro.org \
    --cc=dmurphy@ti.com \
    --cc=dridevel@lists.freedesktop.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=jjhiblot@ti.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pavel@ucw.cz \
    --cc=robh+dt@kernel.org \
    --cc=tomi.valkeinen@ti.com \
    --subject='Re: [PATCH v5 4/4] backlight: add led-backlight driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).