Re: [PATCH] dt-bindings: leds: bcm6358: Convert to DT schema

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 7573 bytes --]

On Mon, May 25, 2026 at 09:02:58AM +0530, Ninad Naik wrote:
> Convert the brcm,bcm6358 to DT schema.
> 
> Signed-off-by: Ninad Naik <ninadnaik07@gmail.com>
> ---
> 
> Referred to this thread:
> https://lore.kernel.org/all/20250930-brcm6358-to-dt-v1-1-ba833ceb1575@thegoodpenguin.co.uk/
> and added the fixes suggested.
> 
>  .../bindings/leds/brcm,bcm6358-leds.yaml      |  98 ++++++++++++
>  .../devicetree/bindings/leds/leds-bcm6358.txt | 143 ------------------
>  2 files changed, 98 insertions(+), 143 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
>  delete mode 100644 Documentation/devicetree/bindings/leds/leds-bcm6358.txt
> 
> 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 000000000000..a61417e5690b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/brcm,bcm6358-leds.yaml
> @@ -0,0 +1,98 @@
> +# 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.
> +  See Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml), or
> +  by hardware using this driver.
> +
> +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.
> +    type: boolean
> +
> +patternProperties:
> +  "^led@(0|1?[0-9a-f])$":
> +    type: object
> +    $ref: common.yaml#
> +    description: Each LED is represented as a sub-node of
> +      this device.
> +
> +    properties:
> +      reg:
> +        description: LED pin number.
> +        minimum: 0
> +        maximum: 31

Is this missing a maxItems: 1?

> +
> +    required:
> +      - reg
> +
> +    unevaluatedProperties: false

I wonder if this should be additionalProperties: false instead, and list
the three properties mentioned by the text binding explicitly?


Cheers,
Conor.

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +    led-controller@fffe00d0 {
> +        compatible = "brcm,bcm6358-leds";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <0xfffe00d0 0x8>;
> +
> +        led@0 {
> +            reg = <0>;
> +            active-low;
> +            label = "white:alarm";
> +        };
> +        led@2 {
> +            reg = <2>;
> +            active-low;
> +            label = "white:tv";
> +        };
> +        led@3 {
> +            reg = <3>;
> +            active-low;
> +            label = "white:tel";
> +        };
> +        led@4 {
> +            reg = <4>;
> +            active-low;
> +            label = "white:adsl";
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6358.txt b/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
> deleted file mode 100644
> index 211ffc3c4a20..000000000000
> --- 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";
> -		};
> -	};
> -- 
> 2.54.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]