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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d5e73894-0e30-499f-a723-2ada72d3b864@kernel.org>
Date: Mon, 10 Mar 2025 08:49:22 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Matthias Fend <matthias.fend@...end.at>, Pavel Machek <pavel@....cz>,
 Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, bsp-development.geo@...ca-geosystems.com
Subject: Re: [PATCH 1/2] dt-bindings: leds: add Texas Instruments TPS6131x
 flash LED driver

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