Re: [RFC PATCH 0/9] leds: Add support for hw initiated hw control trigger transition

["Mark Pearson" <mpearson-lenovo@squebb.ca>]

On Thu, Mar 5, 2026, at 7:37 AM, Rong Zhang wrote:
> Hi Mark,
>
> On Wed, 2026-03-04 at 15:05 -0500, Mark Pearson wrote:
>> Hi Rong,
>> 
>> On Fri, Feb 27, 2026, at 2:05 PM, Rong Zhang wrote:
>> > Hi all,
>> > 
>> > Some laptops can tune their keyboard backlight according to ambient
>> > light sensors (auto mode). This capability is essentially a hw control
>> > trigger. Meanwhile, such laptops also offer a shrotcut for cycling
>> > through brightness levels and auto mode. For example, on ThinkBook,
>> > pressing Fn+Space cycles keyboard backlight levels in the following
>> > sequence:
>> > 
>> >   1 => 2 => 0 => auto => 1 ...
>> > 
>> > Recent ThinkPad models should have similar sequence too.
>> > 
>> > However, there are some issues preventing us from using hw control
>> > trigger:
>> > 
>> > 1. We want a mechanism to tell userspace which trigger is the hw control
>> >    trigger, so that userspace can determine if auto mode is on/off or
>> >    turing it on/off programmatically without obtaining the hw control
>> >    trigger's name via other channels
>> > 2. Turing on/off auto mode via the shortcut cannot activate/deactivate
>> >    the hw control trigger, making the software state out-of-sync
>> > 3. Even with #1 resolved, deactivating the hw control trigger after
>> >    receiving the event indicating "auto => 1" has a side effect of
>> >    emitting LED_OFF, breaking the shortcut cycle
>> > 
>> > This RFC series tries to demonstrate a path on solving these issues:
>> > 
>> > - Introduce an attribute called trigger_may_offload, so that userspace
>> >    can determine:
>> >    - if the LED device supports hw control (supported => visible)
>> >    - which trigger is the hw control trigger
>> >    - if the hw control trigger is selected
>> >    - if the hw control trigger is in hw control (i.e., offloaded)
>> >      - A callback offloaded() is added so that LED triggers can report
>> >        their hw control state
>> > - Add led_trigger_notify_hw_control_changed() interface, so that LED
>> >   drivers can notify the LED core about hardware initiated hw control
>> >   state transitions. The LED core will then determine if the transition
>> >   is allowed and turning on/off the hw control trigger accordingly
>> > - Tune the logic of trigger deactivation so that it won't emit LED_OFF
>> >   when the deactivation is triggered by hardware
>> > 
>> > The last two patches are included into the RFC series to demonstrate how
>> > to utilize these interfaces to add support for auto keyboard backlight
>> > to ThinkBook. They will be submitted separately once the dust settles.
>> > 
>> > Currently no Kconfig entry is provided to disable either interface. If
>> > needed, I will add one later.
>> > 
>> > [ Summary of other approaches ]
>> > 
>> > < custom attribute >
>> > 
>> > Pros:
>> > - simplicity, KISS
>> > - no need to touch the LED core
>> > - extensible as long as it has a sensor-neutral name
>> >   - a sensor-related name could potentially lead to a mess if a future
>> >     device implements auto mode based on multiple different sensors
>> > 
>> > Cons:
>> > - must have zero influence on brightness_set[_blocking] callbacks
>> >   in order not to break triggers
>> >   - potential interference with triggers and the brightness attribute
>> > - weird semantic (an attribute other than "brightness" and "trigger"
>> >   changes the brightness)
>> > 
>> > < hw control trigger (this series) >
>> > 
>> > Pros:
>> > - mutually exclusive with other triggers (hence less chaos)
>> > - semantic correctness
>> > - acts as an aggregate switch to turn on/off auto mode even a future
>> >   device implements auto mode based on multiple different sensors
>> >   - extensibility (through trigger attributes)
>> > 
>> > Cons:
>> > - complexity
>> > 
>> > [ Previous discussion threads ]
>> > 
>> > https://lore.kernel.org/r/08580ec5-1d7b-4612-8a3f-75bc2f40aad2@app.fastmail.com
>> > 
>> > https://lore.kernel.org/r/1dbfcf656cdb4af0299f90d7426d2ec7e2b8ac9e.camel@rong.moe
>> > 
>> > Thanks,
>> > Rong
>> > 
>> > Rong Zhang (9):
>> >   leds: Load trigger modules on-demand if used as hw control trigger
>> >   leds: Add callback offloaded() to query the state of hw control
>> >     trigger
>> >   leds: cros_ec: Implement offloaded() callback for trigger
>> >   leds: turris-omnia: Implement offloaded() callback for trigger
>> >   leds: trigger: netdev: Implement offloaded() callback
>> >   leds: Add trigger_may_offload attribute
>> >   leds: trigger: Add led_trigger_notify_hw_control_changed() interface
>> >   platform/x86: ideapad-laptop: Decouple HW & cdev brightness for kbd
>> >     backlight
>> >   platform/x86: ideapad-laptop: Fully support auto kbd backlight
>> > 
>> >  .../obsolete/sysfs-class-led-trigger-netdev   |  15 ++
>> >  Documentation/ABI/testing/sysfs-class-led     |  22 ++
>> >  .../testing/sysfs-class-led-trigger-netdev    |  13 --
>> >  Documentation/leds/leds-class.rst             |  72 ++++++-
>> >  drivers/leds/led-class.c                      |  23 +++
>> >  drivers/leds/led-triggers.c                   | 176 +++++++++++++++-
>> >  drivers/leds/leds-cros_ec.c                   |   6 +
>> >  drivers/leds/leds-turris-omnia.c              |   7 +
>> >  drivers/leds/leds.h                           |   3 +
>> >  drivers/leds/trigger/ledtrig-netdev.c         |  10 +
>> >  drivers/platform/x86/lenovo/Kconfig           |   1 +
>> >  drivers/platform/x86/lenovo/ideapad-laptop.c  | 194 ++++++++++++++----
>> >  include/linux/leds.h                          |   6 +
>> >  13 files changed, 492 insertions(+), 56 deletions(-)
>> >  create mode 100644 Documentation/ABI/obsolete/sysfs-class-led-trigger-netdev
>> > 
>> > 
>> > base-commit: a75cb869a8ccc88b0bc7a44e1597d9c7995c56e5
>> > -- 
>> > 2.51.0
>> 
>> Thanks for your work on this.
>> 
>> For the series: As it's a RFC, I'm not bothering with notes on any typo's or grammer stuff.
>> 
>> Overall I think the implementation works and I understand it better from our initial discussions. Thank you for putting this together.
>> 
>> I'm not a huge fan of the term offloaded - I would lean towards just calling it hw_control (or similar). But I see it was used in the ledtrig-netdev driver so I don't feel strongly about this.
>> 
>
> FYI, "hw control" is a historical and internal term. I used it because
> it's used a lot in the documentation to the doc consistent. Otherwise
> "offload(ed)" should be used. See commit 44f0fb8dfe26 ("leds: trigger:
> netdev: rename 'hw_control' sysfs entry to 'offloaded'").
>
> I admit that I was somewhat lost in the terminological soup and there
> may be some room for rephrasing in my documentation and commit
> messages.
>
Fair enough :) I think that overrides my mild preferences.

Mark