Re: [PATCH 1/2] dt-bindings: leds: add Texas Instruments TPS6131x flash LED driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/02/2025 11:31, Matthias Fend wrote:
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tps61310
> +      - ti,tps61311
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

Why do you need these two?

> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: GPIO connected to NRESET pin
> +
> +  ti,valley-current-limit:
> +    type: boolean
> +    description:
> +      Reduce the valley peak current limit from 1750mA to 1250mA (TPS61310) or
> +      from 2480mA to 1800mA (TPS61311).
> +
> +  led:

Why do you have only one led node? Description says three: LED1-3,
unless these are just sinks which always have to be connected to the
same LED?

> +    type: object
> +    $ref: common.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      led-sources:
> +        allOf:

Drop allOf

> +          - minItems: 1
> +            maxItems: 3
> +            items:
> +              enum: [1, 2, 3]
> +
> +      led-max-microamp:
> +        anyOf:

oneOf

> +          - minimum: 25000
> +            maximum: 350000
> +            multipleOf: 50000
> +          - minimum: 25000
> +            maximum: 525000

Why two different values?

> +            multipleOf: 25000
> +
> +      flash-max-microamp:
> +        anyOf:

oneOf

> +          - minimum: 25000
> +            maximum: 800000
> +            multipleOf: 50000
> +          - minimum: 25000

Same question

> +            maximum: 1500000
> +            multipleOf: 25000
> +
> +      flash-max-timeout-us:
> +        enum: [ 5300, 10700, 16000, 21300, 26600, 32000, 37300, 68200, 71500,
> +                102200, 136300, 170400, 204500, 340800, 579300, 852000 ]
> +
> +    required:
> +      - led-sources
> +      - led-max-microamp
> +      - flash-max-microamp
> +      - flash-max-timeout-us
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'

Why?

> +  - led
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      led-controller@33 {
> +        compatible = "ti,tps61310";
> +        reg = <0x33>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        reset-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
> +
> +        tps61310_flash: led {

Drop unused label

Best regards,
Krzysztof