[1/2] dt-bindings: backlight: Convert LP8860 into YAML format adding LP886x

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.

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

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

On Fri, Dec 06, 2024 at 05:44:15PM +0000, Sverdlin, Alexander wrote:
> 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.

One brightness perhaps, but what do you do where several LEDs of
different colours are connected? Or if one was to be active-low? I don't
see the benefit of changing the binding in a way that makes it less
able to describe the hardware.

> - The lp8860 driver didn't attempt to do anything with the index.

I don't see this as being relevant, the bindings need only address what
the hardware is able to do. The driver may only implement a subset of
that, and that is perfectly okay.

> I'm glad Andrew Davis had a time for a pre-public review of the new binding
> and actually suggested this simplification.

Okay.

Thanks,
Conor.