[PATCH v5 00/12] leds: Add support for hardware-initiated hardware control trigger transition

[PATCH v5 00/12] leds: Add support for hardware-initiated hardware control trigger transition

[Rong Zhang]

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 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 series to demonstrate how
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.

The last patch in the series depends on commit 5029bff09e79
("platform/x86: ideapad-laptop: Fix driver unregistration order") from
platform-drivers-x86/for-next. 

[ 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

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v5:
- Address a concern from Sashiko:
  - Non-atomic update of led_cdev->flags causes a data race that can
    drop the LED_UNREGISTERING flag
  - Fix it by synchronize the update with trigger_lock
  - https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=8
- Address a concern from Sashiko:
  - led_trigger_notify_hw_control_changed() might sleep, but without any
    internal deferral mechanism or annotation
  - Annotate the method with might_sleep(), since the very first users
    of the interface, i.e., ideapad-laptop and (supposedly)
    thinkpad_acpi, will call the interface from work contexts. It does
    not deserve the overhead of internal deferral mechanism
  - https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=9
- Update "Changes in v4" to include more details
- Link to v4: https://patch.msgid.link/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe

Changes in v4:
- Drop the RFC prefix
- Add a new patch in the series to address a concern from Sashiko
  - [PATCH v4 08/12] leds: trigger: Do not attach trigger to a removing LED
  - Without it, the following patch may expose a race condition if a
    future LED driver synchronizes led_classdev_unregister() and
    led_trigger_notify_hw_control_changed() poorly. The ideapad-laptop
    patches in the series doesn't expose the race condition though
  - In theory, the race condition can also be triggered by a userspace
    program writing to the "trigger" attribute right before
    device_unregister() is called
  - https://sashiko.dev/#/patchset/20260719-leds-trigger-hw-changed-v3-0-5fb55722e36e@rong.moe
- Address other concerns from Sashiko
  - Reject insane hardware brightness
  - Do not notify the LED trigger core when the registration of the
    private trigger has failed
- Enable LEDS_TRIGGERS_HW_CHANGED by default when
  LEDS_BRIGHTNESS_HW_CHANGED is enabled
- Add missing #include (Thanks Ilpo Järvinen)
- Remove needless #include (ditto)
- Remove needless code alignment (ditto)
- Link to v3: https://patch.msgid.link/20260719-leds-trigger-hw-changed-v3-0-5fb55722e36e@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 (12):
      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: Implement offloaded() trigger callback
      leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger
      leds: trigger: netdev: Implement offloaded() callback
      leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()
      leds: trigger: Do not attach trigger to a removing LED
      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                           |  40 ++-
 drivers/leds/led-triggers.c                        | 152 +++++++++++-
 drivers/leds/leds-cros_ec.c                        |   6 +
 drivers/leds/leds-turris-omnia.c                   |   7 +
 drivers/leds/leds.h                                |   2 +
 drivers/leds/trigger/Kconfig                       |  10 +
 drivers/leds/trigger/ledtrig-netdev.c              |   8 +
 drivers/platform/x86/lenovo/Kconfig                |   1 +
 drivers/platform/x86/lenovo/ideapad-laptop.c       | 270 +++++++++++++++++----
 include/linux/leds.h                               |  19 ++
 13 files changed, 549 insertions(+), 66 deletions(-)
---
base-commit: aca39607c1734ed976fdd65deb75b3555a5a0326
change-id: 20260506-leds-trigger-hw-changed-96a62188cbdf

Thanks,
Rong

[PATCH v5 01/12] leds: Move led_trigger_is_hw_controlled() to the right place

[Rong Zhang]

Currently led_trigger_is_hw_controlled() is placed at led-class.c, which
is not an right place as it falls into the triggers namespace and does
triggers stuff.

Move it into led-triggers.c, and split it into locked and unlocked
variant for convenience.

Fixes: b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of hardware controlled LED")
Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- New patch in the series, the dependency of the following patches
---
 drivers/leds/led-class.c    | 10 ----------
 drivers/leds/led-triggers.c | 19 +++++++++++++++++++
 include/linux/leds.h        |  8 ++++++++
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index a51b0ed53886..1b8b688aaaaf 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -27,16 +27,6 @@ static LIST_HEAD(leds_lookup_list);
 
 static struct workqueue_struct *leds_wq;
 
-static bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
-{
-#ifdef CONFIG_LEDS_TRIGGERS
-	guard(rwsem_read)(&led_cdev->trigger_lock);
-	return led_cdev->trigger && led_cdev->trigger->trigger_type;
-#else
-	return false;
-#endif
-}
-
 static ssize_t brightness_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index b1223218bda1..bf2543538ed0 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -7,9 +7,11 @@
  * Author: Richard Purdie <rpurdie@openedhand.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/lockdep.h>
 #include <linux/spinlock.h>
 #include <linux/device.h>
 #include <linux/timer.h>
@@ -33,6 +35,23 @@ trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig)
 	return !trig->trigger_type || trig->trigger_type == led_cdev->trigger_type;
 }
 
+static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
+{
+	lockdep_assert_held(&led_cdev->trigger_lock);
+
+	if (!led_cdev->trigger)
+		return false;
+
+	return led_cdev->trigger->trigger_type;
+}
+
+bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
+{
+	guard(rwsem_read)(&led_cdev->trigger_lock);
+	return __led_trigger_is_hw_controlled(led_cdev);
+}
+EXPORT_SYMBOL_GPL(led_trigger_is_hw_controlled);
+
 ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
 			  const struct bin_attribute *bin_attr, char *buf,
 			  loff_t pos, size_t count)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b16b803cc1ac..a630f5a79f6b 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -534,6 +534,8 @@ void led_trigger_set_default(struct led_classdev *led_cdev);
 int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger);
 void led_trigger_remove(struct led_classdev *led_cdev);
 
+bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev);
+
 static inline void led_set_trigger_data(struct led_classdev *led_cdev,
 					void *trigger_data)
 {
@@ -584,6 +586,12 @@ static inline int led_trigger_set(struct led_classdev *led_cdev,
 }
 
 static inline void led_trigger_remove(struct led_classdev *led_cdev) {}
+
+static inline bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
+{
+	return false;
+}
+
 static inline void led_set_trigger_data(struct led_classdev *led_cdev) {}
 static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 {

-- 
2.55.0

[PATCH v5 02/12] leds: class: Remove hardware control trigger when writing brightness

[Rong Zhang]

Since commit b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of
hardware controlled LED"), the brightness attribute becomes write-only
when the LED is controlled fully by the hardware. A write-only attribute
is very confusing.

Moreover, most LED drivers set hardware brightness innocently with the
side effect of disabling hardware control, but the hardware control
trigger remains active, resulting in the software and hardware being out
of sync.

Fix it by removing the hardware control trigger when writing the
brightness attribute.

This should also match the semantics of hardware control:

    When the LED is in hw control, no software blink is possible and
    doing so will effectively disable hw control.

Fixes: b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of hardware controlled LED")
Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- New patch in the series, integrated from https://lore.kernel.org/all/20260712-leds-hw-control-brightness-set-v1-1-1de593b09d26@rong.moe/
  - The following patches will improve __led_trigger_is_hw_controlled()
    to include offloaded generic triggers and take the advantage of it
---
 drivers/leds/led-class.c    | 3 +++
 drivers/leds/led-triggers.c | 9 +++++++++
 include/linux/leds.h        | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 1b8b688aaaaf..ab61e41a00a3 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -64,6 +64,9 @@ static ssize_t brightness_store(struct device *dev,
 
 	if (state == LED_OFF)
 		led_trigger_remove(led_cdev);
+	else
+		led_trigger_remove_hw_control(led_cdev);
+
 	led_set_brightness(led_cdev, state);
 
 	ret = size;
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index bf2543538ed0..804a04b326c4 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -287,6 +287,15 @@ void led_trigger_remove(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_trigger_remove);
 
+void led_trigger_remove_hw_control(struct led_classdev *led_cdev)
+{
+	guard(rwsem_write)(&led_cdev->trigger_lock);
+
+	if (__led_trigger_is_hw_controlled(led_cdev))
+		led_trigger_set(led_cdev, NULL);
+}
+EXPORT_SYMBOL_GPL(led_trigger_remove_hw_control);
+
 static bool led_match_default_trigger(struct led_classdev *led_cdev,
 				      struct led_trigger *trig)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index a630f5a79f6b..d7d3dd905432 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -533,6 +533,7 @@ void led_trigger_blink_oneshot(struct led_trigger *trigger,
 void led_trigger_set_default(struct led_classdev *led_cdev);
 int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger);
 void led_trigger_remove(struct led_classdev *led_cdev);
+void led_trigger_remove_hw_control(struct led_classdev *led_cdev);
 
 bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev);
 
@@ -586,6 +587,7 @@ static inline int led_trigger_set(struct led_classdev *led_cdev,
 }
 
 static inline void led_trigger_remove(struct led_classdev *led_cdev) {}
+static inline void led_trigger_remove_hw_control(struct led_classdev *led_cdev) {}
 
 static inline bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
 {

-- 
2.55.0

[PATCH v5 03/12] leds: trigger: Add offloaded() callback and provide trigger_may_offload attribute

[Rong Zhang]

There are multiple triggers implementing hardware control. However, the
LED trigger core doesn't really know the hardware control (offloaded)
state since the coordination is done directly between the trigger and
the LED driver. It can only assume private triggers as offloaded and
generic ones as not offloaded.

Add an offloaded() callback so that triggers can report their offloaded
states to the LED trigger core. When unimplemented, it defaults to true
for private triggers and false for generic ones to keep the current
behavior unchanged.

With that, provide a new 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]")

Note: the documentation describes the attribute as "returning a list"
despite the LED core currently only supports one hardware control
trigger per LED device. This is intentional to make the attribute
extensible in the future without breaking userspace.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- Rearrange the series so that the code using the offloaded() callback is
  introduced before the driver implementation (thanks Thomas Weißschuh)
- Reword documentation (ditto)
- Adopt guard() and lockdep (ditto)
- Adopt __led_trigger_is_hw_controlled() from newly-integrated PATCH 1
---
 Documentation/ABI/testing/sysfs-class-led | 22 ++++++++++++++++++++++
 Documentation/leds/leds-class.rst         | 20 ++++++++++++++++++++
 drivers/leds/led-class.c                  | 22 ++++++++++++++++++++++
 drivers/leds/led-triggers.c               | 30 ++++++++++++++++++++++++++++++
 drivers/leds/leds.h                       |  2 ++
 include/linux/leds.h                      |  1 +
 6 files changed, 97 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index d4c918cc11a1..5afb307c935a 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -78,6 +78,28 @@ Description:
 		(which would often be configured in the device tree for the
 		hardware).
 
+What:		/sys/class/leds/<led>/trigger_may_offload
+Date:		August 2026
+KernelVersion:	7.3
+Contact:	linux-leds@vger.kernel.org
+Description:
+		Names and states of triggers that may be offloaded to hardware.
+		Such triggers are also called "hardware control trigger" in some
+		context.
+
+		Only exists when the LED supports trigger offload.
+
+		Reading this file returns a list of triggers that are capable to
+		be offloaded. The optional brackets around the trigger name
+		indicate the state of the current trigger:
+
+		- `foo_trigger`: the trigger is not selected.
+		- `<foo_trigger>`: the trigger is selected, but falls back to
+		  software blink for some reason (e.g., incompatible trigger
+		  parameters)
+		- `[foo_trigger]`: the trigger is selected and offloaded to
+		  hardware.
+
 What:		/sys/class/leds/<led>/inverted
 Date:		January 2011
 KernelVersion:	2.6.38
diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index 3913966cfdac..2d41a6db602c 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -242,6 +242,9 @@ ops and needs to declare specific support for the supported triggers.
 
 With hw control we refer to the LED driven by hardware.
 
+A sysfs attribute `trigger_may_offload` is provided for userspace to
+query supported triggers and their states.
+
 LED driver must define the following value to support hw control:
 
     - hw_control_trigger:
@@ -298,6 +301,15 @@ LED driver must implement the following API to support hw control:
                 Returns a pointer to a struct device or NULL if nothing
                 is currently attached.
 
+LED trigger should implement the following API to indicate hw control:
+    - offloaded:
+                return a boolean indicating if the trigger is currently
+                offloaded to hardware.
+
+                If a trigger doesn't implement this callback, the default
+                value will be true for private triggers and false for generic
+                ones.
+
 LED driver can activate additional modes by default to workaround the
 impossibility of supporting each different mode on the supported trigger.
 Examples are hardcoding the blink speed to a set interval, enable special
@@ -311,6 +323,14 @@ the end use hw_control_set to activate hw control.
 A trigger can use hw_control_get to check if a LED is already in hw control
 and init their flags.
 
+Alternatively, a private trigger can be implemented along with the LED driver if
+the LED's hardware control doesn't fit any generic trigger. To associate the
+private trigger with the LED classdev, their `trigger_type` must be the same. To
+declare that the private trigger provides hardware control for the associated
+LED classdev, set the `hw_control_trigger` string to the trigger's name. Since
+both the LED classdev and the private trigger are in the same LED driver, it's
+not necessary for them to coordinate via `hw_control_*` callbacks.
+
 When the LED is in hw control, no software blink is possible and doing so
 will effectively disable hw control.
 
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index ab61e41a00a3..2460fcf0c469 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -96,8 +96,30 @@ static const struct bin_attribute *const led_trigger_bin_attrs[] = {
 	&bin_attr_trigger,
 	NULL,
 };
+
+static DEVICE_ATTR_RO(trigger_may_offload);
+static struct attribute *led_trigger_attrs[] = {
+	&dev_attr_trigger_may_offload.attr,
+	NULL
+};
+
+static umode_t led_trigger_is_visible(struct kobject *kobj,
+				      struct attribute *attr,
+				      int idx)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+
+	if (attr == &dev_attr_trigger_may_offload.attr)
+		return led_cdev->hw_control_trigger ? attr->mode : 0;
+
+	return attr->mode;
+}
+
 static const struct attribute_group led_trigger_group = {
 	.bin_attrs = led_trigger_bin_attrs,
+	.attrs = led_trigger_attrs,
+	.is_visible = led_trigger_is_visible,
 };
 #endif
 
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 804a04b326c4..de17a8bbb4d4 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -42,6 +42,10 @@ static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
 	if (!led_cdev->trigger)
 		return false;
 
+	if (led_cdev->trigger->offloaded)
+		return led_cdev->trigger->offloaded(led_cdev);
+
+	/* Otherwise assume private triggers as always offloaded. */
 	return led_cdev->trigger->trigger_type;
 }
 
@@ -341,6 +345,32 @@ void led_trigger_set_default(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_trigger_set_default);
 
+ssize_t trigger_may_offload_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_trigger *trig;
+	bool hit, offloaded;
+	int len;
+
+	guard(mutex)(&led_cdev->led_access);
+	guard(rwsem_read)(&led_cdev->trigger_lock);
+
+	trig = led_cdev->trigger;
+
+	offloaded = __led_trigger_is_hw_controlled(led_cdev);
+	hit = offloaded || (trig && !strcmp(led_cdev->hw_control_trigger, trig->name));
+
+	/* [offloaded] <active_but_not_offloaded> inactive */
+	len = sysfs_emit(buf, "%s%s%s\n",
+			 offloaded ? "[" : (hit ? "<" : ""),
+			 led_cdev->hw_control_trigger,
+			 offloaded ? "]" : (hit ? ">" : ""));
+
+	return len;
+}
+EXPORT_SYMBOL_GPL(trigger_may_offload_show);
+
 /* LED Trigger Interface */
 
 int led_trigger_register(struct led_trigger *trig)
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index bee46651e068..b08a289397e4 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -27,6 +27,8 @@ ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
 			const struct bin_attribute *bin_attr, char *buf,
 			loff_t pos, size_t count);
+ssize_t trigger_may_offload_show(struct device *dev,
+				 struct device_attribute *attr, char *buf);
 
 extern struct rw_semaphore leds_list_lock;
 extern struct list_head leds_list;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d7d3dd905432..cc664da33e94 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -485,6 +485,7 @@ struct led_trigger {
 	const char	 *name;
 	int		(*activate)(struct led_classdev *led_cdev);
 	void		(*deactivate)(struct led_classdev *led_cdev);
+	bool		(*offloaded)(struct led_classdev *led_cdev);
 
 	/* Brightness set by led_trigger_event */
 	enum led_brightness brightness;

-- 
2.55.0

[PATCH v5 04/12] leds: cros_ec: Implement offloaded() trigger callback

[Rong Zhang]

"chromeos-auto" is a private hardware control trigger which always stays
in hardware control. Implement offloaded() callback with its return
value to be always true to reflect this.

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
I am aware of a pre-existing bug on probe, i.e., non-idempotent trigger
registration. Since the driver is very platform-specific and the
relevant platforms never have more than one such device, the bug is
never triggered on real devices. Therefore, fixing it is far beyond the
scope of the series.
---
 drivers/leds/leds-cros_ec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c
index 1844d0cd5f52..6db83d015277 100644
--- a/drivers/leds/leds-cros_ec.c
+++ b/drivers/leds/leds-cros_ec.c
@@ -85,12 +85,18 @@ static int cros_ec_led_trigger_activate(struct led_classdev *led_cdev)
 	return cros_ec_led_send_cmd(priv->cros_ec, &arg);
 }
 
+static bool cros_ec_led_trigger_offloaded(struct led_classdev *led_cdev)
+{
+	return true;
+}
+
 static struct led_hw_trigger_type cros_ec_led_trigger_type;
 
 static struct led_trigger cros_ec_led_trigger = {
 	.name = "chromeos-auto",
 	.trigger_type = &cros_ec_led_trigger_type,
 	.activate = cros_ec_led_trigger_activate,
+	.offloaded = cros_ec_led_trigger_offloaded,
 };
 
 static int cros_ec_led_brightness_set_blocking(struct led_classdev *led_cdev,

-- 
2.55.0

[PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

[Rong Zhang]

"omnia-mcu" is a private hardware control trigger which always stays in
hardware control mode. Implement offloaded() callback with its return
value to be always true to reflect this.

Meanwhile, declare it as a hardware control trigger as it's forgotten
before.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
I am aware of a pre-existing bug on probe, i.e., non-idempotent trigger
registration. Since the driver is very platform-specific and the
relevant platforms never have more than one such device, the bug is
never triggered on real devices. Therefore, fixing it is far beyond the
scope of the series.
---
 drivers/leds/leds-turris-omnia.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index ed6a47bbb44f..32d40d176d3f 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -195,10 +195,16 @@ static void omnia_hwtrig_deactivate(struct led_classdev *cdev)
 			err);
 }
 
+static bool omnia_hwtrig_offloaded(struct led_classdev *cdev)
+{
+	return true;
+}
+
 static struct led_trigger omnia_hw_trigger = {
 	.name		= "omnia-mcu",
 	.activate	= omnia_hwtrig_activate,
 	.deactivate	= omnia_hwtrig_deactivate,
+	.offloaded	= omnia_hwtrig_offloaded,
 	.trigger_type	= &omnia_hw_trigger_type,
 };
 
@@ -251,6 +257,7 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
 	 * by LED class from the linux,default-trigger property.
 	 */
 	cdev->default_trigger = omnia_hw_trigger.name;
+	cdev->hw_control_trigger = omnia_hw_trigger.name;
 
 	/* Put the LED into software mode */
 	ret = omnia_cmd_write_u8(client, OMNIA_CMD_LED_MODE, OMNIA_CMD_LED_MODE_LED(led->reg) |

-- 
2.55.0

[PATCH v5 06/12] leds: trigger: netdev: Implement offloaded() callback

[Rong Zhang]

"netdev" can run in hardware control according to hardware capabilities
and trigger options.

Implement offloaded() callback to provide its hardware control state to
the LED core, and document the relation between the custom "offloaded"
attribute and the generic "trigger_may_offload" attribute.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- Do not deprecate netdev's "offloaded" attribute (thanks Thomas
  Weißschuh)
- Document the relation between the custom "offloaded" attribute and the
  generic "trigger_may_offload" attribute (ditto)
---
 Documentation/ABI/testing/sysfs-class-led                | 3 +++
 Documentation/ABI/testing/sysfs-class-led-trigger-netdev | 3 +++
 drivers/leds/trigger/ledtrig-netdev.c                    | 8 ++++++++
 3 files changed, 14 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index 5afb307c935a..2a13c77aee22 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -100,6 +100,9 @@ Description:
 		- `[foo_trigger]`: the trigger is selected and offloaded to
 		  hardware.
 
+		The "netdev" trigger also provides a custom attribute to
+		indicate its state, see `/sys/class/leds/<led>/offloaded`.
+
 What:		/sys/class/leds/<led>/inverted
 Date:		January 2011
 KernelVersion:	2.6.38
diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
index ed46b37ab8a2..a5146ea1e3e6 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
@@ -75,6 +75,9 @@ Description:
 		If 1, the LED blinking in requested mode is offloaded to
 		hardware.
 
+		LED trigger core also provides a generic attribute for this
+		purpose, see `/sys/class/leds/<led>/trigger_may_offload`.
+
 What:		/sys/class/leds/<led>/link_10
 Date:		Jun 2023
 KernelVersion:	6.5
diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 64c078e997f2..a26109ca4b1c 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -754,10 +754,18 @@ static void netdev_trig_deactivate(struct led_classdev *led_cdev)
 	kfree(trigger_data);
 }
 
+static bool netdev_trig_offloaded(struct led_classdev *led_cdev)
+{
+	struct led_netdev_data *trigger_data = led_get_trigger_data(led_cdev);
+
+	return trigger_data->hw_control;
+}
+
 static struct led_trigger netdev_led_trigger = {
 	.name = "netdev",
 	.activate = netdev_trig_activate,
 	.deactivate = netdev_trig_deactivate,
+	.offloaded = netdev_trig_offloaded,
 	.groups = netdev_trig_groups,
 };
 

-- 
2.55.0

[PATCH v5 07/12] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()

[Rong Zhang]

With all existing triggers adopting the new interface, strict checks
could be enforced to make the semantics of hardware control triggers
clearer.

In detail, a hardware control trigger should:

- Implement offloaded() callback to indicate hardware control
- Associate with the LED classdev's hw_control_trigger string

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- New patch in the series, splitted from PATCH 3 (thanks Thomas
  Weißschuh)
---
 drivers/leds/led-triggers.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index de17a8bbb4d4..cb49a02a8b3c 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -42,9 +42,16 @@ static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
 	if (!led_cdev->trigger)
 		return false;
 
+	if (!led_cdev->hw_control_trigger ||
+	    strcmp(led_cdev->hw_control_trigger, led_cdev->trigger->name))
+		return false;
+
 	if (led_cdev->trigger->offloaded)
 		return led_cdev->trigger->offloaded(led_cdev);
 
+	dev_warn_once(led_cdev->dev, "hw control trigger %s doesn't implement offloaded()\n",
+		      led_cdev->trigger->name);
+
 	/* Otherwise assume private triggers as always offloaded. */
 	return led_cdev->trigger->trigger_type;
 }

-- 
2.55.0

[PATCH v5 08/12] leds: trigger: Do not attach trigger to a removing LED

[Rong Zhang]

Currently the LED trigger core knows little about an LED's removal as
the LED_UNREGISTERING flag is set too late. This could lead to a race
condition where a trigger may be attached to a removing LED right after
led_classdev_unregister() has removed the previous trigger.

Set the flag at the very beginning of led_classdev_unregister(), and
check the flag before attaching a trigger.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v5:
- Address a concern from Sashiko:
  - Non-atomic update of led_cdev->flags causes a data race that can
    drop the LED_UNREGISTERING flag
  - Fix it by synchronize the update with trigger_lock
  - https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=8
- Update "Changes in v4" to include more details

Changes in v4:
- New patch in the series to address a concern from Sashiko
  - Without it, the following patch may expose a race condition if a
    future LED driver synchronizes led_classdev_unregister() and
    led_trigger_notify_hw_control_changed() poorly. The ideapad-laptop
    patches in the series doesn't expose the race condition though
  - In theory, the race condition can also be triggered by a userspace
    program writing to the "trigger" attribute right before
    device_unregister() is called
  - https://sashiko.dev/#/patchset/20260719-leds-trigger-hw-changed-v3-0-5fb55722e36e@rong.moe?part=8
---
 drivers/leds/led-class.c    | 5 +++--
 drivers/leds/led-triggers.c | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 2460fcf0c469..771a6e6c659b 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -633,12 +633,13 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 
 #ifdef CONFIG_LEDS_TRIGGERS
 	down_write(&led_cdev->trigger_lock);
+	led_cdev->flags |= LED_UNREGISTERING;
 	if (led_cdev->trigger)
 		led_trigger_set(led_cdev, NULL);
 	up_write(&led_cdev->trigger_lock);
-#endif
-
+#else
 	led_cdev->flags |= LED_UNREGISTERING;
+#endif
 
 	/* Stop blinking */
 	led_stop_software_blink(led_cdev);
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index cb49a02a8b3c..64bc95d0bfeb 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -200,6 +200,9 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 	const char *name;
 	int ret;
 
+	if ((led_cdev->flags & LED_UNREGISTERING) && trig)
+		return -ENODEV;
+
 	if (!led_cdev->trigger && !trig)
 		return 0;
 

-- 
2.55.0

[PATCH v5 09/12] leds: trigger: Add led_trigger_notify_hw_control_changed() interface

[Rong Zhang]

Some hardware can autonomously activate/deactivate hardware control.
After that, the LED hardware notifies the LED driver. Currently, there
is no mechanism for LED drivers to notify the LED core about such events
and initiate a trigger transition to reflect the hardware state.

Add a new interface called led_trigger_notify_hw_control_changed(), so
that LED drivers can call it to notify the LED core about the
transition.

The interface only allows two transitions:

1. "none" => private trigger
2. private trigger => "none"

If the current trigger is neither the private trigger nor "none", no
transition will be made. This protects the currently selected software
trigger.

Note that LED_OFF won't be emitted during the #2 transition, as some
hardware may have selected a new brightness level during its hardware
state transition (e.g., laptop keyboards with a shortcut cycling through
different backlight brightnesses and auto mode).

The interface is designed as a void function as any failure should be
non-fatal and the result of transition should not have any impact on the
LED drivers' event handling procedures.

To use the interface, the config LEDS_TRIGGERS_HW_CHANGED must be
enabled, and the LED driver must set the LED_TRIG_HW_CHANGED flag for
the classdev.

By default, the config is enabled when LEDS_BRIGHTNESS_HW_CHANGED is
enabled.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v5:
- Address a concern from Sashiko:
  - led_trigger_notify_hw_control_changed() might sleep, but without any
    internal deferral mechanism or annotation
  - Annotate the method with might_sleep(), since the very first users
    of the interface, i.e., ideapad-laptop and (supposedly)
    thinkpad_acpi, will call the interface from work contexts. It does
    not deserve the overhead of internal deferral mechanism
  - https://sashiko.dev/#/patchset/20260802-leds-trigger-hw-changed-v4-0-f97e2ca976fe@rong.moe?part=9

Changes in v4:
- Enable LEDS_TRIGGERS_HW_CHANGED by default when
  LEDS_BRIGHTNESS_HW_CHANGED is enabled

Changes in v3:
- Adopt guard() (Thanks Thomas Weißschuh)
- Reword documentations
---
 Documentation/leds/leds-class.rst | 52 ++++++++++++++++++++++++
 drivers/leds/led-triggers.c       | 84 ++++++++++++++++++++++++++++++++++++++-
 drivers/leds/trigger/Kconfig      | 10 +++++
 include/linux/leds.h              |  8 ++++
 4 files changed, 152 insertions(+), 2 deletions(-)

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index 2d41a6db602c..adbc57b9f49c 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -334,6 +334,58 @@ not necessary for them to coordinate via `hw_control_*` callbacks.
 When the LED is in hw control, no software blink is possible and doing so
 will effectively disable hw control.
 
+Hardware-initiated trigger transition
+=====================================
+
+Some hardware can autonomously activate/deactivate hardware control. After that,
+the LED hardware notifies the LED driver.
+
+If the driver can detect such transitions and thus wants to notify the LED core
+to update the current trigger then the `LED_TRIG_HW_CHANGED` flag must be set in
+flags before registering. To update the current trigger accordingly, call
+`led_trigger_notify_hw_control_changed` on the LED classdev.
+
+This capability is restricted to the LED device's private trigger. The private
+trigger must have been properly registered (see above) and named after
+`hw_control_trigger`.
+
+Only two transitions are defined:
+
+- "none" => private trigger:
+        This happens when the hardware autonomously activates hardware control
+        and when "none" (i.e., no trigger) is currently active. If the private
+        trigger is already active when the method is called, this is essentially
+        a no-op.
+
+        The activation sequence for the private trigger will be executed as
+        normal.
+
+        The LED driver and its private trigger must be able to handle the
+        activation sequence even if the hardware is currently in hardware
+        control.
+
+        If error occurs in the activation sequence, the LED Trigger core reverts
+        the effective trigger to "none".
+
+- private trigger => "none"
+        This happens when the hardware autonomously deactivates hardware control
+        and when the private trigger is currently active. If "none" (i.e., no
+        trigger) is active when the method is called, this is essentially a
+        no-op.
+
+        The deactivation sequence for the private trigger will be executed as
+        normal, except that the current LED brightness is retained. The reason
+        for keeping the brightness unchanged is that some hardware may choose a
+        specific brightness instead of simply turning off the LED after
+        autonomously deactivating hardware control.
+
+        The LED driver and its private trigger must be able to handle the
+        deactivation sequence even if the hardware is not currently in hardware
+        control.
+
+If the current trigger is neither the private trigger nor "none", no transition
+will be made.
+
 Known Issues
 ============
 
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 64bc95d0bfeb..32c9d19374c4 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -7,6 +7,7 @@
  * Author: Richard Purdie <rpurdie@openedhand.com>
  */
 
+#include <linux/bug.h>
 #include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
@@ -193,7 +194,8 @@ ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 EXPORT_SYMBOL_GPL(led_trigger_read);
 
 /* Caller must ensure led_cdev->trigger_lock held */
-int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
+static int __led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig,
+			     bool hw_triggered)
 {
 	char *event = NULL;
 	char *envp[2];
@@ -227,7 +229,21 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 		led_cdev->trigger_data = NULL;
 		led_cdev->activated = false;
 		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
-		led_set_brightness(led_cdev, LED_OFF);
+
+		/*
+		 * Hardware may have selected a new brightness level during its
+		 * hardware control transition, so only reset brightness if we
+		 * are switching to another trigger or if the switching is not
+		 * hardware triggered.
+		 *
+		 * Note that this does not apply to the error path, as running
+		 * into the error path implies a none => private trigger
+		 * transition. This hints that the LED driver and its private
+		 * trigger must have some fundamental bugs, so don't bother
+		 * leaving the LED in an undefined state.
+		 */
+		if (trig || !hw_triggered)
+			led_set_brightness(led_cdev, LED_OFF);
 	}
 	if (trig) {
 		spin_lock(&trig->leddev_list_lock);
@@ -291,6 +307,11 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 
 	return ret;
 }
+
+int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
+{
+	return __led_trigger_set(led_cdev, trig, false);
+}
 EXPORT_SYMBOL_GPL(led_trigger_set);
 
 void led_trigger_remove(struct led_classdev *led_cdev)
@@ -471,6 +492,65 @@ int devm_led_trigger_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_led_trigger_register);
 
+#ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED
+static void led_trigger_do_hw_control_transition(struct led_classdev *led_cdev, bool activate,
+						 struct led_trigger *hc_trig)
+{
+	int err = 0;
+
+	if (!led_cdev->trigger) {
+		/* "none" => private trigger. */
+		if (activate)
+			err = __led_trigger_set(led_cdev, hc_trig, true);
+	} else if (led_cdev->trigger == hc_trig) {
+		/* private trigger => "none". */
+		if (!activate)
+			err = __led_trigger_set(led_cdev, NULL, true);
+	} else {
+		/* Other trigger is active. */
+		dev_dbg(led_cdev->dev,
+			"Ignoring hw control transition (%s %s) while %s is active",
+			activate ? "activate" : "deactivate", hc_trig->name,
+			led_cdev->trigger->name);
+
+		return;
+	}
+
+	if (err)
+		dev_warn(led_cdev->dev, "Failed to %s %s in hw control transition: %d",
+			 activate ? "activate" : "deactivate", hc_trig->name, err);
+}
+
+void led_trigger_notify_hw_control_changed(struct led_classdev *led_cdev, bool activate)
+{
+	struct led_trigger *trig;
+
+	might_sleep();
+
+	/* Restricted to private triggers. */
+	if (WARN_ON(!(led_cdev->flags & LED_TRIG_HW_CHANGED) ||
+		    !led_cdev->hw_control_trigger || !led_cdev->trigger_type))
+		return;
+
+	scoped_guard(rwsem_read, &triggers_list_lock) {
+		list_for_each_entry(trig, &trigger_list, next_trig) {
+			if (trig->trigger_type == led_cdev->trigger_type &&
+			    !strcmp(trig->name, led_cdev->hw_control_trigger)) {
+				guard(rwsem_write)(&led_cdev->trigger_lock);
+
+				led_trigger_do_hw_control_transition(led_cdev, activate, trig);
+				return;
+			}
+		}
+	}
+
+	dev_err(led_cdev->dev,
+		"%s() is called, but the private trigger (%s) is not properly registered\n",
+		__func__, led_cdev->hw_control_trigger);
+}
+EXPORT_SYMBOL_GPL(led_trigger_notify_hw_control_changed);
+#endif /* CONFIG_LEDS_TRIGGERS_HW_CHANGED */
+
 /* Simple LED Trigger Interface */
 
 void led_trigger_event(struct led_trigger *trig,
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index c11282a74b5a..a11d04ce4ab2 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -9,6 +9,16 @@ menuconfig LEDS_TRIGGERS
 
 if LEDS_TRIGGERS
 
+config LEDS_TRIGGERS_HW_CHANGED
+	bool "LED hardware-initiated trigger transition support"
+	default LEDS_BRIGHTNESS_HW_CHANGED
+	help
+	  This option enables support for hardware initiated hardware control
+	  transitions, where the LED hardware autonomously switches between
+	  "none" (i.e., no trigger) and its private trigger.
+
+	  See Documentation/leds/leds-class.rst for details.
+
 config LEDS_TRIGGER_TIMER
 	tristate "LED Timer Trigger"
 	help
diff --git a/include/linux/leds.h b/include/linux/leds.h
index cc664da33e94..167598962b73 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -109,6 +109,7 @@ struct led_classdev {
 #define LED_INIT_DEFAULT_TRIGGER BIT(23)
 #define LED_REJECT_NAME_CONFLICT BIT(24)
 #define LED_MULTI_COLOR		BIT(25)
+#define LED_TRIG_HW_CHANGED	BIT(26)
 
 	/* set_brightness_work / blink_timer flags, atomic, private. */
 	unsigned long		work_flags;
@@ -609,6 +610,13 @@ led_trigger_get_brightness(const struct led_trigger *trigger)
 
 #endif /* CONFIG_LEDS_TRIGGERS */
 
+#ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED
+void led_trigger_notify_hw_control_changed(struct led_classdev *led_cdev, bool activate);
+#else
+static inline void led_trigger_notify_hw_control_changed(struct led_classdev *led_cdev,
+							 bool activate) {}
+#endif
+
 /* Trigger specific enum */
 enum led_trigger_netdev_modes {
 	TRIGGER_NETDEV_LINK = 0,

-- 
2.55.0

[PATCH v5 10/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight

[Rong Zhang]

Some recent models come with an ambient light sensor (ALS). On these
models, their EC will automatically set the keyboard backlight to an
appropriate brightness when the effective "hardware brightness" is 3.
"Hardware brightness" can't be perfectly mapped to an LED classdev
brightness, but the EC does use this predefined brightness value to
represent auto mode.

Currently, the code processing keyboard backlight is coupled with LED
classdev, making it hard to expose the auto brightness (ALS) mode to the
userspace.

As the first step toward the goal, decouple hardware brightness from LED
classdev brightness, and update comments about corresponding backlight
modes.

Since upcoming changes will heavily rely on kbd_bl.last_hw_brightness,
also convert it into an atomic_t to prevent potential race conditions.

To minimalize the diff set in upcoming changes, a trivial refactor
also converts the initialization path into another equivalent form.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v4:
- Add missing #include (Thanks Ilpo Järvinen)
- Address concerns from Sashiko
  - Reject insane hardware brightness
  - https://sashiko.dev/#/patchset/20260719-leds-trigger-hw-changed-v3-0-5fb55722e36e@rong.moe?part=9
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 151 +++++++++++++++++++--------
 1 file changed, 106 insertions(+), 45 deletions(-)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 8213524504ee..2fa2665f35d6 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -9,14 +9,17 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/acpi.h>
+#include <linux/atomic.h>
 #include <linux/backlight.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/bug.h>
 #include <linux/cleanup.h>
+#include <linux/compiler.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dev_printk.h>
 #include <linux/dmi.h>
 #include <linux/i8042.h>
 #include <linux/init.h>
@@ -134,10 +137,31 @@ enum {
 };
 
 /*
- * These correspond to the number of supported states - 1
- * Future keyboard types may need a new system, if there's a collision
- * KBD_BL_TRISTATE_AUTO has no way to report or set the auto state
- * so it effectively has 3 states, but needs to handle 4
+ * The enumeration has two purposes:
+ *   - as an internal identifier for all known types of keyboard backlight
+ *   - as a mandatory parameter of the KBLC command
+ *
+ * For each type, the hardware brightness values are defined as follows:
+ * +--------------------------+----------+-----+------+------+
+ * |      Hardware brightness |        0 |   1 |    2 |    3 |
+ * | Type                     |          |     |      |      |
+ * +--------------------------+----------+-----+------+------+
+ * | KBD_BL_STANDARD          |      off |  on |  N/A |  N/A |
+ * +--------------------------+----------+-----+------+------+
+ * | KBD_BL_TRISTATE          |      off | low | high |  N/A |
+ * +--------------------------+----------+-----+------+------+
+ * | KBD_BL_TRISTATE_AUTO     |      off | low | high | auto |
+ * +--------------------------+----------+-----+------+------+
+ *
+ * We map LED classdev brightness for KBD_BL_TRISTATE_AUTO as follows:
+ * +--------------------------+----------+-----+------+
+ * |  LED classdev brightness |        0 |   1 |    2 |
+ * | Operation                |          |     |      |
+ * +--------------------------+----------+-----+------+
+ * | Read                     | off/auto | low | high |
+ * +--------------------------+----------+-----+------+
+ * | Write                    |      off | low | high |
+ * +--------------------------+----------+-----+------+
  */
 enum {
 	KBD_BL_STANDARD      = 1,
@@ -145,6 +169,8 @@ enum {
 	KBD_BL_TRISTATE_AUTO = 3,
 };
 
+#define KBD_BL_AUTO_MODE_HW_BRIGHTNESS	3
+
 #define KBD_BL_QUERY_TYPE		0x1
 #define KBD_BL_TRISTATE_TYPE		0x5
 #define KBD_BL_TRISTATE_AUTO_TYPE	0x7
@@ -203,7 +229,7 @@ struct ideapad_private {
 		bool initialized;
 		int type;
 		struct led_classdev led;
-		unsigned int last_brightness;
+		atomic_t last_hw_brightness;
 	} kbd_bl;
 	struct {
 		bool initialized;
@@ -1592,7 +1618,24 @@ static int ideapad_kbd_bl_check_tristate(int type)
 	return (type == KBD_BL_TRISTATE) || (type == KBD_BL_TRISTATE_AUTO);
 }
 
-static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
+static int ideapad_kbd_bl_brightness_parse(struct ideapad_private *priv, int hw_brightness)
+{
+	/* Off, low or high */
+	if (hw_brightness <= priv->kbd_bl.led.max_brightness)
+		return hw_brightness;
+
+	/* Auto (controlled by EC according to ALS), report as off */
+	if (priv->kbd_bl.type == KBD_BL_TRISTATE_AUTO &&
+	    hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS)
+		return 0;
+
+	/* Unknown value */
+	dev_warn(&priv->platform_device->dev,
+		 "Unknown keyboard backlight value: %d", hw_brightness);
+	return -EINVAL;
+}
+
+static int ideapad_kbd_bl_hw_brightness_get(struct ideapad_private *priv)
 {
 	unsigned long value;
 	int err;
@@ -1606,21 +1649,7 @@ static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
 		if (err)
 			return err;
 
-		/* Convert returned value to brightness level */
-		value = FIELD_GET(KBD_BL_GET_BRIGHTNESS, value);
-
-		/* Off, low or high */
-		if (value <= priv->kbd_bl.led.max_brightness)
-			return value;
-
-		/* Auto, report as off */
-		if (value == priv->kbd_bl.led.max_brightness + 1)
-			return 0;
-
-		/* Unknown value */
-		dev_warn(&priv->platform_device->dev,
-			 "Unknown keyboard backlight value: %lu", value);
-		return -EINVAL;
+		return FIELD_GET(KBD_BL_GET_BRIGHTNESS, value);
 	}
 
 	err = eval_hals(priv->adev->handle, &value);
@@ -1630,6 +1659,16 @@ static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
 	return !!test_bit(HALS_KBD_BL_STATE_BIT, &value);
 }
 
+static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
+{
+	int hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
+
+	if (hw_brightness < 0)
+		return hw_brightness;
+
+	return ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
+}
+
 static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_classdev *led_cdev)
 {
 	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
@@ -1637,32 +1676,37 @@ static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_cla
 	return ideapad_kbd_bl_brightness_get(priv);
 }
 
-static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
+static int ideapad_kbd_bl_hw_brightness_set(struct ideapad_private *priv, int hw_brightness)
 {
-	int err;
 	unsigned long value;
 	int type = priv->kbd_bl.type;
+	int err;
 
 	if (ideapad_kbd_bl_check_tristate(type)) {
-		if (brightness > priv->kbd_bl.led.max_brightness)
-			return -EINVAL;
-
-		value = FIELD_PREP(KBD_BL_SET_BRIGHTNESS, brightness) |
+		value = FIELD_PREP(KBD_BL_SET_BRIGHTNESS, hw_brightness) |
 			FIELD_PREP(KBD_BL_COMMAND_TYPE, type) |
 			KBD_BL_COMMAND_SET;
 		err = exec_kblc(priv->adev->handle, value);
 	} else {
-		err = exec_sals(priv->adev->handle, brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF);
+		value = hw_brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF;
+		err = exec_sals(priv->adev->handle, value);
 	}
-
 	if (err)
 		return err;
 
-	priv->kbd_bl.last_brightness = brightness;
+	atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
 
 	return 0;
 }
 
+static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, int brightness)
+{
+	if (brightness > priv->kbd_bl.led.max_brightness)
+		return -EINVAL;
+
+	return ideapad_kbd_bl_hw_brightness_set(priv, brightness);
+}
+
 static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
 						  enum led_brightness brightness)
 {
@@ -1673,26 +1717,29 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
 
 static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
 {
-	int brightness;
+	int hw_brightness, brightness, last_hw_brightness;
 
 	if (!priv->kbd_bl.initialized)
 		return;
 
-	brightness = ideapad_kbd_bl_brightness_get(priv);
-	if (brightness < 0)
+	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
+	if (hw_brightness < 0)
 		return;
 
-	if (brightness == priv->kbd_bl.last_brightness)
-		return;
+	brightness = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
+	if (brightness < 0)
+		return; /* Reject insane values early. */
 
-	priv->kbd_bl.last_brightness = brightness;
+	last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
+	if (hw_brightness == last_hw_brightness)
+		return;
 
 	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
 }
 
 static int ideapad_kbd_bl_init(struct ideapad_private *priv)
 {
-	int brightness, err;
+	int hw_brightness, err;
 
 	if (!priv->features.kbd_bl)
 		return -ENODEV;
@@ -1700,21 +1747,35 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
 	if (WARN_ON(priv->kbd_bl.initialized))
 		return -EEXIST;
 
-	if (ideapad_kbd_bl_check_tristate(priv->kbd_bl.type))
-		priv->kbd_bl.led.max_brightness = 2;
-	else
-		priv->kbd_bl.led.max_brightness = 1;
+	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
+	if (hw_brightness < 0)
+		return hw_brightness;
 
-	brightness = ideapad_kbd_bl_brightness_get(priv);
-	if (brightness < 0)
-		return brightness;
+	atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
 
-	priv->kbd_bl.last_brightness = brightness;
 	priv->kbd_bl.led.name                    = "platform::" LED_FUNCTION_KBD_BACKLIGHT;
 	priv->kbd_bl.led.brightness_get          = ideapad_kbd_bl_led_cdev_brightness_get;
 	priv->kbd_bl.led.brightness_set_blocking = ideapad_kbd_bl_led_cdev_brightness_set;
 	priv->kbd_bl.led.flags                   = LED_BRIGHT_HW_CHANGED | LED_RETAIN_AT_SHUTDOWN;
 
+	switch (priv->kbd_bl.type) {
+	case KBD_BL_TRISTATE_AUTO:
+	case KBD_BL_TRISTATE:
+		priv->kbd_bl.led.max_brightness = 2;
+		break;
+	case KBD_BL_STANDARD:
+		priv->kbd_bl.led.max_brightness = 1;
+		break;
+	default:
+		/* This has already been validated by ideapad_check_features(). */
+		unreachable();
+	}
+
+	/* Reject insane values. */
+	err = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
+	if (err < 0)
+		return err;
+
 	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
 	if (err)
 		return err;

-- 
2.55.0

[PATCH v5 11/12] platform/x86: ideapad-laptop: Serialize keyboard backlight notifications

[Rong Zhang]

ACPI notifications are delivered in dedicated work contexts and may
arrive simultaneously. In the following change, much work will be done
while handling the notification, which could lead to potential race
conditions.

Introduce a new mutex to serialize keyboard backlight notifications to
prevent potential race conditions.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v4:
- Remove needless #include (Thanks Ilpo Järvinen)
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 2fa2665f35d6..7f7b1e30c326 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -29,6 +29,7 @@
 #include <linux/kernel.h>
 #include <linux/leds.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
 #include <linux/power_supply.h>
@@ -230,6 +231,8 @@ struct ideapad_private {
 		int type;
 		struct led_classdev led;
 		atomic_t last_hw_brightness;
+
+		struct mutex notif_mutex; /* protects notifications */
 	} kbd_bl;
 	struct {
 		bool initialized;
@@ -1722,6 +1725,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
 	if (!priv->kbd_bl.initialized)
 		return;
 
+	guard(mutex)(&priv->kbd_bl.notif_mutex);
+
 	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
 	if (hw_brightness < 0)
 		return;
@@ -1747,6 +1752,10 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
 	if (WARN_ON(priv->kbd_bl.initialized))
 		return -EEXIST;
 
+	err = devm_mutex_init(&priv->platform_device->dev, &priv->kbd_bl.notif_mutex);
+	if (err)
+		return err;
+
 	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
 	if (hw_brightness < 0)
 		return hw_brightness;

-- 
2.55.0

[PATCH v5 12/12] platform/x86: ideapad-laptop: Fully support auto keyboard backlight

[Rong Zhang]

Currently, the auto brightness mode of keyboard backlight maps to
brightness=0 in LED classdev. The only method to switch to such a mode
is by pressing the manufacturer-defined shortcut (Fn+Space). However, 0
is a multiplexed brightness value; writing 0 simply results in the
backlight being turned off.

With brightness processing code decoupled from LED classdev, we can now
fully support the auto brightness mode. In this mode, the keyboard
backlight is controlled by the EC according to the ambient light sensor
(ALS).

To utilize this, a private hardware control trigger "ideapad-auto" is
added, with the event handling procedure calling the
led_trigger_notify_hw_control_changed() interface to activate/deactivate
the private trigger according to the current LED trigger state. To align
with LEDS_BRIGHTNESS_HW_CHANGED, the driver neither depends on
LEDS_TRIGGERS_HW_CHANGED nor selects it.

Meanwhile, block brightness changes on exit to prevent the side effect
of LED device unregistration when the private trigger is active from
resetting the brightness to zero, so that we can retain the state of
auto mode among boots.

Acked-by: Ike Panhc <ikepanhc@gmail.com>
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v4:
- Add missing #include (Thanks Ilpo Järvinen)
- Remove needless code alignment (ditto)
- Address concerns from Sashiko
  - Do not notify the LED trigger core when the registration of the
    private trigger has failed
  - https://sashiko.dev/#/patchset/20260719-leds-trigger-hw-changed-v3-0-5fb55722e36e@rong.moe?part=11

Changes in v3:
- 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
---
 drivers/platform/x86/lenovo/Kconfig          |   1 +
 drivers/platform/x86/lenovo/ideapad-laptop.c | 114 ++++++++++++++++++++++++---
 2 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
index 4443f40ef8aa..e92b1e900795 100644
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -16,6 +16,7 @@ config IDEAPAD_LAPTOP
 	select INPUT_SPARSEKMAP
 	select NEW_LEDS
 	select LEDS_CLASS
+	select LEDS_TRIGGERS
 	help
 	  This is a driver for Lenovo IdeaPad netbooks contains drivers for
 	  rfkill switch, hotkey, fan control and backlight control.
diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 7f7b1e30c326..5442cbfe3969 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -16,6 +16,7 @@
 #include <linux/bug.h>
 #include <linux/cleanup.h>
 #include <linux/compiler.h>
+#include <linux/container_of.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -33,6 +34,7 @@
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
 #include <linux/power_supply.h>
+#include <linux/printk.h>
 #include <linux/rfkill.h>
 #include <linux/seq_file.h>
 #include <linux/string_choices.h>
@@ -1715,9 +1717,58 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
 {
 	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
 
+	/*
+	 * When deinitializing: It must be the side effect of led_cdev
+	 * unregistration when our private trigger is active. We've set
+	 * LED_RETAIN_AT_SHUTDOWN to retain led_cdev brightness level.
+	 * To do the same for auto mode, gate changes and return early.
+	 */
+	if (unlikely(!priv->kbd_bl.initialized))
+		return 0;
+
 	return ideapad_kbd_bl_brightness_set(priv, brightness);
 }
 
+static bool ideapad_kbd_bl_auto_trigger_offloaded(struct led_classdev *led_cdev)
+{
+	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
+
+	return atomic_read(&priv->kbd_bl.last_hw_brightness) == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
+}
+
+static int ideapad_kbd_bl_auto_trigger_activate(struct led_classdev *led_cdev)
+{
+	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
+
+	return ideapad_kbd_bl_hw_brightness_set(priv, KBD_BL_AUTO_MODE_HW_BRIGHTNESS);
+}
+
+static struct led_hw_trigger_type ideapad_kbd_bl_auto_trigger_type;
+
+static struct led_trigger ideapad_kbd_bl_auto_trigger = {
+	.name = "ideapad-auto",
+	.trigger_type = &ideapad_kbd_bl_auto_trigger_type,
+	.activate = ideapad_kbd_bl_auto_trigger_activate,
+	.offloaded = ideapad_kbd_bl_auto_trigger_offloaded,
+};
+
+static bool ideapad_kbd_bl_auto_trigger_registered;
+
+static void ideapad_kbd_bl_notify_hw_control(struct ideapad_private *priv,
+					     int hw_brightness, int last_hw_brightness)
+{
+	bool hw_control, last_hw_control;
+
+	if (!ideapad_kbd_bl_auto_trigger_registered || priv->kbd_bl.type != KBD_BL_TRISTATE_AUTO)
+		return;
+
+	hw_control = hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
+	last_hw_control = last_hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
+
+	if (hw_control != last_hw_control)
+		led_trigger_notify_hw_control_changed(&priv->kbd_bl.led, hw_control);
+}
+
 static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
 {
 	int hw_brightness, brightness, last_hw_brightness;
@@ -1739,6 +1790,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
 	if (hw_brightness == last_hw_brightness)
 		return;
 
+	ideapad_kbd_bl_notify_hw_control(priv, hw_brightness, last_hw_brightness);
+
 	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
 }
 
@@ -1769,6 +1822,24 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
 
 	switch (priv->kbd_bl.type) {
 	case KBD_BL_TRISTATE_AUTO:
+		priv->kbd_bl.led.max_brightness = 2;
+
+		if (!ideapad_kbd_bl_auto_trigger_registered) {
+			dev_warn(&priv->platform_device->dev,
+				 "Could not provide LED trigger %s for keyboard backlight\n",
+				 ideapad_kbd_bl_auto_trigger.name);
+			break;
+		}
+
+		priv->kbd_bl.led.flags |= LED_TRIG_HW_CHANGED;
+		priv->kbd_bl.led.trigger_type = &ideapad_kbd_bl_auto_trigger_type;
+		priv->kbd_bl.led.hw_control_trigger = ideapad_kbd_bl_auto_trigger.name;
+
+		/* Hardware remembers the last brightness level, including auto mode. */
+		if (hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS)
+			priv->kbd_bl.led.default_trigger = ideapad_kbd_bl_auto_trigger.name;
+
+		break;
 	case KBD_BL_TRISTATE:
 		priv->kbd_bl.led.max_brightness = 2;
 		break;
@@ -1785,13 +1856,22 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
 	if (err < 0)
 		return err;
 
-	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
-	if (err)
-		return err;
+	/* Queue notifications, as kbd_bl.initialized is about to be set. */
+	guard(mutex)(&priv->kbd_bl.notif_mutex);
 
+	/*
+	 * Setting kbd_bl.initialized after led_classdev_register() could lead
+	 * to race conditions in ideapad_kbd_bl_led_cdev_brightness_set() where
+	 * kbd_bl.initialized is checked, so set it now. It can be reverted back
+	 * if the LED classdev failed to register.
+	 */
 	priv->kbd_bl.initialized = true;
 
-	return 0;
+	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
+	if (err)
+		priv->kbd_bl.initialized = false;
+
+	return err;
 }
 
 static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
@@ -2618,17 +2698,30 @@ static int __init ideapad_laptop_init(void)
 {
 	int err;
 
+	err = led_trigger_register(&ideapad_kbd_bl_auto_trigger);
+	if (err) {
+		pr_warn("Failed to register LED trigger %s: %d\n",
+			ideapad_kbd_bl_auto_trigger.name, err);
+	} else {
+		ideapad_kbd_bl_auto_trigger_registered = true;
+	}
+
 	err = ideapad_wmi_driver_register();
 	if (err)
-		return err;
+		goto err_ledtrig;
 
 	err = platform_driver_register(&ideapad_acpi_driver);
-	if (err) {
-		ideapad_wmi_driver_unregister();
-		return err;
-	}
+	if (err)
+		goto err_wmi;
 
 	return 0;
+
+err_wmi:
+	ideapad_wmi_driver_unregister();
+err_ledtrig:
+	if (ideapad_kbd_bl_auto_trigger_registered)
+		led_trigger_unregister(&ideapad_kbd_bl_auto_trigger);
+	return err;
 }
 module_init(ideapad_laptop_init)
 
@@ -2636,6 +2729,9 @@ static void __exit ideapad_laptop_exit(void)
 {
 	platform_driver_unregister(&ideapad_acpi_driver);
 	ideapad_wmi_driver_unregister();
+
+	if (ideapad_kbd_bl_auto_trigger_registered)
+		led_trigger_unregister(&ideapad_kbd_bl_auto_trigger);
 }
 module_exit(ideapad_laptop_exit)
 

-- 
2.55.0

Re: [PATCH v5 06/12] leds: trigger: netdev: Implement offloaded() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Unprotected lockless read of shared state in `netdev_trig_offloaded()`

Pre-existing issues:
- [High] Resource leak of `net_device` in `netdev_trig_activate()` error path
--

--- Patch [6]: [PATCH v5 06/12] leds: trigger: netdev: Implement offloaded() callback ---
commit d8144f7505ed647e0e3c94f8c01b2ef2b0814005
Author: Rong Zhang <i@rong.moe>

leds: trigger: netdev: Implement offloaded() callback

This commit implements the offloaded callback for the netdev trigger,
providing its hardware control state to the LED core. It also updates the
sysfs ABI documentation to clarify the relationship between the custom and
generic offload attributes.

> diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
> index 64c078e997f28..a26109ca4b1c9 100644
> --- a/drivers/leds/trigger/ledtrig-netdev.c
> +++ b/drivers/leds/trigger/ledtrig-netdev.c

[ ... ]

> @@ -754,10 +754,18 @@ static void netdev_trig_deactivate(struct led_classdev *led_cdev)
>  	kfree(trigger_data);
>  }
>  
> +static bool netdev_trig_offloaded(struct led_classdev *led_cdev)
> +{
> +	struct led_netdev_data *trigger_data = led_get_trigger_data(led_cdev);
> +
> +	return trigger_data->hw_control;

[Severity: Medium]
Is it possible for hw_control to be read while it is being updated?

When userspace reads the trigger_may_offload sysfs attribute, it calls
netdev_trig_offloaded() holding only the led_cdev->trigger_lock, which is a
read lock.

Concurrently, a network event can invoke netdev_trig_notify() which
acquires trigger_data->lock and mutates trigger_data->hw_control.

Does this code need to hold trigger_data->lock or use the READ_ONCE() macro
to prevent a data race when reading trigger_data->hw_control here?

> +}
> +
>  static struct led_trigger netdev_led_trigger = {
>  	.name = "netdev",
>  	.activate = netdev_trig_activate,
>  	.deactivate = netdev_trig_deactivate,
> +	.offloaded = netdev_trig_offloaded,
>  	.groups = netdev_trig_groups,
>  };

[Severity: High]
This is a pre-existing issue, but does netdev_trig_activate() leak a
net_device reference in its error path?

If hardware control is supported, set_device_name() is called, which
acquires a net device reference via dev_get_by_name() and saves it to
trigger_data->net_dev.

If the subsequent call to register_netdevice_notifier() fails, the error
path executes:

drivers/leds/trigger/ledtrig-netdev.c:netdev_trig_activate() {
    ...
	rc = register_netdevice_notifier(&trigger_data->notifier);
	if (rc)
		kfree(trigger_data);

	return rc;
}

Since trigger_data is freed without calling dev_put(trigger_data->net_dev),
does this leave a permanent reference count leak on the net_device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=6

Re: [PATCH v5 03/12] leds: trigger: Add offloaded() callback and provide trigger_may_offload attribute

[sashiko-bot]

> There are multiple triggers implementing hardware control. However, the
> LED trigger core doesn't really know the hardware control (offloaded)
> state since the coordination is done directly between the trigger and
> the LED driver. It can only assume private triggers as offloaded and
> generic ones as not offloaded.
> 
> Add an offloaded() callback so that triggers can report their offloaded
> states to the LED trigger core. When unimplemented, it defaults to true
> for private triggers and false for generic ones to keep the current
> behavior unchanged.
> 
> With that, provide a new 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]")
> 
> Note: the documentation describes the attribute as "returning a list"
> despite the LED core currently only supports one hardware control
> trigger per LED device. This is intentional to make the attribute
> extensible in the future without breaking userspace.
> 
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=3

Re: [PATCH v5 04/12] leds: cros_ec: Implement offloaded() trigger callback

[sashiko-bot]

> "chromeos-auto" is a private hardware control trigger which always stays
> in hardware control. Implement offloaded() callback with its return
> value to be always true to reflect this.
> 
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=4

Re: [PATCH v5 01/12] leds: Move led_trigger_is_hw_controlled() to the right place

[sashiko-bot]

> Currently led_trigger_is_hw_controlled() is placed at led-class.c, which
> is not an right place as it falls into the triggers namespace and does
> triggers stuff.
> 
> Move it into led-triggers.c, and split it into locked and unlocked
> variant for convenience.
> 
> Fixes: b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of hardware controlled LED")
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=1

Re: [PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The statically allocated `omnia_hw_trigger` object is registered multiple times via `devm_led_trigger_register()` on probe if multiple driver instances are bound, causing lock and list corruption.
--

--- Patch [5]: [PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger ---
commit f8e07428579f0c7761609a5c572f8b597a213a09
Author: Rong Zhang <i@rong.moe>

leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

"omnia-mcu" is a private hardware control trigger which always stays in
hardware control mode. Implement offloaded() callback with its return
value to be always true to reflect this.

> diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
> index ed6a47bbb44f9..32d40d176d3f1 100644
> --- a/drivers/leds/leds-turris-omnia.c
> +++ b/drivers/leds/leds-turris-omnia.c
> @@ -195,10 +195,16 @@ static void omnia_hwtrig_deactivate(struct led_classdev *cdev)
>  			err);
>  }
>  
> +static bool omnia_hwtrig_offloaded(struct led_classdev *cdev)
> +{
> +	return true;
> +}
> +
>  static struct led_trigger omnia_hw_trigger = {
>  	.name		= "omnia-mcu",
>  	.activate	= omnia_hwtrig_activate,
>  	.deactivate	= omnia_hwtrig_deactivate,
> +	.offloaded	= omnia_hwtrig_offloaded,
>  	.trigger_type	= &omnia_hw_trigger_type,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but does using a statically
allocated global trigger structure risk lock and list corruption?

If multiple instances of the driver are bound (for example, via I2C
`new_device` sysfs bindings), `omnia_leds_probe()` will call
`devm_led_trigger_register()` multiple times using this shared
`omnia_hw_trigger` object:

omnia_leds_probe()
    devm_led_trigger_register(dev, &omnia_hw_trigger)

Inside the core `led_trigger_register()` function, `spin_lock_init()` and
`INIT_LIST_HEAD()` are called unconditionally before checking if the trigger
name already exists. As a result, the second probe call will silently
re-initialize the active spinlock and reset the list head of the first
instance's trigger, which could lead to a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=5

Re: [PATCH v5 07/12] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Sysfs attributes are exposed to user-space before the `trigger_lock` rwsem is initialized, causing uninitialized lock access.
--

--- Patch [7]: [PATCH v5 07/12] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled() ---
commit ebab4e455acab29c44baf2a4a586beae42254bc5
Author: Rong Zhang <i@rong.moe>

leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()

With all existing triggers adopting the new interface, strict checks
could be enforced to make the semantics of hardware control triggers
clearer.

> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index de17a8bbb4d48..cb49a02a8b3cf 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -42,9 +42,16 @@ static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
>  	if (!led_cdev->trigger)
>  		return false;
>  
> +	if (!led_cdev->hw_control_trigger ||
> +	    strcmp(led_cdev->hw_control_trigger, led_cdev->trigger->name))
> +		return false;

[Severity: High]
This is a pre-existing issue, but since this patch modifies the hardware
control paths, is it possible to access an uninitialized rw_semaphore here?

In drivers/leds/led-class.c, the device and sysfs attributes are created
and exposed to user-space before the trigger_lock rwsem is initialized:

led_classdev_register_ext() {
        ...
        led_cdev->dev = device_create_with_groups(&leds_class, parent, 0,
                                                  led_cdev, led_cdev->groups,
                                                  "%s", final_name);
        ...
        init_rwsem(&led_cdev->trigger_lock);
        ...
}

If user-space processes the uevent quickly enough and reads the brightness
attribute, brightness_show() will call led_trigger_is_hw_controlled() before
acquiring the led_access mutex:

brightness_show() {
        if (led_trigger_is_hw_controlled(led_cdev))
                return -ENODATA;

        mutex_lock(&led_cdev->led_access);
        ...
}

Since led_trigger_is_hw_controlled() eventually accesses trigger_lock, will
it attempt to acquire the uninitialized semaphore and cause a crash or
lockdep warning?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=7

Re: [PATCH v5 08/12] leds: trigger: Do not attach trigger to a removing LED

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch attempts to fix a data race on `led_cdev->flags` by wrapping the `LED_UNREGISTERING` update in `trigger_lock`. However, this is an incomplete fix because `flags` is an `int` modified via non-atomic bitwise operations (`|=`, `&= ~`), and other modifiers (such as `led_classdev_suspend` and `led_sysfs_disable`) do not hold `trigger_lock`. A concurrent update from these paths will still cause a data race that can drop the `LED_UNREGISTERING` flag, exposing the same Use-After-Free bug.
- [High] `led_classdev_unregister()` flushes the brightness workqueue before unregistering the sysfs device, creating a race condition where concurrent sysfs writes can queue new work that executes after the LED is freed.
--

--- Patch [8]: [PATCH v5 08/12] leds: trigger: Do not attach trigger to a removing LED ---
commit 9ab2868504159e735891fdf50fc3354c0c88c1c8
Author: Rong Zhang <i@rong.moe>

leds: trigger: Do not attach trigger to a removing LED

This patch attempts to fix a race condition where a trigger may be attached
to a removing LED by setting the LED_UNREGISTERING flag under the
trigger_lock in led_classdev_unregister().

> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 2460fcf0c4690..771a6e6c659be 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -633,12 +633,13 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
>  
>  #ifdef CONFIG_LEDS_TRIGGERS
>  	down_write(&led_cdev->trigger_lock);
> +	led_cdev->flags |= LED_UNREGISTERING;

[Severity: High]
This is a pre-existing issue, but does this fully resolve the data race on
the flags bitmask?

While this update is now protected by trigger_lock, other paths like PM
suspend or V4L2 flash release modify led_cdev->flags using non-atomic
bitwise operations without holding trigger_lock.

If a concurrent update occurs without the lock, could the non-atomic
read-modify-write drop the LED_UNREGISTERING flag?

If the flag is lost, a subsequent sysfs trigger write might see it unset
and attach a trigger to the unregistering LED, leading to a use-after-free
when the trigger later fires.

[ ... ]

>  	/* Stop blinking */
>  	led_stop_software_blink(led_cdev);
> 
> 	if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
> 		led_set_brightness(led_cdev, LED_OFF);
> 
> 	flush_work(&led_cdev->set_brightness_work);

[Severity: High]
This is also a pre-existing issue, but is there a race condition between
flushing the workqueue and unregistering the device?

In led_classdev_unregister(), flush_work() is called before
device_unregister() removes the device from sysfs.

Could a user-space thread writing to the sysfs brightness attribute in the
window between these two calls queue new work?

sysfs brightness_store() -> led_set_brightness_nopm() -> queue_work()

Since device_unregister() hasn't been called yet, the sysfs write can
succeed, leaving pending work that will execute after the device memory is
freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=8

Re: [PATCH v5 02/12] leds: class: Remove hardware control trigger when writing brightness

[sashiko-bot]

> Since commit b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of
> hardware controlled LED"), the brightness attribute becomes write-only
> when the LED is controlled fully by the hardware. A write-only attribute
> is very confusing.
> 
> Moreover, most LED drivers set hardware brightness innocently with the
> side effect of disabling hardware control, but the hardware control
> trigger remains active, resulting in the software and hardware being out
> of sync.
> 
> Fix it by removing the hardware control trigger when writing the
> brightness attribute.
> 
> This should also match the semantics of hardware control:
> 
>     When the LED is in hw control, no software blink is possible and
>     doing so will effectively disable hw control.
> 
> Fixes: b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of hardware controlled LED")
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=2

Re: [PATCH v5 09/12] leds: trigger: Add led_trigger_notify_hw_control_changed() interface

[sashiko-bot]

> Some hardware can autonomously activate/deactivate hardware control.
> After that, the LED hardware notifies the LED driver. Currently, there
> is no mechanism for LED drivers to notify the LED core about such events
> and initiate a trigger transition to reflect the hardware state.
> 
> Add a new interface called led_trigger_notify_hw_control_changed(), so
> that LED drivers can call it to notify the LED core about the
> transition.
> 
> The interface only allows two transitions:
> 
> 1. "none" => private trigger
> 2. private trigger => "none"
> 
> If the current trigger is neither the private trigger nor "none", no
> [ ... ]
> By default, the config is enabled when LEDS_BRIGHTNESS_HW_CHANGED is
> enabled.
> 
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=9

Re: [PATCH v5 10/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The conversion of `last_hw_brightness` to an `atomic_t` fails to prevent a TOCTOU (Time-Of-Check to Time-Of-Use) race condition between ACPI notifications and sysfs brightness writes, violating core kernel locking rules.
--

--- Patch [10]: [PATCH v5 10/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight ---
commit b73007a6498db545468f511c09a496e09dfcac31
Author: Rong Zhang <i@rong.moe>

platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight

This commit decouples hardware brightness from LED classdev brightness for
the keyboard backlight. It also converts last_hw_brightness into an atomic_t
to help prevent potential race conditions.

> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
[ ... ]
> @@ -1637,32 +1676,37 @@ static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_cla
>  	return ideapad_kbd_bl_brightness_get(priv);
>  }
>  
> -static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
> +static int ideapad_kbd_bl_hw_brightness_set(struct ideapad_private *priv, int hw_brightness)
>  {
[ ... ]
>  	if (err)
>  		return err;
>  
> -	priv->kbd_bl.last_brightness = brightness;
> +	atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
>  
>  	return 0;
>  }
[ ... ]
> @@ -1673,26 +1717,29 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
>  
>  static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
>  {
> -	int brightness;
> +	int hw_brightness, brightness, last_hw_brightness;
>  
>  	if (!priv->kbd_bl.initialized)
>  		return;
>  
> -	brightness = ideapad_kbd_bl_brightness_get(priv);
> -	if (brightness < 0)
> +	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> +	if (hw_brightness < 0)
>  		return;
>  
> -	if (brightness == priv->kbd_bl.last_brightness)
> -		return;
> +	brightness = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
> +	if (brightness < 0)
> +		return; /* Reject insane values early. */
>  
> -	priv->kbd_bl.last_brightness = brightness;
> +	last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);

[Severity: High]
This is a pre-existing issue, but does converting last_hw_brightness to an
atomic_t fully prevent a time-of-check to time-of-use race condition here?

Since the hardware read and the atomic_xchg() are separate steps, could
the following sequence occur?

CPU1 in ideapad_kbd_bl_notify():
    hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);

CPU2 in ideapad_kbd_bl_hw_brightness_set() via concurrent sysfs write:
    exec_kblc(priv->adev->handle, value);
    atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);

CPU1 resumes in ideapad_kbd_bl_notify():
    last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);

In this scenario, wouldn't CPU1 overwrite the software cache with a stale
hardware state, causing it to become permanently desynchronized from the
actual hardware?

Although follow-up patches in this series introduce a notif_mutex, does
the sysfs write path ever acquire this mutex to serialize these updates
properly?

> +	if (hw_brightness == last_hw_brightness)
> +		return;
>  
>  	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=10

Re: [PATCH v5 11/12] platform/x86: ideapad-laptop: Serialize keyboard backlight notifications

[sashiko-bot]

> ACPI notifications are delivered in dedicated work contexts and may
> arrive simultaneously. In the following change, much work will be done
> while handling the notification, which could lead to potential race
> conditions.
> 
> Introduce a new mutex to serialize keyboard backlight notifications to
> prevent potential race conditions.
> 
> Acked-by: Ike Panhc <ikepanhc@gmail.com>
> Signed-off-by: Rong Zhang <i@rong.moe>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=11

Re: [PATCH v5 12/12] platform/x86: ideapad-laptop: Fully support auto keyboard backlight

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardware brightness state falls out of sync with `last_hw_brightness` due to a TOCTOU race between ACPI notify and sysfs write.
--

--- Patch [12]: [PATCH v5 12/12] platform/x86: ideapad-laptop: Fully support auto keyboard backlight ---
commit 775eb668f48ee8e4ee7f513342cb65f613086809
Author: Rong Zhang <i@rong.moe>

platform/x86: ideapad-laptop: Fully support auto keyboard backlight

This commit adds support for the auto brightness mode of the keyboard
backlight on IdeaPad laptops. It introduces a private hardware control
trigger to handle the auto mode and blocks brightness changes on exit
to retain state across boots.

> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 7f7b1e30c3263..5442cbfe3969b 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c

[ ... ]

> @@ -1739,6 +1790,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
>  	if (hw_brightness == last_hw_brightness)
>  		return;
>  
> +	ideapad_kbd_bl_notify_hw_control(priv, hw_brightness, last_hw_brightness);
> +

[Severity: Medium]
Can a race condition here cause the hardware brightness state to permanently
fall out of sync with the internal tracker?

Looking at ideapad_kbd_bl_notify(), it reads the current hardware brightness
and then updates the tracker using atomic_xchg():

ideapad_kbd_bl_notify() {
    ...
    hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
    ...
    last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
}

If a concurrent sysfs write triggers ideapad_kbd_bl_hw_brightness_set()
in the middle of this process, could we see this sequence:

Thread 1 (ideapad_kbd_bl_notify):
    Reads hw_brightness (e.g., 1)

Thread 2 (sysfs write):
    Calls ideapad_kbd_bl_hw_brightness_set()
    Sets actual hardware brightness to 3 (auto mode)
    Calls atomic_set(&priv->kbd_bl.last_hw_brightness, 3)

Thread 1 (ideapad_kbd_bl_notify resumes):
    Calls atomic_xchg(&priv->kbd_bl.last_hw_brightness, 1)

Wouldn't Thread 1 blindly overwrite the correct atomic state (3) with the
stale read state (1)? 

This seems like it would permanently desync the last_hw_brightness tracker
from the actual hardware, breaking ideapad_kbd_bl_auto_trigger_offloaded()
which would now falsely report that the trigger is not offloaded.

>  	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-leds-trigger-hw-changed-v5-0-3e00d6c4a936@rong.moe?part=12

Re: [PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The statically allocated `omnia_hw_trigger` object is registered multiple times via `devm_led_trigger_register()` on probe if multiple driver instances are bound, causing lock and list corruption.

The driver is very platform-specific and the relevant platforms never
have more than one such device.

Therefore, fixing it is far beyond the scope of the series.

Thanks,
Rong

> --
> 
> --- Patch [5]: [PATCH v5 05/12] leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger ---
> commit f8e07428579f0c7761609a5c572f8b597a213a09
> Author: Rong Zhang <i@rong.moe>
> 
> leds: turris-omnia: Implement offloaded() trigger callback and declare hw_control_trigger
> 
> "omnia-mcu" is a private hardware control trigger which always stays in
> hardware control mode. Implement offloaded() callback with its return
> value to be always true to reflect this.
> 
> > diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
> > index ed6a47bbb44f9..32d40d176d3f1 100644
> > --- a/drivers/leds/leds-turris-omnia.c
> > +++ b/drivers/leds/leds-turris-omnia.c
> > @@ -195,10 +195,16 @@ static void omnia_hwtrig_deactivate(struct led_classdev *cdev)
> >  			err);
> >  }
> >  
> > +static bool omnia_hwtrig_offloaded(struct led_classdev *cdev)
> > +{
> > +	return true;
> > +}
> > +
> >  static struct led_trigger omnia_hw_trigger = {
> >  	.name		= "omnia-mcu",
> >  	.activate	= omnia_hwtrig_activate,
> >  	.deactivate	= omnia_hwtrig_deactivate,
> > +	.offloaded	= omnia_hwtrig_offloaded,
> >  	.trigger_type	= &omnia_hw_trigger_type,
> >  };
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does using a statically
> allocated global trigger structure risk lock and list corruption?
> 
> If multiple instances of the driver are bound (for example, via I2C
> `new_device` sysfs bindings), `omnia_leds_probe()` will call
> `devm_led_trigger_register()` multiple times using this shared
> `omnia_hw_trigger` object:
> 
> omnia_leds_probe()
>     devm_led_trigger_register(dev, &omnia_hw_trigger)
> 
> Inside the core `led_trigger_register()` function, `spin_lock_init()` and
> `INIT_LIST_HEAD()` are called unconditionally before checking if the trigger
> name already exists. As a result, the second probe call will silently
> re-initialize the active spinlock and reset the list head of the first
> instance's trigger, which could lead to a kernel panic.

Re: [PATCH v5 06/12] leds: trigger: netdev: Implement offloaded() callback

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Unprotected lockless read of shared state in `netdev_trig_offloaded()`

This just mimics how the existing "offloaded" attribute does. See
offloaded_show().

This is just an informational hint. It doesn't need to be accurate, so
it's not a bug.

> 
> Pre-existing issues:
> - [High] Resource leak of `net_device` in `netdev_trig_activate()` error path

Fixing it is far beyond the scope of the series.

Thanks,
Rong

> --
> 
> --- Patch [6]: [PATCH v5 06/12] leds: trigger: netdev: Implement offloaded() callback ---
> commit d8144f7505ed647e0e3c94f8c01b2ef2b0814005
> Author: Rong Zhang <i@rong.moe>
> 
> leds: trigger: netdev: Implement offloaded() callback
> 
> This commit implements the offloaded callback for the netdev trigger,
> providing its hardware control state to the LED core. It also updates the
> sysfs ABI documentation to clarify the relationship between the custom and
> generic offload attributes.
> 
> > diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
> > index 64c078e997f28..a26109ca4b1c9 100644
> > --- a/drivers/leds/trigger/ledtrig-netdev.c
> > +++ b/drivers/leds/trigger/ledtrig-netdev.c
> 
> [ ... ]
> 
> > @@ -754,10 +754,18 @@ static void netdev_trig_deactivate(struct led_classdev *led_cdev)
> >  	kfree(trigger_data);
> >  }
> >  
> > +static bool netdev_trig_offloaded(struct led_classdev *led_cdev)
> > +{
> > +	struct led_netdev_data *trigger_data = led_get_trigger_data(led_cdev);
> > +
> > +	return trigger_data->hw_control;
> 
> [Severity: Medium]
> Is it possible for hw_control to be read while it is being updated?
> 
> When userspace reads the trigger_may_offload sysfs attribute, it calls
> netdev_trig_offloaded() holding only the led_cdev->trigger_lock, which is a
> read lock.
> 
> Concurrently, a network event can invoke netdev_trig_notify() which
> acquires trigger_data->lock and mutates trigger_data->hw_control.
> 
> Does this code need to hold trigger_data->lock or use the READ_ONCE() macro
> to prevent a data race when reading trigger_data->hw_control here?
> 
> > +}
> > +
> >  static struct led_trigger netdev_led_trigger = {
> >  	.name = "netdev",
> >  	.activate = netdev_trig_activate,
> >  	.deactivate = netdev_trig_deactivate,
> > +	.offloaded = netdev_trig_offloaded,
> >  	.groups = netdev_trig_groups,
> >  };
> 
> [Severity: High]
> This is a pre-existing issue, but does netdev_trig_activate() leak a
> net_device reference in its error path?
> 
> If hardware control is supported, set_device_name() is called, which
> acquires a net device reference via dev_get_by_name() and saves it to
> trigger_data->net_dev.
> 
> If the subsequent call to register_netdevice_notifier() fails, the error
> path executes:
> 
> drivers/leds/trigger/ledtrig-netdev.c:netdev_trig_activate() {
>     ...
> 	rc = register_netdevice_notifier(&trigger_data->notifier);
> 	if (rc)
> 		kfree(trigger_data);
> 
> 	return rc;
> }
> 
> Since trigger_data is freed without calling dev_put(trigger_data->net_dev),
> does this leave a permanent reference count leak on the net_device?

Re: [PATCH v5 07/12] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Sysfs attributes are exposed to user-space before the `trigger_lock` rwsem is initialized, causing uninitialized lock access.

Valid concern. This bug was introduced in commit b819dc7d8fb2 ("leds:
core: Report ENODATA for brightness of hardware controlled LED"), as the
check is not protected by led_access.

Since the series touches what was introduced by the said commit, I will
prepend a fix commit in the next revision by protecting it with
led_access.

Thanks,
Rong

> --
> 
> --- Patch [7]: [PATCH v5 07/12] leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled() ---
> commit ebab4e455acab29c44baf2a4a586beae42254bc5
> Author: Rong Zhang <i@rong.moe>
> 
> leds: trigger: Enforce strict checks in led_trigger_is_hw_controlled()
> 
> With all existing triggers adopting the new interface, strict checks
> could be enforced to make the semantics of hardware control triggers
> clearer.
> 
> > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> > index de17a8bbb4d48..cb49a02a8b3cf 100644
> > --- a/drivers/leds/led-triggers.c
> > +++ b/drivers/leds/led-triggers.c
> > @@ -42,9 +42,16 @@ static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
> >  	if (!led_cdev->trigger)
> >  		return false;
> >  
> > +	if (!led_cdev->hw_control_trigger ||
> > +	    strcmp(led_cdev->hw_control_trigger, led_cdev->trigger->name))
> > +		return false;
> 
> [Severity: High]
> This is a pre-existing issue, but since this patch modifies the hardware
> control paths, is it possible to access an uninitialized rw_semaphore here?
> 
> In drivers/leds/led-class.c, the device and sysfs attributes are created
> and exposed to user-space before the trigger_lock rwsem is initialized:
> 
> led_classdev_register_ext() {
>         ...
>         led_cdev->dev = device_create_with_groups(&leds_class, parent, 0,
>                                                   led_cdev, led_cdev->groups,
>                                                   "%s", final_name);
>         ...
>         init_rwsem(&led_cdev->trigger_lock);
>         ...
> }
> 
> If user-space processes the uevent quickly enough and reads the brightness
> attribute, brightness_show() will call led_trigger_is_hw_controlled() before
> acquiring the led_access mutex:
> 
> brightness_show() {
>         if (led_trigger_is_hw_controlled(led_cdev))
>                 return -ENODATA;
> 
>         mutex_lock(&led_cdev->led_access);
>         ...
> }
> 
> Since led_trigger_is_hw_controlled() eventually accesses trigger_lock, will
> it attempt to acquire the uninitialized semaphore and cause a crash or
> lockdep warning?
> 
> [ ... ]

Re: [PATCH v5 08/12] leds: trigger: Do not attach trigger to a removing LED

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The patch attempts to fix a data race on `led_cdev->flags` by wrapping the `LED_UNREGISTERING` update in `trigger_lock`. However, this is an incomplete fix because `flags` is an `int` modified via non-atomic bitwise operations (`|=`, `&= ~`), and other modifiers (such as `led_classdev_suspend` and `led_sysfs_disable`) do not hold `trigger_lock`. A concurrent update from these paths will still cause a data race that can drop the `LED_UNREGISTERING` flag, exposing the same Use-After-Free bug.
> - [High] `led_classdev_unregister()` flushes the brightness workqueue before unregistering the sysfs device, creating a race condition where concurrent sysfs writes can queue new work that executes after the LED is freed.

Fixing them would need a huge series and is far beyond the scope of the
series.

Thank,
Rong

> --
> 
> --- Patch [8]: [PATCH v5 08/12] leds: trigger: Do not attach trigger to a removing LED ---
> commit 9ab2868504159e735891fdf50fc3354c0c88c1c8
> Author: Rong Zhang <i@rong.moe>
> 
> leds: trigger: Do not attach trigger to a removing LED
> 
> This patch attempts to fix a race condition where a trigger may be attached
> to a removing LED by setting the LED_UNREGISTERING flag under the
> trigger_lock in led_classdev_unregister().
> 
> > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> > index 2460fcf0c4690..771a6e6c659be 100644
> > --- a/drivers/leds/led-class.c
> > +++ b/drivers/leds/led-class.c
> > @@ -633,12 +633,13 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
> >  
> >  #ifdef CONFIG_LEDS_TRIGGERS
> >  	down_write(&led_cdev->trigger_lock);
> > +	led_cdev->flags |= LED_UNREGISTERING;
> 
> [Severity: High]
> This is a pre-existing issue, but does this fully resolve the data race on
> the flags bitmask?
> 
> While this update is now protected by trigger_lock, other paths like PM
> suspend or V4L2 flash release modify led_cdev->flags using non-atomic
> bitwise operations without holding trigger_lock.
> 
> If a concurrent update occurs without the lock, could the non-atomic
> read-modify-write drop the LED_UNREGISTERING flag?
> 
> If the flag is lost, a subsequent sysfs trigger write might see it unset
> and attach a trigger to the unregistering LED, leading to a use-after-free
> when the trigger later fires.
> 
> [ ... ]
> 
> >  	/* Stop blinking */
> >  	led_stop_software_blink(led_cdev);
> > 
> > 	if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
> > 		led_set_brightness(led_cdev, LED_OFF);
> > 
> > 	flush_work(&led_cdev->set_brightness_work);
> 
> [Severity: High]
> This is also a pre-existing issue, but is there a race condition between
> flushing the workqueue and unregistering the device?
> 
> In led_classdev_unregister(), flush_work() is called before
> device_unregister() removes the device from sysfs.
> 
> Could a user-space thread writing to the sysfs brightness attribute in the
> window between these two calls queue new work?
> 
> sysfs brightness_store() -> led_set_brightness_nopm() -> queue_work()
> 
> Since device_unregister() hasn't been called yet, the sysfs write can
> succeed, leaving pending work that will execute after the device memory is
> freed.

Re: [PATCH v5 12/12] platform/x86: ideapad-laptop: Fully support auto keyboard backlight

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Hardware brightness state falls out of sync with `last_hw_brightness` due to a TOCTOU race between ACPI notify and sysfs write.
> --
> 
> --- Patch [12]: [PATCH v5 12/12] platform/x86: ideapad-laptop: Fully support auto keyboard backlight ---
> commit 775eb668f48ee8e4ee7f513342cb65f613086809
> Author: Rong Zhang <i@rong.moe>
> 
> platform/x86: ideapad-laptop: Fully support auto keyboard backlight
> 
> This commit adds support for the auto brightness mode of the keyboard
> backlight on IdeaPad laptops. It introduces a private hardware control
> trigger to handle the auto mode and blocks brightness changes on exit
> to retain state across boots.
> 
> > diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> > index 7f7b1e30c3263..5442cbfe3969b 100644
> > --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> > +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> 
> [ ... ]
> 
> > @@ -1739,6 +1790,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> >  	if (hw_brightness == last_hw_brightness)
> >  		return;
> >  
> > +	ideapad_kbd_bl_notify_hw_control(priv, hw_brightness, last_hw_brightness);
> > +
> 
> [Severity: Medium]
> Can a race condition here cause the hardware brightness state to permanently
> fall out of sync with the internal tracker?
> 
> Looking at ideapad_kbd_bl_notify(), it reads the current hardware brightness
> and then updates the tracker using atomic_xchg():
> 
> ideapad_kbd_bl_notify() {
>     ...
>     hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
>     ...
>     last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> }
> 
> If a concurrent sysfs write triggers ideapad_kbd_bl_hw_brightness_set()
> in the middle of this process, could we see this sequence:
> 
> Thread 1 (ideapad_kbd_bl_notify):
>     Reads hw_brightness (e.g., 1)
> 
> Thread 2 (sysfs write):
>     Calls ideapad_kbd_bl_hw_brightness_set()
>     Sets actual hardware brightness to 3 (auto mode)
>     Calls atomic_set(&priv->kbd_bl.last_hw_brightness, 3)
> 
> Thread 1 (ideapad_kbd_bl_notify resumes):
>     Calls atomic_xchg(&priv->kbd_bl.last_hw_brightness, 1)
> 
> Wouldn't Thread 1 blindly overwrite the correct atomic state (3) with the
> stale read state (1)? 
> 
> This seems like it would permanently desync the last_hw_brightness tracker
> from the actual hardware, breaking ideapad_kbd_bl_auto_trigger_offloaded()
> which would now falsely report that the trigger is not offloaded.

Well, the concern is valid, Thread 1 could disable the private trigger
after Thread 2 activating it, but the hardware state is still auto due to
__led_trigger_set being called with hw_triggered == true.

led_trigger_notify_hw_control_changed() calls
ideapad_kbd_bl_hw_brightness_set(), that's why the latter is not
protected by the newly introduced notif_mutex.

Maybe we would need to introduce a workqueue deferral mechanism to
resolve that, as Sashiko suggested in the reply to v4...

Thanks,
Rong

> 
> >  	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> >  }

Re: [PATCH v5 10/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight

[Rong Zhang]

Hi all,

On Fri, 2026-08-21 at 17:27 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The conversion of `last_hw_brightness` to an `atomic_t` fails to prevent a TOCTOU (Time-Of-Check to Time-Of-Use) race condition between ACPI notifications and sysfs brightness writes, violating core kernel locking rules.

Please see my reply to Sashiko's review to PATCH 12.

Thanks,
Rong

> --
> 
> --- Patch [10]: [PATCH v5 10/12] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight ---
> commit b73007a6498db545468f511c09a496e09dfcac31
> Author: Rong Zhang <i@rong.moe>
> 
> platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight
> 
> This commit decouples hardware brightness from LED classdev brightness for
> the keyboard backlight. It also converts last_hw_brightness into an atomic_t
> to help prevent potential race conditions.
> 
> > diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> > --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> > +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> [ ... ]
> > @@ -1637,32 +1676,37 @@ static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_cla
> >  	return ideapad_kbd_bl_brightness_get(priv);
> >  }
> >  
> > -static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
> > +static int ideapad_kbd_bl_hw_brightness_set(struct ideapad_private *priv, int hw_brightness)
> >  {
> [ ... ]
> >  	if (err)
> >  		return err;
> >  
> > -	priv->kbd_bl.last_brightness = brightness;
> > +	atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> >  
> >  	return 0;
> >  }
> [ ... ]
> > @@ -1673,26 +1717,29 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
> >  
> >  static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> >  {
> > -	int brightness;
> > +	int hw_brightness, brightness, last_hw_brightness;
> >  
> >  	if (!priv->kbd_bl.initialized)
> >  		return;
> >  
> > -	brightness = ideapad_kbd_bl_brightness_get(priv);
> > -	if (brightness < 0)
> > +	hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> > +	if (hw_brightness < 0)
> >  		return;
> >  
> > -	if (brightness == priv->kbd_bl.last_brightness)
> > -		return;
> > +	brightness = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
> > +	if (brightness < 0)
> > +		return; /* Reject insane values early. */
> >  
> > -	priv->kbd_bl.last_brightness = brightness;
> > +	last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> 
> [Severity: High]
> This is a pre-existing issue, but does converting last_hw_brightness to an
> atomic_t fully prevent a time-of-check to time-of-use race condition here?
> 
> Since the hardware read and the atomic_xchg() are separate steps, could
> the following sequence occur?
> 
> CPU1 in ideapad_kbd_bl_notify():
>     hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> 
> CPU2 in ideapad_kbd_bl_hw_brightness_set() via concurrent sysfs write:
>     exec_kblc(priv->adev->handle, value);
>     atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> 
> CPU1 resumes in ideapad_kbd_bl_notify():
>     last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> 
> In this scenario, wouldn't CPU1 overwrite the software cache with a stale
> hardware state, causing it to become permanently desynchronized from the
> actual hardware?
> 
> Although follow-up patches in this series introduce a notif_mutex, does
> the sysfs write path ever acquire this mutex to serialize these updates
> properly?
> 
> > +	if (hw_brightness == last_hw_brightness)
> > +		return;
> >  
> >  	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> >  }