[PATCH 0/2] platform/x86: asus-wmi: fix and unify keyboard brightness key handling

[PATCH 0/2] platform/x86: asus-wmi: fix and unify keyboard brightness key handling

[Bartu Alev]

The WMI hotkey path and the HID event path in asus-wmi step the
keyboard backlight with two separate copies of the same arithmetic, and
the two copies disagree. The WMI copy stores the new level without
clamping it, so holding brightness-down at zero drives the cached level
one step below zero per keypress, the "value >= 0" apply check then
drops every following transition, and the hardware-changed notification
hands the wrapped negative to userspace as 4294967294+. Pressing
brightness-up at maximum reports 4, above max_brightness.

The first patch fixes the bounds on its own, so stable trees can pick
it up without the rest of the series. The second patch removes the
duplication: both paths now step through one shared helper that reads,
computes, and writes the level inside a single hold of asus_ref.lock.

With the series applied the level stays
within [0, ASUS_EV_MAX_BRIGHTNESS] on every transition and userspace
sees no wrapped or out-of-range values.

Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
Bartu Alev (2):
      platform/x86: asus-wmi: clamp keyboard brightness hotkey updates
      platform/x86: asus-wmi: unify WMI and HID keyboard brightness stepping

 drivers/platform/x86/asus-wmi.c | 79 +++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 39 deletions(-)
---
base-commit: 9d3475065167197742b0882aff21cf8e78393277
change-id: 20261025-b4-asus-kbd-fixes-6c7e3780597b

Best regards,
--  
Bartu Alev <bartualev@gmail.com>

[PATCH 1/2] platform/x86: asus-wmi: clamp keyboard brightness hotkey updates

[Bartu Alev]

Brightness hotkeys (NOTIFY_KBD_BRTUP and NOTIFY_KBD_BRTDWN) pass an
incremented or decremented level into kbd_led_set_by_kbd(). The driver
assumes the new level stays within valid bounds before queuing work to
update the hardware register:

   scoped_guard(spinlock_irqsave, &asus_ref.lock) {
      asus->kbd_led_wk = value;
      asus->kbd_led_notify = true;
   }

Unlike asus_hid_event(), kbd_led_set_by_kbd() never clamps this value.
Pressing brightness-down while the backlight is already off stores -1
in asus->kbd_led_wk. Subsequent presses decrement the counter further
(-2, -3, and so on). When kbd_led_update_all() runs, its "value >= 0"
check drops the work item, leaving the hardware state untouched while
clearing asus->kbd_led_notify.

Userspace listeners receive the unclamped value through
led_classdev_notify_brightness_hw_changed(). Because that interface takes
an unsigned int, the negative integer wraps to 4294967295, returning
-ENODATA to sysfs readers. The keyboard remains unresponsive to upward
brightness hotkeys until the user presses brightness-up enough times to
cancel every negative decrement. At maximum brightness, brightness-up
leaks 4 to userspace, which exceeds max_brightness (3).

Clamp the input in kbd_led_set_by_kbd() to [0, ASUS_EV_MAX_BRIGHTNESS].
In kbd_led_update_all(), refresh the local value from asus->kbd_led_wk
under asus_ref.lock before notifying userspace, so coalesced work items
emit the clamped state instead of the initial snapshot. Stable trees
need this because negative values break sysfs reporting and make the
brightness hotkeys appear dead until the counter climbs back to zero.

Fixes: 7525566abd360 ("platform/x86: asus-wmi: add keyboard brightness event handler")
Cc: stable@vger.kernel.org
Cc: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b33ebdb21aa21..2478361b9b531 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1807,8 +1807,10 @@ static void kbd_led_update_all(struct work_struct *work)
 	if (value >= 0)
 		do_kbd_led_set(&asus->kbd_led, value);
 	if (notify) {
-		scoped_guard(spinlock_irqsave, &asus_ref.lock)
+		scoped_guard(spinlock_irqsave, &asus_ref.lock) {
 			asus->kbd_led_notify = false;
+			value = asus->kbd_led_wk;
+		}
 		led_classdev_notify_brightness_hw_changed(&asus->kbd_led, value);
 	}
 }
@@ -1961,7 +1963,7 @@ static int kbd_led_set(struct led_classdev *led_cdev, enum led_brightness value)
 static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
 {
 	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
-		asus->kbd_led_wk = value;
+		asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
 		asus->kbd_led_notify = true;
 	}
 	queue_work(asus->led_workqueue, &asus->kbd_led_work);

-- 
2.56.0

[PATCH 2/2] platform/x86: asus-wmi: unify WMI and HID keyboard brightness stepping

[Bartu Alev]

WMI notifications and USB HID event passes calculate brightness steps in
two separate functions: asus_wmi_handle_event_code() and asus_hid_event().
The WMI path reads asus->kbd_led_wk under asus_ref.lock in
asus_wmi_handle_event_code(), drops the lock, and then calls
kbd_led_set_by_kbd(), which re-acquires asus_ref.lock. A concurrent event
arriving between the two critical sections can overwrite the newer state.

Consolidate the step arithmetic into a helper, asus_kbd_led_calc_next(),
and add asus_kbd_led_step() to calculate and store the new brightness
level in a single asus_ref.lock critical section. Route the WMI notify
handlers through this helper and remove kbd_led_set_by_kbd(). In
asus_hid_event(), reuse asus_kbd_led_calc_next() under the existing
critical section to keep stepping logic identical across endpoints.

The step logic remains unchanged; locking is consolidated into a single
critical section.

Cc: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 75 ++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 2478361b9b531..533c50c1dba6e 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1815,6 +1815,39 @@ static void kbd_led_update_all(struct work_struct *work)
 	}
 }
 
+static inline int asus_kbd_led_calc_next(int cur, enum asus_hid_event event)
+{
+	switch (event) {
+	case ASUS_EV_BRTUP:
+		cur += 1;
+		break;
+	case ASUS_EV_BRTDOWN:
+		cur -= 1;
+		break;
+	case ASUS_EV_BRTTOGGLE:
+		cur = (cur >= ASUS_EV_MAX_BRIGHTNESS) ? 0 : cur + 1;
+		break;
+	default:
+		return cur;
+	}
+
+	return clamp_val(cur, 0, ASUS_EV_MAX_BRIGHTNESS);
+}
+
+static int asus_kbd_led_step(struct asus_wmi *asus, enum asus_hid_event event)
+{
+	if (!asus)
+		return -ENODEV;
+
+	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
+		asus->kbd_led_wk = asus_kbd_led_calc_next(asus->kbd_led_wk, event);
+		asus->kbd_led_notify = true;
+	}
+
+	queue_work(asus->led_workqueue, &asus->kbd_led_work);
+	return 0;
+}
+
 /*
  * This function is called from hid-asus to inform asus-wmi of brightness
  * changes initiated by the keyboard backlight keys.
@@ -1822,31 +1855,13 @@ static void kbd_led_update_all(struct work_struct *work)
 int asus_hid_event(enum asus_hid_event event)
 {
 	struct asus_wmi *asus;
-	int brightness;
 
 	guard(spinlock_irqsave)(&asus_ref.lock);
 	asus = asus_ref.asus;
 	if (!asus || !asus->kbd_led_registered)
 		return -EBUSY;
 
-	brightness = asus->kbd_led_wk;
-
-	switch (event) {
-	case ASUS_EV_BRTUP:
-		brightness += 1;
-		break;
-	case ASUS_EV_BRTDOWN:
-		brightness -= 1;
-		break;
-	case ASUS_EV_BRTTOGGLE:
-		if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
-			brightness = 0;
-		else
-			brightness += 1;
-		break;
-	}
-
-	asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
+	asus->kbd_led_wk = asus_kbd_led_calc_next(asus->kbd_led_wk, event);
 	asus->kbd_led_notify = true;
 	queue_work(asus->led_workqueue, &asus->kbd_led_work);
 	return 0;
@@ -1960,15 +1975,6 @@ static int kbd_led_set(struct led_classdev *led_cdev, enum led_brightness value)
 	return 0;
 }
 
-static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
-{
-	scoped_guard(spinlock_irqsave, &asus_ref.lock) {
-		asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
-		asus->kbd_led_notify = true;
-	}
-	queue_work(asus->led_workqueue, &asus->kbd_led_work);
-}
-
 static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
 {
 	struct asus_wmi *asus;
@@ -4679,7 +4685,6 @@ static int asus_wmi_get_event_code(union acpi_object *obj)
 
 static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 {
-	enum led_brightness led_value;
 	unsigned int key_value = 1;
 	bool autorelease = 1;
 
@@ -4696,22 +4701,16 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
-	scoped_guard(spinlock_irqsave, &asus_ref.lock)
-		led_value = asus->kbd_led_wk;
-
 	if (code == NOTIFY_KBD_BRTUP) {
-		kbd_led_set_by_kbd(asus, led_value + 1);
+		asus_kbd_led_step(asus, ASUS_EV_BRTUP);
 		return;
 	}
 	if (code == NOTIFY_KBD_BRTDWN) {
-		kbd_led_set_by_kbd(asus, led_value - 1);
+		asus_kbd_led_step(asus, ASUS_EV_BRTDOWN);
 		return;
 	}
 	if (code == NOTIFY_KBD_BRTTOGGLE) {
-		if (led_value >= ASUS_EV_MAX_BRIGHTNESS)
-			kbd_led_set_by_kbd(asus, 0);
-		else
-			kbd_led_set_by_kbd(asus, led_value + 1);
+		asus_kbd_led_step(asus, ASUS_EV_BRTTOGGLE);
 		return;
 	}
 

-- 
2.56.0