[PATCH v2 0/3] HID: asus: improve the driver support for laptops

[PATCH v2 0/3] HID: asus: improve the driver support for laptops

[Denis Benato]

Hi all,

Antheas has requested reviewing the XG mobile patch on this cycle so
I am re-submotting a modified version that is a bit cleaner because
it uses LED_CORE_SUSPENDRESUME instead of re-sending the LED state
manually on resume, but it's functionally equivalent.

I also included a patch to fix a pre-existing issue that was flagged
by sashiko-bot, albeit I'm not entirely sure if it can really happen
in practice, though it is safer to handle it correctly given the
actual code change is so small and leaving it documented helps future
changes.

I also moved some code to reduce nesting and improve the resume path
on ROG ally and other N-KEY devices.

Best regards,
Denis Benato

Link v1: https://lore.kernel.org/all/20260813144736.2477941-1-denis.benato@linux.dev/

Changelog:

-v2
  - HID: asus: add support for xgm led
    - Solve an impossible error reported by sashiko-bot regarding teardown on error

Denis Benato (3):
  HID: asus: document and harden the worker teardown
  HID: asus: reinitialize the device after exiting a sleep state
  HID: asus: add support for xgm led

 drivers/hid/hid-asus.c | 148 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 138 insertions(+), 10 deletions(-)

-- 
2.47.3

[PATCH v2 1/3] HID: asus: document and harden the worker teardown

[Denis Benato]

asus_work() executes actions against the device: they send feature
reports with hid_hw_raw_request() and, for the Fn+F5 fan key fallback,
re-inject the raw report into the HID core with hid_report_raw_event().

For this reason the worker must be quiesced before hid_hw_stop(): after
the low level driver has stopped, the transport is gone (usbhid_stop()
frees the URBs and the I/O buffers) and re-injected reports would race
against the input devices being unregistered by hid_disconnect().

Make so that the teardown cannot race to a use-after-free: every site
queueing an action holds worker->lock across the .removed check, the
list insertion and schedule_work(), and asus_worker_stop() sets .removed
and drains the queue under that same lock before calling
cancel_work_sync(). An action that passed the check is caught by the
latter while any later attempt is discarded by asus_worker_schedule().

Closes: https://lore.kernel.org/all/20260908180032.34C2D1F00A3A@smtp.kernel.org/
Fixes: 47669bec44fe ("HID: asus: refactor the two workqueues and init sequence")
Assisted-by: zcode:glm-5.3-flash
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index bd46aba6622a..3a8b8b7e90f7 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -765,9 +765,14 @@ static void asus_work(struct work_struct *work)
 	struct asus_work_action *action = NULL;
 	unsigned long flags;
 
-	/* Save the action to be performed and clear the flag */
+	/*
+	 * Dequeue the next action, if any. Once teardown has begun .removed
+	 * is set and asus_worker_stop() drains the queue: leave the queued
+	 * actions alone, they are dropped instead of being executed against
+	 * a device that is being removed.
+	 */
 	spin_lock_irqsave(&worker->lock, flags);
-	if (!list_empty(&worker->actions)) {
+	if (!worker->removed && !list_empty(&worker->actions)) {
 		action = list_first_entry(&worker->actions,
 					  struct asus_work_action, node);
 		list_del(&action->node);
@@ -817,6 +822,25 @@ static int asus_worker_create(struct hid_device *hdev, struct asus_drvdata *drvd
 	return 0;
 }
 
+/**
+ * asus_worker_stop - quiesce the worker
+ * @worker: the worker to quiesce
+ *
+ * Once this function returns no more actions can be queued and no instance
+ * of asus_work() is running or pending.
+ *
+ * Callers must do this before hid_hw_stop(): actions are executed while the
+ * device is fully operational, since they send raw requests to it and, in
+ * the fan-key fallback path, re-inject raw reports into the HID core. After
+ * hid_hw_stop() the transport is gone (usbhid_stop() frees the URBs and the
+ * I/O buffers) and the input devices have been unregistered.
+ *
+ * The quiescing is race free because every site that queues an action holds
+ * worker->lock across the .removed check, the list insertion and
+ * schedule_work(): anything scheduled before .removed is set here is caught
+ * by the cancel_work_sync() below, anything after it is discarded by
+ * asus_worker_schedule().
+ */
 static void asus_worker_stop(struct asus_worker *worker)
 {
 	struct asus_work_action *action, *tmp;
-- 
2.47.3

[PATCH v2 2/3] HID: asus: reinitialize the device after exiting a sleep state

[Denis Benato]

The ROG ally needs to have the EC string sent back after resuming from
s2idle since the USB device can be turned completely off by the firmware
when mcu_powersave firmware-attribute is set to 1.

This may also be true for other laptops and certain features might stop
working after the device exit from sleep.

Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 3a8b8b7e90f7..03150d29eec5 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1384,6 +1384,28 @@ static int asus_start_multitouch(struct hid_device *hdev)
 	return 0;
 }
 
+/*
+ * Initialize the reports of the device.
+ *
+ * Failures are intentionally not fatal: asus_kbd_init() tolerates a wrong
+ * handshake until this is verified to work for all devices, so a failure
+ * is only reported and the initialization of the remaining reports is
+ * still attempted.
+ */
+static void asus_initialize_reports(struct hid_device *hdev)
+{
+	int ret;
+
+	for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) {
+		if (asus_has_report_id(hdev, asus_report_id_init[r])) {
+			ret = asus_kbd_init(hdev, asus_report_id_init[r]);
+			if (ret < 0)
+				hid_warn(hdev, "Failed to initialize 0x%x: %d.\n",
+					 asus_report_id_init[r], ret);
+		}
+	}
+}
+
 static int __maybe_unused asus_resume(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1403,6 +1425,9 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
 
+	if (!drvdata->tp)
+		asus_initialize_reports(hdev);
+
 	if (drvdata->tp)
 		return asus_start_multitouch(hdev);
 
@@ -1517,16 +1542,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
-	if (!drvdata->tp) {
-		for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) {
-			if (asus_has_report_id(hdev, asus_report_id_init[r])) {
-				ret = asus_kbd_init(hdev, asus_report_id_init[r]);
-				if (ret < 0)
-					hid_warn(hdev, "Failed to initialize 0x%x: %d.\n",
-						 asus_report_id_init[r], ret);
-			}
-		}
-	}
+	if (!drvdata->tp)
+		asus_initialize_reports(hdev);
 
 	/* Laptops keyboard backlight is always at 0x5a */
 	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
-- 
2.47.3

[PATCH v2 3/3] HID: asus: add support for xgm led

[Denis Benato]

XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.

Let the led core manage the power transitions: the classdev is flagged
with LED_CORE_SUSPENDRESUME, so it is switched off at suspend and its
last brightness is restored at resume. The EC drives its own blinking
pattern during s2idle anyway, so the led state while the machine is
asleep is not meaningful.

Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 87 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 03150d29eec5..27c1c3188513 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
+#define ROG_XGM_REPORT_SIZE 300
+
 #define ROG_ALLY_REPORT_SIZE 64
 #define ROG_ALLY_X_MIN_MCU 313
 #define ROG_ALLY_MIN_MCU 319
@@ -144,6 +146,11 @@ struct asus_worker {
 	bool removed;
 };
 
+struct asus_xgm_led {
+	struct led_classdev cdev;
+	struct hid_device *hdev;
+};
+
 struct asus_touchpad_info {
 	int max_x;
 	int max_y;
@@ -170,6 +177,7 @@ struct asus_drvdata {
 	unsigned long battery_next_query;
 	struct asus_hid_listener listener;
 	bool fn_lock;
+	struct asus_xgm_led *xgm_led;
 };
 
 static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -1161,6 +1169,26 @@ static int asus_battery_probe(struct hid_device *hdev)
 	return ret;
 }
 
+static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+	const u8 buf[ROG_XGM_REPORT_SIZE] = {
+		FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
+	};
+	struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
+	int ret;
+
+	ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
+	if (ret < 0) {
+		hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
+		return ret;
+	} else if (ret != ROG_XGM_REPORT_SIZE) {
+		hid_err(xgm->hdev, "Unexpected partial transfer to XG mobile: %d\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 {
 	struct input_dev *input = hi->input;
@@ -1406,6 +1434,49 @@ static void asus_initialize_reports(struct hid_device *hdev)
 	}
 }
 
+static int asus_xgm_init(struct hid_device *hdev, struct asus_drvdata *drvdata)
+{
+	const char *name;
+	int ret;
+
+	drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
+	if (drvdata->xgm_led == NULL)
+		return -ENOMEM;
+
+	name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "asus:xgm-%s:led",
+			      strlen(hdev->uniq) ? hdev->uniq : dev_name(&hdev->dev));
+
+	if (name == NULL) {
+		ret = -ENOMEM;
+		goto asus_xgm_init_err;
+	}
+
+	drvdata->xgm_led->hdev = hdev;
+	drvdata->xgm_led->cdev.name = name;
+	drvdata->xgm_led->cdev.brightness = 1;
+	drvdata->xgm_led->cdev.max_brightness = 1;
+	drvdata->xgm_led->cdev.brightness_set_blocking = asus_xgm_led_set;
+	drvdata->xgm_led->cdev.flags = LED_CORE_SUSPENDRESUME;
+
+	/* LED state is arbitrary on boot, set a default */
+	ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+	if (ret) {
+		hid_err(hdev, "Asus failed to set xgm led: %d\n", ret);
+		goto asus_xgm_init_err;
+	}
+
+	ret = devm_led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
+	if (ret) {
+		hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
+		goto asus_xgm_init_err;
+	}
+
+	return 0;
+asus_xgm_init_err:
+	drvdata->xgm_led = NULL;
+	return ret;
+}
+
 static int __maybe_unused asus_resume(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
@@ -1545,6 +1616,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (!drvdata->tp)
 		asus_initialize_reports(hdev);
 
+	if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
+	    ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
+	     (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
+		ret = asus_xgm_init(hdev, drvdata);
+		if (ret) {
+			hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
+			goto err_stop_hw;
+		}
+	}
+
 	/* Laptops keyboard backlight is always at 0x5a */
 	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
 	    (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
@@ -1582,6 +1663,9 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (drvdata->listener.brightness_set)
 		asus_hid_unregister_listener(&drvdata->listener);
 
+	if (drvdata->xgm_led)
+		devm_led_classdev_unregister(&hdev->dev, &drvdata->xgm_led->cdev);
+
 	asus_worker_stop(drvdata->worker);
 	hid_hw_stop(hdev);
 	return ret;
@@ -1594,6 +1678,9 @@ static void asus_remove(struct hid_device *hdev)
 	if (drvdata->listener.brightness_set)
 		asus_hid_unregister_listener(&drvdata->listener);
 
+	if (drvdata->xgm_led)
+		devm_led_classdev_unregister(&hdev->dev, &drvdata->xgm_led->cdev);
+
 	asus_worker_stop(drvdata->worker);
 	hid_hw_stop(hdev);
 }
-- 
2.47.3