Messages in this thread | | | Subject | Re: [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers | From | Jacek Anaszewski <> | Date | Tue, 15 Jan 2019 22:42:23 +0100 |
| |
Hi Dan,
Thank you for the patch.
Seeing the mess with regs and led-modules I think it will be better to abide by the regs alone.
Please see below how I would structure that.
On 1/14/19 10:17 PM, Dan Murphy wrote: > Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and the LP5018 > RGB LED device driver. The LP5036/3024/18 can control RGB LEDs individually > or as part of a control bank group. These devices have the ability > to adjust the mixing control for the RGB LEDs to obtain different colors > independent of the overall brightness of the LED grouping. > > Datasheet: > http://www.ti.com/lit/ds/symlink/lp5024.pdf > http://www.ti.com/lit/ds/symlink/lp5036.pdf > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > > v2 - Added the LP5030/36 devices, defined the modules vs banked properties > renamed the file from lp5024 to lp50xx. - https://lore.kernel.org/patchwork/patch/1026514/ > > .../devicetree/bindings/leds/leds-lp50xx.txt | 143 ++++++++++++++++++ > 1 file changed, 143 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.txt b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt > new file mode 100644 > index 000000000000..7bc6843ddba4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt > @@ -0,0 +1,143 @@ > +* Texas Instruments - LP5018/24/30/36 RGB LED driver > + > +The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into > +a LED group or control them individually. > + > +The difference in these RGB LED drivers is the number of supported RGB strings.
Maybe it would be better to replace "strings" with "modules"? Strings usually mean LEDs connected in a row, which is specific to backlights.
> +Required properties: > + - compatible: > + "ti,lp5018" > + "ti,lp5024" > + "ti,lp5030" > + "ti,lp5036" > + - reg : I2C slave address > + lp5018/24 - 0x28 > + lp5030/36 - 0x30 > + - #address-cells : 1 > + - #size-cells : 0 > + > +Optional properties: > + - enable-gpios : gpio pin to enable/disable the device. > + - vled-supply : LED supply > + > +Required child properties: > + - reg : Is the child node iteration.
Let's change it to:
- reg : RGB LED module number. For the node describing RGB cluster bank it should match first element of ti,led-bank array.
> + > +Required Child properties but only one should be defined per child: > +Either one of these two properties are required for each node. The > +property ti,led-bank takes precedence over the ti,led-module within the same > +node. > + > + - ti,led-module : This property denotes the single LED module number > + that will be controlled in the LED class instance.
It will be not needed then.
> + - ti,led-bank : This property denotes the LED module numbers that will > + be controlled as a single RGB cluster. Each LED module > + number will be controlled by a single LED class instance. > + There can only be one instance of the ti,led-bank > + property for each device node. > + > +The LED outpus associated with the LED modules are defined in Table 1 of the > +corresponding data sheets. > + > +LP5018 - 6 Total RGB cluster LED outputs 0-5 > +LP5024 - 8 Total RGB cluster LED outputs 0-7 > +LP5030 - 10 Total RGB cluster LED outputs 0-9 > +LP5036 - 12 Total RGB cluster LED outputs 0-11 > + > +Optional child properties: > + - label : see Documentation/devicetree/bindings/leds/common.txt > + - linux,default-trigger : > + see Documentation/devicetree/bindings/leds/common.txt > + > +Examples: > +LP5018 and LP5024 example: > +led-controller@29 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,lp5024"; > + reg = <0x29>; > + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vmmcsd_fixed>; > + > + led@0 { > + reg = <0>; > + label = "led1_mod"; > + ti,led-module = <1>; > + }; > + > + led@1 { > + reg = <1>; > + label = "banked_leds"; > + ti,led-bank = <0 2 5 3 >; > + }; > + > + led@2 { > + reg = <2>; > + label = "led4_mod"; > + ti,led-module = <4>; > + }; > + > + led@3 { > + reg = <3>; > + label = "led7_mod"; > + ti,led-module = <7>; > + }; > + > + led@4 { > + reg = <4>; > + label = "led6_mod"; > + ti,led-module = <6>; > + };
Without ti,led-module it will look like below:
led@0 { reg = <0>; label = "banked_leds"; ti,led-bank = <0 2 5 3 >; };
led@1 { reg = <1>; label = "led1_mod"; };
led@4 { reg = <4>; label = "led4_mod"; };
led@6 { reg = <6>; label = "led6_mod"; };
led@7 { reg = <7>; label = "led7_mod"; };
> +}; > + > +LP5030 and LP5036 example: > +led-controller@30 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,lp5036"; > + reg = <0x30>; > + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vmmcsd_fixed>; > + > + led@0 { > + reg = <0>; > + label = "led1_mod"; > + ti,led-module = <1>; > + }; > + > + led@1 { > + reg = <1>; > + label = "led_banked"; > + ti,led-bank = <0 2 5 3 9 10 >; > + }; > + > + led@2 { > + reg = <2>; > + label = "led4_mod"; > + ti,led-module = <4>; > + }; > + > + led@3 { > + reg = <3>; > + label = "led7_mod"; > + ti,led-module = <7>; > + }; > + > + led@4 { > + reg = <4>; > + label = "led6_mod"; > + ti,led-module = <6>; > + }; > + > + led@5 { > + reg = <5>; > + label = "led8_mod"; > + ti,led-module = <8>; > + }; > +}; > + > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/symlink/lp5024.pdf > +http://www.ti.com/lit/ds/symlink/lp5036.pdf >
-- Best regards, Jacek Anaszewski
| |