On 08/03/2024 22:50, Patrick Gansterer wrote: > Add Device Tree bindings for Texas Instruments LM3509 - a > High Efficiency Boost for White LED's and/or OLED Displays > > Signed-off-by: Patrick Gansterer <paroga@xxxxxxxxxx> Thank you for your patch. There is something to discuss/improve. > + compatible: > + const: ti,lm3509 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + reset-gpios: > + maxItems: 1 > + > + ti,brightness-rate-of-change-us: > + description: Brightness Rate of Change in microseconds. > + enum: [51, 13000, 26000, 52000] > + > + ti,oled-mode: > + description: Enable OLED mode. > + type: boolean > + > +required: required: goes after all properties. > + - compatible > + - reg > + > +patternProperties: > + "^led@[01]$": > + type: object > + description: Properties for a string of connected LEDs. Are you sure this is a string of LEDs? How does a string/tape work with a backlight, I mean physically? How could it look like? > + > + allOf: > + - $ref: common.yaml# > + > + properties: > + reg: > + description: > + The control register that is used to program the two current sinks. > + The LM3509 has two registers (BMAIN and BSUB) and are represented > + as 0 or 1 in this property. The two current sinks can be controlled > + independently with both registers, or register BMAIN can be > + configured to control both sinks with the led-sources property. > + minimum: 0 > + maximum: 1 > + > + label: > + maxItems: 1 It's a string, not string-array, so maxItems are not needed, just ":true". > + > + led-sources: > + allOf: > + - minItems: 1 > + maxItems: 2 > + items: > + minimum: 0 > + maximum: 1 > + > + default-brightness: > + minimum: 0 > + maximum: 31 Not a required property? Then "default:". > + > + max-brightness: > + minimum: 0 > + maximum: 31 Some of your examples miss it, so there is some kind of default. Add it. > + > + required: > + - reg > + > + unevaluatedProperties: false > + > +unevaluatedProperties: false You rewrote everything, so my previous comment obviously does not make sense in such case. This must be additionalProperties: false. Look at other bindings or example-schema how it is done (to repeat myself). > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + backlight@36 { > + compatible = "ti,lm3509"; > + reg = <0x36>; > + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; > + > + ti,oled-mode; > + ti,brightness-rate-of-change-us = <52000>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@0 { Best regards, Krzysztof