Messages in this thread | | | Subject | Re: [PATCH] leds: pwm: Allow automatic labels for DT based devices | From | Jacek Anaszewski <> | Date | Fri, 28 Aug 2020 22:43:02 +0200 |
| |
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
| |