leds: pwm: Make automatic labels work

PATCH Superseded Alexander Dahl 4 participants 10 messages Aug 31, 2020 – Sep 9, 2020

Subsystems (2)

Device tree bindings LED

lore

Share & tools

lore

Alexander Dahl

author

2020-08-31 21:03 UTC #0 lore raw

v2 cover leds: pwm: Make automatic labels work

Hei hei,

for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.

v1: based on v5.9-rc2, backport on v5.4.59 tested and working

v2: based on v5.9-rc3, added the dt-bindings update patch

Greets
Alex

Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml

 .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
 .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
 drivers/leds/leds-pwm.c                       |  9 +-
 3 files changed, 93 insertions(+), 51 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

Signature · 1 line

2.20.1

Alexander Dahl

author

2020-08-31 21:04 UTC #1 lore raw

PATCH v2 1/2 leds: pwm: Allow automatic labels for DT based devices

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 <[email protected]>

---
 drivers/leds/leds-pwm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

drivers/leds/leds-pwm.c +8 -1

index ef7b91bd2064..a27a1d75a3e9 100644
@@ -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);

Signature · 1 line

2.20.1

Alexander Dahl

author

2020-08-31 21:04 UTC #2 lore raw

PATCH v2 2/2 dt-bindings: leds: Convert pwm to yaml

In reply to Alexander Dahl

The example was adapted slightly to make use of the 'function' and 'color' properties.

Signed-off-by Alexander Dahl <[email protected]>

---
 .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
 .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
 2 files changed, 85 insertions(+), 50 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

Documentation/devicetree/bindings/leds/leds-pwm.txt deleted +0 ~~-50~~

deleted file mode 100644
index 6c6583c35f2f..000000000000
@@ -1,50 +0,0 @@
-LED connected to PWM
-
-Required properties:
-- compatible : should be "pwm-leds".
-
-Each LED is represented as a sub-node of the pwm-leds device.  Each
-node's name represents the name of the corresponding LED.
-
-LED sub-node properties:
-- pwms : PWM property to point to the PWM device (phandle)/port (id) and to
-  specify the period time to be used: <&phandle id period_ns>;
-- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM device
-  For the pwms and pwm-names property please refer to:
-  Documentation/devicetree/bindings/pwm/pwm.txt
-- max-brightness : Maximum brightness possible for the LED
-- active-low : (optional) For PWMs where the LED is wired to supply
-  rather than ground.
-- label :  (optional)
-  see Documentation/devicetree/bindings/leds/common.txt
-- linux,default-trigger :  (optional)
-  see Documentation/devicetree/bindings/leds/common.txt
-
-Example:
-
-twl_pwm: pwm {
-	/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
-	compatible = "ti,twl6030-pwm";
-	#pwm-cells = <2>;
-};
-
-twl_pwmled: pwmled {
-	/* provides one PWM (id 0 for Charing indicator LED) */
-	compatible = "ti,twl6030-pwmled";
-	#pwm-cells = <2>;
-};
-
-pwmleds {
-	compatible = "pwm-leds";
-	kpad {
-		label = "omap4::keypad";
-		pwms = <&twl_pwm 0 7812500>;
-		max-brightness = <127>;
-	};
-
-	charging {
-		label = "omap4:green:chrg";
-		pwms = <&twl_pwmled 0 7812500>;
-		max-brightness = <255>;
-	};
-};

Documentation/devicetree/bindings/leds/leds-pwm.yaml new file +85 -0

85 lines not on this page · raw message

Signature · 1 line

2.20.1

Alexander Dahl

author

2020-08-31 21:31 UTC #3 lore raw

Re: [PATCH v2 2/2] dt-bindings: leds: Convert pwm to yaml

Hei hei,

I forgot to run checkpatch (it's late today already), and it warns:

WARNING: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause) #80: FILE: Documentation/devicetree/bindings/leds/leds-pwm.yaml:1:
+# SPDX-License-Identifier: GPL-2.0-only

tbh, I just copied that line from leds-gpio.yaml, can be changed in a v3 of course.

Greets
Alex

Quoted 161 lines · Alexander Dahl

The example was adapted slightly to make use of the 'function' and 'color' properties.

Signed-off-by Alexander Dahl <[email protected]>

---
 .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
 .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
 2 files changed, 85 insertions(+), 50 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

Documentation/devicetree/bindings/leds/leds-pwm.txt deleted +0 ~~-50~~

deleted file mode 100644
index 6c6583c35f2f..000000000000
@@ -1,50 +0,0 @@
-LED connected to PWM
-
-Required properties:
-- compatible : should be "pwm-leds".
-
-Each LED is represented as a sub-node of the pwm-leds device.  Each
-node's name represents the name of the corresponding LED.
-
-LED sub-node properties:
-- pwms : PWM property to point to the PWM device (phandle)/port (id) and to
-  specify the period time to be used: <&phandle id period_ns>;
-- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM device
-  For the pwms and pwm-names property please refer to:
-  Documentation/devicetree/bindings/pwm/pwm.txt
-- max-brightness : Maximum brightness possible for the LED
-- active-low : (optional) For PWMs where the LED is wired to supply
-  rather than ground.
-- label :  (optional)
-  see Documentation/devicetree/bindings/leds/common.txt
-- linux,default-trigger :  (optional)
-  see Documentation/devicetree/bindings/leds/common.txt
-
-Example:
-
-twl_pwm: pwm {
-	/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
-	compatible = "ti,twl6030-pwm";
-	#pwm-cells = <2>;
-};
-
-twl_pwmled: pwmled {
-	/* provides one PWM (id 0 for Charing indicator LED) */
-	compatible = "ti,twl6030-pwmled";
-	#pwm-cells = <2>;
-};
-
-pwmleds {
-	compatible = "pwm-leds";
-	kpad {
-		label = "omap4::keypad";
-		pwms = <&twl_pwm 0 7812500>;
-		max-brightness = <127>;
-	};
-
-	charging {
-		label = "omap4:green:chrg";
-		pwms = <&twl_pwmled 0 7812500>;
-		max-brightness = <255>;
-	};
-};

Documentation/devicetree/bindings/leds/leds-pwm.yaml new file +85 -0

new file mode 100644
index 000000000000..8c5217f2a9f7
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LEDs connected to PWM
+
+maintainers:
+  - Pavel Machek <[email protected]>
+
+description:
+  Each LED is represented as a sub-node of the pwm-leds device.  Each
+  node's name represents the name of the corresponding LED.
+
+properties:
+  compatible:
+    const: pwm-leds
+
+patternProperties:
+  "^pwm-led-([0-9a-f])$":
+    type: object
+
+    $ref: common.yaml#
+
+    properties:
+      pwms:
+        description:
+          "PWM property to point to the PWM device (phandle)/port (id)
+          and to specify the period time to be used:
+          <&phandle id period_ns>;"
+
+      pwm-names:
+        description:
+          "Name to be used by the PWM subsystem for the PWM device For
+          the pwms and pwm-names property please refer to:
+          Documentation/devicetree/bindings/pwm/pwm.txt"
+
+      max-brightness:
+        description:
+          Maximum brightness possible for the LED
+
+      active-low:
+        description:
+          For PWMs where the LED is wired to supply rather than ground.
+
+    required:
+      - pwms
+      - max-brightness
+
+examples:
+  - |
+
+    #include <dt-bindings/leds/common.h>
+
+    twl_pwm: pwm {
+        /* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
+        compatible = "ti,twl6030-pwm";
+        #pwm-cells = <2>;
+    };
+
+    twl_pwmled: pwmled {
+        /* provides one PWM (id 0 for Charing indicator LED) */
+        compatible = "ti,twl6030-pwmled";
+        #pwm-cells = <2>;
+    };
+
+    pwm_leds {
+        compatible = "pwm-leds";
+
+        pwm-led-1 {
+            label = "omap4::keypad";
+            pwms = <&twl_pwm 0 7812500>;
+            max-brightness = <127>;
+        };
+
+        pwm-led-2 {
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_CHARGING;
+            pwms = <&twl_pwmled 0 7812500>;
+            max-brightness = <255>;
+        };
+    };
+
+...

Signature · 1 line

2.20.1

Signature · 4 lines

/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)

Jacek Anaszewski

Acked-by on the v2 series

2020-09-01 21:08 UTC #4 lore raw

In reply to Alexander Dahl

Hi Alexander,

Thanks for the v2.

Quoted 20 lines · Alexander Dahl

Hei hei,
for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.
v1: based on v5.9-rc2, backport on v5.4.59 tested and working
v2: based on v5.9-rc3, added the dt-bindings update patch
Greets
Alex
Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml

Quoted 7 lines

  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
  .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
  drivers/leds/leds-pwm.c                       |  9 +-
  3 files changed, 93 insertions(+), 51 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

For both patches:

Acked-by Jacek Anaszewski <[email protected]>

Signature · 2 lines

Best regards,
Jacek Anaszewski

Alexander Dahl

2020-09-04 07:53 UTC #5 lore raw

Hi Jacek,

Am Dienstag, 1. September 2020, 23:08:09 CEST schrieb Jacek Anaszewski:

Quoted 32 lines

Hi Alexander,
Thanks for the v2.
Quoted 20 lines · Alexander Dahl
Hei hei,
for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.
v1: based on v5.9-rc2, backport on v5.4.59 tested and working
v2: based on v5.9-rc3, added the dt-bindings update patch
Greets
Alex
Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml
Quoted 6 lines
  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
  .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
  drivers/leds/leds-pwm.c                       |  9 +-
  3 files changed, 93 insertions(+), 51 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

Quoted 3 lines

For both patches:
Acked-by Jacek Anaszewski <[email protected]>

I'd like to make a v3 and change the license of the .yaml file to "(GPL-2.0- only OR BSD-2-Clause)" as suggested by checkpatch and [1]. Can I keep your Acked-by for that?

Besides: those suggestions are obviously valid for new bindings. What about old bindings (.txt), which had no explicit SPDX tag or license note before? What license would apply there? Is the .yaml file technically new, when it was mostly just converted from .txt?

Greets
Alex

[1] https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html

Jacek Anaszewski

2020-09-04 21:20 UTC #6 lore raw

Hi Alexander,

Quoted 35 lines · Alexander Dahl

Hi Jacek,
Am Dienstag, 1. September 2020, 23:08:09 CEST schrieb Jacek Anaszewski:
Quoted 32 lines
Hi Alexander,
Thanks for the v2.
Quoted 26 lines · Alexander Dahl
Hei hei,
for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.
v1: based on v5.9-rc2, backport on v5.4.59 tested and working
v2: based on v5.9-rc3, added the dt-bindings update patch
Greets
Alex
Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml
   .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
   .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
   drivers/leds/leds-pwm.c                       |  9 +-
   3 files changed, 93 insertions(+), 51 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml

Quoted 7 lines

Quoted 3 lines
For both patches:
Acked-by Jacek Anaszewski <[email protected]>
I'd like to make a v3 and change the license of the .yaml file to "(GPL-2.0- only OR BSD-2-Clause)" as suggested by checkpatch and [1]. Can I keep your Acked-by for that?

Go ahead.

Quoted 4 lines

Besides: those suggestions are obviously valid for new bindings. What about old bindings (.txt), which had no explicit SPDX tag or license note before? What license would apply there? Is the .yaml file technically new, when it was mostly just converted from .txt?

I don't know what was the rationale behind adding license to DT bindings, probably Rob will be able to share some details.

Possibly the fact that DT examples can be now compile-tested makes some difference here.

Signature · 2 lines

Best regards,
Jacek Anaszewski

Pavel Machek

2020-09-09 09:00 UTC #7 lore raw

In reply to Alexander Dahl

Hi!

Quoted 29 lines

Quoted 25 lines
Quoted 18 lines
for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.
v1: based on v5.9-rc2, backport on v5.4.59 tested and working
v2: based on v5.9-rc3, added the dt-bindings update patch
Greets
Alex
Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml
Quoted 6 lines
  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
  .../devicetree/bindings/leds/leds-pwm.yaml    | 85 +++++++++++++++++++
  drivers/leds/leds-pwm.c                       |  9 +-
  3 files changed, 93 insertions(+), 51 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
Quoted 3 lines
For both patches:
Acked-by Jacek Anaszewski <[email protected]>

Quoted 8 lines

I'd like to make a v3 and change the license of the .yaml file to "(GPL-2.0- only OR BSD-2-Clause)" as suggested by checkpatch and [1]. Can I keep your Acked-by for that?
Besides: those suggestions are obviously valid for new bindings. What about old bindings (.txt), which had no explicit SPDX tag or license note before? What license would apply there? Is the .yaml file technically new, when it was mostly just converted from .txt?

If it is based on previous .txt binding, you have to respect previous author's license. That probably means GPL-2.0 only.

Alternatively, you can contact original author(s) to get permission to relicense under (GPL-2.0-only OR BSD-2-Clause).

Best regards,
Pavel

Signature · 2 lines

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

Alexander Dahl

2020-09-09 09:22 UTC #8 lore raw

Hello Pavel,

Am Mittwoch, 9. September 2020, 11:00:33 CEST schrieb Pavel Machek:

Quoted 39 lines

Hi!
Quoted 37 lines
Quoted 29 lines
Quoted 24 lines
for leds-gpio you can use the properties 'function' and 'color' in the devicetree node and omit 'label', the label is constructed automatically. This is a common feature supposed to be working for all
LED drivers. However it did not yet work for the 'leds-pwm' driver. This series fixes the driver and takes the opportunity to update the dt-bindings accordingly.
v1: based on v5.9-rc2, backport on v5.4.59 tested and working
v2: based on v5.9-rc3, added the dt-bindings update patch
Greets
Alex
Alexander Dahl (2):
leds: pwm: Allow automatic labels for DT based devices
dt-bindings: leds: Convert pwm to yaml
  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -----------
  .../devicetree/bindings/leds/leds-pwm.yaml    | 85
  +++++++++++++++++++
  drivers/leds/leds-pwm.c                       |  9 +-
  3 files changed, 93 insertions(+), 51 deletions(-)
Quoted 4 lines
delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
Quoted 3 lines
For both patches:
Acked-by Jacek Anaszewski <[email protected]>
I'd like to make a v3 and change the license of the .yaml file to "(GPL-2.0- only OR BSD-2-Clause)" as suggested by checkpatch and [1]. Can I keep your Acked-by for that?

Quoted 7 lines

Quoted 4 lines
Besides: those suggestions are obviously valid for new bindings. What about old bindings (.txt), which had no explicit SPDX tag or license note before? What license would apply there? Is the .yaml file technically new, when it was mostly just converted from .txt?
If it is based on previous .txt binding, you have to respect previous author's license. That probably means GPL-2.0 only.

Probably?

Quoted 2 lines

Alternatively, you can contact original author(s) to get permission to relicense under (GPL-2.0-only OR BSD-2-Clause).

Judging from your feedback on v3, there will be a v4 anyways, so I contacted Peter Ujfalusi, who added the original .txt binding back in 2012 (merged in 2013).

Thanks for your feedback
Alex

Pavel Machek

2020-09-09 09:27 UTC #9 lore raw

Hi!

Quoted 9 lines

Quoted 7 lines
Quoted 4 lines
Besides: those suggestions are obviously valid for new bindings. What about old bindings (.txt), which had no explicit SPDX tag or license note before? What license would apply there? Is the .yaml file technically new, when it was mostly just converted from .txt?
If it is based on previous .txt binding, you have to respect previous author's license. That probably means GPL-2.0 only.
Probably?

I have not checked exact licensing situation of that text, have not decided if it was copyrightable in the first place, and am not a lawyer.

So... probably :-).

Best regards,
Pavel

Signature · 2 lines

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