2020-08-31 22:57:27

by Alexander Dahl

[permalink] [raw]
Subject: [PATCH v2 0/2] 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

--
2.20.1


2020-08-31 22:57:48

by Alexander Dahl

[permalink] [raw]
Subject: [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(-)

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

2020-08-31 22:59:59

by Alexander Dahl

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: leds: Convert pwm to yaml

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

Suggested-by: Jacek Anaszewski <[email protected]>
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

diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.txt b/Documentation/devicetree/bindings/leds/leds-pwm.txt
deleted file mode 100644
index 6c6583c35f2f..000000000000
--- a/Documentation/devicetree/bindings/leds/leds-pwm.txt
+++ /dev/null
@@ -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>;
- };
-};
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.yaml b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
new file mode 100644
index 000000000000..8c5217f2a9f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
@@ -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>;
+ };
+ };
+
+...
--
2.20.1

2020-09-01 21:11:24

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hi Alexander,

Thanks for the v2.

On 8/31/20 11:02 PM, Alexander Dahl wrote:
> 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
>

For both patches:

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

--
Best regards,
Jacek Anaszewski

2020-09-04 07:56:51

by Alexander Dahl

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hi Jacek,

Am Dienstag, 1. September 2020, 23:08:09 CEST schrieb Jacek Anaszewski:
> Hi Alexander,
>
> Thanks for the v2.
>
> On 8/31/20 11:02 PM, Alexander Dahl wrote:
> > 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
>
> 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



2020-09-04 21:20:53

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hi Alexander,

On 9/4/20 9:53 AM, Alexander Dahl wrote:
> Hi Jacek,
>
> Am Dienstag, 1. September 2020, 23:08:09 CEST schrieb Jacek Anaszewski:
>> Hi Alexander,
>>
>> Thanks for the v2.
>>
>> On 8/31/20 11:02 PM, Alexander Dahl wrote:
>>> 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
>>
>> 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.

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

--
Best regards,
Jacek Anaszewski

2020-09-09 09:01:40

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hi!

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

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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


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

2020-09-09 09:23:31

by Alexander Dahl

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hello Pavel,

Am Mittwoch, 9. September 2020, 11:00:33 CEST schrieb Pavel Machek:
> Hi!
>
> > > > 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
> > >
> > > 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?
>
> If it is based on previous .txt binding, you have to respect previous
> author's license. That probably means GPL-2.0 only.

Probably?

> 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



2020-09-09 09:30:29

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] leds: pwm: Make automatic labels work

Hi!

> > > 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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


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