> 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. > Thank you for pointing this out. I will use mps,mp3326.yaml in the next version. > > > > 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: > https://urldefense.com/v3/__http://devicetree.org/schemas/mps,mp3326.yaml*__;Iw!!FIHMVlGrYVGa5kwGHCY!X_Ejb8ueivebvMv2 > RooVy9MVgqWRElf75NTk8JPqFXaYwWNyzupK7XKN5UKAvJRo-WMQmvxIuJPq5isKHeoqFbKZ4_cH81h24frs$ > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta- > schemas/core.yaml*__;Iw!!FIHMVlGrYVGa5kwGHCY!X_Ejb8ueivebvMv2RooVy9MVgqWRElf75NTk8JPqFXaYwWNyzupK7XKN5UKAvJR > o-WMQmvxIuJPq5isKHeoqFbKZ4_cH88smmVyA$ > > + > > +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? > Yes, it indicates short protection threshold in mp3326. But they do not have the units. They just indicate the corresponding bits value of register which can configure short protection value. > > + 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. > Thank you for pointing out this, I will Fixed it in the next version. > > + type: object > > + > > + properties: > > + "#address-cells": > > + const: 1 > > + "#size-cells": > > + const: 0 > > Why do you have the,? > Sorry, here in no , . what do you mean? > > + reg: > > + description: Index of the LED. > > + minimum: 1 > > + maximum: 16 > > Please format it properly. You miss blank lines between each property > description. > Thank you for pointing this out. I will add the blank lines in the next version. > > + 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. > Thank you for pointing this out. I will add the additional Properties in the next version. > > + > > +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://urldefense.com/v3/__https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html*generic- > names- > recommendation__;Iw!!FIHMVlGrYVGa5kwGHCY!X_Ejb8ueivebvMv2RooVy9MVgqWRElf75NTk8JPqFXaYwWNyzupK7XKN5UKAvJRo- > WMQmvxIuJPq5isKHeoqFbKZ4_cH8-K2cXo8$ > Thank you for sharing this, I will check it and change it in the next version. > > > + 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://urldefense.com/v3/__https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html*generic- > names- > recommendation__;Iw!!FIHMVlGrYVGa5kwGHCY!X_Ejb8ueivebvMv2RooVy9MVgqWRElf75NTk8JPqFXaYwWNyzupK7XKN5UKAvJRo- > WMQmvxIuJPq5isKHeoqFbKZ4_cH8-K2cXo8$ > Thank you for sharing this, I will check it and change it in the next version. > > > + 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 Best Regards. Yuxi Wang
