Message ID | 20241206170717.1090206-2-alexander.sverdlin@siemens.com |
---|---|
State | New |
Headers | show |
Series | leds: TI LP8864/LP8866 support | expand |
On Fri, Dec 06, 2024 at 06:07:12PM +0100, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Add Texas Instruments' LP8864/LP8866 bindings into LP8860 converting them > into YAML format simultaneously. While here, drop the index of the "led" > subnode, this one is neither used nor mandated by the drivers. All the > *-cells properties are therefore not required. Are you sure this is a correct thing to do? The lp8860-q1 product link cites it as being a 4-channel device. Even if the kernel only ever supports it as a single-channel device, the binding should reflect what it is capable of doing. Cheers, Conor. > > Move the file into backlight directory because all of the LP886x drivers > are special backlight products. > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > .../bindings/leds/backlight/ti,lp8860.yaml | 86 +++++++++++++++++++ > .../devicetree/bindings/leds/leds-lp8860.txt | 50 ----------- > 2 files changed, 86 insertions(+), 50 deletions(-) > create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml > delete mode 100644 Documentation/devicetree/bindings/leds/leds-lp8860.txt > > diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml > new file mode 100644 > index 0000000000000..3ece2f6fc3f02 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml > @@ -0,0 +1,86 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/ti,lp8860.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments - LP886x 4/6-Channel LED Driver family > + > +maintainers: > + - Andrew Davis <afd@ti.com> > + - Alexander Sverdlin <alexander.sverdlin@siemens.com> > + > +description: | > + The LP8860-Q1 is an high-efficiency LED driver with boost controller. > + It has 4 high-precision current sinks that can be controlled by a PWM input > + signal, a SPI/I2C master, or both. > + > + LP8866-Q1, LP8866S-Q1, LP8864-Q1, LP8864S-Q1 are newer products offering > + similar functionality with 4/6 channels. > + > + For more product information please see the links below: > + https://www.ti.com/product/lp8860-q1 > + https://www.ti.com/product/LP8864-Q1 > + https://www.ti.com/product/LP8864S-Q1 > + https://www.ti.com/product/LP8866-Q1 > + https://www.ti.com/product/LP8866S-Q1 > + > +properties: > + compatible: > + enum: > + - ti,lp8860 > + - ti,lp8864 > + > + reg: > + maxItems: 1 > + description: I2C slave address > + > + enable-gpios: > + maxItems: 1 > + description: GPIO pin to enable (active high) / disable the device > + > + vled-supply: > + description: LED supply > + > + led: > + type: object > + $ref: common.yaml# > + properties: > + function: true > + color: true > + label: true > + linux,default-trigger: true > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - led > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/leds/common.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led-controller@2d { > + compatible = "ti,lp8860"; > + reg = <0x2d>; > + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > + > + led { > + function = LED_FUNCTION_BACKLIGHT; > + color = <LED_COLOR_ID_WHITE>; > + linux,default-trigger = "backlight"; > + }; > + }; > + }; > + > +... > diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt > deleted file mode 100644 > index 8bb25749a3da3..0000000000000 > --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt > +++ /dev/null > @@ -1,50 +0,0 @@ > -* Texas Instruments - lp8860 4-Channel LED Driver > - > -The LP8860-Q1 is an high-efficiency LED > -driver with boost controller. It has 4 high-precision > -current sinks that can be controlled by a PWM input > -signal, a SPI/I2C master, or both. > - > -Required properties: > - - compatible : > - "ti,lp8860" > - - reg : I2C slave address > - - #address-cells : 1 > - - #size-cells : 0 > - > -Optional properties: > - - enable-gpios : gpio pin to enable (active high)/disable the device. > - - vled-supply : LED supply > - > -Required child properties: > - - reg : 0 > - > -Optional child properties: > - - function : see Documentation/devicetree/bindings/leds/common.txt > - - color : see Documentation/devicetree/bindings/leds/common.txt > - - label : see Documentation/devicetree/bindings/leds/common.txt (deprecated) > - - linux,default-trigger : > - see Documentation/devicetree/bindings/leds/common.txt > - > -Example: > - > -#include <dt-bindings/leds/common.h> > - > -led-controller@2d { > - compatible = "ti,lp8860"; > - #address-cells = <1>; > - #size-cells = <0>; > - reg = <0x2d>; > - enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > - vled-supply = <&vbatt>; > - > - led@0 { > - reg = <0>; > - function = LED_FUNCTION_BACKLIGHT; > - color = <LED_COLOR_ID_WHITE>; > - linux,default-trigger = "backlight"; > - }; > -} > - > -For more product information please see the link below: > -https://www.ti.com/product/lp8860-q1 > -- > 2.47.1 >
Hello Conor, On Fri, 2024-12-06 at 17:14 +0000, Conor Dooley wrote: > > Add Texas Instruments' LP8864/LP8866 bindings into LP8860 converting them > > into YAML format simultaneously. While here, drop the index of the "led" > > subnode, this one is neither used nor mandated by the drivers. All the > > *-cells properties are therefore not required. > > Are you sure this is a correct thing to do? The lp8860-q1 product link > cites it as being a 4-channel device. Even if the kernel only ever > supports it as a single-channel device, the binding should reflect what > it is capable of doing. my understanding is: - The whole family is multi-channel (4 or 6), but this is rather used internally in the chip for power balancing; separate diagnostics is provided. From the user perspective one has only one brightness per chip. - The lp8860 driver didn't attempt to do anything with the index. I'm glad Andrew Davis had a time for a pre-public review of the new binding and actually suggested this simplification.
Hi Andrew, On Fri, 2024-12-06 at 11:43 -0600, Andrew Davis wrote: > > Are you sure this is a correct thing to do? The lp8860-q1 product link > > cites it as being a 4-channel device. Even if the kernel only ever > > supports it as a single-channel device, the binding should reflect what > > it is capable of doing. > > > > Agree, the driver today just checks the first child node, but it could > be extended for all 4 supported LED channels, and we shouldn't have > to change the binding for that new support. but the channels are (in my understanding) for power-balancing only, there are no separate controls for them. What do I miss?
On 12/6/24 11:46 AM, Sverdlin, Alexander wrote: > Hi Andrew, > > On Fri, 2024-12-06 at 11:43 -0600, Andrew Davis wrote: >>> Are you sure this is a correct thing to do? The lp8860-q1 product link >>> cites it as being a 4-channel device. Even if the kernel only ever >>> supports it as a single-channel device, the binding should reflect what >>> it is capable of doing. >>> >> >> Agree, the driver today just checks the first child node, but it could >> be extended for all 4 supported LED channels, and we shouldn't have >> to change the binding for that new support. > > but the channels are (in my understanding) for power-balancing only, there > are no separate controls for them. What do I miss? > I'm not very familiar with this part either, but looking at the datasheet I see: > Supports Display Mode (Global Dimming) and > Cluster Mode (Independent Dimming) > In Cluster mode LED strings have independent control but fewer features enabled than in Display Mode. And one channel controlling the others is only in this "Display Mode", but the currents to the others can be independently controlled in a different mode (seems these modes have less features which is probably why the driver doesn't make use of that today). Andrew
diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml new file mode 100644 index 0000000000000..3ece2f6fc3f02 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml @@ -0,0 +1,86 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/backlight/ti,lp8860.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments - LP886x 4/6-Channel LED Driver family + +maintainers: + - Andrew Davis <afd@ti.com> + - Alexander Sverdlin <alexander.sverdlin@siemens.com> + +description: | + The LP8860-Q1 is an high-efficiency LED driver with boost controller. + It has 4 high-precision current sinks that can be controlled by a PWM input + signal, a SPI/I2C master, or both. + + LP8866-Q1, LP8866S-Q1, LP8864-Q1, LP8864S-Q1 are newer products offering + similar functionality with 4/6 channels. + + For more product information please see the links below: + https://www.ti.com/product/lp8860-q1 + https://www.ti.com/product/LP8864-Q1 + https://www.ti.com/product/LP8864S-Q1 + https://www.ti.com/product/LP8866-Q1 + https://www.ti.com/product/LP8866S-Q1 + +properties: + compatible: + enum: + - ti,lp8860 + - ti,lp8864 + + reg: + maxItems: 1 + description: I2C slave address + + enable-gpios: + maxItems: 1 + description: GPIO pin to enable (active high) / disable the device + + vled-supply: + description: LED supply + + led: + type: object + $ref: common.yaml# + properties: + function: true + color: true + label: true + linux,default-trigger: true + + additionalProperties: false + +required: + - compatible + - reg + - led + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/leds/common.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@2d { + compatible = "ti,lp8860"; + reg = <0x2d>; + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; + vled-supply = <&vbatt>; + + led { + function = LED_FUNCTION_BACKLIGHT; + color = <LED_COLOR_ID_WHITE>; + linux,default-trigger = "backlight"; + }; + }; + }; + +... diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt deleted file mode 100644 index 8bb25749a3da3..0000000000000 --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt +++ /dev/null @@ -1,50 +0,0 @@ -* Texas Instruments - lp8860 4-Channel LED Driver - -The LP8860-Q1 is an high-efficiency LED -driver with boost controller. It has 4 high-precision -current sinks that can be controlled by a PWM input -signal, a SPI/I2C master, or both. - -Required properties: - - compatible : - "ti,lp8860" - - reg : I2C slave address - - #address-cells : 1 - - #size-cells : 0 - -Optional properties: - - enable-gpios : gpio pin to enable (active high)/disable the device. - - vled-supply : LED supply - -Required child properties: - - reg : 0 - -Optional child properties: - - function : see Documentation/devicetree/bindings/leds/common.txt - - color : see Documentation/devicetree/bindings/leds/common.txt - - label : see Documentation/devicetree/bindings/leds/common.txt (deprecated) - - linux,default-trigger : - see Documentation/devicetree/bindings/leds/common.txt - -Example: - -#include <dt-bindings/leds/common.h> - -led-controller@2d { - compatible = "ti,lp8860"; - #address-cells = <1>; - #size-cells = <0>; - reg = <0x2d>; - enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; - vled-supply = <&vbatt>; - - led@0 { - reg = <0>; - function = LED_FUNCTION_BACKLIGHT; - color = <LED_COLOR_ID_WHITE>; - linux,default-trigger = "backlight"; - }; -} - -For more product information please see the link below: -https://www.ti.com/product/lp8860-q1