[PATCH v10 0/6] Add a generic driver for LED-based backlight

[PATCH v10 0/6] 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 v10:
- Provide a YAML schema instead of text file for the documentation of the
  LED backlight device-tree bindings.
- Add a short description of how the LED properties should be named in
  the common LED binding documentation. (new patch)
- rename property "default-brightness-level" as "default-brightness",
  following the majority here.

changes in v9:
- let to_of_node() check if the fwnode is actually a of_node
- add some checks in of_led_get()
- let dev_of_node() do the check about OF availabilty
- refactor led_bl_probe() to register a cleanup function with
  devm_add_action_or_reset(). This simplifies the error handling (which
  was not 100% done in v7) and allows to get rid of led_bl_remove()

changes in v8:
- use class_find_device_by_of_node() instead of class_find_device()
- renamed devm_led_get() as devm_of_led_get()

changes in v7:
- rebased on top of linux-leds/for-next
- populate the of_node member of the LED device if fwnode is a of_node
  (this is a new patch and the first of the series).
- devm_led_get() works only when the device tree is used. Add a few checks
  for that.  

changes in v6:
- trim the list of included headers
- remove useless definition of BKL_FULL_BRIGHTNESS

changes in v5:
- removed LED brightness scaling
- disable sysfs the interface of the LEDs when used by the backlight device

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 (4):
  leds: populate the device's of_node
  leds: Add managed API to get a LED from a device driver
  dts-bindings: leds: Document the naming requirement for LED properties
  dt-bindings: backlight: Add led-backlight binding

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

 .../leds/backlight/led-backlight.yaml         |  55 ++++
 .../devicetree/bindings/leds/common.txt       |  20 +-
 drivers/leds/led-class.c                      | 103 ++++++-
 drivers/video/backlight/Kconfig               |   7 +
 drivers/video/backlight/Makefile              |   1 +
 drivers/video/backlight/led_bl.c              | 258 ++++++++++++++++++
 include/linux/leds.h                          |   6 +
 7 files changed, 446 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
 create mode 100644 drivers/video/backlight/led_bl.c

-- 
2.17.1

[PATCH v10 1/6] leds: populate the device's of_node

[Jean-Jacques Hiblot]

If initialization data is available and its fwnode is actually a of_node,
store this information in the led device's structure. This will allow the
device to use or provide OF-based API such (devm_xxx).

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/leds/led-class.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 647b1263c579..bfa1b1033274 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -276,8 +276,10 @@ int led_classdev_register_ext(struct device *parent,
 		mutex_unlock(&led_cdev->led_access);
 		return PTR_ERR(led_cdev->dev);
 	}
-	if (init_data && init_data->fwnode)
+	if (init_data && init_data->fwnode) {
 		led_cdev->dev->fwnode = init_data->fwnode;
+		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
+	}
 
 	if (ret)
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
-- 
2.17.1

Re: [PATCH v10 1/6] leds: populate the device's of_node

[Tomi Valkeinen]

Hi JJ,

On 09/10/2019 11:51, Jean-Jacques Hiblot wrote:
> If initialization data is available and its fwnode is actually a of_node,
> store this information in the led device's structure. This will allow the
> device to use or provide OF-based API such (devm_xxx).
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>   drivers/leds/led-class.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 647b1263c579..bfa1b1033274 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -276,8 +276,10 @@ int led_classdev_register_ext(struct device *parent,
>   		mutex_unlock(&led_cdev->led_access);
>   		return PTR_ERR(led_cdev->dev);
>   	}
> -	if (init_data && init_data->fwnode)
> +	if (init_data && init_data->fwnode) {
>   		led_cdev->dev->fwnode = init_data->fwnode;
> +		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
> +	}
>   
>   	if (ret)
>   		dev_warn(parent, "Led %s renamed to %s due to name collision",

This doesn't compile, as it's missing #include <linux/of.h> (which you 
add in the next patch).

  Tomi

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

[PATCH v10 2/6] 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>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 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 bfa1b1033274..1d1f1d546dc7 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -19,6 +19,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;
@@ -214,6 +215,55 @@ static int led_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
 
+/**
+ * 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;
+
+	/* Not using device tree? */
+	if (!IS_ENABLED(CONFIG_OF))
+		return ERR_PTR(-ENOTSUPP);
+	if (!np)
+		return ERR_PTR(-EINVAL);
+
+	led_node = of_parse_phandle(np, "leds", index);
+	if (!led_node)
+		return ERR_PTR(-ENOENT);
+
+	led_dev = class_find_device_by_of_node(leds_class, 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 led_classdev_next_name(const char *init_name, char *name,
 				  size_t len)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b8df71193329..6f7371bc7757 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -20,6 +20,7 @@
 
 struct device;
 struct led_pattern;
+struct device_node;
 /*
  * LED Core
  */
@@ -196,6 +197,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 v10 3/6] 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 | 49 ++++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h     |  2 ++
 2 files changed, 51 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 1d1f1d546dc7..639224392ffa 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -264,6 +264,55 @@ 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_of_led_get - Resource-managed request of a LED device
+ * @dev:	LED consumer
+ * @index:	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_of_led_get(struct device *dev,
+						  int index)
+{
+	struct led_classdev *led;
+	struct led_classdev **dr;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	/* Consummer not using device tree? */
+	if (!dev_of_node(dev))
+		return ERR_PTR(-ENOTSUPP);
+
+	led = of_led_get(dev_of_node(dev), 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_of_led_get);
+
 static int led_classdev_next_name(const char *init_name, char *name,
 				  size_t len)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6f7371bc7757..9b94cf752012 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -199,6 +199,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_of_led_get(struct device *dev,
+						  int index);
 
 /**
  * led_blink_set - set blinking with software fallback
-- 
2.17.1

[PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Jean-Jacques Hiblot]

LED properties must be named "leds" in the same way that PWM, clocks or
PHY properties are names respectively "pwms", "clocks" and "phys".

Cc: devicetree@vger.kernel.org
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 9fa6f9795d50..31b8c1f68d27 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
 have to be tightly coupled with the LED device binding. They are represented
 by child nodes of the parent LED device binding.
 
+LED properties should be named "leds". The exact meaning of each leds
+property must be documented in the device tree binding for each device.
+
 
 Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
@@ -165,9 +168,20 @@ led-controller@30 {
 		function-enumerator = <2>;
         };
 
-        led@3 {
+        bkl_led0: led@3 {
 		reg = <3>;
-		function = LED_FUNCTION_INDICATOR;
-		function-enumerator = <3>;
+		function = LED_FUNCTION_BACKLIGHT;
+		function-enumerator = <1>;
         };
+
+        bkl_led1: led@4 {
+		reg = <4>;
+		function = LED_FUNCTION_BACKLIGHT;
+		function-enumerator = <2>;
+        };
+};
+
+
+backlight@40 {
+	leds = <&bkl_led0> , <&bkl_led1>;
 };
-- 
2.17.1

Re: [PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Rob Herring]

On Wed, Oct 09, 2019 at 10:51:25AM +0200, Jean-Jacques Hiblot wrote:
> LED properties must be named "leds" in the same way that PWM, clocks or
> PHY properties are names respectively "pwms", "clocks" and "phys".
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>  .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 9fa6f9795d50..31b8c1f68d27 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
>  have to be tightly coupled with the LED device binding. They are represented
>  by child nodes of the parent LED device binding.
>  
> +LED properties should be named "leds". The exact meaning of each leds
> +property must be documented in the device tree binding for each device.
> +

This is worded oddly. The property is 'leds' and it is always a list of 
phandles to LED device nodes. It is present in an LED consumer device.

>  
>  Optional properties for child nodes:
>  - led-sources : List of device current outputs the LED is connected to. The
> @@ -165,9 +168,20 @@ led-controller@30 {
>  		function-enumerator = <2>;
>          };
>  
> -        led@3 {
> +        bkl_led0: led@3 {
>  		reg = <3>;
> -		function = LED_FUNCTION_INDICATOR;
> -		function-enumerator = <3>;
> +		function = LED_FUNCTION_BACKLIGHT;
> +		function-enumerator = <1>;
>          };
> +
> +        bkl_led1: led@4 {
> +		reg = <4>;
> +		function = LED_FUNCTION_BACKLIGHT;
> +		function-enumerator = <2>;
> +        };
> +};
> +
> +
> +backlight@40 {

Either needs 'reg' or the unit-address dropped.

> +	leds = <&bkl_led0> , <&bkl_led1>;

drop the space            ^

>  };
> -- 
> 2.17.1
> 

Re: [PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Jean-Jacques Hiblot]

On 09/10/2019 21:26, Rob Herring wrote:
> On Wed, Oct 09, 2019 at 10:51:25AM +0200, Jean-Jacques Hiblot wrote:
>> LED properties must be named "leds" in the same way that PWM, clocks or
>> PHY properties are names respectively "pwms", "clocks" and "phys".
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
>>   1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 9fa6f9795d50..31b8c1f68d27 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are represented
>>   by child nodes of the parent LED device binding.
>>   
>> +LED properties should be named "leds". The exact meaning of each leds
>> +property must be documented in the device tree binding for each device.
>> +
> This is worded oddly. The property is 'leds' and it is always a list of
> phandles to LED device nodes. It is present in an LED consumer device.

How about:

[...]

A LED consumer device has a 'leds' property. This property is always a list
of phandles to LED nodes (child node of a LED device node).

led_device {
     ...

     led0: led@0 {
         ...
     };

     led1: led@1 {
         ...
     };
};

consumer {
     ...
     leds = <led0>, <led1>;
};

>
>>   
>>   Optional properties for child nodes:
>>   - led-sources : List of device current outputs the LED is connected to. The
>> @@ -165,9 +168,20 @@ led-controller@30 {
>>   		function-enumerator = <2>;
>>           };
>>   
>> -        led@3 {
>> +        bkl_led0: led@3 {
>>   		reg = <3>;
>> -		function = LED_FUNCTION_INDICATOR;
>> -		function-enumerator = <3>;
>> +		function = LED_FUNCTION_BACKLIGHT;
>> +		function-enumerator = <1>;
>>           };
>> +
>> +        bkl_led1: led@4 {
>> +		reg = <4>;
>> +		function = LED_FUNCTION_BACKLIGHT;
>> +		function-enumerator = <2>;
>> +        };
>> +};
>> +
>> +
>> +backlight@40 {
> Either needs 'reg' or the unit-address dropped.
>
>> +	leds = <&bkl_led0> , <&bkl_led1>;
> drop the space            ^
>
>>   };
>> -- 
>> 2.17.1
>>

[PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Jean-Jacques Hiblot]

Add DT binding for led-backlight.

Cc: devicetree@vger.kernel.org
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

---

.../leds/backlight/led-backlight.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
new file mode 100644
index 000000000000..47ae3d6b4f31
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED-based Backlight Device Tree Bindings
+
+maintainers:
+  - Jean-Jacques Hiblot <jjhiblot@ti.com>
+
+description: |
+  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.
+
+properties:
+  compatible:
+    const: led-backlight
+
+  leds:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: List of LEDs constitutive of the backlight.
+
+  default-brightness:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Default brightness level on boot.
+    minimum: 0
+
+  brightness-levels:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: 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.
+
+required:
+  - compatible
+  - backlight-leds
+
+examples:
+  - |
+    backlight {
+      compatible = "led-backlight";
+      backlight-leds = <&led1>;
+      brightness-levels = <0 4 8 16 32 64 128 255>;
+      default-brightness = <6>; /*6th level => brightness of the LEDs is 128*/
+    };
+  - |
+    backlight {
+      compatible = "led-backlight";
+      backlight-leds = <&led1>, <&led2>;
+      default-brightness = <255>;
+    };
+...
-- 
2.17.1

Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Rob Herring]

On Wed, Oct 09, 2019 at 10:51:26AM +0200, Jean-Jacques Hiblot wrote:
> Add DT binding for led-backlight.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> 
> ---
> 
> .../leds/backlight/led-backlight.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> new file mode 100644
> index 000000000000..47ae3d6b4f31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED-based Backlight Device Tree Bindings
> +
> +maintainers:
> +  - Jean-Jacques Hiblot <jjhiblot@ti.com>
> +
> +description: |
> +  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.
> +
> +properties:
> +  compatible:
> +    const: led-backlight
> +
> +  leds:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: List of LEDs constitutive of the backlight.
> +
> +  default-brightness:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Default brightness level on boot.

It's not clear that this is an index when 'brightness-levels' is present 
and absolute level when not. I wonder if we've been consistent on that?

> +    minimum: 0

Implied by being a uint32.

> +
> +  brightness-levels:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: 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.
> +
> +required:
> +  - compatible
> +  - backlight-leds

leds?


Add 'additionalProperties: false' here.

> +
> +examples:
> +  - |
> +    backlight {

Please also define $nodename is 'backlight'.

> +      compatible = "led-backlight";
> +      backlight-leds = <&led1>;

leds?

> +      brightness-levels = <0 4 8 16 32 64 128 255>;
> +      default-brightness = <6>; /*6th level => brightness of the LEDs is 128*/
> +    };
> +  - |
> +    backlight {
> +      compatible = "led-backlight";
> +      backlight-leds = <&led1>, <&led2>;
> +      default-brightness = <255>;
> +    };
> +...
> -- 
> 2.17.1
> 

Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Tony Lindgren]

Hi,

* Rob Herring <robh@kernel.org> [700101 00:00]:
> On Wed, Oct 09, 2019 at 10:51:26AM +0200, Jean-Jacques Hiblot wrote:
> > Add DT binding for led-backlight.
...
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
...

> > +  default-brightness:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Default brightness level on boot.
> 
> It's not clear that this is an index when 'brightness-levels' is present 
> and absolute level when not. I wonder if we've been consistent on that?

Yeah.. And should we use "default-brightness-level" here like we do
in the kernel now?

Sorry if I've missed some discussion on this before..

> > +      brightness-levels = <0 4 8 16 32 64 128 255>;

What we're using for droid4 with an earlier version of this
patch set for the brightness-levels is generated backwards
with:

$ for i in 0 1 2 3 4 5 6 7; do echo "255 - ${i} * (256 / 8)" | bc; done

This produces the following range that seem to behave nicely:

brightness-levels = <31 63 95 127 159 191 223 255>;

Of course depends on the backing hardware, this is with
leds_lm3532 on droid4. But I think also the current example
in the binding might be from Pavel also for droid4?

If so, you might want to update the range :)

Regards,

Tony

[PATCH v10 6/6] 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>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/video/backlight/Kconfig  |   7 +
 drivers/video/backlight/Makefile |   1 +
 drivers/video/backlight/led_bl.c | 258 +++++++++++++++++++++++++++++++
 3 files changed, 266 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..718fa3e03eac
--- /dev/null
+++ b/drivers/video/backlight/led_bl.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015-2019 Texas Instruments Incorporated -  http://www.ti.com/
+ * Authors: Tomi Valkeinen <tomi.valkeinen@ti.com>
+ *          Jean-Jacques Hiblot <jjhiblot@ti.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+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 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++)
+		led_set_brightness(priv->leds[i], bkl_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_of_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", &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 void led_bl_cleanup(void *data)
+{
+	struct led_bl_data *priv = data;
+	int i;
+
+	led_bl_power_off(priv);
+	for (i = 0; i < priv->nb_leds; i++)
+		led_sysfs_enable(priv->leds[i]);
+}
+
+static int led_bl_probe(struct platform_device *pdev)
+{
+	struct backlight_properties props;
+	struct led_bl_data *priv;
+	int ret, i;
+
+	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;
+	}
+
+	ret = devm_add_action_or_reset(&pdev->dev, led_bl_cleanup, priv);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < priv->nb_leds; i++)
+		led_sysfs_disable(priv->leds[i]);
+
+	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 = devm_backlight_device_register(&pdev->dev,
+			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 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	= led_bl_of_match,
+	},
+	.probe		= led_bl_probe,
+};
+
+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 v10 6/6] backlight: add led-backlight driver

[Tony Lindgren]

Hi,

* Jean-Jacques Hiblot <jjhiblot@ti.com> [700101 00:00]:
> 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.
...

> +	ret = of_property_read_u32(node, "default-brightness", &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");

Hmm so just wondering.. Are we using "default-brightness" instead of the
usual "default-brightness-level" here?

Please Cc me on the next patchset too :)

Regards,

Tony

Re: [PATCH v10 6/6] backlight: add led-backlight driver

[Lee Jones]

On Thu, 21 Nov 2019, Tony Lindgren wrote:

> Hi,
> 
> * Jean-Jacques Hiblot <jjhiblot@ti.com> [700101 00:00]:
> > 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.
> ...
> 
> > +	ret = of_property_read_u32(node, "default-brightness", &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");
> 
> Hmm so just wondering.. Are we using "default-brightness" instead of the
> usual "default-brightness-level" here?

Did this get answered?

> Please Cc me on the next patchset too :)

I've been waiting for v11.

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

Re: [PATCH v10 6/6] backlight: add led-backlight driver

[Pavel Machek]

Hi!

> > * Jean-Jacques Hiblot <jjhiblot@ti.com> [700101 00:00]:
> > > 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.
> > ...
> > 
> > > +	ret = of_property_read_u32(node, "default-brightness", &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");
> > 
> > Hmm so just wondering.. Are we using "default-brightness" instead of the
> > usual "default-brightness-level" here?
> 
> Did this get answered?
> 
> > Please Cc me on the next patchset too :)
> 
> I've been waiting for v11.

I guess I could just remove it from a merge for now if there's no other
fix.

Best regards,
								Pavel

Re: [PATCH v10 6/6] backlight: add led-backlight driver

[Lee Jones]

On Tue, 07 Jan 2020, Pavel Machek wrote:

> Hi!
> 
> > > * Jean-Jacques Hiblot <jjhiblot@ti.com> [700101 00:00]:
> > > > 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.
> > > ...
> > > 
> > > > +	ret = of_property_read_u32(node, "default-brightness", &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");
> > > 
> > > Hmm so just wondering.. Are we using "default-brightness" instead of the
> > > usual "default-brightness-level" here?
> > 
> > Did this get answered?
> > 
> > > Please Cc me on the next patchset too :)
> > 
> > I've been waiting for v11.
> 
> I guess I could just remove it from a merge for now if there's no other
> fix.

What do you mean, sorry?

This patch should not be merged anywhere yet.

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