From: Andriy Shevencho <andriy.shevchenko@linux.intel.com>
To: Jonathan Brophy <professorjonny98@gmail.com>
Cc: lee Jones <lee@kernel.org>, Pavel Machek <pavel@kernel.org>,
Jonathan Brophy <professor_jonny@hotmail.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Radoslav Tsvetkov <rtsvetkov@gradotech.eu>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-leds@vger.kernel.org
Subject: Re: [PATCH v5 6/7] leds: Add fwnode_led_get() for firmware-agnostic LED resolution
Date: Tue, 30 Dec 2025 14:00:48 +0200 [thread overview]
Message-ID: <aVO-8IK5yuES-m6d@smile.fi.intel.com> (raw)
In-Reply-To: <20251230082336.3308403-7-professorjonny98@gmail.com>
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
next prev parent reply other threads:[~2025-12-30 12:00 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-30 8:23 [PATCH v5 0/7] leds: Add virtual LED group driver with priority arbitration Jonathan Brophy
2025-12-30 8:23 ` [PATCH v5 1/7] dt-bindings: leds: add function virtual_status to led common properties Jonathan Brophy
2025-12-30 8:23 ` [PATCH v5 2/7] dt-bindings: leds: Add virtual LED class bindings Jonathan Brophy
2025-12-30 8:23 ` [PATCH v5 3/7] dt-bindings: leds: Add virtual LED group controller bindings Jonathan Brophy
2025-12-30 8:23 ` [PATCH v5 4/7] ABI: Add sysfs documentation for leds-group-virtualcolor Jonathan Brophy
2025-12-30 11:52 ` Andriy Shevencho
2025-12-30 8:23 ` [PATCH v5 5/7] leds: Add driver " Jonathan Brophy
2025-12-30 8:23 ` [PATCH v5 6/7] leds: Add fwnode_led_get() for firmware-agnostic LED resolution Jonathan Brophy
2025-12-30 12:00 ` Andriy Shevencho [this message]
2025-12-31 2:30 ` kernel test robot
2025-12-31 23:37 ` kernel test robot
2025-12-31 23:45 ` kernel test robot
2026-01-02 12:20 ` kernel test robot
2026-01-02 15:07 ` kernel test robot
2026-01-02 16:29 ` kernel test robot
2025-12-30 8:23 ` [PATCH v5 7/7] leds: Add virtual LED group driver with priority arbitration Jonathan Brophy
2025-12-30 12:19 ` Andriy Shevencho
2026-01-03 8:22 ` [PATCH v5 7/7] leds: Add virtual LED group driver Jonathan Brophy
2026-01-03 12:56 ` Andriy Shevencho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aVO-8IK5yuES-m6d@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lee@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@kernel.org \
--cc=professor_jonny@hotmail.com \
--cc=professorjonny98@gmail.com \
--cc=robh@kernel.org \
--cc=rtsvetkov@gradotech.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).