devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: leds: bcm6358: Convert to DT Schema
@ 2025-09-30 10:27 Harrison Carter
  2025-10-08  1:52 ` Rob Herring
  0 siblings, 1 reply; 3+ messages in thread
From: Harrison Carter @ 2025-09-30 10:27 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jonas Gorski
  Cc: linux-leds, devicetree, linux-kernel, Harrison Carter

Convert the brcm,bcm6358 LEDs to DT Schema format

Signed-off-by: Harrison Carter <hcarter@thegoodpenguin.co.uk>
---
 .../bindings/leds/brcm,bcm6358-leds.yaml           | 187 +++++++++++++++++++++
 .../devicetree/bindings/leds/leds-bcm6358.txt      | 143 ----------------
 2 files changed, 187 insertions(+), 143 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml b/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..a9052a29aa7bd6ddc252258bfe4982325499713f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
@@ -0,0 +1,187 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/brcm,bcm6358-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LEDs connected to Broadcom BCM6358 controller
+
+description: This controller is present on BCM6358 and
+  BCM6368. In these SoCs there are Serial LEDs (LEDs
+  connected to a 74x164 controller), which can either be
+  controlled by software (exporting the 74x164 as spi-gpio) 
+  or by hardware using this driver. See example at
+  Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml.
+
+maintainers:
+  - Jonas Gorski <jonas.gorski@gmail.com>
+
+properties:
+  compatible:
+    const: brcm,bcm6358-leds
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  brcm,clk-div:
+    description: SCK signal Divider. Default 1
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2, 4, 8]
+
+  brcm,clk-dat-low:
+    description: Makes clock and data signals active low.
+      Default false.
+    type: boolean
+
+patternProperties:
+  "^led@[0,1]?([0-9]|[a-z])$":
+    type: object
+    $ref: common.yaml
+    description: Each LED is represented as a sub-node of
+      this device.
+
+    properties:
+      reg:
+        description: LED pin number
+        maximum: 31
+        minimum: 0
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    /* The bcm6358 SOC */
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        led-controller@fffe00d0 {
+            compatible = "brcm,bcm6358-leds";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <0xfffe00d0 0x8>;
+
+            alarm_white@0 {
+                reg = <0>;
+                active-low;
+                label = "white:alarm";
+            };
+            tv_white@2 {
+                reg = <2>;
+                active-low;
+                label = "white:tv";
+            };
+            tel_white@3 {
+                reg = <3>;
+                active-low;
+                label = "white:tel";
+            };
+            adsl_white@4 {
+                reg = <4>;
+                active-low;
+                label = "white:adsl";
+            };
+        };
+    };
+  - |
+    /* The bcm6368 SOC */
+    led-controller@100000d0 {
+        compatible = "brcm,bcm6358-leds";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0x100000d0 0x8>;
+
+        brcm,pol-low;
+        brcm,clk-div = <4>;
+
+        power_red@0 {
+            reg = <0>;
+            active-low;
+            label = "red:power";
+        };
+        power_green@1 {
+            reg = <1>;
+            active-low;
+            label = "green:power";
+            default-state = "on";
+        };
+        power_blue@2 {
+            reg = <2>;
+            label = "blue:power";
+        };
+        broadband_red@3 {
+            reg = <3>;
+            active-low;
+            label = "red:broadband";
+        };
+        broadband_green@4 {
+            reg = <4>;
+            label = "green:broadband";
+        };
+        broadband_blue@5 {
+            reg = <5>;
+            active-low;
+            label = "blue:broadband";
+        };
+        wireless_red@6 {
+            reg = <6>;
+            active-low;
+            label = "red:wireless";
+        };
+        wireless_green@7 {
+            reg = <7>;
+            active-low;
+            label = "green:wireless";
+        };
+        wireless_blue@8 {
+            reg = <8>;
+            label = "blue:wireless";
+        };
+        phone_red@9 {
+            reg = <9>;
+            active-low;
+            label = "red:phone";
+        };
+        phone_green@10 {
+            reg = <10>;
+            active-low;
+            label = "green:phone";
+        };
+        phone_blue@11 {
+            reg = <11>;
+            label = "blue:phone";
+        };
+        upgrading_red@12 {
+            reg = <12>;
+            active-low;
+            label = "red:upgrading";
+        };
+        upgrading_green@13 {
+            reg = <13>;
+            active-low;
+            label = "green:upgrading";
+        };
+        upgrading_blue@14 {
+            reg = <14>;
+            label = "blue:upgrading";
+        };
+    };
+...
+
diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6358.txt b/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
deleted file mode 100644
index 211ffc3c4a201235e8d242b0230747b5dfe2a417..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
+++ /dev/null
@@ -1,143 +0,0 @@
-LEDs connected to Broadcom BCM6358 controller
-
-This controller is present on BCM6358 and BCM6368.
-In these SoCs there are Serial LEDs (LEDs connected to a 74x164 controller),
-which can either be controlled by software (exporting the 74x164 as spi-gpio.
-See Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml), or
-by hardware using this driver.
-
-Required properties:
-  - compatible : should be "brcm,bcm6358-leds".
-  - #address-cells : must be 1.
-  - #size-cells : must be 0.
-  - reg : BCM6358 LED controller address and size.
-
-Optional properties:
-  - brcm,clk-div : SCK signal divider. Possible values are 1, 2, 4 and 8.
-    Default : 1
-  - brcm,clk-dat-low : Boolean, makes clock and data signals active low.
-    Default : false
-
-Each LED is represented as a sub-node of the brcm,bcm6358-leds device.
-
-LED sub-node required properties:
-  - reg : LED pin number (only LEDs 0 to 31 are valid).
-
-LED sub-node optional properties:
-  - label : see Documentation/devicetree/bindings/leds/common.txt
-  - default-state : see
-    Documentation/devicetree/bindings/leds/common.txt
-  - linux,default-trigger : see
-    Documentation/devicetree/bindings/leds/common.txt
-
-Examples:
-Scenario 1 : BCM6358
-	leds0: led-controller@fffe00d0 {
-		compatible = "brcm,bcm6358-leds";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0xfffe00d0 0x8>;
-
-		alarm_white {
-			reg = <0>;
-			active-low;
-			label = "white:alarm";
-		};
-		tv_white {
-			reg = <2>;
-			active-low;
-			label = "white:tv";
-		};
-		tel_white {
-			reg = <3>;
-			active-low;
-			label = "white:tel";
-		};
-		adsl_white {
-			reg = <4>;
-			active-low;
-			label = "white:adsl";
-		};
-	};
-
-Scenario 2 : BCM6368
-	leds0: led-controller@100000d0 {
-		compatible = "brcm,bcm6358-leds";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0x100000d0 0x8>;
-		brcm,pol-low;
-		brcm,clk-div = <4>;
-
-		power_red {
-			reg = <0>;
-			active-low;
-			label = "red:power";
-		};
-		power_green {
-			reg = <1>;
-			active-low;
-			label = "green:power";
-			default-state = "on";
-		};
-		power_blue {
-			reg = <2>;
-			label = "blue:power";
-		};
-		broadband_red {
-			reg = <3>;
-			active-low;
-			label = "red:broadband";
-		};
-		broadband_green {
-			reg = <4>;
-			label = "green:broadband";
-		};
-		broadband_blue {
-			reg = <5>;
-			active-low;
-			label = "blue:broadband";
-		};
-		wireless_red {
-			reg = <6>;
-			active-low;
-			label = "red:wireless";
-		};
-		wireless_green {
-			reg = <7>;
-			active-low;
-			label = "green:wireless";
-		};
-		wireless_blue {
-			reg = <8>;
-			label = "blue:wireless";
-		};
-		phone_red {
-			reg = <9>;
-			active-low;
-			label = "red:phone";
-		};
-		phone_green {
-			reg = <10>;
-			active-low;
-			label = "green:phone";
-		};
-		phone_blue {
-			reg = <11>;
-			label = "blue:phone";
-		};
-		upgrading_red {
-			reg = <12>;
-			active-low;
-			label = "red:upgrading";
-		};
-		upgrading_green {
-			reg = <13>;
-			active-low;
-			label = "green:upgrading";
-		};
-		upgrading_blue {
-			reg = <14>;
-			label = "blue:upgrading";
-		};
-	};

---
base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
change-id: 20250923-brcm6358-to-dt-a69fa1228512

Best regards,
-- 
Harrison Carter <hcarter@thegoodpenguin.co.uk>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] dt-bindings: leds: bcm6358: Convert to DT Schema
  2025-09-30 10:27 [PATCH] dt-bindings: leds: bcm6358: Convert to DT Schema Harrison Carter
@ 2025-10-08  1:52 ` Rob Herring
  2025-10-09 11:53   ` Harrison Carter
  0 siblings, 1 reply; 3+ messages in thread
From: Rob Herring @ 2025-10-08  1:52 UTC (permalink / raw)
  To: Harrison Carter
  Cc: Lee Jones, Pavel Machek, Krzysztof Kozlowski, Conor Dooley,
	Jonas Gorski, linux-leds, devicetree, linux-kernel

On Tue, Sep 30, 2025 at 11:27:26AM +0100, Harrison Carter wrote:
> Convert the brcm,bcm6358 LEDs to DT Schema format
> 
> Signed-off-by: Harrison Carter <hcarter@thegoodpenguin.co.uk>
> ---
>  .../bindings/leds/brcm,bcm6358-leds.yaml           | 187 +++++++++++++++++++++
>  .../devicetree/bindings/leds/leds-bcm6358.txt      | 143 ----------------
>  2 files changed, 187 insertions(+), 143 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml b/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..a9052a29aa7bd6ddc252258bfe4982325499713f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
> @@ -0,0 +1,187 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/brcm,bcm6358-leds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LEDs connected to Broadcom BCM6358 controller
> +
> +description: This controller is present on BCM6358 and

Start paragraph on new line.

> +  BCM6368. In these SoCs there are Serial LEDs (LEDs
> +  connected to a 74x164 controller), which can either be
> +  controlled by software (exporting the 74x164 as spi-gpio) 
> +  or by hardware using this driver. See example at
> +  Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml.

Wrap lines at 80 chars.

> +
> +maintainers:
> +  - Jonas Gorski <jonas.gorski@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: brcm,bcm6358-leds
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  brcm,clk-div:
> +    description: SCK signal Divider. Default 1
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4, 8]

default: 1

And drop it from the description.

> +
> +  brcm,clk-dat-low:
> +    description: Makes clock and data signals active low.
> +      Default false.

Default false makes no sense. That's always the case for a boolean. The 
only way it can't be is if it is required, but then what is the point of 
a required boolean property.
 
> +    type: boolean
> +
> +patternProperties:
> +  "^led@[0,1]?([0-9]|[a-z])$":

This should be 0-0x1f. So '^led@(0|1?[0-9a-f])$'

> +    type: object
> +    $ref: common.yaml

       unevaluatedProperties: false

(which will make your example fail)

> +    description: Each LED is represented as a sub-node of
> +      this device.
> +
> +    properties:
> +      reg:
> +        description: LED pin number
> +        maximum: 31
> +        minimum: 0
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: true

Cannot be true. Only false is allowed.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    /* The bcm6358 SOC */
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;

Drop this node. Unnecessary for the example.

> +
> +        led-controller@fffe00d0 {
> +            compatible = "brcm,bcm6358-leds";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            reg = <0xfffe00d0 0x8>;
> +
> +            alarm_white@0 {
> +                reg = <0>;
> +                active-low;
> +                label = "white:alarm";
> +            };
> +            tv_white@2 {
> +                reg = <2>;
> +                active-low;
> +                label = "white:tv";
> +            };
> +            tel_white@3 {
> +                reg = <3>;
> +                active-low;
> +                label = "white:tel";
> +            };
> +            adsl_white@4 {
> +                reg = <4>;
> +                active-low;
> +                label = "white:adsl";
> +            };
> +        };
> +    };
> +  - |
> +    /* The bcm6368 SOC */
> +    led-controller@100000d0 {
> +        compatible = "brcm,bcm6358-leds";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <0x100000d0 0x8>;
> +
> +        brcm,pol-low;
> +        brcm,clk-div = <4>;
> +
> +        power_red@0 {
> +            reg = <0>;
> +            active-low;
> +            label = "red:power";
> +        };
> +        power_green@1 {
> +            reg = <1>;
> +            active-low;
> +            label = "green:power";
> +            default-state = "on";
> +        };
> +        power_blue@2 {
> +            reg = <2>;
> +            label = "blue:power";
> +        };
> +        broadband_red@3 {
> +            reg = <3>;
> +            active-low;
> +            label = "red:broadband";
> +        };
> +        broadband_green@4 {
> +            reg = <4>;
> +            label = "green:broadband";
> +        };
> +        broadband_blue@5 {
> +            reg = <5>;
> +            active-low;
> +            label = "blue:broadband";
> +        };
> +        wireless_red@6 {
> +            reg = <6>;
> +            active-low;
> +            label = "red:wireless";
> +        };
> +        wireless_green@7 {
> +            reg = <7>;
> +            active-low;
> +            label = "green:wireless";
> +        };
> +        wireless_blue@8 {
> +            reg = <8>;
> +            label = "blue:wireless";
> +        };
> +        phone_red@9 {
> +            reg = <9>;
> +            active-low;
> +            label = "red:phone";
> +        };
> +        phone_green@10 {
> +            reg = <10>;
> +            active-low;
> +            label = "green:phone";
> +        };
> +        phone_blue@11 {
> +            reg = <11>;
> +            label = "blue:phone";
> +        };
> +        upgrading_red@12 {
> +            reg = <12>;
> +            active-low;
> +            label = "red:upgrading";
> +        };
> +        upgrading_green@13 {
> +            reg = <13>;
> +            active-low;
> +            label = "green:upgrading";
> +        };
> +        upgrading_blue@14 {
> +            reg = <14>;
> +            label = "blue:upgrading";
> +        };
> +    };

I don't think we need 2 examples.

Rob

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] dt-bindings: leds: bcm6358: Convert to DT Schema
  2025-10-08  1:52 ` Rob Herring
@ 2025-10-09 11:53   ` Harrison Carter
  0 siblings, 0 replies; 3+ messages in thread
From: Harrison Carter @ 2025-10-09 11:53 UTC (permalink / raw)
  To: robh
  Cc: conor+dt, devicetree, hcarter, jonas.gorski, krzk+dt, lee,
	linux-kernel, linux-leds, pavel

Hi Rob,

Thanks for looking at my patch. There's an issue in the patch anyway that's 
been noticed between the patternProperties's regex and the names of the 
led nodes. Here it's just led@... but the nodes are named thing_colour.

On and off this regex has been done as .*_.*@, led@, and .*@ . What is the 
preference? led@ would amend the led information to a label in the node. The 
latter option is very promiscuous (but not uncommon in bindings), the former 
sticks to what the examples are sort of expecting.

Cheers,

HarryC


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-10-09 11:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-30 10:27 [PATCH] dt-bindings: leds: bcm6358: Convert to DT Schema Harrison Carter
2025-10-08  1:52 ` Rob Herring
2025-10-09 11:53   ` Harrison Carter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).