lkml.org 
[lkml]   [2020]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] leds: pwm: Allow automatic labels for DT based devices
From
Date
Hi Alexander,

On 8/28/20 9:00 AM, Alexander Dahl wrote:
> Hello Jacek,
>
> Am Donnerstag, 27. August 2020, 23:28:45 CEST schrieb Jacek Anaszewski:
>> On 8/26/20 11:37 AM, Alexander Dahl wrote:
>>> From: Alexander Dahl <post@lespocky.de>
>>>
>>> If LEDs are configured through device tree and the property 'label' is
>>> omitted, the label is supposed to be generated from the properties
>>> 'function' and 'color' if present. While this works fine for e.g. the
>>> 'leds-gpio' driver, it did not for 'leds-pwm'.
>>>
>>> The reason is, you get this label naming magic only if you add a LED
>>> device through 'devm_led_classdev_register_ext()' and pass a pointer to
>>> the current device tree node. The approach to fix this was adopted from
>>> the 'leds-gpio' driver.
>>>
>>> For the following node from dts the LED appeared as 'led5' in sysfs
>>> before and as 'red:debug' after this change.
>>>
>>> pwm_leds {
>>>
>>> compatible = "pwm-leds";
>>>
>>> led5 {
>>>
>>> function = LED_FUNCTION_DEBUG;
>>> color = <LED_COLOR_ID_RED>;
>>> pwms = <&pwm0 2 10000000 0>;
>>> max-brightness = <127>;
>>>
>>> linux,default-trigger = "heartbeat";
>>> panic-indicator;
>>>
>>> };
>>>
>>> };
>>>
>>> Signed-off-by: Alexander Dahl <post@lespocky.de>
>>> ---
>>>
>>> Notes:
>>> v1: based on v5.9-rc2, backport on v5.4.59 also works
>>>
>>> drivers/leds/leds-pwm.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
>>> index ef7b91bd2064..a27a1d75a3e9 100644
>>> --- a/drivers/leds/leds-pwm.c
>>> +++ b/drivers/leds/leds-pwm.c
>>> @@ -65,6 +65,7 @@ static int led_pwm_add(struct device *dev, struct
>>> led_pwm_priv *priv,>
>>> struct led_pwm *led, struct fwnode_handle *fwnode)
>>>
>>> {
>>>
>>> struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
>>>
>>> + struct led_init_data init_data = {};
>>>
>>> int ret;
>>>
>>> led_data->active_low = led->active_low;
>>>
>>> @@ -90,7 +91,13 @@ static int led_pwm_add(struct device *dev, struct
>>> led_pwm_priv *priv,>
>>> pwm_init_state(led_data->pwm, &led_data->pwmstate);
>>>
>>> - ret = devm_led_classdev_register(dev, &led_data->cdev);
>>> + if (fwnode) {
>>> + init_data.fwnode = fwnode;
>>> + ret = devm_led_classdev_register_ext(dev, &led_data->cdev,
>>> + &init_data);
>>> + } else {
>>> + ret = devm_led_classdev_register(dev, &led_data->cdev);
>>> + }
>>>
>>> if (ret) {
>>>
>>> dev_err(dev, "failed to register PWM led for %s: %d\n",
>>>
>>> led->name, ret);
>>
>> This part looks good, but corresponding update of
>> Documentation/devicetree/bindings/leds/leds-pwm.txt is needed as well.
>
> I'm not sure, what needs updating. The properties 'function' and 'color' are
> already documented in Documentation/devicetree/bindings/leds/common.yaml … the
> only thing I can think of here is updating the examples? That would be nice,
> as would be updating to yaml, but I don't see the strong relation, yet.

It is necessary to tell the user that given driver is capable of
utilizing a property. I thought of something like in commit [0].

>> It would be good to switch to yaml by this occassion.
>
> Is there some guidance on that in general?

I am not aware of, but surely sooner or later all bindings will
need to be unified. Touching the file is always a good opportunity
to address that. It's up to you, though.

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/leds/leds-lm3692x.txt?id=4dcbc8f8c59f4b618d651f5ba884ee5bf562c8de

--
Best regards,
Jacek Anaszewski

\
 
 \ /
  Last update: 2020-08-28 22:44    [W:5.532 / U:0.884 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site