2019-07-10 12:42:24

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v3 0/4] Add a generic driver for LED-based backlight

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 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 optionnal
- 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


2019-07-10 12:59:17

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v3 1/4] leds: Add of_led_get() and led_put()

From: Tomi Valkeinen <[email protected]>

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 <[email protected]>
Signed-off-by: Jean-Jacques Hiblot <[email protected]>
---
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 e01b2d982564..0f67b13b0f1f 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -19,6 +19,7 @@
#include <linux/timer.h>
#include <linux/regulator/consumer.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);

+static int led_match_led_node(struct device *led_dev, const void *data)
+{
+ return led_dev->of_node == data ? 1 : 0;
+}
+
+/**
+ * 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

2019-07-10 19:11:52

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] leds: Add of_led_get() and led_put()

On Wed 2019-07-10 14:39:29, Jean-Jacques Hiblot wrote:
> From: Tomi Valkeinen <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Jean-Jacques Hiblot <[email protected]>


> @@ -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);
>
> +static int led_match_led_node(struct device *led_dev, const void *data)
> +{
> + return led_dev->of_node == data ? 1 : 0;
> +}

Get rid of the "? 1 : 0"?


> + 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);

Won't this defer probe "forever" when the driver is not available?

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


Attachments:
(No filename) (1.25 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-07-12 13:55:12

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] leds: Add of_led_get() and led_put()

hi Pavel

On 10/07/2019 21:09, Pavel Machek wrote:
> On Wed 2019-07-10 14:39:29, Jean-Jacques Hiblot wrote:
>> From: Tomi Valkeinen <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Jean-Jacques Hiblot <[email protected]>
>>
>>
>> @@ -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);
>>
>> +static int led_match_led_node(struct device *led_dev, const void *data)
>> +{
>> + return led_dev->of_node == data ? 1 : 0;
>> +}
> Get rid of the "? 1 : 0"?
OK
>
>
>> + 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);
> Won't this defer probe "forever" when the driver is not available?

Yes it will.

However I don't see how we can fix this because we don't know for sure
that the LED driver will not become available at a later time.

JJ



>