The example was adapted slightly to make use of the 'function' and
'color' properties. License discussed with the original author.
Suggested-by: Jacek Anaszewski <[email protected]>
Signed-off-by: Alexander Dahl <[email protected]>
Cc: Peter Ujfalusi <[email protected]>
---
Notes:
v4 -> v5:
* updated based on feedback by Rob Herring
* removed Acked-by
v3 -> v4:
* added Cc to original author of the binding
v2 -> v3:
* changed license identifier to recommended one
* added Acked-by
v2:
* added this patch to series (Suggested-by: Jacek Anaszewski)
.../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
.../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
2 files changed, 82 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..c9316811c7f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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:
+ "^led(-[0-9a-f]+)?$":
+ type: object
+
+ $ref: common.yaml#
+
+ properties:
+ pwms:
+ maxItems: 1
+
+ pwm-names: true
+
+ max-brightness:
+ description:
+ Maximum brightness possible for the LED
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ active-low:
+ description:
+ For PWMs where the LED is wired to supply rather than ground.
+ type: boolean
+
+ required:
+ - pwms
+ - max-brightness
+
+additionalProperties: false
+
+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";
+
+ led-1 {
+ label = "omap4::keypad";
+ pwms = <&twl_pwm 0 7812500>;
+ max-brightness = <127>;
+ };
+
+ led-2 {
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_CHARGING;
+ pwms = <&twl_pwmled 0 7812500>;
+ max-brightness = <255>;
+ };
+ };
+
+...
--
2.20.1
On Sat, 19 Sep 2020 07:31:45 +0200, Alexander Dahl wrote:
> The example was adapted slightly to make use of the 'function' and
> 'color' properties. License discussed with the original author.
>
> Suggested-by: Jacek Anaszewski <[email protected]>
> Signed-off-by: Alexander Dahl <[email protected]>
> Cc: Peter Ujfalusi <[email protected]>
> ---
>
> Notes:
> v4 -> v5:
> * updated based on feedback by Rob Herring
> * removed Acked-by
>
> v3 -> v4:
> * added Cc to original author of the binding
>
> v2 -> v3:
> * changed license identifier to recommended one
> * added Acked-by
>
> v2:
> * added this patch to series (Suggested-by: Jacek Anaszewski)
>
> .../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
> .../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
> 2 files changed, 82 insertions(+), 50 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
> create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml: pwmleds: 'panel' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-pwm.yaml
See https://patchwork.ozlabs.org/patch/1367461
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
On Sat, Sep 19, 2020 at 07:31:45AM +0200, Alexander Dahl wrote:
> The example was adapted slightly to make use of the 'function' and
> 'color' properties. License discussed with the original author.
>
> Suggested-by: Jacek Anaszewski <[email protected]>
> Signed-off-by: Alexander Dahl <[email protected]>
> Cc: Peter Ujfalusi <[email protected]>
> ---
>
> Notes:
> v4 -> v5:
> * updated based on feedback by Rob Herring
> * removed Acked-by
>
> v3 -> v4:
> * added Cc to original author of the binding
>
> v2 -> v3:
> * changed license identifier to recommended one
> * added Acked-by
>
> v2:
> * added this patch to series (Suggested-by: Jacek Anaszewski)
>
> .../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
> .../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
> 2 files changed, 82 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..c9316811c7f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%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:
> + "^led(-[0-9a-f]+)?$":
> + type: object
> +
> + $ref: common.yaml#
> +
> + properties:
> + pwms:
> + maxItems: 1
> +
> + pwm-names: true
> +
> + max-brightness:
> + description:
> + Maximum brightness possible for the LED
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + active-low:
> + description:
> + For PWMs where the LED is wired to supply rather than ground.
> + type: boolean
> +
> + required:
> + - pwms
> + - max-brightness
> +
> +additionalProperties: false
> +
> +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>;
> + };
It would be best to just remove these 2 nodes. The provider is not
really relevant here and these will likely have schema errors when
there's a schema for them. For example, they should be child nodes of
the TWL6030 device.
> +
> + pwm_leds {
Use generic node names:
led-controller {
> + compatible = "pwm-leds";
> +
> + led-1 {
> + label = "omap4::keypad";
> + pwms = <&twl_pwm 0 7812500>;
> + max-brightness = <127>;
> + };
> +
> + led-2 {
> + color = <LED_COLOR_ID_GREEN>;
> + function = LED_FUNCTION_CHARGING;
> + pwms = <&twl_pwmled 0 7812500>;
> + max-brightness = <255>;
> + };
> + };
> +
> +...
> --
> 2.20.1
>
Hello Rob,
Am Dienstag, 22. September 2020, 17:42:58 CEST schrieb Rob Herring:
> On Sat, 19 Sep 2020 07:31:45 +0200, Alexander Dahl wrote:
> > The example was adapted slightly to make use of the 'function' and
> > 'color' properties. License discussed with the original author.
> >
> > Suggested-by: Jacek Anaszewski <[email protected]>
> > Signed-off-by: Alexander Dahl <[email protected]>
> > Cc: Peter Ujfalusi <[email protected]>
> > ---
> >
> > Notes:
> > v4 -> v5:
> > * updated based on feedback by Rob Herring
> > * removed Acked-by
> >
> > v3 -> v4:
> > * added Cc to original author of the binding
> >
> > v2 -> v3:
> > * changed license identifier to recommended one
> > * added Acked-by
> >
> > v2:
> > * added this patch to series (Suggested-by: Jacek Anaszewski)
> >
> > .../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
> > .../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
> > 2 files changed, 82 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
> > create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/iqs
> 62x.example.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
> '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+' From schema:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/l
> eds-pwm.yaml
I somehow expected errors on those checks, because I got actually two
different recommendations from you:
In feedback on v4 of this patch (series) you recommended '^led(-[0-9a-f]+)?$'
for the (pwm) led node name, which I used in v5. Or just allow any node name
with ".*" like in gpio-keys.yaml …
I just checked all in-tree dts files using "pwm-leds" and each also defines
the "label" property, so renaming those nodes should not alter the paths in
sysfs, if I understood everything correctly. So I see two options now:
1) Go with the stricter check and fix all failing dts files and examples.
2) Just use the very loose check.
If 1), which patch would go first, renaming nodes in dts and examples or
converting bindings to yaml enabling the stricter check?
> Please check and re-submit.
Will do, maybe I split the patch series and send both remaining patches
separately?
Alex
On Mon, Sep 28, 2020 at 6:19 AM Alexander Dahl <[email protected]> wrote:
>
> Hello Rob,
>
> Am Dienstag, 22. September 2020, 17:42:58 CEST schrieb Rob Herring:
> > On Sat, 19 Sep 2020 07:31:45 +0200, Alexander Dahl wrote:
> > > The example was adapted slightly to make use of the 'function' and
> > > 'color' properties. License discussed with the original author.
> > >
> > > Suggested-by: Jacek Anaszewski <[email protected]>
> > > Signed-off-by: Alexander Dahl <[email protected]>
> > > Cc: Peter Ujfalusi <[email protected]>
> > > ---
> > >
> > > Notes:
> > > v4 -> v5:
> > > * updated based on feedback by Rob Herring
> > > * removed Acked-by
> > >
> > > v3 -> v4:
> > > * added Cc to original author of the binding
> > >
> > > v2 -> v3:
> > > * changed license identifier to recommended one
> > > * added Acked-by
> > >
> > > v2:
> > > * added this patch to series (Suggested-by: Jacek Anaszewski)
> > >
> > > .../devicetree/bindings/leds/leds-pwm.txt | 50 -----------
> > > .../devicetree/bindings/leds/leds-pwm.yaml | 82 +++++++++++++++++++
> > > 2 files changed, 82 insertions(+), 50 deletions(-)
> > > delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
> > > create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/iqs
> > 62x.example.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
> > '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+' From schema:
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/l
> > eds-pwm.yaml
>
> I somehow expected errors on those checks, because I got actually two
> different recommendations from you:
>
> In feedback on v4 of this patch (series) you recommended '^led(-[0-9a-f]+)?$'
> for the (pwm) led node name, which I used in v5. Or just allow any node name
> with ".*" like in gpio-keys.yaml …
>
> I just checked all in-tree dts files using "pwm-leds" and each also defines
> the "label" property, so renaming those nodes should not alter the paths in
> sysfs, if I understood everything correctly. So I see two options now:
>
> 1) Go with the stricter check and fix all failing dts files and examples.
>
> 2) Just use the very loose check.
Either one is fine. Given label is present and there's not a ton of
cases, then I'd probably go with 1.
> If 1), which patch would go first, renaming nodes in dts and examples or
> converting bindings to yaml enabling the stricter check?
There's currently no requirement on dts files being warning free. So
the schema can come first and any dts fixes later.
Rob
Hei hei,
Am Dienstag, 22. September 2020, 17:57:47 CEST schrieb Rob Herring:
> Use generic node names:
>
> led-controller {
>
> > + compatible = "pwm-leds";
> > +
> > + led-1 {
> > + label = "omap4::keypad";
> > + pwms = <&twl_pwm 0 7812500>;
> > + max-brightness = <127>;
> > + };
> > +
> > + led-2 {
> > + color = <LED_COLOR_ID_GREEN>;
> > + function = LED_FUNCTION_CHARGING;
> > + pwms = <&twl_pwmled 0 7812500>;
> > + max-brightness = <255>;
> > + };
> > + };
> > +
> > +...
This is clear for the "one led-controller" case. However, when trying to fix
those node names in existing .dts files, I wondered how those should be named
for multiple, different led-controllers, e.g. one using "pwm-leds" and another
one using "gpio-leds"?
See arch/arm/boot/dts/at91-kizbox3-hs.dts for example, the nodes are called
"pwm_leds" and "leds" currently. If both were part of a .dtsi and both named
the same, you could not overwrite/complement those in a .dts file including
that .dtsi due to a name conflict.
Just append a numerical index like this?
led-controller-1 {
…
};
led-controller-2 {
…
};
Greets
Alex
On Tue, Sep 29, 2020 at 2:39 AM Alexander Dahl <[email protected]> wrote:
>
> Hei hei,
>
> Am Dienstag, 22. September 2020, 17:57:47 CEST schrieb Rob Herring:
> > Use generic node names:
> >
> > led-controller {
> >
> > > + compatible = "pwm-leds";
> > > +
> > > + led-1 {
> > > + label = "omap4::keypad";
> > > + pwms = <&twl_pwm 0 7812500>;
> > > + max-brightness = <127>;
> > > + };
> > > +
> > > + led-2 {
> > > + color = <LED_COLOR_ID_GREEN>;
> > > + function = LED_FUNCTION_CHARGING;
> > > + pwms = <&twl_pwmled 0 7812500>;
> > > + max-brightness = <255>;
> > > + };
> > > + };
> > > +
> > > +...
>
> This is clear for the "one led-controller" case. However, when trying to fix
> those node names in existing .dts files, I wondered how those should be named
> for multiple, different led-controllers, e.g. one using "pwm-leds" and another
> one using "gpio-leds"?
>
> See arch/arm/boot/dts/at91-kizbox3-hs.dts for example, the nodes are called
> "pwm_leds" and "leds" currently. If both were part of a .dtsi and both named
> the same, you could not overwrite/complement those in a .dts file including
> that .dtsi due to a name conflict.
>
> Just append a numerical index like this?
>
> led-controller-1 {
> …
> };
>
> led-controller-2 {
> …
> };
Yes, that's generally what we've been doing.
Rob