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: 25+ 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
2026-01-06 16:59 ` [PATCH v5 0/7] leds: Add virtual LED group driver with priority arbitration Rob Herring
2026-01-13 11:52 ` Lee Jones
2026-01-13 11:57 ` Lee Jones
2026-01-13 20:35 ` Jonathan Brophy
2026-01-15 15:07 ` Lee Jones
2026-01-15 16:58 ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.