linux-kernel - Re: [PATCH v5 6/7] leds: Add fwnode_led_get() for firmware-agnostic LED resolution

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: <aVO-8IK5yuES-m6d@smile.fi.intel.com>
Date: Tue, 30 Dec 2025 14:00:48 +0200
From: Andriy Shevencho <andriy.shevchenko@...ux.intel.com>
To: Jonathan Brophy <professorjonny98@...il.com>
Cc: lee Jones <lee@...nel.org>, Pavel Machek <pavel@...nel.org>,
	Jonathan Brophy <professor_jonny@...mail.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Radoslav Tsvetkov <rtsvetkov@...dotech.eu>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-leds@...r.kernel.org
Subject: Re: [PATCH v5 6/7] leds: Add fwnode_led_get() for firmware-agnostic
 LED resolution

On Tue, Dec 30, 2025 at 09:23:19PM +1300, Jonathan Brophy wrote:

> Add fwnode_led_get() to resolve LED class devices from firmware node
> references, providing a firmware-agnostic alternative to of_led_get().
> 
> The function supports:
> - Device Tree and ACPI systems

...and software nodes (board files) I think also fall into this category.

> - GPIO LEDs (which may lack struct device)
> - Platform LED controllers
> - Deferred probing via -EPROBE_DEFER
> - Reference counting via led_module_get()
> 
> Implementation details:
> - Uses fwnode_property_get_reference_args() for property traversal
> - Falls back to of_led_get() for Device Tree GPIO LEDs
> - Returns optional parent device reference for power management
> - Handles NULL parent devices gracefully (common for GPIO LEDs)
> 
> This enables LED resolution using generic firmware APIs while
> maintaining compatibility with existing OF-specific LED drivers.
> Future migration to full fwnode support in LED core will be
> straightforward.

...

> -	return sysfs_emit(buf, "%u\n", brightness);
> +	return sprintf(buf, "%u\n", brightness);

Huh?!

This seems like indeliberate revert. Otherwise it's so wrong.

Ditto. for all same issues.

...

> -static const BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);
> -static const struct bin_attribute *const led_trigger_bin_attrs[] = {
> +static BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);

Why?! Don't we have a mechanism to add more groups on-the-fly?

...

> +#define LED_BLINK_BRIGHTNESS_CHANGE 	4

Mixed TABs and spaces.

...

> +	unsigned 	gpio;

Ditto.
Besides we should get rid of this completely (it's deprecated APIs that is on
removal stage).

...

> +	int 		num_leds;

TABs/spaces mix.

...

I have a felling that this patch is doing too many things at once. Please, try
to split (my brief look suggests that 3+ patches should come out of this one).

-- 
With Best Regards,
Andy Shevchenko