leds: pwm: Make automatic labels work

Since commit 141f15c66d94 ("leds: pwm: remove header") that platform interface is not usable from outside and there seems to be no in tree user anymore. All in-tree users of the leds-pwm driver seem to use DT currently. Getting rid of the old platform interface allows the leds-pwm driver to switch over from 'devm_led_classdev_register()' to 'devm_led_classdev_register_ext()'.

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

Cc Denis Osterland-Heim <[email protected]>

Reviewed-by Marek Behún <[email protected]>

The example was adapted in the following ways:

- make use of the now supported 'function' and 'color' properties
- remove pwm nodes, those are documented elsewhere
- align node names to new dt schema rules and dt recommendations

License was not explicitly set before. The license set now is recommended by DT project.

Suggested-by Jacek Anaszewski <[email protected]>

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

Reviewed-by Krzysztof Kozlowski <[email protected]>

Reviewed-by Rob Herring <[email protected]>

Cc Peter Ujfalusi <[email protected]>

Cc Russell King <[email protected]>

---

Notes:
NOTE: Due to license set/change this needs Acked-by or Signed-off-by from:
* Peter Ujfalusi
* Russell King

That was discussed already with Peter (original author), still waiting for Acked-by though …

Quoted 15 lines · Alexander Dahl

The example was adapted in the following ways:

- make use of the now supported 'function' and 'color' properties
- remove pwm nodes, those are documented elsewhere
- align node names to new dt schema rules and dt recommendations

License was not explicitly set before. The license set now is recommended by DT project.

Suggested-by Jacek Anaszewski <[email protected]>

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

Reviewed-by Krzysztof Kozlowski <[email protected]>

Reviewed-by Rob Herring <[email protected]>

Cc Peter Ujfalusi <[email protected]>

Cc Russell King <[email protected]>

Quoted 6 lines

---

Notes:
NOTE: Due to license set/change this needs Acked-by or Signed-off-by from:
* Peter Ujfalusi
* Russell King

Russell King authored two lines of the documentation. He can state objections now, but I'll not wait for his Acked-by.

Quoted 2 lines

That was discussed already with Peter (original author), still waiting for Acked-by though …

If Peter was okay with license change, I guess we can continue here.

The example was adapted in the following ways:

- make use of the now supported 'function' and 'color' properties
- remove pwm nodes, those are documented elsewhere
- align node names to new dt schema rules and dt recommendations

License was not explicitly set before. The license set now is recommended by DT project.

Suggested-by Jacek Anaszewski <[email protected]>

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

Reviewed-by Krzysztof Kozlowski <[email protected]>

Reviewed-by Rob Herring <[email protected]>

Cc Peter Ujfalusi <[email protected]>

Cc Russell King <[email protected]>

---

Notes:
NOTE: Due to license set/change this needs Acked-by or Signed-off-by from:
* Peter Ujfalusi
* Russell King

That was discussed already with Peter (original author), still waiting for Acked-by though …

Changelog
---------
v6 -> v7:
* added Reviewed-by (Krzysztof Kozlowski)
* reworded commit message (suggested by Krzysztof)
* added Reviewed-by (Rob Herring)

v5 -> v6:
* removed pwm nodes from example (Rob)
* renamed led-controller node in example (Rob)

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    | 70 +++++++++++++++++++
 2 files changed, 70 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 +70 -0

new file mode 100644
index 000000000000..fe4d5fd25913
@@ -0,0 +1,70 @@
+# 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>
+
+    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>;
+        };
+    };
+
+...

Signature · 1 line

2.20.1

Hello,

Peter, Russel, could you please give your Acked-by or Signed-off-by on this patch? Your ack is needed, because the license is now explicitly set (it was not explicit before), and you were the contributors to this binding before the conversion to yaml.

Thanks and Greets
Alex

Quoted 190 lines · Alexander Dahl

The example was adapted in the following ways:

- make use of the now supported 'function' and 'color' properties
- remove pwm nodes, those are documented elsewhere
- align node names to new dt schema rules and dt recommendations

License was not explicitly set before. The license set now is recommended by DT project.

Suggested-by Jacek Anaszewski <[email protected]>

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

Reviewed-by Krzysztof Kozlowski <[email protected]>

Reviewed-by Rob Herring <[email protected]>

Cc Peter Ujfalusi <[email protected]>

Cc Russell King <[email protected]>

---

Notes:
NOTE: Due to license set/change this needs Acked-by or Signed-off-by from:
* Peter Ujfalusi
* Russell King

That was discussed already with Peter (original author), still waiting for Acked-by though …

Changelog
---------
v6 -> v7:
* added Reviewed-by (Krzysztof Kozlowski)
* reworded commit message (suggested by Krzysztof)
* added Reviewed-by (Rob Herring)

v5 -> v6:
* removed pwm nodes from example (Rob)
* renamed led-controller node in example (Rob)

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    | 70 +++++++++++++++++++
 2 files changed, 70 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 +70 -0

new file mode 100644
index 000000000000..fe4d5fd25913
@@ -0,0 +1,70 @@
+# 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>
+
+    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>;
+        };
+    };
+
+...

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)

The example was adapted in the following ways:

- make use of the now supported 'function' and 'color' properties
- remove pwm nodes, those are documented elsewhere
- align node names to new dt schema rules and dt recommendations

License was not explicitly set before. The license set now is recommended by DT project.

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
/home/alex/build/linux/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: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

---

Notes:
v6 -> v7:
* added warning message to commit message (Krzysztof Kozlowski)

v6:
* added this patch to series

 Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/mfd/iqs62x.yaml +3 -2

index 541b06d80e73..92dc48a8dfa7 100644
@@ -90,10 +90,11 @@ examples:
             };
     };
 
-    pwmleds {
+    led-controller {
             compatible = "pwm-leds";
 
-            panel {
+            led-1 {
+                    label = "panel";
                     pwms = <&iqs620a_pwm 0 1000000>;
                     max-brightness = <255>;
             };

Signature · 1 line

2.20.1

Hi Alexander,

Quoted 46 lines · Alexander Dahl

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml

/home/alex/build/linux/Documentation/devicetree/bindings/mfd/iqs62x.exampl e.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
'^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'>
From schema:
/home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/ leds-pwm.yaml>

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

---

Notes:
v6 -> v7:
* added warning message to commit message (Krzysztof Kozlowski)

v6:
* added this patch to series

 Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/mfd/iqs62x.yaml index 541b06d80e73..92dc48a8dfa7 100644

Documentation/devicetree/bindings/mfd/iqs62x.yaml +3 -2

@@ -90,10 +90,11 @@ examples:
             };
 
     };
 
-    pwmleds {
+    led-controller {
 
             compatible = "pwm-leds";
 
-            panel {
+            led-1 {
+                    label = "panel";
 

Quoted 4 lines

                     pwms = <&iqs620a_pwm 0 1000000>;
                     max-brightness = <255>;
             
             };

I like the consistency this brings. My only feedback is that in the other examples I found (common.yaml and leds-gpio.yaml), the children count off from 0 (e.g. led-0) instead of 1 as your series appears to.

That's not a huge deal; it simply seems more consistent to count from the first index allowed by the regex (0). The patch is still fine, and so:

Acked-by Jeff LaBundy <[email protected]>

Hello Jeff,

Am Dienstag, 6. Oktober 2020, 04:17:29 CEST schrieb Jeff LaBundy:

Quoted 49 lines

Hi Alexander,

Quoted 46 lines · Alexander Dahl

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml

/home/alex/build/linux/Documentation/devicetree/bindings/mfd/iqs62x.exampl e.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
'^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'>
From schema:
/home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/ leds-pwm.yaml>

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

---

Notes:
v6 -> v7:
* added warning message to commit message (Krzysztof Kozlowski)

v6:
* added this patch to series

 Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/mfd/iqs62x.yaml index 541b06d80e73..92dc48a8dfa7 100644

Documentation/devicetree/bindings/mfd/iqs62x.yaml +3 -2

@@ -90,10 +90,11 @@ examples:
             };
 
     };
 
-    pwmleds {
+    led-controller {
 
             compatible = "pwm-leds";
 
-            panel {
+            led-1 {
+                    label = "panel";
 

Quoted 8 lines

Quoted 4 lines

                     pwms = <&iqs620a_pwm 0 1000000>;
                     max-brightness = <255>;
             
             };

I like the consistency this brings. My only feedback is that in the other examples I found (common.yaml and leds-gpio.yaml), the children count off from 0 (e.g. led-0) instead of 1 as your series appears to.

You're right. And that's also the same in leds-lp50xx.yaml and … well I did not look close enough, maybe the numbering scheme on the PCB on my desk confused me.

Okay, we are already talking about starting index. What about the parent node's "led-controller" then in case there are more than one? IIRC Rob acknowledged starting from 1 like "led-controller-1", "led-controller-2" and so on.

Quoted 4 lines

That's not a huge deal; it simply seems more consistent to count from the first index allowed by the regex (0). The patch is still fine, and so:

Acked-by Jeff LaBundy <[email protected]>

Thanks.

I'm not sure how many more iterations of this series we will need, at least one for the binding license acks I guess, so I could also adapt the child node indexes in schema and actual dts files in v8 or so.

Greets
Alex

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
/home/alex/build/linux/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: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

---

Notes:
v6 -> v7:
* added warning message to commit message (Krzysztof Kozlowski)

v6:
* added this patch to series

 Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
/home/alex/build/linux/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: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

Quoted 7 lines

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml

Quoted 7 lines

/home/alex/build/linux/Documentation/devicetree/bindings/mfd/iqs62x.exampl e.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
'^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'>
From schema:
/home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/ leds-pwm.yaml>

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

Okay, so I need Rob's ack here, I guess... and rest of the series is not really for my tree.

Hi!

Quoted 9 lines

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
/home/alex/build/linux/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: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

Quoted 1 line

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

Okay, so I need Rob's ack here, I guess... and rest of the series is not really for my tree.

Quoted 9 lines

Quoted 9 lines

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
/home/alex/build/linux/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: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

Quoted 4 lines

Quoted 1 line

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

Okay, so I need Rob's ack here, I guess... and rest of the series is not really for my tree.

This patch is not for your tree either. ;)

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml
CHECK arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml: pwmleds: 'blueled', 'greenled' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/exynos5422-odroidhc1.dt.yaml
CHECK arch/arm/boot/dts/exynos5422-odroidhc1.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5422-odroidhc1.dt.yaml: pwmleds: 'blueled' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml
CHECK arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml: pwmleds: 'blueled', 'greenled' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/exynos5422-odroidxu3-lite.dt.yaml
CHECK arch/arm/boot/dts/exynos5422-odroidxu3-lite.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dt.yaml: pwmleds: 'blueled', 'greenled' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/exynos5422-odroidxu4.dt.yaml
CHECK arch/arm/boot/dts/exynos5422-odroidxu4.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5422-odroidxu4.dt.yaml: pwmleds: 'blueled' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

---

Notes:
v6 -> v7:
* split up patch (one per sub arch)
* added actual warnings to commit message

 arch/arm/boot/dts/exynos5422-odroidhc1.dts      |  4 ++--
 arch/arm/boot/dts/exynos5422-odroidxu4.dts      |  4 ++--
 arch/arm/boot/dts/exynos54xx-odroidxu-leds.dtsi | 11 ++++++-----
 3 files changed, 10 insertions(+), 9 deletions(-)

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml
CHECK arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/exynos5410-odroidxu.dt.yaml: pwmleds: 'blueled', 'greenled' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC arch/arm/boot/dts/imx53-ppd.dt.yaml
CHECK arch/arm/boot/dts/imx53-ppd.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/imx53-ppd.dt.yaml: leds-brightness: 'alarm-brightness' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/imx6dl-cubox-i.dt.yaml
CHECK arch/arm/boot/dts/imx6dl-cubox-i.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/imx6dl-cubox-i.dt.yaml: pwmleds: 'front' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/imx6dl-cubox-i-emmc-som-v15.dt.yaml
CHECK arch/arm/boot/dts/imx6dl-cubox-i-emmc-som-v15.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/imx6dl-cubox-i-emmc-som-v15.dt.yaml: pwmleds: 'front' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/imx6dl-cubox-i-som-v15.dt.yaml
CHECK arch/arm/boot/dts/imx6dl-cubox-i-som-v15.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/imx6dl-cubox-i-som-v15.dt.yaml: pwmleds: 'front' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
DTC arch/arm/boot/dts/imx6sx-softing-vining-2000.dt.yaml
CHECK arch/arm/boot/dts/imx6sx-softing-vining-2000.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/imx6sx-softing-vining-2000.dt.yaml: pwmleds: 'blue', 'green', 'red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
CHECK arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml: led-rgb: 'led-blue', 'led-green', 'led-red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

---

Notes:
v6 -> v7:
* split up patch (one per sub arch)
* added actual warnings to commit message

 arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

index 5700e6b700d3..25d548cb975b 100644
@@ -36,34 +36,34 @@
 		stdout-path = &uart4;
 	};
 
-	led-act {
+	led-controller-1 {
 		compatible = "gpio-leds";
 

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-green {
+		led-3 {
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-blue {
+		led-4 {
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Hello Alexander,

Quoted 10 lines · Alexander Dahl

The node names for devices using the pwm-leds driver follow a certain naming scheme (now). Parent node name is not enforced, but recommended by DT project.

DTC arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
CHECK arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
/home/alex/build/linux/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml: led-rgb: 'led-blue', 'led-green', 'led-red' do not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
From schema: /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml

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

Acked-by Ahmad Fatoum <[email protected]>

But got two questions below:

Quoted 30 lines

---

Notes:
v6 -> v7:
* split up patch (one per sub arch)
* added actual warnings to commit message

 arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

index 5700e6b700d3..25d548cb975b 100644
@@ -36,34 +36,34 @@
 		stdout-path = &uart4;
 	};
 
-	led-act {
+	led-controller-1 {
 		compatible = "gpio-leds";
 
-		led-green {
+		led-1 {
 			label = "mc1:green:act";
 			gpios = <&gpioa 13 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "heartbeat";
 		};
 	};
 

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

 		compatible = "pwm-leds";
 
-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Quoted 21 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-green {
+		led-3 {
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-blue {
+		led-4 {
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Quoted 7 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 5 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

 		compatible = "pwm-leds";
 
-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

Greets
Alex

Quoted 27 lines

Quoted 21 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-green {
+		led-3 {
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-blue {
+		led-4 {
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hello,

Quoted 4 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 6 lines

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Do you happen to know if the new multicolor LED support could be used here?

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

Quoted 8 lines

Quoted 7 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 5 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

 		compatible = "pwm-leds";
 
-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Quoted 3 lines

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Thanks for fixing the dtschema warnings by the way!

Cheers,
Ahmad

Quoted 26 lines

Greets
Alex

Quoted 23 lines

Quoted 23 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 
 		};
 
-		led-green {
+		led-3 {
 
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 
 		};
 
-		led-blue {
+		led-4 {
 
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Hello Ahmad,

Am Dienstag, 27. Oktober 2020, 11:58:10 CET schrieb Ahmad Fatoum:

Quoted 14 lines

Hello,

Quoted 10 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Quoted 1 line

Do you happen to know if the new multicolor LED support could be used here?

AFAIK not yet. The multicolor class should be ready and it is used by some drivers for I²C connected LED controllers, but if I understood Pavel correctly, additional work has to be done for a gpio and/or pwm multicolor driver. See this thread from August for example:

https://lore.kernel.org/linux-leds/2530787.iFCFyWWcSu@g550jk/

Quoted 5 lines

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

The "new" attributes 'function' and 'color' attributes should cover this information. IIRC those were introduced sometime before v5.4 and documentation is in the leds/common.yaml binding. I don't see it in the scope of this patch series, but if we would merge this warning fix first, the information is lost, so maybe those attributes should be added before?

My heuristics on that would be looking at the label and if there's a distinct color in it, add the color property. I could do that for all pwm LEDs known to the tree currently. That would be a bigger task for GPIO leds though. ;-)

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 6 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

 		compatible = "pwm-leds";
 
-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Quoted 6 lines

Quoted 3 lines

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Ack.

Quoted 2 lines

Thanks for fixing the dtschema warnings by the way!

Well, I "introduced" them by converting the leds-pwm binding to yaml (not merged yet), so I could as well fix the warnings then? ;-)

Greets
Alex

Quoted 30 lines

Cheers,
Ahmad

Quoted 26 lines

Greets
Alex

Quoted 23 lines

Quoted 23 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 
 		};
 
-		led-green {
+		led-3 {
 
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 
 		};
 
-		led-blue {
+		led-4 {
 
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Hello,

Quoted 11 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Do you happen to know if the new multicolor LED support could be used here?

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

compatible = "pwm-leds";

Quoted 8 lines

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Thanks for fixing the dtschema warnings by the way!

Cheers,
Ahmad

Quoted 32 lines

Greets
Alex

Quoted 27 lines

Quoted 21 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-green {
+		led-3 {
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-blue {
+		led-4 {
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hei hei,

Quoted 15 lines · Ahmad Fatoum

Hello,

Quoted 11 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Quoted 7 lines

Do you happen to know if the new multicolor LED support could be used here?

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

Slightly off-topic, but while I was working on the patch based on your feedback I tried to find some information on that Linux Automation MC-1 board. However I could not find any? Is there some website, some datasheet or maybe a schematic online? The vendor prefix says "Linux Automation GmbH", but I find only that USB-SD-Mux on their page?

Greets
Alex

Quoted 61 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

compatible = "pwm-leds";

Quoted 8 lines

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Thanks for fixing the dtschema warnings by the way!

Cheers,
Ahmad

Quoted 32 lines

Greets
Alex

Quoted 27 lines

Quoted 21 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

 			label = "mc1:red:rgb";
 			pwms = <&leds_pwm 1 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-green {
+		led-3 {
 			label = "mc1:green:rgb";
 			pwms = <&leds_pwm 2 1000000 0>;
 			max-brightness = <255>;
 			active-low;
 		};
 
-		led-blue {
+		led-4 {
 			label = "mc1:blue:rgb";
 			pwms = <&leds_pwm 3 1000000 0>;
 			max-brightness = <255>;

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hei hei,

Quoted 15 lines · Ahmad Fatoum

Hello,

Quoted 11 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Quoted 7 lines

Do you happen to know if the new multicolor LED support could be used here?

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

Slightly off-topic, but while I was working on the patch based on your feedback I tried to find some information on that Linux Automation MC-1 board. However I could not find any? Is there some website, some datasheet or maybe a schematic online? The vendor prefix says "Linux Automation GmbH", but I find only that USB-SD-Mux on their page?

Greets
Alex

Quoted 43 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

  		compatible = "pwm-leds";

Quoted 8 lines

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Thanks for fixing the dtschema warnings by the way!

Cheers,
Ahmad

Quoted 19 lines

Greets
Alex

Quoted 15 lines

Quoted 15 lines

  			label = "mc1:red:rgb";
  			pwms = <&leds_pwm 1 1000000 0>;
  			max-brightness = <255>;
  			active-low;
  		};

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-green {
+		led-3 {
  			label = "mc1:green:rgb";
  			pwms = <&leds_pwm 2 1000000 0>;
  			max-brightness = <255>;
  			active-low;
  		};

Quoted 19 lines

Quoted 13 lines

Quoted 12 lines

Quoted 6 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-blue {
+		led-4 {
  			label = "mc1:blue:rgb";
  			pwms = <&leds_pwm 3 1000000 0>;
  			max-brightness = <255>;

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-stm32 mailing list
[email protected]
https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

Hi Alex

Quoted 32 lines · Alexander Dahl

Hei hei,

Quoted 15 lines · Ahmad Fatoum

Hello,

Quoted 11 lines · Alexander Dahl

Hello Ahmad,

thanks for your feedback, comments below.

Quoted 4 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 49

Quoted 2 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-	led-rgb {
+	led-controller-2 {

Is a single RGB LED really a controller?

I just followed the recommendations by Rob here.

Quoted 7 lines

Do you happen to know if the new multicolor LED support could be used here?

I find it unfortunate that the device tree loses information relevant to humans to adhere to a fixed nomenclature. Apparently led-controller isn't even codified in the YAML binding (It's just in the examples). If you respin, please add a comment that this is a single RGB led. I'd prefer to keep the information in the DTB as well though.

Slightly off-topic, but while I was working on the patch based on your feedback I tried to find some information on that Linux Automation MC-1 board. However I could not find any? Is there some website, some datasheet or maybe a schematic online? The vendor prefix says "Linux Automation GmbH", but I find only that USB-SD-Mux on their page?

Quoted 2 lines

Greets
Alex

I saw that Ahmad Acked this patch but regarding your discussion it seems there are opening questions. Are you going to send an update of it or can I take it ?

regards
alex

Quoted 61 lines

Quoted 45 lines

Quoted 11 lines

Quoted 10 lines

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts line 52

Quoted 8 lines

  		compatible = "pwm-leds";

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +1 -1

-		led-red {
+		led-2 {

Shouldn't this have been led-1 as well or is the numbering "global" ?

Quoted 3 lines

Also good question. This numbering is for dts only, it usually does not correspond with LEDs on the board, so it could be numbered per led-controller as well?

I'd prefer that it starts by 1. That way it's aligned with PWM channel ID.

Thanks for fixing the dtschema warnings by the way!

Cheers,
Ahmad

Quoted 22 lines

Greets
Alex

Quoted 18 lines

Quoted 18 lines

  			label = "mc1:red:rgb";
  			pwms = <&leds_pwm 1 1000000 0>;
  			max-brightness = <255>;
  			active-low;
  		};

arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts +2 -2

-		led-green {
+		led-3 {
  			label = "mc1:green:rgb";
  			pwms = <&leds_pwm 2 1000000 0>;
  			max-brightness = <255>;
  			active-low;
  		};
-		led-blue {
+		led-4 {
  			label = "mc1:blue:rgb";
  			pwms = <&leds_pwm 3 1000000 0>;
  			max-brightness = <255>;

Quoted 14 lines

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Signature · 4 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Quoted 5 lines

_______________________________________________
Linux-stm32 mailing list
[email protected]
https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 work until recently for the 'leds-pwm' driver.

[...]

Quoted 8 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 work until recently for the 'leds-pwm' driver.

Quoted 1 line

[...]

Applied, thanks!

[1/2] ARM: dts: at91: smartkiz: Reference led node directly

      commit: 02e46262af5db410da5a27783833d68e2bdfb352
[2/2] ARM: dts: at91: Fix schema warnings for pwm-leds
      commit: 997ebd64df39f3916c37ca67a6eff58e79a13c73