[PATCH v4 0/4] Add a generic driver for LED-based backlight

[PATCH v4 0/4] Add a generic driver for LED-based backlight

[Jean-Jacques Hiblot]

This series aims to add a led-backlight driver, similar to pwm-backlight,
but using a LED class device underneath.

A few years ago (2015), Tomi Valkeinen posted a series implementing a
backlight driver on top of a LED device:
https://patchwork.kernel.org/patch/7293991/
https://patchwork.kernel.org/patch/7294001/
https://patchwork.kernel.org/patch/7293981/

The discussion stopped because Tomi lacked the time to work on it.

changes in v4:
- fix dev_err() messages and commit logs following the advices of Pavel
- cosmetic changes (indents, getting rid of  "? 1 : 0" in
  led_match_led_node())

changes in v3:
- dt binding: don't limit the brightness range to 0-255. Use the range of
  the underlying LEDs. as a side-effect, all LEDs must now have the same
  range
- driver: Adapt to dt binding update.
- driver: rework probe() for clarity and remove the remaining goto.

changes in v2:
- handle more than one LED.
- don't make the backlight device a child of the LED controller.
- make brightness-levels and default-brightness-level optional
- removed the option to use a GPIO enable.
- removed the option to use a regulator. It should be handled by the LED
  core
- don't make any change to the LED core (not needed anymore)

Jean-Jacques Hiblot (2):
  leds: Add managed API to get a LED from a device driver
  dt-bindings: backlight: Add led-backlight binding

Tomi Valkeinen (2):
  leds: Add of_led_get() and led_put()
  backlight: add led-backlight driver

 .../bindings/leds/backlight/led-backlight.txt |  28 ++
 drivers/leds/led-class.c                      |  92 ++++++
 drivers/video/backlight/Kconfig               |   7 +
 drivers/video/backlight/Makefile              |   1 +
 drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
 include/linux/leds.h                          |   6 +
 6 files changed, 402 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
 create mode 100644 drivers/video/backlight/led_bl.c

-- 
2.17.1

[PATCH v4 1/4] leds: Add of_led_get() and led_put()

[Jean-Jacques Hiblot]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

This patch adds basic support for a kernel driver to get a LED device.
This will be used by the led-backlight driver.

Only OF version is implemented for now, and the behavior is similar to
PWM's of_pwm_get() and pwm_put().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/leds/led-class.c | 50 ++++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h     |  4 ++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index cadd43c30d50..9f48798a713d 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -18,6 +18,7 @@
 #include <linux/spinlock.h>
 #include <linux/timer.h>
 #include <uapi/linux/uleds.h>
+#include <linux/of.h>
 #include "leds.h"
 
 static struct class *leds_class;
@@ -213,6 +214,55 @@ static int led_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
 
+static int led_match_led_node(struct device *led_dev, const void *data)
+{
+	return led_dev->of_node == data;
+}
+
+/**
+ * of_led_get() - request a LED device via the LED framework
+ * @np: device node to get the LED device from
+ * @index: the index of the LED
+ *
+ * Returns the LED device parsed from the phandle specified in the "leds"
+ * property of a device tree node or a negative error-code on failure.
+ */
+struct led_classdev *of_led_get(struct device_node *np, int index)
+{
+	struct device *led_dev;
+	struct led_classdev *led_cdev;
+	struct device_node *led_node;
+
+	led_node = of_parse_phandle(np, "leds", index);
+	if (!led_node)
+		return ERR_PTR(-ENOENT);
+
+	led_dev = class_find_device(leds_class, NULL, led_node,
+		led_match_led_node);
+	of_node_put(led_node);
+
+	if (!led_dev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	led_cdev = dev_get_drvdata(led_dev);
+
+	if (!try_module_get(led_cdev->dev->parent->driver->owner))
+		return ERR_PTR(-ENODEV);
+
+	return led_cdev;
+}
+EXPORT_SYMBOL_GPL(of_led_get);
+
+/**
+ * led_put() - release a LED device
+ * @led_cdev: LED device
+ */
+void led_put(struct led_classdev *led_cdev)
+{
+	module_put(led_cdev->dev->parent->driver->owner);
+}
+EXPORT_SYMBOL_GPL(led_put);
+
 static int match_name(struct device *dev, const void *data)
 {
 	if (!dev_name(dev))
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bee8e3f8dddd..0a71c7cdd191 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -19,6 +19,7 @@
 
 struct device;
 struct led_pattern;
+struct device_node;
 /*
  * LED Core
  */
@@ -145,6 +146,9 @@ extern void devm_led_classdev_unregister(struct device *parent,
 extern void led_classdev_suspend(struct led_classdev *led_cdev);
 extern void led_classdev_resume(struct led_classdev *led_cdev);
 
+extern struct led_classdev *of_led_get(struct device_node *np, int index);
+extern void led_put(struct led_classdev *led_cdev);
+
 /**
  * led_blink_set - set blinking with software fallback
  * @led_cdev: the LED to start blinking
-- 
2.17.1

[PATCH v4 2/4] leds: Add managed API to get a LED from a device driver

[Jean-Jacques Hiblot]

If the LED is acquired by a consumer device with devm_led_get(), it is
automatically released when the device is detached.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/led-class.c | 42 ++++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h     |  2 ++
 2 files changed, 44 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 9f48798a713d..714b55f1da0f 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -263,6 +263,48 @@ void led_put(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_put);
 
+static void devm_led_release(struct device *dev, void *res)
+{
+	struct led_classdev **p = res;
+
+	led_put(*p);
+}
+
+/**
+ * devm_led_get - Resource-managed request of a LED device
+ * @dev:	LED consumer
+ * @idx:	index of the LED to obtain in the consumer
+ *
+ * The device node of the device is parse to find the request LED device.
+ * The LED device returned from this function is automatically released
+ * on driver detach.
+ *
+ * @return a pointer to a LED device or ERR_PTR(errno) on failure.
+ */
+struct led_classdev *__must_check devm_led_get(struct device *dev,
+					       int index)
+{
+	struct led_classdev *led;
+	struct led_classdev **dr;
+
+	led = of_led_get(dev->of_node, index);
+	if (IS_ERR(led))
+		return led;
+
+	dr = devres_alloc(devm_led_release, sizeof(struct led_classdev *),
+			  GFP_KERNEL);
+	if (!dr) {
+		led_put(led);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	*dr = led;
+	devres_add(dev, dr);
+
+	return led;
+}
+EXPORT_SYMBOL_GPL(devm_led_get);
+
 static int match_name(struct device *dev, const void *data)
 {
 	if (!dev_name(dev))
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0a71c7cdd191..7fcec566d774 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -148,6 +148,8 @@ extern void led_classdev_resume(struct led_classdev *led_cdev);
 
 extern struct led_classdev *of_led_get(struct device_node *np, int index);
 extern void led_put(struct led_classdev *led_cdev);
+struct led_classdev *__must_check devm_led_get(struct device *dev,
+					       int index);
 
 /**
  * led_blink_set - set blinking with software fallback
-- 
2.17.1

[PATCH v4 3/4] dt-bindings: backlight: Add led-backlight binding

[Jean-Jacques Hiblot]

Add DT binding for led-backlight.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 .../bindings/leds/backlight/led-backlight.txt | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt

diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
new file mode 100644
index 000000000000..4c7dfbe7f67a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
@@ -0,0 +1,28 @@
+led-backlight bindings
+
+This binding is used to describe a basic backlight device made of LEDs.
+It can also be used to describe a backlight device controlled by the output of
+a LED driver.
+
+Required properties:
+  - compatible: "led-backlight"
+  - leds: a list of LEDs
+
+Optional properties:
+  - brightness-levels: Array of distinct brightness levels. The levels must be
+                       in the range accepted by the underlying LED devices.
+                       This is used to translate a backlight brightness level
+                       into a LED brightness level. If it is not provided, the
+                       identity mapping is used.
+
+  - default-brightness-level: The default brightness level.
+
+Example:
+
+	backlight {
+		compatible = "led-backlight";
+
+		leds = <&led1>, <&led2>;
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
+	};
-- 
2.17.1

[PATCH v4 4/4] backlight: add led-backlight driver

[Jean-Jacques Hiblot]

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 | 268 +++++++++++++++++++++++++++++++
 3 files changed, 276 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..ac5ff78e7859
--- /dev/null
+++ b/drivers/video/backlight/led_bl.c
@@ -0,0 +1,268 @@
+// 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>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define BKL_FULL_BRIGHTNESS 255
+
+struct led_bl_data {
+	struct device		*dev;
+	struct backlight_device	*bl_dev;
+	struct led_classdev	**leds;
+	bool			enabled;
+	int			nb_leds;
+	unsigned int		*levels;
+	unsigned int		default_brightness;
+	unsigned int		max_brightness;
+};
+
+static int to_led_brightness(struct led_classdev *led, int value)
+{
+	return (value * led->max_brightness) / BKL_FULL_BRIGHTNESS;
+}
+
+static void led_bl_set_brightness(struct led_bl_data *priv, int level)
+{
+	int i;
+	int bkl_brightness;
+
+	if (priv->levels)
+		bkl_brightness = priv->levels[level];
+	else
+		bkl_brightness = level;
+
+	for (i = 0; i < priv->nb_leds; i++) {
+		int led_brightness;
+		struct led_classdev *led = priv->leds[i];
+
+		led_brightness = to_led_brightness(led, bkl_brightness);
+		led_set_brightness(led, led_brightness);
+	}
+
+	priv->enabled = true;
+}
+
+static void led_bl_power_off(struct led_bl_data *priv)
+{
+	int i;
+
+	if (!priv->enabled)
+		return;
+
+	for (i = 0; i < priv->nb_leds; i++)
+		led_set_brightness(priv->leds[i], LED_OFF);
+
+	priv->enabled = false;
+}
+
+static int led_bl_update_status(struct backlight_device *bl)
+{
+	struct led_bl_data *priv = bl_get_data(bl);
+	int brightness = bl->props.brightness;
+
+	if (bl->props.power != FB_BLANK_UNBLANK ||
+	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
+	    bl->props.state & BL_CORE_FBBLANK)
+		brightness = 0;
+
+	if (brightness > 0)
+		led_bl_set_brightness(priv, brightness);
+	else
+		led_bl_power_off(priv);
+
+	return 0;
+}
+
+static const struct backlight_ops led_bl_ops = {
+	.update_status	= led_bl_update_status,
+};
+
+static int led_bl_get_leds(struct device *dev,
+			   struct led_bl_data *priv)
+{
+	int i, nb_leds, ret;
+	struct device_node *node = dev->of_node;
+	struct led_classdev **leds;
+	unsigned int max_brightness;
+	unsigned int default_brightness;
+
+	ret = of_count_phandle_with_args(node, "leds", NULL);
+	if (ret < 0) {
+		dev_err(dev, "Unable to get led count\n");
+		return -EINVAL;
+	}
+
+	nb_leds = ret;
+	if (nb_leds < 1) {
+		dev_err(dev, "At least one LED must be specified!\n");
+		return -EINVAL;
+	}
+
+	leds = devm_kzalloc(dev, sizeof(struct led_classdev *) * nb_leds,
+			    GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	for (i = 0; i < nb_leds; i++) {
+		leds[i] = devm_led_get(dev, i);
+		if (IS_ERR(leds[i]))
+			return PTR_ERR(leds[i]);
+	}
+
+	/* check that the LEDs all have the same brightness range */
+	max_brightness = leds[0]->max_brightness;
+	for (i = 1; i < nb_leds; i++) {
+		if (max_brightness != leds[i]->max_brightness) {
+			dev_err(dev, "LEDs must have identical ranges\n");
+			return -EINVAL;
+		}
+	}
+
+	/* get the default brightness from the first LED from the list */
+	default_brightness = leds[0]->brightness;
+
+	priv->nb_leds = nb_leds;
+	priv->leds = leds;
+	priv->max_brightness = max_brightness;
+	priv->default_brightness = default_brightness;
+
+	return 0;
+}
+
+static int led_bl_parse_levels(struct device *dev,
+			   struct led_bl_data *priv)
+{
+	struct device_node *node = dev->of_node;
+	int num_levels;
+	u32 value;
+	int ret;
+
+	if (!node)
+		return -ENODEV;
+
+	num_levels = of_property_count_u32_elems(node, "brightness-levels");
+	if (num_levels > 1) {
+		int i;
+		unsigned int db;
+		u32 *levels = NULL;
+
+		levels = devm_kzalloc(dev, sizeof(u32) * num_levels,
+				      GFP_KERNEL);
+		if (!levels)
+			return -ENOMEM;
+
+		ret = of_property_read_u32_array(node, "brightness-levels",
+						levels,
+						num_levels);
+		if (ret < 0)
+			return ret;
+
+		/*
+		 * Try to map actual LED brightness to backlight brightness
+		 * level
+		 */
+		db = priv->default_brightness;
+		for (i = 0 ; i < num_levels; i++) {
+			if ((i && db > levels[i-1]) && db <= levels[i])
+				break;
+		}
+		priv->default_brightness = i;
+		priv->max_brightness = num_levels - 1;
+		priv->levels = levels;
+	} else if (num_levels >= 0)
+		dev_warn(dev, "Not enough levels defined\n");
+
+	ret = of_property_read_u32(node, "default-brightness-level", &value);
+	if (!ret && value <= priv->max_brightness)
+		priv->default_brightness = value;
+	else if (!ret  && value > priv->max_brightness)
+		dev_warn(dev, "Invalid default brightness. Ignoring it\n");
+
+	return 0;
+}
+
+static int led_bl_probe(struct platform_device *pdev)
+{
+	struct backlight_properties props;
+	struct led_bl_data *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->dev = &pdev->dev;
+
+	ret = led_bl_get_leds(&pdev->dev, priv);
+	if (ret)
+		return ret;
+
+	ret = led_bl_parse_levels(&pdev->dev, priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to parse DT data\n");
+		return ret;
+	}
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.type = BACKLIGHT_RAW;
+	props.max_brightness = priv->max_brightness;
+	props.brightness = priv->default_brightness;
+	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
+		      FB_BLANK_UNBLANK;
+	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
+			&pdev->dev, priv, &led_bl_ops, &props);
+	if (IS_ERR(priv->bl_dev)) {
+		dev_err(&pdev->dev, "Failed to register backlight\n");
+		return PTR_ERR(priv->bl_dev);
+	}
+
+	backlight_update_status(priv->bl_dev);
+
+	return 0;
+}
+
+static int led_bl_remove(struct platform_device *pdev)
+{
+	struct led_bl_data *priv = platform_get_drvdata(pdev);
+	struct backlight_device *bl = priv->bl_dev;
+
+	backlight_device_unregister(bl);
+
+	led_bl_power_off(priv);
+
+	return 0;
+}
+
+static const struct of_device_id led_bl_of_match[] = {
+	{ .compatible = "led-backlight" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, led_bl_of_match);
+
+static struct platform_driver led_bl_driver = {
+	.driver		= {
+		.name		= "led-backlight",
+		.of_match_table	= of_match_ptr(led_bl_of_match),
+	},
+	.probe		= led_bl_probe,
+	.remove		= led_bl_remove,
+};
+
+module_platform_driver(led_bl_driver);
+
+MODULE_DESCRIPTION("LED based Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:led-backlight");
-- 
2.17.1

Re: [PATCH v4 3/4] dt-bindings: backlight: Add led-backlight binding

[Jacek Anaszewski]

Cc devicetree@vger.kernel.org list - Rob once informed us this gets
higher priority in his queue this way.

On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
> Add DT binding for led-backlight.
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>  .../bindings/leds/backlight/led-backlight.txt | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> new file mode 100644
> index 000000000000..4c7dfbe7f67a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> @@ -0,0 +1,28 @@
> +led-backlight bindings
> +
> +This binding is used to describe a basic backlight device made of LEDs.
> +It can also be used to describe a backlight device controlled by the output of
> +a LED driver.
> +
> +Required properties:
> +  - compatible: "led-backlight"
> +  - leds: a list of LEDs
> +
> +Optional properties:
> +  - brightness-levels: Array of distinct brightness levels. The levels must be
> +                       in the range accepted by the underlying LED devices.
> +                       This is used to translate a backlight brightness level
> +                       into a LED brightness level. If it is not provided, the
> +                       identity mapping is used.
> +
> +  - default-brightness-level: The default brightness level.
> +
> +Example:
> +
> +	backlight {
> +		compatible = "led-backlight";
> +
> +		leds = <&led1>, <&led2>;
> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <6>;
> +	};
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

[Jacek Anaszewski]

On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
> This series aims to add a led-backlight driver, similar to pwm-backlight,
> but using a LED class device underneath.
> 
> A few years ago (2015), Tomi Valkeinen posted a series implementing a
> backlight driver on top of a LED device:
> https://patchwork.kernel.org/patch/7293991/
> https://patchwork.kernel.org/patch/7294001/
> https://patchwork.kernel.org/patch/7293981/
> 
> The discussion stopped because Tomi lacked the time to work on it.
> 
> changes in v4:
> - fix dev_err() messages and commit logs following the advices of Pavel
> - cosmetic changes (indents, getting rid of  "? 1 : 0" in
>   led_match_led_node())
> 
> changes in v3:
> - dt binding: don't limit the brightness range to 0-255. Use the range of
>   the underlying LEDs. as a side-effect, all LEDs must now have the same
>   range
> - driver: Adapt to dt binding update.
> - driver: rework probe() for clarity and remove the remaining goto.
> 
> changes in v2:
> - handle more than one LED.
> - don't make the backlight device a child of the LED controller.
> - make brightness-levels and default-brightness-level optional
> - removed the option to use a GPIO enable.
> - removed the option to use a regulator. It should be handled by the LED
>   core
> - don't make any change to the LED core (not needed anymore)
> 
> Jean-Jacques Hiblot (2):
>   leds: Add managed API to get a LED from a device driver
>   dt-bindings: backlight: Add led-backlight binding
> 
> Tomi Valkeinen (2):
>   leds: Add of_led_get() and led_put()
>   backlight: add led-backlight driver
> 
>  .../bindings/leds/backlight/led-backlight.txt |  28 ++
>  drivers/leds/led-class.c                      |  92 ++++++
>  drivers/video/backlight/Kconfig               |   7 +
>  drivers/video/backlight/Makefile              |   1 +
>  drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
>  include/linux/leds.h                          |   6 +
>  6 files changed, 402 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>  create mode 100644 drivers/video/backlight/led_bl.c
> 

For the whole set:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

Lee - we need to create immutable branch for this set since there will
be some interfering changes in the LED core in this cycle.

I can create the branch and send the pull request once we will
obtain the ack from Rob for DT bindings, unless you have other
preference.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

[Lee Jones]

On Thu, 18 Jul 2019, Jacek Anaszewski wrote:

> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
> > This series aims to add a led-backlight driver, similar to pwm-backlight,
> > but using a LED class device underneath.
> > 
> > A few years ago (2015), Tomi Valkeinen posted a series implementing a
> > backlight driver on top of a LED device:
> > https://patchwork.kernel.org/patch/7293991/
> > https://patchwork.kernel.org/patch/7294001/
> > https://patchwork.kernel.org/patch/7293981/
> > 
> > The discussion stopped because Tomi lacked the time to work on it.
> > 
> > changes in v4:
> > - fix dev_err() messages and commit logs following the advices of Pavel
> > - cosmetic changes (indents, getting rid of  "? 1 : 0" in
> >   led_match_led_node())
> > 
> > changes in v3:
> > - dt binding: don't limit the brightness range to 0-255. Use the range of
> >   the underlying LEDs. as a side-effect, all LEDs must now have the same
> >   range
> > - driver: Adapt to dt binding update.
> > - driver: rework probe() for clarity and remove the remaining goto.
> > 
> > changes in v2:
> > - handle more than one LED.
> > - don't make the backlight device a child of the LED controller.
> > - make brightness-levels and default-brightness-level optional
> > - removed the option to use a GPIO enable.
> > - removed the option to use a regulator. It should be handled by the LED
> >   core
> > - don't make any change to the LED core (not needed anymore)
> > 
> > Jean-Jacques Hiblot (2):
> >   leds: Add managed API to get a LED from a device driver
> >   dt-bindings: backlight: Add led-backlight binding
> > 
> > Tomi Valkeinen (2):
> >   leds: Add of_led_get() and led_put()
> >   backlight: add led-backlight driver
> > 
> >  .../bindings/leds/backlight/led-backlight.txt |  28 ++
> >  drivers/leds/led-class.c                      |  92 ++++++
> >  drivers/video/backlight/Kconfig               |   7 +
> >  drivers/video/backlight/Makefile              |   1 +
> >  drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
> >  include/linux/leds.h                          |   6 +
> >  6 files changed, 402 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >  create mode 100644 drivers/video/backlight/led_bl.c
> > 
> 
> For the whole set:
> 
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> 
> Lee - we need to create immutable branch for this set since there will
> be some interfering changes in the LED core in this cycle.
> 
> I can create the branch and send the pull request once we will
> obtain the ack from Rob for DT bindings, unless you have other
> preference.

We also require a review to be conducted by Daniel Thompson.

After which, an immutable branch sounds like a good idea.  I'd like to
create this myself if you don't mind.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

[Jacek Anaszewski]

On 7/22/19 9:06 AM, Lee Jones wrote:
> On Thu, 18 Jul 2019, Jacek Anaszewski wrote:
> 
>> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
>>> This series aims to add a led-backlight driver, similar to pwm-backlight,
>>> but using a LED class device underneath.
>>>
>>> A few years ago (2015), Tomi Valkeinen posted a series implementing a
>>> backlight driver on top of a LED device:
>>> https://patchwork.kernel.org/patch/7293991/
>>> https://patchwork.kernel.org/patch/7294001/
>>> https://patchwork.kernel.org/patch/7293981/
>>>
>>> The discussion stopped because Tomi lacked the time to work on it.
>>>
>>> changes in v4:
>>> - fix dev_err() messages and commit logs following the advices of Pavel
>>> - cosmetic changes (indents, getting rid of  "? 1 : 0" in
>>>   led_match_led_node())
>>>
>>> changes in v3:
>>> - dt binding: don't limit the brightness range to 0-255. Use the range of
>>>   the underlying LEDs. as a side-effect, all LEDs must now have the same
>>>   range
>>> - driver: Adapt to dt binding update.
>>> - driver: rework probe() for clarity and remove the remaining goto.
>>>
>>> changes in v2:
>>> - handle more than one LED.
>>> - don't make the backlight device a child of the LED controller.
>>> - make brightness-levels and default-brightness-level optional
>>> - removed the option to use a GPIO enable.
>>> - removed the option to use a regulator. It should be handled by the LED
>>>   core
>>> - don't make any change to the LED core (not needed anymore)
>>>
>>> Jean-Jacques Hiblot (2):
>>>   leds: Add managed API to get a LED from a device driver
>>>   dt-bindings: backlight: Add led-backlight binding
>>>
>>> Tomi Valkeinen (2):
>>>   leds: Add of_led_get() and led_put()
>>>   backlight: add led-backlight driver
>>>
>>>  .../bindings/leds/backlight/led-backlight.txt |  28 ++
>>>  drivers/leds/led-class.c                      |  92 ++++++
>>>  drivers/video/backlight/Kconfig               |   7 +
>>>  drivers/video/backlight/Makefile              |   1 +
>>>  drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
>>>  include/linux/leds.h                          |   6 +
>>>  6 files changed, 402 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>>>  create mode 100644 drivers/video/backlight/led_bl.c
>>>
>>
>> For the whole set:
>>
>> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>
>> Lee - we need to create immutable branch for this set since there will
>> be some interfering changes in the LED core in this cycle.
>>
>> I can create the branch and send the pull request once we will
>> obtain the ack from Rob for DT bindings, unless you have other
>> preference.
> 
> We also require a review to be conducted by Daniel Thompson.
> 
> After which, an immutable branch sounds like a good idea.  I'd like to
> create this myself if you don't mind.

Sure, thanks.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 1/4] leds: Add of_led_get() and led_put()

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On Wed 2019-07-17 16:15:11, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> This patch adds basic support for a kernel driver to get a LED device.
> This will be used by the led-backlight driver.
> 
> Only OF version is implemented for now, and the behavior is similar to
> PWM's of_pwm_get() and pwm_put().
> 
> 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>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v4 2/4] leds: Add managed API to get a LED from a device driver

[Tomi Valkeinen]

On 17/07/2019 17:15, Jean-Jacques Hiblot wrote:

> +struct led_classdev *__must_check devm_led_get(struct device *dev,
> +					       int index)
> +{
> +	struct led_classdev *led;
> +	struct led_classdev **dr;
> +

Should you check here if dev->of_node == NULL? Or should of_led_get() 
check it.

> +	led = of_led_get(dev->of_node, index);
> +	if (IS_ERR(led))
> +		return led;
> +
> +	dr = devres_alloc(devm_led_release, sizeof(struct led_classdev *),
> +			  GFP_KERNEL);
> +	if (!dr) {
> +		led_put(led);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	*dr = led;
> +	devres_add(dev, dr);
> +
> +	return led;
> +}

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

[Jacek Anaszewski]

On 7/22/19 11:23 PM, Jacek Anaszewski wrote:
> On 7/22/19 9:06 AM, Lee Jones wrote:
>> On Thu, 18 Jul 2019, Jacek Anaszewski wrote:
>>
>>> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
>>>> This series aims to add a led-backlight driver, similar to pwm-backlight,
>>>> but using a LED class device underneath.
>>>>
>>>> A few years ago (2015), Tomi Valkeinen posted a series implementing a
>>>> backlight driver on top of a LED device:
>>>> https://patchwork.kernel.org/patch/7293991/
>>>> https://patchwork.kernel.org/patch/7294001/
>>>> https://patchwork.kernel.org/patch/7293981/
>>>>
>>>> The discussion stopped because Tomi lacked the time to work on it.
>>>>
>>>> changes in v4:
>>>> - fix dev_err() messages and commit logs following the advices of Pavel
>>>> - cosmetic changes (indents, getting rid of  "? 1 : 0" in
>>>>   led_match_led_node())
>>>>
>>>> changes in v3:
>>>> - dt binding: don't limit the brightness range to 0-255. Use the range of
>>>>   the underlying LEDs. as a side-effect, all LEDs must now have the same
>>>>   range
>>>> - driver: Adapt to dt binding update.
>>>> - driver: rework probe() for clarity and remove the remaining goto.
>>>>
>>>> changes in v2:
>>>> - handle more than one LED.
>>>> - don't make the backlight device a child of the LED controller.
>>>> - make brightness-levels and default-brightness-level optional
>>>> - removed the option to use a GPIO enable.
>>>> - removed the option to use a regulator. It should be handled by the LED
>>>>   core
>>>> - don't make any change to the LED core (not needed anymore)
>>>>
>>>> Jean-Jacques Hiblot (2):
>>>>   leds: Add managed API to get a LED from a device driver
>>>>   dt-bindings: backlight: Add led-backlight binding
>>>>
>>>> Tomi Valkeinen (2):
>>>>   leds: Add of_led_get() and led_put()
>>>>   backlight: add led-backlight driver
>>>>
>>>>  .../bindings/leds/backlight/led-backlight.txt |  28 ++
>>>>  drivers/leds/led-class.c                      |  92 ++++++
>>>>  drivers/video/backlight/Kconfig               |   7 +
>>>>  drivers/video/backlight/Makefile              |   1 +
>>>>  drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
>>>>  include/linux/leds.h                          |   6 +
>>>>  6 files changed, 402 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>>>>  create mode 100644 drivers/video/backlight/led_bl.c
>>>>
>>>
>>> For the whole set:
>>>
>>> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>>
>>> Lee - we need to create immutable branch for this set since there will
>>> be some interfering changes in the LED core in this cycle.
>>>
>>> I can create the branch and send the pull request once we will
>>> obtain the ack from Rob for DT bindings, unless you have other
>>> preference.
>>
>> We also require a review to be conducted by Daniel Thompson.
>>
>> After which, an immutable branch sounds like a good idea.  I'd like to
>> create this myself if you don't mind.
> 
> Sure, thanks.

Unfortunately that hasn't happened and it will miss 5.4 merge window.

Daniel, we need your ack for this patch set.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 4/4] backlight: add led-backlight driver

[Daniel Thompson]

On Wed, Jul 17, 2019 at 04:15:14PM +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 | 268 +++++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/video/backlight/led_bl.c
> 
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> new file mode 100644
> index 000000000000..ac5ff78e7859
> --- /dev/null
> +++ b/drivers/video/backlight/led_bl.c
> @@ -0,0 +1,268 @@
> +// 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>

Why do we need this header file?

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

If we really need to have a full intensity constant then shouldn't we
use LED_FULL directly.

> +
> +struct led_bl_data {
> +	struct device		*dev;
> +	struct backlight_device	*bl_dev;
> +	struct led_classdev	**leds;
> +	bool			enabled;
> +	int			nb_leds;
> +	unsigned int		*levels;
> +	unsigned int		default_brightness;
> +	unsigned int		max_brightness;
> +};
> +
> +static int to_led_brightness(struct led_classdev *led, int value)
> +{
> +	return (value * led->max_brightness) / BKL_FULL_BRIGHTNESS;

This code looks broken.

For example led->max_brightness is 127 then the value this
function will pick values is in the interval 0..63 which is
wrong since we are not using the full range of the LED.

Similarly led->max_brightness is > 255 then we'll generate values
that are out-of-range


Daniel.

Re: [PATCH v4 0/4] Add a generic driver for LED-based backlight

[Daniel Thompson]

On Sun, Sep 08, 2019 at 06:17:50PM +0200, Jacek Anaszewski wrote:
> On 7/22/19 11:23 PM, Jacek Anaszewski wrote:
> > On 7/22/19 9:06 AM, Lee Jones wrote:
> >> On Thu, 18 Jul 2019, Jacek Anaszewski wrote:
> >>
> >>> On 7/17/19 4:15 PM, Jean-Jacques Hiblot wrote:
> >>>> This series aims to add a led-backlight driver, similar to pwm-backlight,
> >>>> but using a LED class device underneath.
> >>>>
> >>>> A few years ago (2015), Tomi Valkeinen posted a series implementing a
> >>>> backlight driver on top of a LED device:
> >>>> https://patchwork.kernel.org/patch/7293991/
> >>>> https://patchwork.kernel.org/patch/7294001/
> >>>> https://patchwork.kernel.org/patch/7293981/
> >>>>
> >>>> The discussion stopped because Tomi lacked the time to work on it.
> >>>>
> >>>> changes in v4:
> >>>> - fix dev_err() messages and commit logs following the advices of Pavel
> >>>> - cosmetic changes (indents, getting rid of  "? 1 : 0" in
> >>>>   led_match_led_node())
> >>>>
> >>>> changes in v3:
> >>>> - dt binding: don't limit the brightness range to 0-255. Use the range of
> >>>>   the underlying LEDs. as a side-effect, all LEDs must now have the same
> >>>>   range
> >>>> - driver: Adapt to dt binding update.
> >>>> - driver: rework probe() for clarity and remove the remaining goto.
> >>>>
> >>>> changes in v2:
> >>>> - handle more than one LED.
> >>>> - don't make the backlight device a child of the LED controller.
> >>>> - make brightness-levels and default-brightness-level optional
> >>>> - removed the option to use a GPIO enable.
> >>>> - removed the option to use a regulator. It should be handled by the LED
> >>>>   core
> >>>> - don't make any change to the LED core (not needed anymore)
> >>>>
> >>>> Jean-Jacques Hiblot (2):
> >>>>   leds: Add managed API to get a LED from a device driver
> >>>>   dt-bindings: backlight: Add led-backlight binding
> >>>>
> >>>> Tomi Valkeinen (2):
> >>>>   leds: Add of_led_get() and led_put()
> >>>>   backlight: add led-backlight driver
> >>>>
> >>>>  .../bindings/leds/backlight/led-backlight.txt |  28 ++
> >>>>  drivers/leds/led-class.c                      |  92 ++++++
> >>>>  drivers/video/backlight/Kconfig               |   7 +
> >>>>  drivers/video/backlight/Makefile              |   1 +
> >>>>  drivers/video/backlight/led_bl.c              | 268 ++++++++++++++++++
> >>>>  include/linux/leds.h                          |   6 +
> >>>>  6 files changed, 402 insertions(+)
> >>>>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >>>>  create mode 100644 drivers/video/backlight/led_bl.c
> >>>>
> >>>
> >>> For the whole set:
> >>>
> >>> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> >>>
> >>> Lee - we need to create immutable branch for this set since there will
> >>> be some interfering changes in the LED core in this cycle.
> >>>
> >>> I can create the branch and send the pull request once we will
> >>> obtain the ack from Rob for DT bindings, unless you have other
> >>> preference.
> >>
> >> We also require a review to be conducted by Daniel Thompson.
> >>
> >> After which, an immutable branch sounds like a good idea.  I'd like to
> >> create this myself if you don't mind.
> > 
> > Sure, thanks.
> 
> Unfortunately that hasn't happened and it will miss 5.4 merge window.
> 
> Daniel, we need your ack for this patch set.

Sorry for dropping the ball here.

I'm afraid I couldn't ack since I spotted a bug but I have shared
review feedback now!


Daniel.