On 08/11/2023 04:29, Yuxi Wang wrote: > Document mps mp3326 LED driver devicetree bindings. > > Signed-off-by: Yuxi Wang <wyx137120466@xxxxxxxxx> > --- > .../devicetree/bindings/leds/leds-mp3326.yaml | 184 ++++++++++++++++++ > 1 file changed, 184 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-mp3326.yaml Except that this was not tested :(, few more comments. Filename like compatible. > > diff --git a/Documentation/devicetree/bindings/leds/leds-mp3326.yaml b/Documentation/devicetree/bindings/leds/leds-mp3326.yaml > new file mode 100644 > index 000000000000..899cf568f647 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-mp3326.yaml > @@ -0,0 +1,184 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mps,mp3326.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MPS MP3326 RGB/White LED Driver > + > +maintainers: > + - Yuxi Wang <wyx137120466@xxxxxxxxx> > + > +description: | > + The MP3326 is a RGB/White LED driver with I2C interface. > + > + For more product information please see the link below: > + https://www.monolithicpower.com/en/products/mp3326.html > + > +properties: > + compatible: > + - const: mps,mp3326 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + reg: > + maxItems: 1 > + > + mps,led-protect: > + description: | > + LED short protection threshold. threshold? So in some units? What does it mean? What do the values mean? > + enum: [0, 1, 2, 3] > + > + multi-led: > + type: object > + > + properties: > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 > + > + color: > + description: RGB module > + const: LED_COLOR_ID_RGB > + > + led_r: Nope. First, no underscores in names. Second, please open existing bindings and look how it is done there. > + type: object > + > + properties: > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 Why do you have the,? > + reg: > + description: Index of the LED. > + minimum: 1 > + maximum: 16 Please format it properly. You miss blank lines between each property description. > + color: > + description: Red. > + const: LED_COLOR_ID_RED > + required: > + - reg > + - color > + > + led_g: > + type: object > + > + properties: > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 > + reg: > + description: Index of the LED. > + minimum: 1 > + maximum: 16 > + color: > + description: Green. > + const: LED_COLOR_ID_GREEN > + required: > + - reg > + - color > + > + led_b: > + type: object > + > + properties: > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 > + reg: > + description: Index of the LED. > + minimum: 1 > + maximum: 16 > + color: > + description: Blue. > + const: LED_COLOR_ID_BLUE > + required: > + - reg > + - color > + > + patternProperties: > + "^led@[0-3]$": > + type: object > + > + properties: > + reg: > + description: Index of the LED. > + minimum: 1 > + maximum: 16 > + > + required: > + - reg > + - color Missing required, additionalProperties. Open existing binding and use it as example. > + > +examples: > + - | > + #include <dt-bindings/leds/common.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mp3326@30 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "mps,mp3326"; > + reg = <0x30>; > + led-protect =<3>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + multi-led { > + color = <LED_COLOR_ID_RGB>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + rgb_r@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + color = <LED_COLOR_ID_RED>; > + }; > + rgb_g@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <2>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + rgb_b@3 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <3>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + }; > + }; > + > + - | > + #include <dt-bindings/leds/common.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mp3326@30 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "mps,mp3326"; > + reg = <0x30>; > + led-protect =<3>; > + #address-cells = <1>; > + #size-cells = <0>; > + led0@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg= <5>; > + color = <LED_COLOR_ID_WHITE>; > + }; > + }; > + }; > + > +... Best regards, Krzysztof
