Re: [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
---

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux