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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251125134836.GC1127788@google.com>
Date: Tue, 25 Nov 2025 13:48:36 +0000
From: Lee Jones <lee@...nel.org>
To: Nam Tran <trannamatk@...il.com>
Cc: pavel@...nel.org, gregkh@...uxfoundation.org, rdunlap@...radead.org,
	christophe.jaillet@...adoo.fr, krzk+dt@...nel.org, robh@...nel.org,
	conor+dt@...nel.org, corbet@....net, linux-leds@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH v18 2/3] leds: add basic support for TI/National
 Semiconductor LP5812 LED Driver

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@...il.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 [李琼斯]