[PATCH 0/3] Convert LP8864 LED driver to backlight class

[PATCH 0/3] Convert LP8864 LED driver to backlight class

[A. Sverdlin]

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

This series converts the TI LP8864/LP8866 driver from a pure LED class
driver to a backlight class driver, while preserving backward-compatible
LED class device registration.

The motivation is a use case on a hot-pluggable segment of an I2C bus.
The generic led-backlight driver (drivers/video/backlight/led_bl.c) is
a platform driver and thus inherently non-hotpluggable -- it cannot react
to dynamic appearance/disappearance of the underlying I2C device. By
making the LP8864 driver directly register a backlight class device, it
becomes a self-contained I2C driver that properly supports hot-plug/unplug
events without depending on a separate platform device for backlight
functionality.

The LED class device registration is retained for backward compatibility:
if the "led" child node is present in the device-tree, an LED class device
is registered in addition to the backlight device.

No in-tree device-trees reference this driver, so there is no mainline
impact. Out-of-tree users who previously relied on the LP8864 as a backend
for the generic led-backlight platform driver will notice a backlight class
device appearing directly.

Alexander Sverdlin (3):
  dt-bindings: backlight: ti,lp8864: Add backlight class properties
  leds: lp8864: Rename struct lp8864_led and local variables
  backlight: lp8864: Convert from LED to backlight class driver

 .../bindings/leds/backlight/ti,lp8864.yaml    |  16 +-
 MAINTAINERS                                   |   2 +-
 drivers/leds/Kconfig                          |  12 --
 drivers/leds/Makefile                         |   1 -
 drivers/video/backlight/Kconfig               |  15 ++
 drivers/video/backlight/Makefile              |   1 +
 .../backlight/lp8864_bl.c}                    | 169 ++++++++++++------
 7 files changed, 147 insertions(+), 69 deletions(-)
 rename drivers/{leds/leds-lp8864.c => video/backlight/lp8864_bl.c} (55%)

-- 
2.54.0

[PATCH 1/3] dt-bindings: backlight: ti,lp8864: Add backlight class properties

[A. Sverdlin]

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

Extend the TI LP8864/LP8866 device-tree binding to support backlight
class properties alongside the existing LED class child node.

This is a preparatory change for converting the LP8864 driver from a
pure LED class driver to additionally register a backlight class device,
motivated by a use case on a hot-pluggable segment of an I2C bus. The
generic led-backlight driver (led_bl.c) is a platform driver and thus
inherently non-hotpluggable, which makes it unsuitable for hardware
topologies where the backlight controller resides on a hot-pluggable I2C
bus segment. By making the LP8864 driver itself register a backlight
class device, it becomes a native I2C driver that properly supports
hot-plug/unplug events.

The binding is updated to:
- Reference backlight common.yaml at the top level, making
  default-brightness and max-brightness valid optional properties
- Make the "led" child node optional rather than required, since the
  backlight class device is now the primary interface
- Use unevaluatedProperties instead of additionalProperties to properly
  allow properties inherited from the referenced common schema

The LED child node is preserved for backward compatibility with existing
device-trees. No in-tree device-trees reference this binding, so this
change has no impact on existing mainline users.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 .../bindings/leds/backlight/ti,lp8864.yaml       | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml
index d44232d462bde..11d7e3840c6fb 100644
--- a/Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/leds/backlight/ti,lp8864.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Texas Instruments - LP8864/LP8866 4/6-Channel LED Driver family
+title: Texas Instruments - LP8864/LP8866 4/6-Channel LED Backlight Driver family
 
 maintainers:
   - Andrew Davis <afd@ti.com>
@@ -21,6 +21,9 @@ description: |
     https://www.ti.com/product/LP8866-Q1
     https://www.ti.com/product/LP8866S-Q1
 
+allOf:
+  - $ref: common.yaml#
+
 properties:
   compatible:
     const: ti,lp8864
@@ -36,9 +39,15 @@ properties:
   vled-supply:
     description: LED supply
 
+  default-brightness:
+    maximum: 65535
+
+  max-brightness:
+    maximum: 65535
+
   led:
     type: object
-    $ref: common.yaml#
+    $ref: /schemas/leds/common.yaml#
     properties:
       function: true
       color: true
@@ -50,9 +59,8 @@ properties:
 required:
   - compatible
   - reg
-  - led
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.54.0

[PATCH 2/3] leds: lp8864: Rename struct lp8864_led and local variables

[A. Sverdlin]

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

Rename struct lp8864_led to struct lp8864 and the local variable 'led'
to 'priv' throughout the driver. The LED functionality will become
optional in a subsequent patch that adds backlight class support, so
having the driver's private data structure and variables named after
LED-specifics is misleading.

No functional change.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 drivers/leds/leds-lp8864.c | 70 +++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/leds/leds-lp8864.c b/drivers/leds/leds-lp8864.c
index 3afd729d2f8a6..d05211b970c94 100644
--- a/drivers/leds/leds-lp8864.c
+++ b/drivers/leds/leds-lp8864.c
@@ -69,32 +69,32 @@ static const char *const lp8864_led_status_msg[] = {
 };
 
 /**
- * struct lp8864_led
+ * struct lp8864
  * @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 lp8864 {
 	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)
+static int lp8864_fault_check(struct lp8864 *priv)
 {
 	int ret, i;
 	unsigned int val;
 
-	ret = regmap_read(led->regmap, LP8864_SUPPLY_STATUS, &val);
+	ret = regmap_read(priv->regmap, LP8864_SUPPLY_STATUS, &val);
 	if (ret)
 		goto err;
 
 	/* Odd bits are status bits, even bits are clear bits */
 	for (i = 0; i < ARRAY_SIZE(lp8864_supply_status_msg); i++)
 		if (val & BIT(i * 2 + 1))
-			dev_warn(&led->client->dev, "%s\n", lp8864_supply_status_msg[i]);
+			dev_warn(&priv->client->dev, "%s\n", lp8864_supply_status_msg[i]);
 
 	/*
 	 * Clear bits have an index preceding the corresponding Status bits;
@@ -102,25 +102,25 @@ static int lp8864_fault_check(struct lp8864_led *led)
 	 * Status bit.
 	 */
 	if (val)
-		ret = regmap_write(led->regmap, LP8864_SUPPLY_STATUS, val >> 1 | val);
+		ret = regmap_write(priv->regmap, LP8864_SUPPLY_STATUS, val >> 1 | val);
 	if (ret)
 		goto err;
 
-	ret = regmap_read(led->regmap, LP8864_BOOST_STATUS, &val);
+	ret = regmap_read(priv->regmap, LP8864_BOOST_STATUS, &val);
 	if (ret)
 		goto err;
 
 	/* Odd bits are status bits, even bits are clear bits */
 	for (i = 0; i < ARRAY_SIZE(lp8864_boost_status_msg); i++)
 		if (val & BIT(i * 2 + 1))
-			dev_warn(&led->client->dev, "%s\n", lp8864_boost_status_msg[i]);
+			dev_warn(&priv->client->dev, "%s\n", lp8864_boost_status_msg[i]);
 
 	if (val)
-		ret = regmap_write(led->regmap, LP8864_BOOST_STATUS, val >> 1 | val);
+		ret = regmap_write(priv->regmap, LP8864_BOOST_STATUS, val >> 1 | val);
 	if (ret)
 		goto err;
 
-	ret = regmap_read(led->regmap, LP8864_LED_STATUS, &val);
+	ret = regmap_read(priv->regmap, LP8864_LED_STATUS, &val);
 	if (ret)
 		goto err;
 
@@ -128,31 +128,31 @@ static int lp8864_fault_check(struct lp8864_led *led)
 	 * Clear already reported faults that maintain their value until device
 	 * power-down
 	 */
-	val &= ~led->led_status_mask;
+	val &= ~priv->led_status_mask;
 
 	for (i = 0; i < ARRAY_SIZE(lp8864_led_status_msg); i++)
 		if (lp8864_led_status_msg[i] && val & BIT(i))
-			dev_warn(&led->client->dev, "%s\n", lp8864_led_status_msg[i]);
+			dev_warn(&priv->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 |= val & ~LP8864_LED_STATUS_WR_MASK;
+	priv->led_status_mask |= val & ~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 (val & LP8864_LED_STATUS_WR_MASK)
-		ret = regmap_write(led->regmap, LP8864_LED_STATUS, val >> 1 | val);
+		ret = regmap_write(priv->regmap, LP8864_LED_STATUS, val >> 1 | val);
 	if (ret)
 		goto err;
 
 	return 0;
 
 err:
-	dev_err(&led->client->dev, "Failed to read/clear faults (%pe)\n", ERR_PTR(ret));
+	dev_err(&priv->client->dev, "Failed to read/clear faults (%pe)\n", ERR_PTR(ret));
 
 	return ret;
 }
@@ -160,31 +160,31 @@ static int lp8864_fault_check(struct lp8864_led *led)
 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);
+	struct lp8864 *priv = container_of(led_cdev, struct lp8864, led_dev);
 	/* Scale 0..LED_FULL into 16-bit HW brightness */
 	unsigned int val = brt_val * 0xffff / LED_FULL;
 	int ret;
 
-	ret = lp8864_fault_check(led);
+	ret = lp8864_fault_check(priv);
 	if (ret)
 		return ret;
 
-	ret = regmap_write(led->regmap, LP8864_BRT_CONTROL, val);
+	ret = regmap_write(priv->regmap, LP8864_BRT_CONTROL, val);
 	if (ret)
-		dev_err(&led->client->dev, "Failed to write brightness value\n");
+		dev_err(&priv->client->dev, "Failed to write brightness value\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);
+	struct lp8864 *priv = container_of(led_cdev, struct lp8864, led_dev);
 	unsigned int val;
 	int ret;
 
-	ret = regmap_read(led->regmap, LP8864_BRT_CONTROL, &val);
+	ret = regmap_read(priv->regmap, LP8864_BRT_CONTROL, &val);
 	if (ret) {
-		dev_err(&led->client->dev, "Failed to read brightness value\n");
+		dev_err(&priv->client->dev, "Failed to read brightness value\n");
 		return ret;
 	}
 
@@ -208,14 +208,14 @@ static void lp8864_disable_gpio(void *data)
 static int lp8864_probe(struct i2c_client *client)
 {
 	int ret;
-	struct lp8864_led *led;
+	struct lp8864 *priv;
 	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)
+	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
 	child_node = of_get_next_available_child(np, NULL);
@@ -237,24 +237,24 @@ static int lp8864_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	led->client = client;
-	led->led_dev.brightness_set_blocking = lp8864_brightness_set;
-	led->led_dev.brightness_get = lp8864_brightness_get;
+	priv->client = client;
+	priv->led_dev.brightness_set_blocking = lp8864_brightness_set;
+	priv->led_dev.brightness_get = lp8864_brightness_get;
 
-	led->regmap = devm_regmap_init_i2c(client, &lp8864_regmap_config);
-	if (IS_ERR(led->regmap))
-		return dev_err_probe(&client->dev, PTR_ERR(led->regmap),
+	priv->regmap = devm_regmap_init_i2c(client, &lp8864_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(priv->regmap),
 				     "Failed to allocate regmap\n");
 
 	/* Control brightness by DISPLAY_BRT register */
-	ret = regmap_update_bits(led->regmap, LP8864_USER_CONFIG1, LP8864_BRT_MODE_MASK,
+	ret = regmap_update_bits(priv->regmap, LP8864_USER_CONFIG1, LP8864_BRT_MODE_MASK,
 								   LP8864_BRT_MODE_REG);
 	if (ret) {
-		dev_err(&led->client->dev, "Failed to set brightness control mode\n");
+		dev_err(&priv->client->dev, "Failed to set brightness control mode\n");
 		return ret;
 	}
 
-	ret = lp8864_fault_check(led);
+	ret = lp8864_fault_check(priv);
 	if (ret)
 		return ret;
 
@@ -262,7 +262,7 @@ static int lp8864_probe(struct i2c_client *client)
 	init_data.devicename = "lp8864";
 	init_data.default_label = ":display_cluster";
 
-	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev, &init_data);
+	ret = devm_led_classdev_register_ext(&client->dev, &priv->led_dev, &init_data);
 	if (ret)
 		dev_err(&client->dev, "Failed to register LED device (%pe)\n", ERR_PTR(ret));
 
-- 
2.54.0

[PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver

[A. Sverdlin]

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

Move the TI LP8864/LP8866 driver from drivers/leds/ to
drivers/video/backlight/ and convert it to register a backlight class
device as its primary interface.

The motivation is a use case on a hot-pluggable segment of an I2C bus.
The generic led-backlight driver (drivers/video/backlight/led_bl.c) is a
platform driver and as such inherently non-hotpluggable. It cannot react
to dynamic appearance/disappearance of the underlying I2C device. By
making the LP8864 driver directly register a backlight class device, it
becomes a native I2C driver that properly supports hot-plug/unplug
events on the I2C bus.

Key changes:
- Register a backlight class device using
  devm_backlight_device_register() as the primary interface
- Implement backlight_ops (update_status, get_brightness)
- The hardware 16-bit brightness register (0x0000-0xFFFF) is directly
  exposed as the backlight brightness range
- Support DT properties "default-brightness" and "max-brightness"
  from the backlight common binding
- Include BL_CORE_SUSPENDRESUME for proper power management integration
- Preserve backward-compatible LED class device registration: if the
  "led" child node is present in the DT, an LED class device is also
  registered (same as the original driver behavior)
- Preserve the CONFIG_LEDS_LP8864 Kconfig symbol name so that existing
  kernel configurations are not affected
- Update MAINTAINERS to reflect the new file location

This will be noticeable for applications which already used the LP8864
as a backend for the generic led-backlight platform driver, as a
backlight device will now appear directly in addition to the LED class
device. However, no in-tree device-trees reference this driver, so
there is no mainline impact.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 MAINTAINERS                                   |   2 +-
 drivers/leds/Kconfig                          |  12 --
 drivers/leds/Makefile                         |   1 -
 drivers/video/backlight/Kconfig               |  15 +++
 drivers/video/backlight/Makefile              |   1 +
 .../backlight/lp8864_bl.c}                    | 111 ++++++++++++++----
 6 files changed, 106 insertions(+), 36 deletions(-)
 rename drivers/{leds/leds-lp8864.c => video/backlight/lp8864_bl.c} (70%)

diff --git a/MAINTAINERS b/MAINTAINERS
index dbd4552236e64..250e8b1ed4bb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26481,7 +26481,7 @@ M:	Alexander Sverdlin <alexander.sverdlin@siemens.com>
 L:	linux-leds@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml
-F:	drivers/leds/leds-lp8864.c
+F:	drivers/video/backlight/lp8864_bl.c
 
 TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER
 M:	Nishanth Menon <nm@ti.com>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f4a0a3c8c8705..990cb9ef18c1e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -529,18 +529,6 @@ 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 8fdb45d5b4393..5e624a48aa2a5 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -59,7 +59,6 @@ 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/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a7a3fbaf7c29e..82ecd7e46236d 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -514,6 +514,21 @@ config BACKLIGHT_LED
 	  If you have a LCD backlight adjustable by LED class driver, say Y
 	  to enable this driver.
 
+config LEDS_LP8864
+	tristate "Backlight driver for TI LP8864/LP8866 4/6 channel LED drivers"
+	depends on I2C && OF
+	select REGMAP_I2C
+	select NEW_LEDS
+	select LEDS_CLASS
+	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. The driver registers a backlight
+	  class device and optionally an LED class device.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called lp8864_bl.
+
 endif # BACKLIGHT_CLASS_DEVICE
 
 endmenu
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 794820a98ed49..6a7287d01d81b 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -62,3 +62,4 @@ obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
 obj-$(CONFIG_BACKLIGHT_ARCXCNN) 	+= arcxcnn_bl.o
 obj-$(CONFIG_BACKLIGHT_RAVE_SP)		+= rave-sp-backlight.o
 obj-$(CONFIG_BACKLIGHT_LED)		+= led_bl.o
+obj-$(CONFIG_LEDS_LP8864)		+= lp8864_bl.o
diff --git a/drivers/leds/leds-lp8864.c b/drivers/video/backlight/lp8864_bl.c
similarity index 70%
rename from drivers/leds/leds-lp8864.c
rename to drivers/video/backlight/lp8864_bl.c
index d05211b970c94..67b28f7daedd2 100644
--- a/drivers/leds/leds-lp8864.c
+++ b/drivers/video/backlight/lp8864_bl.c
@@ -1,12 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * TI LP8864/LP8866 4/6 Channel LED Driver
+ * TI LP8864/LP8866 4/6 Channel LED Backlight Driver
  *
- * Copyright (C) 2024 Siemens AG
+ * Copyright (C) 2024-2026 Siemens AG
  *
  * Based on LP8860 driver by Dan Murphy <dmurphy@ti.com>
  */
 
+#include <linux/backlight.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -27,6 +28,8 @@
 #define LP8864_LED_STATUS		0x12
 #define   LP8864_LED_STATUS_WR_MASK	GENMASK(14, 9)	/* Writeable bits in the LED_STATUS reg */
 
+#define LP8864_MAX_BRIGHTNESS		0xffff
+
 /* Textual meaning for status bits, starting from bit 1 */
 static const char *const lp8864_supply_status_msg[] = {
 	"Vin under-voltage fault",
@@ -71,13 +74,15 @@ static const char *const lp8864_led_status_msg[] = {
 /**
  * struct lp8864
  * @client: Pointer to the I2C client
- * @led_dev: led class device pointer
+ * @led_dev: optional led class device pointer
+ * @bl: backlight device pointer
  * @regmap: Devices register map
  * @led_status_mask: Helps to report LED fault only once
  */
 struct lp8864 {
 	struct i2c_client *client;
-	struct led_classdev led_dev;
+	struct led_classdev *led_dev;
+	struct backlight_device *bl;
 	struct regmap *regmap;
 	u16 led_status_mask;
 };
@@ -157,28 +162,59 @@ static int lp8864_fault_check(struct lp8864 *priv)
 	return ret;
 }
 
-static int lp8864_brightness_set(struct led_classdev *led_cdev,
-				 enum led_brightness brt_val)
+static int lp8864_brightness_set(struct lp8864 *priv, unsigned int brightness)
 {
-	struct lp8864 *priv = container_of(led_cdev, struct lp8864, led_dev);
-	/* Scale 0..LED_FULL into 16-bit HW brightness */
-	unsigned int val = brt_val * 0xffff / LED_FULL;
 	int ret;
 
 	ret = lp8864_fault_check(priv);
 	if (ret)
 		return ret;
 
-	ret = regmap_write(priv->regmap, LP8864_BRT_CONTROL, val);
+	ret = regmap_write(priv->regmap, LP8864_BRT_CONTROL, brightness);
 	if (ret)
 		dev_err(&priv->client->dev, "Failed to write brightness value\n");
 
 	return ret;
 }
 
-static enum led_brightness lp8864_brightness_get(struct led_classdev *led_cdev)
+static int lp8864_backlight_update_status(struct backlight_device *bl)
+{
+	return lp8864_brightness_set(bl_get_data(bl), backlight_get_brightness(bl));
+}
+
+static int lp8864_backlight_get_brightness(struct backlight_device *bl)
 {
-	struct lp8864 *priv = container_of(led_cdev, struct lp8864, led_dev);
+	struct lp8864 *priv = bl_get_data(bl);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(priv->regmap, LP8864_BRT_CONTROL, &val);
+	if (ret) {
+		dev_err(&priv->client->dev, "Failed to read brightness value\n");
+		return ret;
+	}
+
+	return val;
+}
+
+static const struct backlight_ops lp8864_backlight_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = lp8864_backlight_update_status,
+	.get_brightness = lp8864_backlight_get_brightness,
+};
+
+static int lp8864_led_brightness_set(struct led_classdev *led_cdev,
+				     enum led_brightness brt_val)
+{
+	struct lp8864 *priv = dev_get_drvdata(led_cdev->dev->parent);
+
+	/* Scale 0..LED_FULL into 16-bit HW brightness */
+	return lp8864_brightness_set(priv, brt_val * 0xffff / LED_FULL);
+}
+
+static enum led_brightness lp8864_led_brightness_get(struct led_classdev *led_cdev)
+{
+	struct lp8864 *priv = dev_get_drvdata(led_cdev->dev->parent);
 	unsigned int val;
 	int ret;
 
@@ -212,18 +248,15 @@ static int lp8864_probe(struct i2c_client *client)
 	struct device_node *np = dev_of_node(&client->dev);
 	struct device_node *child_node;
 	struct led_init_data init_data = {};
+	struct backlight_device *bl;
+	struct backlight_properties props;
 	struct gpio_desc *enable_gpio;
+	u32 val;
 
 	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		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\n");
@@ -238,8 +271,7 @@ static int lp8864_probe(struct i2c_client *client)
 		return ret;
 
 	priv->client = client;
-	priv->led_dev.brightness_set_blocking = lp8864_brightness_set;
-	priv->led_dev.brightness_get = lp8864_brightness_get;
+	i2c_set_clientdata(client, priv);
 
 	priv->regmap = devm_regmap_init_i2c(client, &lp8864_regmap_config);
 	if (IS_ERR(priv->regmap))
@@ -258,11 +290,46 @@ static int lp8864_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	/* Register backlight class device */
+	memset(&props, 0, sizeof(props));
+	props.type = BACKLIGHT_RAW;
+	props.max_brightness = LP8864_MAX_BRIGHTNESS;
+	props.brightness = LP8864_MAX_BRIGHTNESS;
+	props.scale = BACKLIGHT_SCALE_LINEAR;
+
+	if (!device_property_read_u32(&client->dev, "max-brightness", &val))
+		props.max_brightness = val;
+
+	if (!device_property_read_u32(&client->dev, "default-brightness", &val))
+		props.brightness = val;
+
+	bl = devm_backlight_device_register(&client->dev, "lp8864-backlight",
+					    &client->dev, priv,
+					    &lp8864_backlight_ops, &props);
+	if (IS_ERR(bl))
+		return dev_err_probe(&client->dev, PTR_ERR(bl),
+				     "Failed to register backlight device\n");
+
+	priv->bl = bl;
+	backlight_update_status(bl);
+
+	/* Register LED class device if "led" child node is present */
+	child_node = of_get_available_child_by_name(np, "led");
+	if (!child_node)
+		return 0;
+
+	priv->led_dev = devm_kzalloc(&client->dev, sizeof(*priv->led_dev), GFP_KERNEL);
+	if (!priv->led_dev)
+		return -ENOMEM;
+
+	priv->led_dev->brightness_set_blocking = lp8864_led_brightness_set;
+	priv->led_dev->brightness_get = lp8864_led_brightness_get;
+
 	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, &priv->led_dev, &init_data);
+	ret = devm_led_classdev_register_ext(&client->dev, priv->led_dev, &init_data);
 	if (ret)
 		dev_err(&client->dev, "Failed to register LED device (%pe)\n", ERR_PTR(ret));
 
@@ -291,6 +358,6 @@ static struct i2c_driver lp8864_driver = {
 };
 module_i2c_driver(lp8864_driver);
 
-MODULE_DESCRIPTION("Texas Instruments LP8864/LP8866 LED driver");
+MODULE_DESCRIPTION("Texas Instruments LP8864/LP8866 LED Backlight driver");
 MODULE_AUTHOR("Alexander Sverdlin <alexander.sverdlin@siemens.com>");
 MODULE_LICENSE("GPL");
-- 
2.54.0

Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Hardcoded backlight device name prevents probing multiple device instances.
- [High] Unclamped device tree brightness properties bypass limits and cause hardware wraparound.
- [High] The LED class interface completely ignores DT backlight limits, allowing userspace to overdrive the display.
- [Medium] Concurrent sysfs writes from the backlight and LED subsystems cause a data race on `priv->led_status_mask`.
--

commit 3a876b5316e78acf44b2217cdc076c3285471e8a
Author: Alexander Sverdlin <alexander.sverdlin@gmail.com>

backlight: lp8864: Convert from LED to backlight class driver
    
Move the TI LP8864/LP8866 driver from drivers/leds/ to drivers/video/backlight/
and convert it to register a backlight class device. The driver preserves
backward-compatible LED class device registration if the "led" child node is
present.

> diff --git a/drivers/video/backlight/lp8864_bl.c b/drivers/video/backlight/lp8864_bl.c
> index d05211b970c94..67b28f7daedd2 100644
> --- a/drivers/leds/leds-lp8864.c
> +++ b/drivers/video/backlight/lp8864_bl.c

[ ... ]

> @@ -157,28 +162,59 @@ static int lp8864_fault_check(struct lp8864 *priv)
>  	return ret;
>  }
>  
> -static int lp8864_brightness_set(struct led_classdev *led_cdev,
> -				 enum led_brightness brt_val)
> +static int lp8864_brightness_set(struct lp8864 *priv, unsigned int brightness)
>  {
> -	struct lp8864 *priv = container_of(led_cdev, struct lp8864, led_dev);
> -	/* Scale 0..LED_FULL into 16-bit HW brightness */
> -	unsigned int val = brt_val * 0xffff / LED_FULL;
>  	int ret;
>  
>  	ret = lp8864_fault_check(priv);

[Severity: Medium]
Since the driver registers both a backlight device and an LED class
device, can concurrent sysfs writes to both interfaces cause a data race?

The LED and Backlight subsystems use independent locks (led_access mutex
and update_lock mutex). Parallel writes will result in concurrent calls to
lp8864_brightness_set(), which calls lp8864_fault_check().

Inside lp8864_fault_check(), the driver performs an unprotected
read-modify-write on priv->led_status_mask. Could this concurrent access
cause fault status updates to be lost?

>  	if (ret)
>  		return ret;

[ ... ]

> +static int lp8864_led_brightness_set(struct led_classdev *led_cdev,
> +				     enum led_brightness brt_val)
> +{
> +	struct lp8864 *priv = dev_get_drvdata(led_cdev->dev->parent);
> +
> +	/* Scale 0..LED_FULL into 16-bit HW brightness */
> +	return lp8864_brightness_set(priv, brt_val * 0xffff / LED_FULL);
> +}

[Severity: High]
Does the LED class interface ignore the device tree backlight limits?

The backlight device respects props.max_brightness, but the parallel LED
interface scales the requested brightness using a hardcoded LED_FULL divisor
(brt_val * 0xffff / LED_FULL).

If a custom max-brightness is specified in the device tree, writing to
the LED sysfs interface could bypass this limit and write arbitrary high
values to the hardware. Could this overdrive and potentially damage the
display?

[ ... ]

> @@ -258,11 +290,46 @@ static int lp8864_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	/* Register backlight class device */
> +	memset(&props, 0, sizeof(props));
> +	props.type = BACKLIGHT_RAW;
> +	props.max_brightness = LP8864_MAX_BRIGHTNESS;
> +	props.brightness = LP8864_MAX_BRIGHTNESS;
> +	props.scale = BACKLIGHT_SCALE_LINEAR;
> +
> +	if (!device_property_read_u32(&client->dev, "max-brightness", &val))
> +		props.max_brightness = val;
> +
> +	if (!device_property_read_u32(&client->dev, "default-brightness", &val))
> +		props.brightness = val;

[Severity: High]
Could reading max-brightness without default-brightness bypass the
protective limits during boot?

If max-brightness is provided in the device tree but default-brightness
is not, props.brightness retains its initial 0xFFFF value. When
backlight_update_status() is called shortly after, it will write this
unconstrained 0xFFFF to the hardware.

Additionally, since device_property_read_u32() is used without upper bound
checks, would values greater than 65535 in the device tree wrap around
when passed to the 16-bit regmap_write() in lp8864_brightness_set()?

> +
> +	bl = devm_backlight_device_register(&client->dev, "lp8864-backlight",
> +					    &client->dev, priv,
> +					    &lp8864_backlight_ops, &props);

[Severity: High]
Will using a fixed string for the backlight device name prevent probing
multiple instances?

The commit message mentions a use case on a hot-pluggable segment of an
I2C bus. If there are multiple LP8864 instances on the same system, the
second instance will fail to probe because the sysfs core mandates unique
names for devices in the same class.

Should this use a dynamically unique name, such as dev_name(&client->dev),
to avoid device_add() returning -EEXIST?

> +	if (IS_ERR(bl))
> +		return dev_err_probe(&client->dev, PTR_ERR(bl),
> +				     "Failed to register backlight device\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615120353.3409035-1-alexander.sverdlin@siemens.com?part=3