[PATCH 0/2] leds: lp8864: Expose a backlight via led_bl helper

[PATCH 0/2] leds: lp8864: Expose a backlight via led_bl helper

[A. Sverdlin]

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

This series lets the TI LP8864/LP8866 LED driver expose a standard
backlight class device in addition to its LED class device, so it can
be consumed by display/panel stacks that expect a backlight.

This is a replacement for the earlier series "Convert LP8864 LED driver
to backlight class" [1], which took a much more invasive route: it moved
the driver from drivers/leds/ to drivers/video/backlight/ and added
backlight class registration on top.

Feedback on that approach was that a working LED driver should not be
relocated and re-typed just to gain a backlight interface.

The new approach keeps the LP8864 a plain LED driver and instead teaches
the existing generic led-backlight code to register a backlight on behalf
of a self-contained LED provider:

 * Patch 1 factors the backlight registration out of led_bl's probe path
   into a shared helper and exports devm_led_backlight_register(). This
   registers a backlight class device driven by a single LED, without a
   device-tree node, bound to the caller's device lifetime. The registration
   is now fully devres-managed, so the explicit .remove callback goes away.
   The exported symbol is a no-op when led-backlight support is not
   reachable (IS_REACHABLE(CONFIG_BACKLIGHT_LED)), so callers need no
   Kconfig plumbing and are not force-selected to build led_bl.

 * Patch 2 calls that helper from the LP8864 driver, spawning a backlight
   tied to the I2C device lifetime.

Compared to [1] this means:

 * The driver stays in drivers/leds/ and remains an ordinary LED driver.
 * The generic led_bl helper becomes reusable by other self-contained LED
   providers.

The motivating use case is unchanged: an LP8864 on a hot-pluggable segment
of an I2C bus. The generic led-backlight platform driver cannot react to a
dynamically (dis)appearing I2C device; letting the I2C driver register the
backlight directly makes it a self-contained, hot-plug-safe driver.

[1] https://lore.kernel.org/all/20260615120353.3409035-1-alexander.sverdlin@siemens.com/

Alexander Sverdlin (2):
  backlight: led_bl: Add devm_led_backlight_register() helper
  leds: lp8864: Register a backlight device

 drivers/leds/leds-lp8864.c       |   5 +-
 drivers/video/backlight/led_bl.c | 130 ++++++++++++++++++++-----------
 include/linux/led_bl.h           |  20 +++++
 3 files changed, 109 insertions(+), 46 deletions(-)
 create mode 100644 include/linux/led_bl.h

-- 
2.55.0

[PATCH 1/2] backlight: led_bl: Add devm_led_backlight_register() helper

[A. Sverdlin]

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

The led-backlight driver could so far only be instantiated from a
device-tree node with the "led-backlight" compatible. This makes it
impossible for a self-contained LED provider (e.g. a hot-pluggable I2C
LED controller) to expose a backlight interface tied to its own
lifetime.

Factor the actual backlight registration out of the probe path into a
shared led_bl_register() helper and export devm_led_backlight_register(),
which registers a backlight class device driven by a single LED, without
device tree and bound to the caller's device lifetime. The backlight
device and the LED sysfs handover are now devres-managed, so the probe
path shrinks and the explicit .remove callback is no longer needed.

The exported helper is a no-op when the led-backlight support is not
reachable (IS_REACHABLE(CONFIG_BACKLIGHT_LED)), so callers do not need
any Kconfig plumbing and are not force-selected to build it.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 MAINTAINERS                      |   1 +
 drivers/video/backlight/led_bl.c | 130 ++++++++++++++++++++-----------
 include/linux/led_bl.h           |  20 +++++
 3 files changed, 107 insertions(+), 44 deletions(-)
 create mode 100644 include/linux/led_bl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8014b9f8253ed..d525a7c4043aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4508,6 +4508,7 @@ F:	Documentation/ABI/testing/sysfs-class-backlight
 F:	Documentation/devicetree/bindings/leds/backlight
 F:	drivers/video/backlight/
 F:	include/linux/backlight.h
+F:	include/linux/led_bl.h
 F:	include/linux/pwm_backlight.h
 
 BARCO P50 GPIO DRIVER
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index f7ab9b3607313..f87a48f74186a 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/backlight.h>
+#include <linux/led_bl.h>
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -173,29 +174,25 @@ static int led_bl_parse_levels(struct device *dev,
 	return 0;
 }
 
-static int led_bl_probe(struct platform_device *pdev)
+static void led_bl_disable(void *data)
 {
-	struct backlight_properties props;
-	struct led_bl_data *priv;
-	int ret, i;
-
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, priv);
+	struct led_bl_data *priv = data;
+	int i;
 
-	priv->dev = &pdev->dev;
+	led_bl_power_off(priv);
+	for (i = 0; i < priv->nb_leds; i++) {
+		mutex_lock(&priv->leds[i]->led_access);
+		led_sysfs_enable(priv->leds[i]);
+		mutex_unlock(&priv->leds[i]->led_access);
+	}
+}
 
-	ret = led_bl_get_leds(&pdev->dev, priv);
-	if (ret)
-		return ret;
+static int led_bl_register(struct device *dev, struct led_bl_data *priv)
+{
+	struct backlight_properties props;
+	int ret, i;
 
-	ret = led_bl_parse_levels(&pdev->dev, priv);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to parse DT data\n");
-		return ret;
-	}
+	priv->dev = dev;
 
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
@@ -203,24 +200,28 @@ static int led_bl_probe(struct platform_device *pdev)
 	props.brightness = priv->default_brightness;
 	props.power = (priv->default_brightness > 0) ? BACKLIGHT_POWER_OFF :
 		      BACKLIGHT_POWER_ON;
-	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
-			&pdev->dev, priv, &led_bl_ops, &props);
-	if (IS_ERR(priv->bl_dev)) {
-		dev_err(&pdev->dev, "Failed to register backlight\n");
-		return PTR_ERR(priv->bl_dev);
-	}
+	priv->bl_dev = devm_backlight_device_register(dev, dev_name(dev), dev,
+						      priv, &led_bl_ops, &props);
+	if (IS_ERR(priv->bl_dev))
+		return dev_err_probe(dev, PTR_ERR(priv->bl_dev),
+				     "Failed to register backlight\n");
 
 	for (i = 0; i < priv->nb_leds; i++) {
+		struct device *supplier = priv->leds[i]->dev->parent;
 		struct device_link *link;
 
-		link = device_link_add(&pdev->dev, priv->leds[i]->dev->parent,
-				       DL_FLAG_AUTOREMOVE_CONSUMER);
-		if (!link) {
-			dev_err(&pdev->dev, "Failed to add devlink (consumer %s, supplier %s)\n",
-				dev_name(&pdev->dev), dev_name(priv->leds[i]->dev->parent));
-			backlight_device_unregister(priv->bl_dev);
-			return -EINVAL;
-		}
+		/*
+		 * BL and the LED are the same device if instantiated via
+		 * devm_led_backlight_register()
+		 */
+		if (supplier == dev)
+			continue;
+
+		link = device_link_add(dev, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
+		if (!link)
+			return dev_err_probe(dev, -EINVAL,
+					     "Failed to add devlink (consumer %s, supplier %s)\n",
+					     dev_name(dev), dev_name(supplier));
 	}
 
 	for (i = 0; i < priv->nb_leds; i++) {
@@ -229,26 +230,68 @@ static int led_bl_probe(struct platform_device *pdev)
 		mutex_unlock(&priv->leds[i]->led_access);
 	}
 
+	ret = devm_add_action_or_reset(dev, led_bl_disable, priv);
+	if (ret)
+		return ret;
+
 	backlight_update_status(priv->bl_dev);
 
 	return 0;
 }
 
-static void led_bl_remove(struct platform_device *pdev)
+static int led_bl_probe(struct platform_device *pdev)
 {
-	struct led_bl_data *priv = platform_get_drvdata(pdev);
-	struct backlight_device *bl = priv->bl_dev;
-	int i;
+	struct led_bl_data *priv;
+	int ret;
 
-	backlight_device_unregister(bl);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
-	led_bl_power_off(priv);
-	for (i = 0; i < priv->nb_leds; i++) {
-		mutex_lock(&priv->leds[i]->led_access);
-		led_sysfs_enable(priv->leds[i]);
-		mutex_unlock(&priv->leds[i]->led_access);
+	ret = led_bl_get_leds(&pdev->dev, priv);
+	if (ret)
+		return ret;
+
+	ret = led_bl_parse_levels(&pdev->dev, priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to parse DT data\n");
+		return ret;
 	}
+
+	return led_bl_register(&pdev->dev, priv);
+}
+
+/**
+ * devm_led_backlight_register - expose a LED as a backlight device
+ * @dev: LED provider device, also the parent and lifecycle owner
+ * @led: LED class device to drive the backlight
+ *
+ * Registers a backlight class device driven by @led, without device tree and
+ * tied to the lifetime of @dev. This lets self-contained (e.g. hot-pluggable
+ * I2C) LED drivers offer a backlight interface without static platform
+ * plumbing. It is a no-op when the led-backlight support is not built in.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int devm_led_backlight_register(struct device *dev, struct led_classdev *led)
+{
+	struct led_bl_data *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->leds = devm_kmalloc(dev, sizeof(*priv->leds), GFP_KERNEL);
+	if (!priv->leds)
+		return -ENOMEM;
+	priv->leds[0] = led;
+	priv->nb_leds = 1;
+	priv->max_brightness = led->max_brightness;
+	priv->default_brightness = led->brightness;
+
+	return led_bl_register(dev, priv);
 }
+EXPORT_SYMBOL_GPL(devm_led_backlight_register);
 
 static const struct of_device_id led_bl_of_match[] = {
 	{ .compatible = "led-backlight" },
@@ -263,7 +306,6 @@ static struct platform_driver led_bl_driver = {
 		.of_match_table	= led_bl_of_match,
 	},
 	.probe		= led_bl_probe,
-	.remove		= led_bl_remove,
 };
 
 module_platform_driver(led_bl_driver);
diff --git a/include/linux/led_bl.h b/include/linux/led_bl.h
new file mode 100644
index 0000000000000..e38e4d62bf653
--- /dev/null
+++ b/include/linux/led_bl.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_LED_BL_H
+#define _LINUX_LED_BL_H
+
+#include <linux/kconfig.h>
+
+struct device;
+struct led_classdev;
+
+#if IS_REACHABLE(CONFIG_BACKLIGHT_LED)
+int devm_led_backlight_register(struct device *dev, struct led_classdev *led);
+#else
+static inline int devm_led_backlight_register(struct device *dev,
+					      struct led_classdev *led)
+{
+	return 0;
+}
+#endif
+
+#endif /* _LINUX_LED_BL_H */
-- 
2.55.0

[PATCH 2/2] leds: lp8864: Register a backlight device

[A. Sverdlin]

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

The LP8864/LP8866 is a display-cluster LED backlight driver. Expose a
standard backlight class interface in addition to the LED class device,
so it can be used by display/panel stacks that expect a backlight.

Use the new devm_led_backlight_register() helper, which spawns a
led-backlight device driven by our LED and tied to the I2C device
lifetime. This keeps the driver a plain LED driver, requires no
device-tree changes and remains backwards compatible with existing DTs.
It is a no-op when the led-backlight support is not enabled.

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

diff --git a/drivers/leds/leds-lp8864.c b/drivers/leds/leds-lp8864.c
index 204727f2f350e..47896dfc13c11 100644
--- a/drivers/leds/leds-lp8864.c
+++ b/drivers/leds/leds-lp8864.c
@@ -10,6 +10,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
+#include <linux/led_bl.h>
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -264,9 +265,9 @@ static int lp8864_probe(struct i2c_client *client)
 
 	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 dev_err_probe(&client->dev, ret, "Failed to register LED device\n");
 
-	return ret;
+	return devm_led_backlight_register(&client->dev, &led->led_dev);
 }
 
 static const struct i2c_device_id lp8864_id[] = {
-- 
2.55.0

Re: [PATCH 2/2] leds: lp8864: Register a backlight device

[sashiko-bot]

> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> The LP8864/LP8866 is a display-cluster LED backlight driver. Expose a
> standard backlight class interface in addition to the LED class device,
> so it can be used by display/panel stacks that expect a backlight.
> 
> Use the new devm_led_backlight_register() helper, which spawns a
> led-backlight device driven by our LED and tied to the I2C device
> lifetime. This keeps the driver a plain LED driver, requires no
> device-tree changes and remains backwards compatible with existing DTs.
> It is a no-op when the led-backlight support is not enabled.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH 1/2] backlight: led_bl: Add devm_led_backlight_register() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The devres registration order for the backlight device and the LED disable action is reversed, creating a TOCTOU race condition on device removal that allows the hardware to be improperly turned back on.
- [Medium] Hardcoding `dev_name(dev)` as the backlight device name prevents a single device from registering multiple LED-backed backlights.

Pre-existing issues:
- [High] Dereferencing a potentially NULL `supplier` pointer inside `dev_name()` during an error path causes a kernel panic.
--

--- Patch [1]: [PATCH 1/2] backlight: led_bl: Add devm_led_backlight_register() helper ---
commit 9866a4a9924f34ae7a659947d01fcc9dc9744ec2
Author: Alexander Sverdlin <alexander.sverdlin@siemens.com>

backlight: led_bl: Add devm_led_backlight_register() helper

This commit factors out the backlight registration into a led_bl_register()
helper and adds devm_led_backlight_register(). This allows self-contained
LED providers to expose a backlight interface. It also switches to
devres-managed functions to handle teardown, removing the explicit .remove
callback.

> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> index f7ab9b3607313..f87a48f74186a 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c

[ ... ]

> @@ -203,24 +200,28 @@ static int led_bl_probe(struct platform_device *pdev)
>  	props.brightness = priv->default_brightness;
>  	props.power = (priv->default_brightness > 0) ? BACKLIGHT_POWER_OFF :
>  		      BACKLIGHT_POWER_ON;
> -	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
> -			&pdev->dev, priv, &led_bl_ops, &props);
> -	if (IS_ERR(priv->bl_dev)) {
> -		dev_err(&pdev->dev, "Failed to register backlight\n");
> -		return PTR_ERR(priv->bl_dev);
> -	}
> +	priv->bl_dev = devm_backlight_device_register(dev, dev_name(dev), dev,
> +						      priv, &led_bl_ops, &props);

[Severity: Medium]
Since dev_name(dev) is used for the backlight device name, will this prevent
a single device from registering multiple LED-backed backlights?

If a single controller driver calls devm_led_backlight_register() more than
once for different LEDs, it seems the second call will fail due to a sysfs
naming collision (-EEXIST) because the name is hardcoded to the parent
device's name.

> +	if (IS_ERR(priv->bl_dev))
> +		return dev_err_probe(dev, PTR_ERR(priv->bl_dev),
> +				     "Failed to register backlight\n");
>  
>  	for (i = 0; i < priv->nb_leds; i++) {
> +		struct device *supplier = priv->leds[i]->dev->parent;
>  		struct device_link *link;
>  
> -		link = device_link_add(&pdev->dev, priv->leds[i]->dev->parent,
> -				       DL_FLAG_AUTOREMOVE_CONSUMER);
> -		if (!link) {
> -			dev_err(&pdev->dev, "Failed to add devlink (consumer %s, supplier %s)\n",
> -				dev_name(&pdev->dev), dev_name(priv->leds[i]->dev->parent));
> -			backlight_device_unregister(priv->bl_dev);
> -			return -EINVAL;
> -		}
> +		/*
> +		 * BL and the LED are the same device if instantiated via
> +		 * devm_led_backlight_register()
> +		 */
> +		if (supplier == dev)
> +			continue;
> +
> +		link = device_link_add(dev, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
> +		if (!link)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Failed to add devlink (consumer %s, supplier %s)\n",
> +					     dev_name(dev), dev_name(supplier));

[Severity: High]
This is a pre-existing issue, but could this dev_name(supplier) call trigger a
NULL pointer dereference?

If an LED classdev without a parent device (where parent = NULL) is passed
to devm_led_backlight_register(), supplier becomes NULL.

Since device_link_add() unconditionally returns NULL when the supplier is
NULL, the error path will execute and dev_name(supplier) will panic by
dereferencing NULL.

>  	}
>  
>  	for (i = 0; i < priv->nb_leds; i++) {
> @@ -229,26 +230,68 @@ static int led_bl_probe(struct platform_device *pdev)
>  		mutex_unlock(&priv->leds[i]->led_access);
>  	}
>  
> +	ret = devm_add_action_or_reset(dev, led_bl_disable, priv);
> +	if (ret)
> +		return ret;

[Severity: High]
Does registering the led_bl_disable action here create a race condition
during device removal?

Because devres actions execute in LIFO order (reverse of registration),
led_bl_disable() will run before the backlight device is unregistered.

This means led_bl_disable() turns off the LED, but the backlight sysfs nodes
are still active. Could userspace write to the backlight's sysfs brightness
attribute during this window, turning the LED back on just before the driver
is removed, thus leaving the hardware powered on?

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