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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251126160024.141129-1-trannamatk@gmail.com>
Date: Wed, 26 Nov 2025 23:00:24 +0700
From: Nam Tran <trannamatk@...il.com>
To: lee@...nel.org
Cc: gregkh@...uxfoundation.org,
	pavel@...nel.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 Tue, 25 Nov 2025, Lee Jones wrote:

> > +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?

These fields are part of unions (u_drive_mode and u_scan_order).
The bitfields are packed into drive_mode_val and scan_order_val, which are
written to DEV_CONFIG1 and DEV_CONFIG2 in lp5812_set_drive_mode_scan_order().

> [...]
> 
> > +union u_scan_order {
> 
> What is 'u'?

The u_* and s_* prefixes were originally meant to indicate union/struct, but they are not idiomatic.
I will rename it to
        union lp5812_scan_order {
            struct {
                u8 order0:2;
                u8 order1:2;
                u8 order2:2;
                u8 order3:2;
            } bits;
            u8 val;
        };
and do the same for u_drive_mode.

Thanks for reviewing.

Best regards,
Nam Tran