Re: [PATCH 1/2] leds: Texas Instruments LP5816 driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/07/2026 20:43, Alistair Bell wrote:
> Add support for Texas Instruments LP5816 4-channel I2C device,
> the driver provides:
> 
> - Independent 4-channel control via the multicolor sysfs class
> - Configurable fade effects, duration, fade profile and maximum
>   operating current via sysfs
> 
> Signed-off-by: Alistair Bell <dev@alistairbell.org>
> ---
>  MAINTAINERS                |   8 +
>  drivers/leds/Kconfig       |  12 +
>  drivers/leds/Makefile      |   1 +
>  drivers/leds/leds-lp5816.c | 443 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 464 insertions(+)
>  create mode 100644 drivers/leds/leds-lp5816.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f37a81950..b6a8c812a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26761,6 +26761,14 @@ F:	drivers/leds/rgb/Makefile
>  F:	drivers/leds/rgb/leds-lp5812.c
>  F:	drivers/leds/rgb/leds-lp5812.h
>  
> +TEXAS INSTRUMENTS' LP5816 RGBW LED DRIVER
> +M:	Alistair Bell <dev@alistairbell.org>
> +L:	linux-leds@vger.kernel.org
> +S:	Maintained
> +F:	drivers/leds/Kconfig
> +F:	drivers/leds/Makefile
> +F:	drivers/leds/leds-lp5816.c
> +
>  TEXAS INSTRUMENTS' LB8864 LED BACKLIGHT DRIVER
>  M:	Alexander Sverdlin <alexander.sverdlin@siemens.com>
>  L:	linux-leds@vger.kernel.org
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f4a0a3c8c..ce3776adf 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -541,6 +541,18 @@ config LEDS_LP8864
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called leds-lp8864.
>  
> +config LEDS_LP5816
> +    tristate "LED driver for LP5816 chip"
> +    depends on I2C
> +    depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> +	select REGMAP_I2C
> +    help
> +      Say Y to enable support for the Texas Instruments LP5816
> +      RGBW LED connected via I2C.
> +
> +      To compile this driver as a module, choose M here:
> +      the module will be called lp5816.

Messed indentation. Please be sure you are sending consistent code, not
something written completely different than the rest.

> +
>  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 7db376891..5ba7de099 100644
> --- a/drivers/leds/Makefile


...

> +
> +static ssize_t max_current_store(struct device *dev,
> +				 struct device_attribute *attr,
> +								const char *buf, size_t count)
> +{
> +	struct led_classdev *cdev;
> +	struct lp5816 *chip;
> +	int res, val;
> +
> +	cdev = dev_get_drvdata(dev);
> +	chip = container_of(cdev, struct lp5816, mcdev.led_cdev);
> +
> +	res = kstrtoint(buf, 0, &val);
> +	if (res < 0)
> +		return res;
> +	if (val < 0 || val > 1)
> +		return -EINVAL;
> +
> +	res = lp5816_multi_lock_write(chip, (const struct reg_sequence[]) {
> +		{ .reg = REG_DEV_CONFIG0, .def = val },
> +		{ .reg = REG_UPDATE_CMD, .def = UPDATE_CMD } }, 2);
> +	return (res < 0) ? res : count;
> +}


You cannot introduce own ABI duplicating existing sysfs interface.


> +
> +static int lp5816_probe(struct i2c_client *client)
> +{
> +	struct lp5816 *chip;
> +	char *name;
> +	int res;
> +
> +	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return dev_err_probe(&client->dev, -ENOMEM,
> +			"failed to allocate lp5816 internal structure\n");

Since when any driver has such error message?

Please look at most recently addedd and reviewed drivers and learn from
them how typical code looks like.


Best regards,
Krzysztof