Re: [PATCH v8 2/5] leds: Add of_led_get() and led_put()

[Jacek Anaszewski]

Hi Jean,

On 10/3/19 2:47 PM, Jean-Jacques Hiblot wrote:
> Hi Sebastian,
> 
> On 03/10/2019 12:42, Sebastian Reichel wrote:
>> Hi,
>>
>> On Thu, Oct 03, 2019 at 10:28:09AM +0200, Jean-Jacques Hiblot wrote:
>>> From: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
>>>
>>> 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@xxxxxx>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@xxxxxx>
>>> Acked-by: Pavel Machek <pavel@xxxxxx>
>>> ---
>>>   drivers/leds/led-class.c | 44 ++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/leds.h     |  4 ++++
>>>   2 files changed, 48 insertions(+)
>>>
>>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>>> index c2167b66b61f..455545f5d663 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,49 @@ 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;
>>> +
>>> +    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);
>> If you convert led_node into a fwnode, you can use
>> class_find_device_by_fwnode() instead. That way the
>> first patch can just be dropped.
> 
> Thanks for the reviews.
> 
> The first patch could be dropped  indeed, but it would break something
> else I'm working on: getting regulator support in the LED core.
> 
> This has been discussed during the v7 iteration of this series.

I wonder if it wouldn't make sense to add support for fwnode
parsing to regulator core. Or maybe it is either somehow supported
or not supported on purpose?

These are questions to regulator maintainers.

Adding Liam and Mark.

-- 
Best regards,
Jacek Anaszewski