[PATCH 0/2] leds: TI LP8864/LP8866 support

[PATCH 0/2] leds: TI LP8864/LP8866 support

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Add new DT compatible ti,lp8864 to support all four software-compatible
devices:
- LP8864
- LP8864S
- LP8866
- LP8866S
This is a new family with a functionality similar to LP8860 -- hence the
same (re-used) DT bindings. They had to be converted to YAML along the way.

Add the new driver for the above four chips. Despite similar functionality,
the I2C interface is completely different to LP8860 -- hence the separate
driver.

Alexander Sverdlin (2):
  dt-bindings: backlight: Convert LP8860 into YAML format adding LP886x
  leds: lp8864: New driver

 .../bindings/leds/backlight/ti,lp8860.yaml    |  86 +++++
 .../devicetree/bindings/leds/leds-lp8860.txt  |  50 ---
 MAINTAINERS                                   |   7 +
 drivers/leds/Kconfig                          |  12 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-lp8864.c                    | 320 ++++++++++++++++++
 6 files changed, 426 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-lp8860.txt
 create mode 100644 drivers/leds/leds-lp8864.c

-- 
2.47.1

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

[A. Sverdlin]

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

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5736 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Andrew Davis]

On 12/6/24 11:14 AM, Conor Dooley wrote:
> 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.
> 

The index isn't needed, but we should still allow for multiple child
LED nodes. The usual way to do this is with node names led-0, led-1, etc..

See here for the usual patternProperties for that:

https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-pwm.yaml

> 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.

Andrew

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

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

[Sverdlin, Alexander]

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?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

[Andrew Davis]

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

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

[Sverdlin, Alexander]

Hi Andrew,

On Fri, 2024-12-06 at 12:02 -0600, Andrew Davis wrote:
> 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.

thanks for looking into this!

> 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).

You are right! This seems to be the feature of the legacy lp8860.
Shall I then leave its binding alone and re-submit new YAML binding as-is
for the newer LP8864/LP8866 family? Seems that they don't have the cluster mode
any more.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

[Andrew Davis]

On 12/6/24 12:20 PM, Sverdlin, Alexander wrote:
> Hi Andrew,
> 
> On Fri, 2024-12-06 at 12:02 -0600, Andrew Davis wrote:
>> 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.
> 
> thanks for looking into this!
> 
>> 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).
> 
> You are right! This seems to be the feature of the legacy lp8860.
> Shall I then leave its binding alone and re-submit new YAML binding as-is
> for the newer LP8864/LP8866 family? Seems that they don't have the cluster mode
> any more.
> 

Well the txt to yaml binding conversion looks good other than the
patternProperties for multiple LEDs part. But if you don't plan on
reusing the binding then you don't need it as part of this series.
(still good to send it by itself since you already did the work)

A new binding doc for these new parts might be the way to go then.

Andrew

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

[Sverdlin, Alexander]

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.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/2] leds: lp8864: New driver

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Add driver for TI LP8864, LP8864S, LP8866 4/6 channel LED-backlight drivers
with I2C interface.

Link: https://www.ti.com/lit/gpn/lp8866-q1
Link: https://www.ti.com/lit/gpn/lp8864-q1
Link: https://www.ti.com/lit/gpn/lp8864s-q1
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 MAINTAINERS                |   7 +
 drivers/leds/Kconfig       |  12 ++
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lp8864.c | 320 +++++++++++++++++++++++++++++++++++++
 4 files changed, 340 insertions(+)
 create mode 100644 drivers/leds/leds-lp8864.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 21f855fe468bc..14e87b6b37b8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23262,6 +23262,13 @@ S:	Supported
 F:	Documentation/devicetree/bindings/iio/dac/ti,dac7612.yaml
 F:	drivers/iio/dac/ti-dac7612.c
 
+TEXAS INSTRUMENTS' LB8864 LED BACKLIGHT DRIVER
+M:	Alexander Sverdlin <alexander.sverdlin@siemens.com>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/backlight/ti,lp8860.yaml
+F:	drivers/leds/leds-lp8864.c
+
 TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER
 M:	Nishanth Menon <nm@ti.com>
 M:	Tero Kristo <kristo@kernel.org>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b784bb74a8378..6d0e88e501614 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -511,6 +511,18 @@ config LEDS_LP8860
 	  on the LP8860 4 channel LED driver using the I2C communication
 	  bus.
 
+config LEDS_LP8864
+	tristate "LED support for the TI LP8864/LP8866 4/6 channel LED drivers"
+	depends on LEDS_CLASS && I2C && OF
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for the TI LP8864-Q1,
+	  LP8864S-Q1, LP8866-Q1, LP8866S-Q1 4/6 channel LED backlight
+	  drivers with I2C interface.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called leds-lp8864.
+
 config LEDS_CLEVO_MAIL
 	tristate "Mail LED on Clevo notebook"
 	depends on LEDS_CLASS && BROKEN
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 18afbb5a23ee5..f66bf2e13665f 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_LEDS_LP55XX_COMMON)	+= leds-lp55xx-common.o
 obj-$(CONFIG_LEDS_LP8501)		+= leds-lp8501.o
 obj-$(CONFIG_LEDS_LP8788)		+= leds-lp8788.o
 obj-$(CONFIG_LEDS_LP8860)		+= leds-lp8860.o
+obj-$(CONFIG_LEDS_LP8864)		+= leds-lp8864.o
 obj-$(CONFIG_LEDS_LT3593)		+= leds-lt3593.o
 obj-$(CONFIG_LEDS_MAX5970)		+= leds-max5970.o
 obj-$(CONFIG_LEDS_MAX77650)		+= leds-max77650.o
diff --git a/drivers/leds/leds-lp8864.c b/drivers/leds/leds-lp8864.c
new file mode 100644
index 0000000000000..da02e29bbf4b7
--- /dev/null
+++ b/drivers/leds/leds-lp8864.c
@@ -0,0 +1,320 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TI LP8864/LP8866 4/6 Channel LED Driver
+ *
+ * Copyright (C) 2024 Siemens AG
+ *
+ * Based on LP8860 driver by Dan Murphy <dmurphy@ti.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define LP8864_BRT_CONTROL		0x00
+#define LP8864_USER_CONFIG1		0x04
+#define   LP8864_BRT_MODE_MASK		GENMASK(9, 8)
+/* Brightness controlled by DISPLAY_BRT register */
+#define   LP8864_BRT_MODE_REG		BIT(9)
+#define LP8864_SUPPLY_STATUS		0x0e
+#define LP8864_BOOST_STATUS		0x10
+#define LP8864_LED_STATUS		0x12
+/* Writeable bits in the LED_STATUS register */
+#define   LP8864_LED_STATUS_WR_MASK	GENMASK(14, 9)
+
+/* Textual meaning for every register bit */
+static const char *const lp8864_supply_status_msg[] = {
+	NULL, "Vin under-voltage fault",
+	NULL, "Vin over-voltage fault",
+	NULL, "Vdd under-voltage fault",
+	NULL, "Vin over-current fault",
+	NULL, "Missing charge pump fault",
+	NULL, "Charge pump fault",
+	NULL, "Missing boost sync fault",
+	NULL, "CRC error fault ",
+};
+
+/* Textual meaning for every register bit */
+static const char *const lp8864_boost_status_msg[] = {
+	NULL, "Boost OVP low fault",
+	NULL, "Boost OVP high fault",
+	NULL, "Boost over-current fault",
+	NULL, "Missing boost FSET resistor fault",
+	NULL, "Missing MODE SEL resistor fault",
+	NULL, "Missing LED resistor fault",
+	NULL, "ISET resistor short to ground fault",
+	NULL, "Thermal shutdown fault",
+};
+
+/* Textual meaning for every register bit */
+static const char *const lp8864_led_status_msg[] = {
+	"LED 1 fault",
+	"LED 2 fault",
+	"LED 3 fault",
+	"LED 4 fault",
+	"LED 5 fault",
+	"LED 6 fault",
+	"LED open fault",
+	"LED internal short fault",
+	"LED short to GND fault",
+	NULL, NULL, NULL,
+	"Invalid string configuration fault",
+	NULL,
+	"I2C time out fault",
+};
+
+/**
+ * struct lp8864_led
+ * @client: Pointer to the I2C client
+ * @led_dev: led class device pointer
+ * @regmap: Devices register map
+ * @led_status_mask: Helps to report LED fault only once
+ */
+struct lp8864_led {
+	struct i2c_client *client;
+	struct led_classdev led_dev;
+	struct regmap *regmap;
+	u16 led_status_mask;
+};
+
+static int lp8864_fault_check(struct lp8864_led *led)
+{
+	int ret, i;
+	unsigned int buf;
+
+	ret = regmap_read(led->regmap, LP8864_SUPPLY_STATUS, &buf);
+	if (ret)
+		goto err;
+
+	for (i = 0; i < ARRAY_SIZE(lp8864_supply_status_msg); i++)
+		if (lp8864_supply_status_msg[i] && buf & BIT(i))
+			dev_err(&led->client->dev, "%s\n",
+				lp8864_supply_status_msg[i]);
+
+	/*
+	 * Clear bits have an index preceding the corresponding Status bits;
+	 * both have to be written "1" simultaneously to clear the corresponding
+	 * Status bit.
+	 */
+	if (buf)
+		ret = regmap_write(led->regmap, LP8864_SUPPLY_STATUS,
+				   buf >> 1 | buf);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(led->regmap, LP8864_BOOST_STATUS, &buf);
+	if (ret)
+		goto err;
+
+	for (i = 0; i < ARRAY_SIZE(lp8864_boost_status_msg); i++)
+		if (lp8864_boost_status_msg[i] && buf & BIT(i))
+			dev_err(&led->client->dev, "%s\n",
+				lp8864_boost_status_msg[i]);
+
+	if (buf)
+		ret = regmap_write(led->regmap, LP8864_BOOST_STATUS,
+				   buf >> 1 | buf);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(led->regmap, LP8864_LED_STATUS, &buf);
+	if (ret)
+		goto err;
+
+	/*
+	 * Clear already reported faults that maintain their value until device
+	 * power-down
+	 */
+	buf &= ~led->led_status_mask;
+
+	for (i = 0; i < ARRAY_SIZE(lp8864_led_status_msg); i++)
+		if (lp8864_led_status_msg[i] && buf & BIT(i))
+			dev_err(&led->client->dev, "%s\n",
+				lp8864_led_status_msg[i]);
+
+	/*
+	 * Mark those which maintain their value until device power-down as
+	 * "already reported"
+	 */
+	led->led_status_mask |= buf & ~LP8864_LED_STATUS_WR_MASK;
+
+	/*
+	 * Only bits 14, 12, 10 have to be cleared here, but others are RO,
+	 * we don't care what we write to them.
+	 */
+	if (buf & LP8864_LED_STATUS_WR_MASK)
+		ret = regmap_write(led->regmap, LP8864_LED_STATUS,
+				   buf >> 1 | buf);
+	if (ret)
+		goto err;
+
+	return 0;
+
+err:
+	dev_err(&led->client->dev, "Cannot read/clear faults (%pe)\n",
+		ERR_PTR(ret));
+
+	return ret;
+}
+
+static int lp8864_brightness_set(struct led_classdev *led_cdev,
+				 enum led_brightness brt_val)
+{
+	struct lp8864_led *led = container_of(led_cdev, struct lp8864_led,
+					      led_dev);
+	unsigned int val = brt_val * 0xffff / LED_FULL;
+	int ret;
+
+	ret = lp8864_fault_check(led);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(led->regmap, LP8864_BRT_CONTROL, val);
+	if (ret)
+		dev_err(&led->client->dev, "Cannot write BRT_CONTROL\n");
+
+	return ret;
+}
+
+static enum led_brightness lp8864_brightness_get(struct led_classdev *led_cdev)
+{
+	struct lp8864_led *led = container_of(led_cdev, struct lp8864_led,
+					      led_dev);
+	unsigned int buf;
+	int ret;
+
+	ret = regmap_read(led->regmap, LP8864_BRT_CONTROL, &buf);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read BRT_CONTROL\n");
+		return ret;
+	}
+
+	return buf * LED_FULL / 0xffff;
+}
+
+static int lp8864_init(struct lp8864_led *led)
+{
+	int ret;
+
+	/* Control brightness by DISPLAY_BRT register */
+	ret = regmap_update_bits(led->regmap, LP8864_USER_CONFIG1,
+				 LP8864_BRT_MODE_MASK, LP8864_BRT_MODE_REG);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write USER_CONFIG1\n");
+		return ret;
+	}
+
+	return lp8864_fault_check(led);
+}
+
+static const struct regmap_config lp8864_regmap_config = {
+	.reg_bits		= 8,
+	.val_bits		= 16,
+	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
+	.cache_type		= REGCACHE_NONE,
+};
+
+static void lp8864_disable_gpio(void *data)
+{
+	struct gpio_desc *gpio = data;
+
+	gpiod_set_value(gpio, 0);
+}
+
+static int lp8864_probe(struct i2c_client *client)
+{
+	int ret;
+	struct lp8864_led *led;
+	struct device_node *np = dev_of_node(&client->dev);
+	struct device_node *child_node;
+	struct led_init_data init_data = {};
+	struct gpio_desc *enable_gpio;
+
+	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	child_node = of_get_next_available_child(np, NULL);
+	if (!child_node) {
+		dev_err(&client->dev, "No LED function defined\n");
+		return -EINVAL;
+	}
+
+	ret = devm_regulator_get_enable_optional(&client->dev, "vled");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to enable vled regulator (%pe)\n",
+				     ERR_PTR(ret));
+
+	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
+					      GPIOD_OUT_HIGH);
+	if (IS_ERR(enable_gpio))
+		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
+				     "Failed to get enable GPIO (%pe)\n",
+				     enable_gpio);
+
+	ret = devm_add_action_or_reset(&client->dev, lp8864_disable_gpio,
+				       enable_gpio);
+
+	led->client = client;
+	led->led_dev.brightness_set_blocking = lp8864_brightness_set;
+	led->led_dev.brightness_get = lp8864_brightness_get;
+
+	i2c_set_clientdata(client, led);
+
+	led->regmap = devm_regmap_init_i2c(client, &lp8864_regmap_config);
+	if (IS_ERR(led->regmap)) {
+		dev_err(&client->dev, "Failed to allocate register map (%pe)\n",
+			led->regmap);
+		return PTR_ERR(led->regmap);
+	}
+
+	ret = lp8864_init(led);
+	if (ret)
+		return ret;
+
+	init_data.fwnode = of_fwnode_handle(child_node);
+	init_data.devicename = "lp8864";
+	init_data.default_label = ":display_cluster";
+
+	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
+					     &init_data);
+	if (ret)
+		dev_err(&client->dev, "Failed to register LED device (%pe)\n",
+			ERR_PTR(ret));
+
+	return ret;
+}
+
+static const struct i2c_device_id lp8864_id[] = {
+	{ "lp8864" },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, lp8864_id);
+
+static const struct of_device_id of_lp8864_leds_match[] = {
+	{ .compatible = "ti,lp8864" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_lp8864_leds_match);
+
+static struct i2c_driver lp8864_driver = {
+	.driver = {
+		.name	= "lp8864",
+		.of_match_table = of_lp8864_leds_match,
+	},
+	.probe		= lp8864_probe,
+	.id_table	= lp8864_id,
+};
+module_i2c_driver(lp8864_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LP8864/LP8866 LED driver");
+MODULE_AUTHOR("Alexander Sverdlin <alexander.sverdlin@siemens.com>");
+MODULE_LICENSE("GPL");
-- 
2.47.1