dt-bindings: leds: lacie,netxbig-leds: Convert to DT schema

Convert lacie,netxbig-leds devicetree binding from legacy text format to DT schema.

Signed-off-by Udaya Kiran Challa <[email protected]>

---
Changelog:
Changes since v2:
- Add reference to led common.yaml
- Drop type for all common properties

Link to v2:https://lore.kernel.org/all/[email protected]/

Changes since v1:
- Use pattenProperties for LED child nodes instead of additionalProperties
- Explicitly list the two optional properties supported by the binding
- Place the top-level required block after child-node definition

Link to v1:https://lore.kernel.org/all/[email protected]/

---
 .../bindings/leds/lacie,netxbig-leds.yaml     | 139 ++++++++++++++++++
 .../devicetree/bindings/leds/leds-netxbig.txt |  92 ------------
 2 files changed, 139 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-netxbig.txt

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +21 -0

new file mode 100644
index 000000000000..5cb2eaf74d1a
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/lacie,netxbig-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LaCie/Seagate netxbig CPLD LEDs
+
+maintainers:
+  - Simon Guinot <[email protected]>
+
+description: |
+  Binding for the CPLD LEDs (GPIO extension bus) found on some LaCie/Seagate
+  boards (Example: 2Big/5Big Network v2, 2Big NAS).
+
+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +19 -0

+          Mode to value mapping. Each entry is represented by two integers:
+          a mode and the corresponding value on the gpio-ext bus.
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+
+      bright-addr:
+        description: Brightness register address on gpio-ext bus.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      max-brightness:
+        description: Maximum brightness value.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - mode-addr
+      - mode-val
+      - bright-addr
+      - max-brightness
+
+required:

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 31

Quoted 10 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 38

Quoted 11 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +7 -0

+    type: object
+    $ref: common.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      label: true
+      linux,default-trigger: true

Drop both, common.yaml provides these.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 45

Quoted 8 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +7 -0

+
+      mode-addr:
+        description: Mode register address on gpio-ext bus.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      mode-val:
+        description: |

Do not need '|' unless you need to preserve formatting.

Quoted 24 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +23 -0

+          Mode to value mapping. Each entry is represented by two integers:
+          a mode and the corresponding value on the gpio-ext bus.
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+
+      bright-addr:
+        description: Brightness register address on gpio-ext bus.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      max-brightness:
+        description: Maximum brightness value.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - mode-addr
+      - mode-val
+      - bright-addr
+      - max-brightness
+
+required:
+  - compatible
+  - gpio-ext
+
+additionalProperties: false

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 74

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+
+examples:
+  - |
+    #include <dt-bindings/leds/leds-netxbig.h>
+
+    netxbig-leds {

Just "leds"

Quoted 2 lines

+ compatible = "lacie,netxbig-leds";

Quoted 19 lines

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 31

Quoted 10 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

Quoted 34 lines · Uday Kiran

Quoted 19 lines

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 31

Quoted 10 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Quoted 2 lines

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

The children are leds, so they should use generic node name pattern like "^led-[a-z0-9]+$"

Quoted 36 lines · Krzysztof Kozlowski

Quoted 34 lines · Uday Kiran

Quoted 19 lines

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 31

Quoted 10 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Quoted 6 lines

Quoted 2 lines

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

The children are leds, so they should use generic node name pattern like "^led-[a-z0-9]+$"

Yes, Krzysztof but the original binding uses blue-power, red-power, blue-sata0, etc.
as LED child node names. Would you like me to rename these nodes to led-blue-power, led-red-power, etc. to use the suggested ^led-[a-z0-9]+$ pattern,
or should I retain the existing node names and use a more specific pattern for them
like "^(blue|red)-(power|sata[0-9]+)$"

Quoted 44 lines · Uday Kiran

Quoted 36 lines · Krzysztof Kozlowski

Quoted 34 lines · Uday Kiran

Quoted 19 lines

Quoted 6 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml line 31

Quoted 10 lines

Documentation/devicetree/bindings/leds/lacie,netxbig-leds.yaml new file +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Quoted 6 lines

Quoted 2 lines

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

The children are leds, so they should use generic node name pattern like "^led-[a-z0-9]+$"

Quoted 8 lines

Yes, Krzysztof but the original binding uses blue-power, red-power, blue-sata0, etc.
as LED child node names. Would you like me to rename these nodes to led-blue-power, led-red-power, etc. to use the suggested ^led-[a-z0-9]+$ pattern,
or should I retain the existing node names and use a more specific pattern for them
like "^(blue|red)-(power|sata[0-9]+)$"

I did not see original binding mentioning any names. Neither in the driver.

Quoted 55 lines · Krzysztof Kozlowski

Quoted 44 lines · Uday Kiran

Quoted 36 lines · Krzysztof Kozlowski

Quoted 34 lines · Uday Kiran

Quoted 19 lines

Quoted 6 lines

(quoted diff) +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Review

Quoted 10 lines

(quoted diff) +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Quoted 6 lines

Quoted 2 lines

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

The children are leds, so they should use generic node name pattern like "^led-[a-z0-9]+$"

Quoted 8 lines

Yes, Krzysztof but the original binding uses blue-power, red-power, blue-sata0, etc.
as LED child node names. Would you like me to rename these nodes to led-blue-power, led-red-power, etc. to use the suggested ^led-[a-z0-9]+$ pattern,
or should I retain the existing node names and use a more specific pattern for them
like "^(blue|red)-(power|sata[0-9]+)$"

Quoted 1 line

I did not see original binding mentioning any names. Neither in the driver.

Sorry for the delayed response, Krzysztof.

You are right that the binding text and the driver do not mandate node names. However, the existing device tree source use names like blue-power, red-sata0, etc.:

arch/arm/boot/dts/marvell/kirkwood-netxbig.dtsi

Also, the driver uses the node name as a fallback LED label when the label property is absent.

drivers/leds/leds-netxbig.c, lines 585-588.

If we enforce ^led-[a-z0-9]+$ in the schema, the existing DTS file would fail validation, and renaming the nodes would change the userspace-visible LED names.

Quoted 66 lines · Uday Kiran

Quoted 55 lines · Krzysztof Kozlowski

Quoted 44 lines · Uday Kiran

Quoted 36 lines · Krzysztof Kozlowski

Quoted 34 lines · Uday Kiran

Quoted 19 lines

Quoted 6 lines

(quoted diff) +6 -0

+properties:
+  compatible:
+    const: lacie,netxbig-leds
+
+  gpio-ext:
+    description: Phandle for the gpio-ext bus.

Review

Quoted 10 lines

(quoted diff) +10 -0

+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  timers:
+    description: |
+      Timer array. Each timer entry is represented by three integers:
+      Mode (gpio-ext bus), delay_on and delay_off.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+patternProperties:
+  "^(?!gpio-ext$|pinctrl-[0-9]+$)[a-z][a-z0-9]*(-[a-z0-9]+)+$":

This is an odd pattern. Why is pinctrl an LED? or gpio-ext?

Quoted 4 lines

Actually entire pattern is very odd - it has $ in the middle, but then followed with other REQUIRED stuff. I am surprised it works, but regardless it is absolutely not readable. Looks like a begative pattern, but why would you have negative pattern for known properties?

Thanks for the review Krzysztof.

The negative lookahead was added because patternProperties matches all top- level property names that fit the pattern, so excluded known top-level properties like gpio-ext and pinctrl-*;

In patch v1, I've used additionalProperties which removes the regex entirely and allows arbitrary child-node names. But removed and used patternProperties as per the review feedback.

Quoted 6 lines

Quoted 2 lines

Could you please suggest to me which way to proceed? with additionalProperties or patternProperties

The children are leds, so they should use generic node name pattern like "^led-[a-z0-9]+$"

Quoted 8 lines

Yes, Krzysztof but the original binding uses blue-power, red-power, blue-sata0, etc.
as LED child node names. Would you like me to rename these nodes to led-blue-power, led-red-power, etc. to use the suggested ^led-[a-z0-9]+$ pattern,
or should I retain the existing node names and use a more specific pattern for them
like "^(blue|red)-(power|sata[0-9]+)$"

Quoted 1 line

I did not see original binding mentioning any names. Neither in the driver.

Sorry for the delayed response, Krzysztof.

You are right that the binding text and the driver do not mandate node names. However, the existing device tree source use names like blue-power, red-sata0, etc.:

arch/arm/boot/dts/marvell/kirkwood-netxbig.dtsi

Quoted 4 lines

Also, the driver uses the node name as a fallback LED label when the label property is absent.

drivers/leds/leds-netxbig.c, lines 585-588.

So what is the problem exactly? You rename the nodes and use label. Does anything break?