mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Rong Zhang <i@rong.moe>
To: Lee Jones <lee@kernel.org>
Cc: "Pavel Machek" <pavel@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Thomas Weißschuh" <linux@weissschuh.net>,
	"Benson Leung" <bleung@chromium.org>,
	"Guenter Roeck" <groeck@chromium.org>,
	"Marek Behún" <kabel@kernel.org>,
	"Mark Pearson" <mpearson-lenovo@squebb.ca>,
	"Derek J. Clark" <derekjohn.clark@gmail.com>,
	"Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Ike Panhc" <ikepanhc@gmail.com>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Vishnu Sankar" <vishnuocv@gmail.com>,
	"Vishnu Sankar" <vsankar@lenovo.com>,
	linux-leds@vger.kernel.org, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	chrome-platform@lists.linux.dev,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH RFC v3 00/11] leds: Add support for hardware-initiated hardware control trigger transition
Date: Thu, 23 Jul 2026 00:31:52 +0800	[thread overview]
Message-ID: <bb2a9c53669fde370f14cb9ef3e0a73fc729a5bb.camel@rong.moe> (raw)
In-Reply-To: <20260722123913.GB11805@google.com>

Hi Lee,

On Wed, 2026-07-22 at 13:39 +0100, Lee Jones wrote:
> Why is this still an RFC?

I plan to make it non-RFC in v4, after resolving concerns from Ilpo and
Sashiko.

Thanks,
Rong

> 
> > Some laptops can tune their keyboard backlight according to ambient
> > light sensors (auto mode). This capability is essentially a hardware
> > control trigger. Meanwhile, such laptops also offer a shrotcut for
> > cycling through brightness levels and auto mode. For example, on
> > ThinkBook, pressing Fn+Space ("shortcut") 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 a private
> > hardware control trigger:
> > 
> > 1. We want a mechanism to tell userspace which trigger is the hardware
> >    control one, so that userspace can determine if auto mode is on/off,
> >    as well as turing it on/off programmatically without obtaining the
> >    trigger's name via other channels
> > 2. Writing brightness has the side effect of disabling hardware control,
> >    but the hardware control trigger remains active, resulting in the
> >    software and hardware being out of sync. Most LED drivers that
> >    supports hardware control also suffer from the same issue
> > 3. Turing on/off auto mode via the shortcut cannot activate/deactivate
> >    the corresponding hardware control trigger, making the software state
> >    out of sync
> > 4. Even with #3 solved, deactivating the hardware control trigger has
> >    the side effect of emitting LED_OFF, breaking the shortcut cycle,
> >    especially "auto => 1"
> > 
> > This RFC series tries to demonstrate a path on solving these issues:
> > 
> > - Introduce an attribute "trigger_may_offload", so that userspace can
> >   determine:
> >   - if the LED device supports hardware control (supported => visible)
> >   - which trigger is the hardware control trigger selected by the LED
> >     device
> >   - if the trigger is selected ("<foo_trigger>")
> >   - if the trigger is offloaded ("[foo_trigger]")
> >     - A callback offloaded() is added so that LED triggers can report
> >       their hardware control state
> > - Remove hardware control trigger when writing brightness
> > - Add led_trigger_notify_hw_control_changed() interface, so that LED
> >   drivers can notify the LED core about hardware-initiated hardware
> >   control transitions. The LED core will then determine if the
> >   transition is allowed and switching between "none" (i.e., no trigger)
> >   and the device's private trigger accordingly
> >   - This capability is restricted to the device's private trigger. If
> >     the current trigger is neither the private trigger nor "none", no
> >     transition will be made
> >   - This interface is gated behind Kconfig LEDS_TRIGGERS_HW_CHANGED and
> >     LED device flag LED_TRIG_HW_CHANGED
> > - Tune the logic of trigger deactivation so that it won't emit LED_OFF
> >   when the deactivation is triggered by hardware
> > 
> > The last three patches are included in the RFC series to demonstrate how
> > to these interfaces are supposed to be utilized, so that ideapad-laptop
> > can expose the auto mode of ThinkBook's keyboard backlight. They can be
> > submitted separately once the dust settles, if preferred.
> > 
> > [ 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,
> >     can't solve #2
> > - weird semantic (an attribute other than "brightness" and "trigger"
> >   changes the brightness)
> > 
> > < private hardware control trigger (this series) >
> > 
> > Pros:
> > - mutually exclusive with other triggers and the brightness attribute
> >   (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
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> > Changes in v3:
> > - Integrate https://lore.kernel.org/all/20260712-leds-hw-control-brightness-set-v1-1-1de593b09d26@rong.moe/
> >   into the series
> >   - Adopt __led_trigger_is_hw_controlled() in the rest of the series
> > - Rearrange the series so that the code using the offloaded() callback is
> >   introduced before the driver implementation (thanks Thomas Weißschuh)
> > - Reword documentations and commit messages (ditto)
> > - Adopt guard() and lockdep (ditto)
> > - Address concerns from Sashiko
> >   - Fix a race condition in ideapad_kbd_bl_led_cdev_brightness_set()
> >   - Fix trigger re-registration of ideapad_kbd_bl_auto_trigger
> >   - https://sashiko.dev/#/patchset/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3%40rong.moe
> > - Make registration failures of ideapad_kbd_bl_auto_trigger non-fatal
> > - Link to v2: https://patch.msgid.link/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3@rong.moe
> > 
> > Changes in v2:
> > - Restrict the led_trigger_notify_hw_control_changed() interface to
> >   private triggers only
> >   - Drop PATCH v1 1/9 ("leds: Load trigger modules on-demand if used as
> >     hw control trigger"), not relavant any more
> > - Gate the led_trigger_notify_hw_control_changed() interface behind
> >   Kconfig LEDS_TRIGGERS_HW_CHANGED and LED device flag
> >   LED_TRIG_HW_CHANGED
> > - Fix lock ordering inversion
> > - ideapad-laptop:
> >   - Only call led_trigger_notify_hw_control_changed() when needed
> >   - Serialize keyboard backlight notifications
> > - Reword commit messages and documentations
> > - Link to v1: https://patch.msgid.link/20260227190617.271388-1-i@rong.moe
> > 
> > ---
> > Rong Zhang (11):
> >       leds: Move led_trigger_is_hw_controlled() to the right place
> >       leds: class: Remove hardware control trigger when writing brightness
> >       leds: trigger: Add offloaded() callback and provide trigger_may_offload attribute
> >       leds: cros_ec: trigger: Implement offloaded() callback
> >       leds: turris-omnia: trigger: Implement offloaded() and declare hw_control_trigger
> >       leds: trigger: netdev: Implement offloaded() callback
> >       leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()
> >       leds: trigger: Add led_trigger_notify_hw_control_changed() interface
> >       platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight
> >       platform/x86: ideapad-laptop: Serialize keyboard backlight notifications
> >       platform/x86: ideapad-laptop: Fully support auto keyboard backlight
> > 
> >  Documentation/ABI/testing/sysfs-class-led          |  25 ++
> >  .../ABI/testing/sysfs-class-led-trigger-netdev     |   3 +
> >  Documentation/leds/leds-class.rst                  |  72 ++++++
> >  drivers/leds/led-class.c                           |  35 ++-
> >  drivers/leds/led-triggers.c                        | 146 +++++++++++-
> >  drivers/leds/leds-cros_ec.c                        |   6 +
> >  drivers/leds/leds-turris-omnia.c                   |   7 +
> >  drivers/leds/leds.h                                |   2 +
> >  drivers/leds/trigger/Kconfig                       |   9 +
> >  drivers/leds/trigger/ledtrig-netdev.c              |   8 +
> >  drivers/platform/x86/lenovo/Kconfig                |   1 +
> >  drivers/platform/x86/lenovo/ideapad-laptop.c       | 264 ++++++++++++++++-----
> >  include/linux/leds.h                               |  19 ++
> >  13 files changed, 532 insertions(+), 65 deletions(-)
> > ---
> > base-commit: 1229e2e57a5c2980ccd457b9b53ea0eed5a22ab3
> > change-id: 20260506-leds-trigger-hw-changed-96a62188cbdf
> > 
> > Thanks,
> > Rong
> > 

      reply	other threads:[~2026-07-22 16:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-18 17:05 Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 01/11] leds: Move led_trigger_is_hw_controlled() to the right place Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 02/11] leds: class: Remove hardware control trigger when writing brightness Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 03/11] leds: trigger: Add offloaded() callback and provide trigger_may_offload attribute Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 04/11] leds: cros_ec: trigger: Implement offloaded() callback Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 05/11] leds: turris-omnia: trigger: Implement offloaded() and declare hw_control_trigger Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 06/11] leds: trigger: netdev: Implement offloaded() callback Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 07/11] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled() Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 08/11] leds: trigger: Add led_trigger_notify_hw_control_changed() interface Rong Zhang
2026-07-18 17:05 ` [PATCH RFC v3 09/11] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight Rong Zhang
2026-07-21 17:08   ` Ilpo Järvinen
2026-07-18 17:05 ` [PATCH RFC v3 10/11] platform/x86: ideapad-laptop: Serialize keyboard backlight notifications Rong Zhang
2026-07-21 17:09   ` Ilpo Järvinen
2026-07-18 17:05 ` [PATCH RFC v3 11/11] platform/x86: ideapad-laptop: Fully support auto keyboard backlight Rong Zhang
2026-07-21 17:14   ` Ilpo Järvinen
2026-07-21 18:25     ` Rong Zhang
2026-07-22  8:08       ` Ilpo Järvinen
2026-07-22 12:39 ` [PATCH RFC v3 00/11] leds: Add support for hardware-initiated hardware control trigger transition Lee Jones
2026-07-22 16:31   ` Rong Zhang [this message]

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=bb2a9c53669fde370f14cb9ef3e0a73fc729a5bb.camel@rong.moe \
    --to=i@rong.moe \
    --cc=andrew+netdev@lunn.ch \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=corbet@lwn.net \
    --cc=derekjohn.clark@gmail.com \
    --cc=groeck@chromium.org \
    --cc=hansg@kernel.org \
    --cc=ikepanhc@gmail.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=kabel@kernel.org \
    --cc=kuba@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=mpearson-lenovo@squebb.ca \
    --cc=netdev@vger.kernel.org \
    --cc=pavel@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=vishnuocv@gmail.com \
    --cc=vsankar@lenovo.com \
    /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

all inboxes | Powered by JetHome®