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

[Andrew Davis <afd@ti.com>]

On 6/15/26 7:03 AM, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> Move the TI LP8864/LP8866 driver from drivers/leds/ to
> drivers/video/backlight/

Why move it? You can register a backlight device from any directory.

> and convert it to register a backlight class
> device as its primary interface.
> 

What do you mean by "primary"? You should be able to register with
both frameworks and have the driver interop between as needed.

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

That isn't strictly true, there is platform_device_{del,unregister}(), so
whatever your mechanism for removing the I2C device would be, the same
could be done to the led_bl device before then removing the I2C device.

We don't want to have to move every LED driver that could possibly
be used as a backlight to the backlight framework, the led_bl.c
handles adapting LED->backlight as needed. So what you really need
here is to de-couple led_bl.c from DT so it can better handle dynamic
add/remove. Then this LED driver simply could register a "led-backlight"
platform driver to handle the backlight interface, and remove the
backlight device when it itself (the LED device) is removed.

Andrew

  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");