Re: [PATCH v18 2/3] leds: add basic support for TI/National Semiconductor LP5812 LED Driver

[Lee Jones <lee@kernel.org>]

On Mon, 24 Nov 2025, Nam Tran wrote:

> The LP5812 is a 4x3 matrix RGB LED driver with an autonomous animation
> engine and time-cross-multiplexing (TCM) support for up to 12 LEDs or
> 4 RGB LEDs. Each LED can be configured through the related registers
> to realize vivid and fancy lighting effects.
> 
> This patch adds minimal driver support for the LP5812, implementing
> only the essential functionality: I2C communication with the device,
> LED registration, brightness control in manual mode, and basic sysfs
> interfaces for LED configuration and fault monitoring.
> 
> This patch adds minimal driver support for the LP5812, implementing
> only the essential functionality: I2C communication with the device,
> LED registration, brightness control in manual mode, and basic sysfs
> interfaces for LED configuration and fault monitoring.
> 
> Signed-off-by: Nam Tran <trannamatk@gmail.com>
> ---
>  MAINTAINERS                    |   4 +
>  drivers/leds/rgb/Kconfig       |  13 +
>  drivers/leds/rgb/Makefile      |   1 +
>  drivers/leds/rgb/leds-lp5812.c | 646 +++++++++++++++++++++++++++++++++
>  drivers/leds/rgb/leds-lp5812.h | 172 +++++++++
>  5 files changed, 836 insertions(+)
>  create mode 100644 drivers/leds/rgb/leds-lp5812.c
>  create mode 100644 drivers/leds/rgb/leds-lp5812.h

[...]

> +static ssize_t parse_drive_mode(struct lp5812_chip *chip, const char *str)
> +{
> +	int i;
> +
> +	chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = false;
> +	chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = false;
> +	chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = false;
> +	chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = false;
> +
> +	if (sysfs_streq(str, LP5812_MODE_DIRECT_NAME)) {
> +		chip->u_drive_mode.s_drive_mode.led_mode = LP5812_MODE_DIRECT_VALUE;
> +		return 0;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(chip_mode_map); i++) {
> +		if (!sysfs_streq(str, chip_mode_map[i].mode_name))
> +			continue;
> +
> +		chip->u_drive_mode.s_drive_mode.led_mode = chip_mode_map[i].mode;
> +		chip->u_scan_order.s_scan_order.scan_order_0 = chip_mode_map[i].scan_order_0;
> +		chip->u_scan_order.s_scan_order.scan_order_1 = chip_mode_map[i].scan_order_1;
> +		chip->u_scan_order.s_scan_order.scan_order_2 = chip_mode_map[i].scan_order_2;
> +		chip->u_scan_order.s_scan_order.scan_order_3 = chip_mode_map[i].scan_order_3;

Where are all of these used?

> +
> +		switch (chip_mode_map[i].selection_led) {
> +		case LP5812_MODE_MIX_SELECT_LED_0:
> +			chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = true;
> +			break;
> +		case LP5812_MODE_MIX_SELECT_LED_1:
> +			chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = true;
> +			break;
> +		case LP5812_MODE_MIX_SELECT_LED_2:
> +			chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = true;
> +			break;
> +		case LP5812_MODE_MIX_SELECT_LED_3:
> +			chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = true;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}

[...]

> +union u_scan_order {

What is 'u'?

> +	struct {
> +		u8 scan_order_0:2;
> +		u8 scan_order_1:2;
> +		u8 scan_order_2:2;
> +		u8 scan_order_3:2;
> +	} s_scan_order;
> +	u8 scan_order_val;
> +};

[...]

-- 
Lee Jones [李琼斯]