* [PATCH v2 01/12] HID: asus: reinitialize the device after exiting a sleep state
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 19:58 ` [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (10 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, 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 | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index ec966fc0a411..9ce21fcc7d2e 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1360,6 +1360,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);
@@ -1379,6 +1401,8 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ asus_initialize_reports(hdev);
+
if (drvdata->tp)
return asus_start_multitouch(hdev);
@@ -1493,14 +1517,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
return 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);
- }
- }
+ asus_initialize_reports(hdev);
/* Laptops keyboard backlight is always at 0x5a */
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
2026-08-19 19:58 ` [PATCH v2 01/12] HID: asus: reinitialize the device after exiting a sleep state Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:11 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 03/12] HID: asus: add gamepad configuration Denis Benato
` (9 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
Implement the core functionality for ASUS handhelds:
- extend functionality of hid-asus if driver is enabled
- initialise the device to a "ready" state
- implement Ally X input mapping
- emit a single proper event for AC button long press
Assisted-by: opencode:glm-5.2
Assisted-by: Claude:claude-fable-5
Signed-off-by: Khamunetri Clark <khamunetriclark@gmail.com>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Matthew Schwartz <matthew.schwartz@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
drivers/hid/hid-asus.c | 815 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 814 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 9ce21fcc7d2e..88b30726f6c6 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -24,12 +24,18 @@
#include <linux/cleanup.h>
#include <linux/dmi.h>
#include <linux/hid.h>
+#include <linux/jiffies.h>
+#include <linux/list.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_data/x86/asus-wmi.h>
+#include <linux/types.h>
#include <linux/input/mt.h>
#include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
#include <linux/power_supply.h>
+#include <linux/stddef.h>
#include <linux/leds.h>
+#include <linux/unaligned.h>
#include "hid-ids.h"
@@ -37,6 +43,9 @@ MODULE_AUTHOR("Yusuke Fujimaki <usk.fujimaki@gmail.com>");
MODULE_AUTHOR("Brendan McGrath <redmcg@redmandi.dyndns.org>");
MODULE_AUTHOR("Victor Vlasenko <victor.vlasenko@sysgears.com>");
MODULE_AUTHOR("Frederik Wenigwieser <frederik.wenigwieser@gmail.com>");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Luke Jones <luke@ljones.dev>");
+MODULE_AUTHOR("Khamunetri Clark <khamunetriclark@gmail.com>");
MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define T100_TPAD_INTF 2
@@ -55,6 +64,19 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define ROG_ALLY_X_MIN_MCU 313
#define ROG_ALLY_MIN_MCU 319
+#define HID_ALLY_INTF_KEYBOARD_IN 0x81
+#define HID_ALLY_INTF_CFG_IN 0x83
+#define HID_ALLY_X_INTF_IN 0x87
+
+#define HID_ALLY_GET_REPORT_ID 0x0D
+#define HID_ALLY_SET_REPORT_ID 0x5A
+#define HID_ALLY_FEATURE_CODE_PAGE 0xD1
+
+#define HID_ALLY_X_INPUT_REPORT_SIZE 16
+#define HID_ALLY_X_INPUT_REPORT 0x0B
+
+#define HID_ALLY_READY_MAX_TRIES 6
+
/* Spurious HID codes sent by QUIRK_ROG_NKEY_KEYBOARD devices */
#define ASUS_SPURIOUS_CODE_0XEA 0xea
#define ASUS_SPURIOUS_CODE_0XEC 0xec
@@ -154,6 +176,23 @@ struct asus_touchpad_info {
int report_size;
};
+struct ally_handheld {
+ /* All read/write to IN interfaces must lock */
+ struct mutex intf_mutex;
+ struct hid_device *cfg_hdev;
+
+ struct input_dev *ally_x_input;
+ struct hid_device *ally_x_hdev;
+
+ struct hid_device *keyboard_hdev;
+ struct input_dev *keyboard_input;
+
+ u8 cad_sequence_state;
+ unsigned long cad_last_event_time;
+
+ struct delayed_work resume_work;
+};
+
struct asus_drvdata {
unsigned long quirks;
struct hid_device *hdev;
@@ -161,6 +200,7 @@ struct asus_drvdata {
struct input_dev *tp_kbd_input;
struct asus_worker *worker;
unsigned int kbd_backlight_brightness;
+ struct ally_handheld *rog_ally;
const struct asus_touchpad_info *tp;
struct power_supply *battery;
struct power_supply_desc battery_desc;
@@ -232,12 +272,735 @@ static const struct asus_touchpad_info medion_e1239t_tp = {
.report_size = 32 /* 2 byte header + 5 * 5 + 5 byte footer */,
};
+enum ally_command_codes {
+ CMD_SET_GAMEPAD_MODE = 0x01,
+ CMD_SET_MAPPING = 0x02,
+ CMD_SET_JOYSTICK_MAPPING = 0x03,
+ CMD_SET_JOYSTICK_DEADZONE = 0x04,
+ CMD_SET_TRIGGER_RANGE = 0x05,
+ CMD_SET_VIBRATION_INTENSITY = 0x06,
+ CMD_LED_CONTROL = 0x08,
+ CMD_CHECK_READY = 0x0A,
+ CMD_SET_XBOX_CONTROLLER = 0x0B,
+ CMD_CHECK_XBOX_SUPPORT = 0x0C,
+ CMD_USER_CAL_DATA = 0x0D,
+ CMD_CHECK_USER_CAL_SUPPORT = 0x0E,
+ CMD_SET_TURBO_PARAMS = 0x0F,
+ CMD_CHECK_TURBO_SUPPORT = 0x10,
+ CMD_CHECK_RESP_CURVE_SUPPORT = 0x12,
+ CMD_SET_RESP_CURVE = 0x13,
+ CMD_CHECK_DIR_TO_BTN_SUPPORT = 0x14,
+ CMD_SET_GYRO_PARAMS = 0x15,
+ CMD_CHECK_GYRO_TO_JOYSTICK = 0x16,
+ CMD_CHECK_ANTI_DEADZONE = 0x17,
+ CMD_SET_ANTI_DEADZONE = 0x18,
+};
+
+static const u8 ALLY_FORCE_FEEDBACK_OFF[] = {
+ 0x0D, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xEB
+};
+
+/*
+ * The ROG Ally device presents multiple USB interfaces (keyboard, mouse, gamepad,
+ * and custom configuration interface) that bind to the same module. Since only
+ * one ROG Ally device can be connected at a time, we use a single global static
+ * ally_handheld structure to share state across these separate HID interfaces.
+ */
+static void ally_resume_work_fn(struct work_struct *work);
+
+/*
+ * Changes to ally_drvdata must lock: the raw_event callbacks, which may
+ * run in atomic (URB completion) context, also take this lock, so it must
+ * be a spinlock.
+ */
+static DEFINE_SPINLOCK(ally_data_lock);
+static struct ally_handheld ally_drvdata = {
+ .intf_mutex = __MUTEX_INITIALIZER(ally_drvdata.intf_mutex),
+ /*
+ * Initialised statically so it is always safe to cancel, whichever
+ * of the interfaces probed or failed to probe.
+ */
+ .resume_work = __DELAYED_WORK_INITIALIZER(ally_drvdata.resume_work,
+ ally_resume_work_fn, 0),
+};
+
static const u8 asus_report_id_init[] = {
FEATURE_KBD_REPORT_ID,
FEATURE_KBD_LED_REPORT_ID1,
FEATURE_KBD_LED_REPORT_ID2
};
+static inline int ally_dev_set_report(struct hid_device *hdev, const u8 *buf, size_t len)
+{
+ u8 *dmabuf __free(kfree) = kmemdup(buf, len, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ return hid_hw_raw_request(hdev, buf[0], dmabuf, len,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+}
+
+static inline int ally_dev_get_report(struct hid_device *hdev, u8 *out, size_t len)
+{
+ return hid_hw_raw_request(hdev, HID_ALLY_GET_REPORT_ID, out, len,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+}
+
+static void ally_resume_work_fn(struct work_struct *work)
+{
+ struct ally_handheld *ally = container_of(work, struct ally_handheld,
+ resume_work.work);
+ struct input_dev *keyboard_input, *x_input;
+ unsigned long flags;
+
+ /*
+ * Snapshot the very pointers that get dereferenced under the lock
+ * protecting them: probe sets keyboard_hdev even when the interface
+ * exposes no input_dev, and removal clears the two fields one after
+ * the other.
+ */
+ spin_lock_irqsave(&ally_data_lock, flags);
+ keyboard_input = ally->keyboard_input;
+ x_input = ally->ally_x_input;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ /*
+ * Force release all vendor buttons to prevent "stuck" ghosting on
+ * resume (workaround for Ally X USB re-probing during suspend/resume).
+ */
+ if (keyboard_input) {
+ input_report_key(keyboard_input, KEY_F16, 0);
+ input_report_key(keyboard_input, KEY_F17, 0);
+ input_report_key(keyboard_input, KEY_F18, 0);
+ input_report_key(keyboard_input, KEY_F19, 0);
+ input_report_key(keyboard_input, KEY_F20, 0);
+ input_report_key(keyboard_input, KEY_PROG1, 0);
+ input_sync(keyboard_input);
+ }
+
+ if (x_input) {
+ input_report_key(x_input, KEY_F16, 0);
+ input_report_key(x_input, KEY_F17, 0);
+ input_report_key(x_input, KEY_F18, 0);
+ input_report_key(x_input, KEY_PROG1, 0);
+ input_sync(x_input);
+ }
+}
+
+/**
+ * handle_ctrl_alt_del() - detect a left button long press
+ * @hdev: HID device the report arrived on
+ * @ally: ally handheld structure holding the sequence state
+ * @data: raw report buffer, rewritten in place when the sequence matches
+ * @size: length of @data in bytes
+ *
+ * The Ally left button emits a sequence of ctrl+alt+del events. Capture that
+ * and emit only a single code for that single event.
+ *
+ * Return: true iff the event has been managed
+ */
+static bool handle_ctrl_alt_del(struct hid_device *hdev,
+ struct ally_handheld *ally, u8 *data, int size)
+{
+ bool time_is_past = time_after(jiffies, ally->cad_last_event_time + msecs_to_jiffies(100));
+
+ if (size < 16 || data[0] != 0x01)
+ return false;
+
+ if (ally->cad_sequence_state > 0 && time_is_past)
+ ally->cad_sequence_state = 0;
+
+ ally->cad_last_event_time = jiffies;
+
+ switch (ally->cad_sequence_state) {
+ case 0:
+ if (data[1] == 0x01 && data[2] == 0x00 && data[3] == 0x00) {
+ ally->cad_sequence_state = 1;
+ data[1] = 0x00;
+ return true;
+ }
+ break;
+ case 1:
+ if (data[1] == 0x05 && data[2] == 0x00 && data[3] == 0x00) {
+ ally->cad_sequence_state = 2;
+ data[1] = 0x00;
+ return true;
+ }
+ break;
+ case 2:
+ if (data[1] == 0x05 && data[2] == 0x00 && data[3] == 0x4c) {
+ ally->cad_sequence_state = 3;
+ data[1] = 0x00;
+ data[3] = 0x6F; // F20;
+ return true;
+ }
+ break;
+ case 3:
+ if (data[1] == 0x04 && data[2] == 0x00 && data[3] == 0x4c) {
+ ally->cad_sequence_state = 4;
+ data[1] = data[3] = 0x00;
+ return true;
+ }
+ break;
+ case 4:
+ if (data[1] == 0x00 && data[2] == 0x00 && data[3] == 0x4c) {
+ ally->cad_sequence_state = 5;
+ data[3] = 0x00;
+ return true;
+ }
+ break;
+ }
+ ally->cad_sequence_state = 0;
+ return false;
+}
+
+static bool handle_ally_event(struct hid_device *hdev, struct ally_handheld *ally,
+ u8 *data, int size)
+{
+ struct input_dev *keyboard_input;
+ unsigned long flags;
+ int keycode = 0;
+
+ if (size < 2)
+ return false;
+
+ if (data[0] == 0x5A) {
+ switch (data[1]) {
+ case 0x38:
+ keycode = KEY_F19;
+ break;
+ case 0xA6:
+ keycode = KEY_F16;
+ break;
+ case 0xA7:
+ keycode = KEY_F17;
+ break;
+ case 0xA8:
+ keycode = KEY_F18;
+ break;
+ default:
+ return false;
+ }
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ keyboard_input = ally->keyboard_input;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ if (!keyboard_input)
+ return false;
+
+ input_report_key(keyboard_input, keycode, 1);
+ input_sync(keyboard_input);
+ input_report_key(keyboard_input, keycode, 0);
+ input_sync(keyboard_input);
+ return true;
+ }
+ return false;
+}
+
+/**
+ * ally_gamepad_send_packet() - Send a raw packet to the gamepad device
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ * @buf: buffer containing the packet data
+ * @len: length of data to send
+ *
+ * Return: count of data transferred, negative if error
+ */
+static int ally_gamepad_send_packet(struct ally_handheld *ally,
+ struct hid_device *hdev, const u8 *buf, size_t len)
+{
+ scoped_guard(mutex, &ally->intf_mutex)
+ return ally_dev_set_report(hdev, buf, len);
+}
+
+/**
+ * ally_gamepad_send_receive_packet() - Send a packet and receive the response
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ * @buf: buffer containing the packet data to send and receive response in
+ * @len: length of buffer
+ *
+ * Return: count of data transferred, negative if error
+ */
+static int ally_gamepad_send_receive_packet(struct ally_handheld *ally,
+ struct hid_device *hdev,
+ u8 *buf, size_t len)
+{
+ int ret;
+
+ scoped_guard(mutex, &ally->intf_mutex) {
+ ret = ally_dev_set_report(hdev, buf, len);
+ if (ret >= 0) {
+ memset(buf, 0, len);
+ ret = ally_dev_get_report(hdev, buf, len);
+ }
+ }
+
+ return ret;
+}
+
+/**
+ * ally_alloc_cmd() - Construct a command buffer for the gamepad
+ * @cmd: Command code to send
+ * @payload: Optional payload data to include in the command
+ * @payload_size: Size of the payload data
+ *
+ * The constructed buffer is 64 bytes long, and it is the caller
+ * responsibility to free the buffer using kfree().
+ *
+ * Return: the newly allocated buffer containing the command, or NULL on
+ * allocation failure
+ */
+static u8 *ally_alloc_cmd(u8 cmd, const u8 *payload, u8 payload_size)
+{
+ u8 *hidbuf = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+
+ if (!hidbuf)
+ return NULL;
+
+ hidbuf[0] = HID_ALLY_SET_REPORT_ID;
+ hidbuf[1] = HID_ALLY_FEATURE_CODE_PAGE;
+ hidbuf[2] = cmd;
+ hidbuf[3] = payload_size;
+
+ if (payload_size > 0 && payload)
+ memcpy(&hidbuf[4], payload, payload_size);
+
+ return hidbuf;
+}
+
+/**
+ * ally_gamepad_check_ready() - Wait for the gamepad MCU to report ready
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ *
+ * This should be called before any remapping attempts, and on driver
+ * init/resume, after the asus handshake has been performed on the
+ * configuration endpoint.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_gamepad_check_ready(struct ally_handheld *ally, struct hid_device *hdev)
+{
+ u8 payload[] = { 0x00 };
+ int ret;
+
+ for (int i = 0; i < HID_ALLY_READY_MAX_TRIES; i++) {
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_CHECK_READY, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_gamepad_send_receive_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_dbg(hdev, "ROG Ally check %d/%d failed: %d\n", i,
+ HID_ALLY_READY_MAX_TRIES, ret);
+ continue;
+ }
+
+ if (buf[2] == CMD_CHECK_READY)
+ return 0;
+
+ usleep_range(1000, 2000);
+ }
+
+ hid_err(hdev, "ROG Ally never responded with a ready\n");
+ return -ENODEV;
+}
+
+static int ally_get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return -ENODEV;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ if (!intf || !intf->cur_altsetting)
+ return -ENODEV;
+
+ ep = intf->cur_altsetting->endpoint;
+ if (!ep)
+ return -ENODEV;
+
+ return ep->desc.bEndpointAddress;
+}
+
+struct ally_x_input_report {
+ __le16 x, y;
+ __le16 rx, ry;
+ __le16 z, rz;
+ u8 buttons[3];
+} __packed;
+
+/* The hatswitch outputs integers, we use them to index this X|Y pair */
+static const int hat_values[][2] = {
+ { 0, 0 }, { 0, -1 }, { 1, -1 }, { 1, 0 }, { 1, 1 },
+ { 0, 1 }, { -1, 1 }, { -1, 0 }, { -1, -1 },
+};
+
+/**
+ * ally_x_raw_event() - Parse and report an Ally X gamepad input report
+ * @input: input device to report the events on
+ * @hdev: HID device
+ * @report: HID report the raw data arrived in
+ * @data: raw report buffer
+ * @size: length of @data in bytes
+ *
+ * Return: true if the event was handled, otherwise false
+ */
+static bool ally_x_raw_event(struct input_dev *input, struct hid_device *hdev,
+ struct hid_report *report, u8 *data, int size)
+{
+ struct ally_x_input_report *in_report;
+ u16 x, y, rx, ry, z, rz;
+ u8 byte;
+
+ if (!input)
+ return false;
+
+ if (size < 1)
+ return false;
+
+ /*
+ * The size check above only guarantees one valid byte: do not
+ * read the code of a truncated report.
+ */
+ if (data[0] == 0x5A) {
+ if (size < 2)
+ return false;
+
+ input_report_key(input, KEY_PROG1, data[1] == 0x38);
+ input_report_key(input, KEY_F16, data[1] == 0xA6);
+ input_report_key(input, KEY_F17, data[1] == 0xA7);
+ input_report_key(input, KEY_F18, data[1] == 0xA8);
+ input_sync(input);
+
+ return data[1] == 0xA6 || data[1] == 0xA7 || data[1] == 0xA8 || data[1] == 0x38;
+ }
+
+ if (data[0] != HID_ALLY_X_INPUT_REPORT)
+ return false;
+
+ /*
+ * hid-core only guarantees size >= 1 and does not zero-pad short
+ * reports before ->raw_event, so a truncated transfer would leave the
+ * payload below pointing at stale DMA buffer contents.
+ */
+ if (size < 1 + sizeof(*in_report))
+ return false;
+
+ in_report = (struct ally_x_input_report *)&data[1];
+
+ /* USB HID payloads are little-endian: convert them explicitly. */
+ x = get_unaligned_le16(&in_report->x);
+ y = get_unaligned_le16(&in_report->y);
+ rx = get_unaligned_le16(&in_report->rx);
+ ry = get_unaligned_le16(&in_report->ry);
+ z = get_unaligned_le16(&in_report->z);
+ rz = get_unaligned_le16(&in_report->rz);
+
+ input_report_abs(input, ABS_X, x - 32768);
+ input_report_abs(input, ABS_Y, y - 32768);
+ input_report_abs(input, ABS_RX, rx - 32768);
+ input_report_abs(input, ABS_RY, ry - 32768);
+ input_report_abs(input, ABS_Z, z);
+ input_report_abs(input, ABS_RZ, rz);
+
+ byte = in_report->buttons[0];
+ input_report_key(input, BTN_A, byte & BIT(0));
+ input_report_key(input, BTN_B, byte & BIT(1));
+ input_report_key(input, BTN_X, byte & BIT(2));
+ input_report_key(input, BTN_Y, byte & BIT(3));
+ input_report_key(input, BTN_TL, byte & BIT(4));
+ input_report_key(input, BTN_TR, byte & BIT(5));
+ input_report_key(input, BTN_SELECT, byte & BIT(6));
+ input_report_key(input, BTN_START, byte & BIT(7));
+
+ byte = in_report->buttons[1];
+ input_report_key(input, BTN_THUMBL, byte & BIT(0));
+ input_report_key(input, BTN_THUMBR, byte & BIT(1));
+ input_report_key(input, BTN_MODE, byte & BIT(2));
+
+ /* The hatswitch byte is device-controlled; treat anything the table
+ * does not cover as centred rather than indexing out of bounds.
+ */
+ byte = in_report->buttons[2];
+ if (byte >= ARRAY_SIZE(hat_values))
+ byte = 0;
+ input_report_abs(input, ABS_HAT0X, hat_values[byte][0]);
+ input_report_abs(input, ABS_HAT0Y, hat_values[byte][1]);
+
+ input_sync(input);
+
+ return true;
+}
+
+static struct input_dev *ally_x_alloc_input_dev(struct hid_device *hdev)
+{
+ struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
+
+ if (!input_dev)
+ return ERR_PTR(-ENOMEM);
+
+ input_dev->id.bustype = hdev->bus;
+ input_dev->id.vendor = hdev->vendor;
+ input_dev->id.product = hdev->product;
+ input_dev->id.version = hdev->version;
+ input_dev->uniq = hdev->uniq;
+ input_dev->name = "ASUS ROG Ally X Gamepad";
+
+ input_set_drvdata(input_dev, hdev);
+
+ return input_dev;
+}
+
+static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ struct input_dev *input = ally_x_alloc_input_dev(hdev);
+ unsigned long flags;
+ int ret;
+
+ if (IS_ERR(input))
+ return PTR_ERR(input);
+
+ input_set_abs_params(input, ABS_X, -32768, 32767, 0, 0);
+ input_set_abs_params(input, ABS_Y, -32768, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RX, -32768, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RY, -32768, 32767, 0, 0);
+ input_set_abs_params(input, ABS_Z, 0, 1023, 0, 0);
+ input_set_abs_params(input, ABS_RZ, 0, 1023, 0, 0);
+ input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
+ input_set_capability(input, EV_KEY, BTN_A);
+ input_set_capability(input, EV_KEY, BTN_B);
+ input_set_capability(input, EV_KEY, BTN_X);
+ input_set_capability(input, EV_KEY, BTN_Y);
+ input_set_capability(input, EV_KEY, BTN_TL);
+ input_set_capability(input, EV_KEY, BTN_TR);
+ input_set_capability(input, EV_KEY, BTN_SELECT);
+ input_set_capability(input, EV_KEY, BTN_START);
+ input_set_capability(input, EV_KEY, BTN_MODE);
+ input_set_capability(input, EV_KEY, BTN_THUMBL);
+ input_set_capability(input, EV_KEY, BTN_THUMBR);
+
+ input_set_capability(input, EV_KEY, KEY_PROG1);
+ input_set_capability(input, EV_KEY, KEY_F16);
+ input_set_capability(input, EV_KEY, KEY_F17);
+ input_set_capability(input, EV_KEY, KEY_F18);
+ input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY);
+ input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY1);
+
+ ret = input_register_device(input);
+ if (ret) {
+ hid_err(hdev, "Failed to register Ally X gamepad device: %d\n", ret);
+ goto ally_x_setup_input_err;
+ }
+
+ /* Publish the input_dev only when it is fully set up. */
+ spin_lock_irqsave(&ally_data_lock, flags);
+ ally->ally_x_input = input;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ return 0;
+ally_x_setup_input_err:
+ return ret;
+}
+
+static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ int ret;
+
+ /*
+ * This function assumes the asus-specific initialization
+ * to have been performed already at this point.
+ */
+ ret = ally_gamepad_check_ready(ally, hdev);
+ if (ret < 0) {
+ hid_err(hdev, "ROG Ally device is not ready: %d\n", ret);
+ return ret;
+ }
+
+ /* Failure at this point is non-critical */
+ ret = ally_gamepad_send_packet(ally, hdev, ALLY_FORCE_FEEDBACK_OFF,
+ sizeof(ALLY_FORCE_FEEDBACK_OFF));
+ if (ret < 0)
+ hid_err(hdev, "Ally failed to init force-feedback off: %d\n", ret);
+
+ return 0;
+}
+
+/**
+ * hid_asus_ally_raw_event() - Route raw reports from the Ally interfaces
+ * @hdev: HID device
+ * @ally: ally handheld structure
+ * @report: HID report the raw data arrived in
+ * @data: raw report buffer
+ * @size: length of @data in bytes
+ *
+ * Called from the raw_event callback, which may run in atomic (URB
+ * completion) context: only spinlock-protected accesses to the shared
+ * state are allowed here.
+ *
+ * Return: true if the event was handled, otherwise false
+ */
+static bool hid_asus_ally_raw_event(struct hid_device *hdev, struct ally_handheld *ally,
+ struct hid_report *report, u8 *data, int size)
+{
+ struct input_dev *x_input;
+ struct hid_device *x_hdev;
+ unsigned long flags;
+
+ if (!ally)
+ return false;
+
+ switch (ally_get_endpoint_address(hdev)) {
+ case HID_ALLY_X_INTF_IN:
+ spin_lock_irqsave(&ally_data_lock, flags);
+ x_input = ally->ally_x_input;
+ x_hdev = ally->ally_x_hdev;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ if (ally_x_raw_event(x_input, x_hdev, report, data, size))
+ return true;
+ break;
+ case HID_ALLY_INTF_CFG_IN:
+ if (handle_ally_event(hdev, ally, data, size))
+ return true;
+ break;
+ case HID_ALLY_INTF_KEYBOARD_IN:
+ if (handle_ctrl_alt_del(hdev, ally, data, size))
+ return false;
+ break;
+ default:
+ break;
+ }
+
+ return false;
+}
+
+/**
+ * hid_asus_ally_probe() - Initialize the ROG Ally HID extension
+ * @hdev: HID device
+ *
+ * This module works alongside the main Asus HID driver to handle
+ * Ally-specific features and quirks.
+ *
+ * Return: an ally_handheld struct pointer on success, an ERR_PTR on
+ * failure. The caller is not expected to use the returned pointer, but it
+ * should check for errors by using IS_ERR and PTR_ERR and pass NULL to
+ * other functions if there was an error.
+ */
+static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
+{
+ unsigned long flags;
+ int ret, ep = ally_get_endpoint_address(hdev);
+ struct hid_input *hidinput;
+
+ if (ep < 0)
+ return ERR_PTR(ep);
+
+ /*
+ * The interface initialization sleeps (it performs USB transfers),
+ * so it must run before taking the spinlock guarding the shared
+ * state; pointers are published only once it succeeded.
+ */
+ switch (ep) {
+ case HID_ALLY_INTF_CFG_IN:
+ ret = hid_asus_ally_init(hdev, &ally_drvdata);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ ally_drvdata.cfg_hdev = hdev;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+ break;
+ case HID_ALLY_X_INTF_IN:
+ /* This will create and populate ally_x_input */
+ ret = ally_x_setup_input(hdev, &ally_drvdata);
+ if (ret) {
+ hid_err(hdev, "Failed to create Ally X gamepad device.\n");
+ return ERR_PTR(ret);
+ }
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ ally_drvdata.ally_x_hdev = hdev;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+ break;
+ case HID_ALLY_INTF_KEYBOARD_IN:
+ spin_lock_irqsave(&ally_data_lock, flags);
+ ally_drvdata.keyboard_hdev = hdev;
+ if (!list_empty(&hdev->inputs)) {
+ hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
+ ally_drvdata.keyboard_input = hidinput->input;
+ }
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+ break;
+ default:
+ /* This is normally supposed to happen */
+ break;
+ }
+
+ return &ally_drvdata;
+}
+
+static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ unsigned long flags;
+
+ if (!ally)
+ return;
+
+ /*
+ * Any of the three interfaces can own an input_dev the resume work
+ * reports through, and they are torn down in an arbitrary order, so
+ * drain it before clearing anything. Cancel outside ally_data_lock so
+ * a handler that wants the lock cannot deadlock against us.
+ */
+ cancel_delayed_work_sync(&ally->resume_work);
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ if (ally->ally_x_hdev == hdev) {
+ ally->ally_x_input = NULL;
+ ally->ally_x_hdev = NULL;
+ }
+
+ /*
+ * The keyboard interface is torn down before the config one, and
+ * its input_dev is freed with it. handle_ally_event() and
+ * ally_resume_work_fn() both report keys through it from the
+ * config endpoint, so drop the references here or they dangle.
+ */
+ if (ally->keyboard_hdev == hdev) {
+ ally->keyboard_input = NULL;
+ ally->keyboard_hdev = NULL;
+ }
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+}
+
+static int hid_asus_ally_reset_resume(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ int ep, ret;
+
+ /*
+ * The extensions failed to probe and the device is operating as a
+ * generic HID device: do not fail the resume because of that.
+ */
+ if (!ally)
+ return 0;
+
+ ep = ally_get_endpoint_address(hdev);
+ if (ep != HID_ALLY_INTF_CFG_IN)
+ return 0;
+
+ ret = hid_asus_ally_init(hdev, ally);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
/*
* Send events to asus-wmi driver for handling special keys
*/
@@ -505,6 +1268,17 @@ static int asus_raw_event(struct hid_device *hdev,
if (drvdata->quirks & QUIRK_MEDION_E1239T)
return asus_e1239t_event(drvdata, data, size);
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ /*
+ * Return -1 to suppress further processing by the generic HID
+ * input parser for reports we fully handle for the Gamepad (0x0B):
+ * letting 0x0B reach the default parser creates a generic gamepad
+ * causing Steam Input overlaps (i.e. L1 stuck on screenshot).
+ */
+ if (hid_asus_ally_raw_event(hdev, drvdata->rog_ally, report, data, size))
+ return -1;
+ }
+
/*
* Skip these report ID, the device emits a continuous stream associated
* with the AURA mode it is in which looks like an 'echo'.
@@ -1254,6 +2028,7 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0x5f: asus_map_key_clear(KEY_PROG2); break; /* S-shaped programmable key */
case 0x6b: asus_map_key_clear(KEY_F21); break; /* ASUS touchpad toggle */
case 0x38: asus_map_key_clear(KEY_PROG1); break; /* ROG key */
+ case 0x93: asus_map_key_clear(KEY_PROG1); break; /* ROG Ally X AC button */
case 0xba: asus_map_key_clear(KEY_PROG2); break; /* Fn+C ASUS Splendid */
case 0x5c: asus_map_key_clear(KEY_PROG3); break; /* Fn+Space Power4Gear */
case 0x99: asus_map_key_clear(KEY_PROG4); break; /* Fn+F5 "fan" symbol */
@@ -1266,7 +2041,7 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xa5: asus_map_key_clear(KEY_F15); break; /* ROG Ally left back */
case 0xa6: asus_map_key_clear(KEY_F16); break; /* ROG Ally QAM button */
case 0xa7: asus_map_key_clear(KEY_F17); break; /* ROG Ally ROG long-press */
- case 0xa8: asus_map_key_clear(KEY_F18); break; /* ROG Ally ROG long-press-release */
+ case 0xa8: asus_map_key_clear(KEY_F18); break;
default:
/* ASUS lazily declares 256 usages, ignore the rest,
@@ -1385,6 +2160,8 @@ static void asus_initialize_reports(struct hid_device *hdev)
static int __maybe_unused asus_resume(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ int ep;
/*
* If we have a backlight listener registered, restore the previous state,
@@ -1394,18 +2171,33 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
if (drvdata->listener.brightness_set)
asus_kbd_backlight_set(&drvdata->listener, drvdata->kbd_backlight_brightness);
+ if (ally && (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)) {
+ ep = ally_get_endpoint_address(hdev);
+ if (ep == HID_ALLY_INTF_CFG_IN)
+ schedule_delayed_work(&ally->resume_work, msecs_to_jiffies(500));
+ }
+
return 0;
}
static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
asus_initialize_reports(hdev);
if (drvdata->tp)
return asus_start_multitouch(hdev);
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ ret = hid_asus_ally_reset_resume(hdev, drvdata->rog_ally);
+ if (ret) {
+ hid_err(hdev, "Failed to resume ROG Ally HID extensions: %d\n", ret);
+ return ret;
+ }
+ }
+
return 0;
}
@@ -1413,6 +2205,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct hid_report_enum *rep_enum;
struct asus_drvdata *drvdata;
+ struct ally_handheld *ally;
struct hid_report *rep;
bool is_vendor = false;
int ret;
@@ -1525,6 +2318,15 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
(asus_kbd_register_leds(hdev)))
hid_warn(hdev, "Failed to initialize backlight.\n");
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ ally = hid_asus_ally_probe(hdev);
+ if (IS_ERR(ally))
+ hid_err(hdev, "Failed to initialize ROG Ally HID extensions: %ld\n",
+ PTR_ERR(ally));
+ else
+ drvdata->rog_ally = ally;
+ }
+
/*
* For ROG keyboards, skip rename for consistency and ->input check as
* some devices do not have inputs.
@@ -1556,6 +2358,14 @@ 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);
+ /*
+ * Roll back the state published in the global ally_drvdata by
+ * hid_asus_ally_probe(), or it would keep dangling pointers to
+ * this hdev and its devres-managed input_dev.
+ */
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)
+ hid_asus_ally_remove(hdev, drvdata->rog_ally);
+
asus_worker_stop(drvdata->worker);
hid_hw_stop(hdev);
return ret;
@@ -1568,6 +2378,9 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->listener.brightness_set)
asus_hid_unregister_listener(&drvdata->listener);
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)
+ hid_asus_ally_remove(hdev, drvdata->rog_ally);
+
asus_worker_stop(drvdata->worker);
hid_hw_stop(hdev);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds
2026-08-19 19:58 ` [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds Denis Benato
@ 2026-08-19 20:11 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:11 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Cross-device pointer access causes a Use-After-Free race when the keyboard interface is unbound.
- [High] Custom keyboard events silently fail because keyboard_input is extracted before the inputs list is populated.
- [High] The use of a global singleton `ally_drvdata` structure without enforcing a single connected device allows malicious state overwrites and cross-device Use-After-Free.
- [Medium] The custom Ally X gamepad input device lacks open and close callbacks, risking runtime PM suspension.
--
--- Patch [2]: [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds ---
commit e59e4270b19f2b93584009213d08bcb80cc4001c
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for ROG Ally handhelds
Implement the core functionality for ASUS handhelds by extending hid-asus
functionality if the driver is enabled. The device is initialized to a ready
state, Ally X input mapping is implemented, and a proper event is emitted
for the AC button long press.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 9ce21fcc7d2e0..88b30726f6c67 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +/*
> + * Changes to ally_drvdata must lock: the raw_event callbacks, which may
> + * run in atomic (URB completion) context, also take this lock, so it must
> + * be a spinlock.
> + */
> +static DEFINE_SPINLOCK(ally_data_lock);
> +static struct ally_handheld ally_drvdata = {
[Severity: High]
Could using a single global ally_drvdata structure cause state corruption if
multiple ROG Ally devices are connected?
If a user plugs in two devices or a spoofed USB peripheral, the second probe
unconditionally overwrites the pointers for the first device. Unbinding them
later could lead to use-after-free or dangling pointers.
Should this state be allocated dynamically per device instance instead?
> + .intf_mutex = __MUTEX_INITIALIZER(ally_drvdata.intf_mutex),
> + /*
> + * Initialised statically so it is always safe to cancel, whichever
> + * of the interfaces probed or failed to probe.
> + */
[ ... ]
> +static bool handle_ally_event(struct hid_device *hdev, struct ally_handheld *ally,
> + u8 *data, int size)
> +{
[ ... ]
> + spin_lock_irqsave(&ally_data_lock, flags);
> + keyboard_input = ally->keyboard_input;
> + spin_unlock_irqrestore(&ally_data_lock, flags);
> +
> + if (!keyboard_input)
> + return false;
> +
> + input_report_key(keyboard_input, keycode, 1);
[Severity: High]
Is it possible for an input_dev use-after-free to occur here? The
keyboard_input pointer is fetched under ally_data_lock, but the lock is
dropped before calling input_report_key().
If the device is concurrently unbound, hid_asus_ally_remove() will clear the
pointer from the global state and the input core will free the input_dev,
meaning the concurrent event handler would access freed memory.
Would taking a reference to the input device before dropping the lock
prevent this race?
> + input_sync(keyboard_input);
> + input_report_key(keyboard_input, keycode, 0);
> + input_sync(keyboard_input);
> + return true;
> + }
[ ... ]
> +static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *ally)
> +{
> + struct input_dev *input = ally_x_alloc_input_dev(hdev);
[ ... ]
> + input_set_capability(input, EV_KEY, KEY_F18);
> + input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY);
> + input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY1);
> +
> + ret = input_register_device(input);
[Severity: Medium]
Does the custom Ally X gamepad input device need open and close callbacks?
Usually, standard hid-input devices assign these callbacks to invoke
hid_hw_open() and hid_hw_close(). Without them, opening just the gamepad
node might not wake the hardware from suspend or prevent it from sleeping,
which could lead to dropped events or input latency during runtime PM
suspension.
> + if (ret) {
> + hid_err(hdev, "Failed to register Ally X gamepad device: %d\n", ret);
> + goto ally_x_setup_input_err;
> + }
[ ... ]
> +static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
> +{
[ ... ]
> + case HID_ALLY_INTF_KEYBOARD_IN:
> + spin_lock_irqsave(&ally_data_lock, flags);
> + ally_drvdata.keyboard_hdev = hdev;
> + if (!list_empty(&hdev->inputs)) {
> + hidinput = list_first_entry(&hdev->inputs, struct hid_input, list);
> + ally_drvdata.keyboard_input = hidinput->input;
> + }
[Severity: High]
Will ally_drvdata.keyboard_input ever be populated here? Looking at
asus_probe(), hid_asus_ally_probe() is called before hid_hw_start().
At this point, the HID core has not yet called hidinput_connect(), so
hdev->inputs is guaranteed to be empty. Because of this, the keyboard_input
pointer remains NULL, and custom keyboard events might be silently discarded
in handle_ally_event().
Could this initialization be deferred until after hid_hw_start() populates
the inputs list?
> + spin_unlock_irqrestore(&ally_data_lock, flags);
> + break;
> + default:
> + /* This is normally supposed to happen */
> + break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 03/12] HID: asus: add gamepad configuration
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
2026-08-19 19:58 ` [PATCH v2 01/12] HID: asus: reinitialize the device after exiting a sleep state Denis Benato
2026-08-19 19:58 ` [PATCH v2 02/12] HID: asus: add support for ROG Ally handhelds Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:10 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 04/12] HID: asus: add vibration strength configuration Denis Benato
` (8 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
Add the base configuration structures for the gamepad configuration,
detect capabilities and initialize the device in a known state.
Assisted-by: opencode:glm-5.2
Assisted-by: Claude:claude-fable-5
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
drivers/hid/hid-asus.c | 330 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 319 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 88b30726f6c6..900914d21ca4 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -22,6 +22,7 @@
#include <linux/acpi.h>
#include <linux/cleanup.h>
+#include <linux/device.h>
#include <linux/dmi.h>
#include <linux/hid.h>
#include <linux/jiffies.h>
@@ -34,6 +35,7 @@
#include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
#include <linux/power_supply.h>
#include <linux/stddef.h>
+#include <linux/sysfs.h>
#include <linux/leds.h>
#include <linux/unaligned.h>
@@ -176,6 +178,36 @@ struct asus_touchpad_info {
int report_size;
};
+struct ally_config {
+ /* Must be locked if the data is being changed */
+ struct mutex config_mutex;
+ bool initialized;
+
+ /* Device capabilities flags */
+ bool is_ally_x;
+ bool xbox_controller_support;
+ bool user_cal_support;
+ bool turbo_support;
+ bool resp_curve_support;
+ bool dir_to_btn_support;
+ bool gyro_support;
+ bool anti_deadzone_support;
+
+ /* Current settings */
+ bool xbox_controller_enabled;
+ u8 gamepad_mode;
+ u8 left_deadzone;
+ u8 left_outer_threshold;
+ u8 right_deadzone;
+ u8 right_outer_threshold;
+ u8 left_anti_deadzone;
+ u8 right_anti_deadzone;
+ u8 left_trigger_min;
+ u8 left_trigger_max;
+ u8 right_trigger_min;
+ u8 right_trigger_max;
+};
+
struct ally_handheld {
/* All read/write to IN interfaces must lock */
struct mutex intf_mutex;
@@ -191,6 +223,8 @@ struct ally_handheld {
unsigned long cad_last_event_time;
struct delayed_work resume_work;
+
+ struct ally_config *config;
};
struct asus_drvdata {
@@ -570,6 +604,253 @@ static u8 *ally_alloc_cmd(u8 cmd, const u8 *payload, u8 payload_size)
return hidbuf;
}
+/**
+ * ally_check_capability() - Check if a specific capability is supported
+ * @hdev: HID device
+ * @ally: ally handheld structure
+ * @check_cmd: capability command code to query
+ *
+ * Return: true if the capability is supported, false otherwise
+ */
+static bool ally_check_capability(struct hid_device *hdev, struct ally_handheld *ally,
+ enum ally_command_codes check_cmd)
+{
+ u8 payload[] = { 0x00 };
+ int ret;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(check_cmd, payload, sizeof(payload));
+ if (!buf) {
+ hid_err(hdev, "Failed to allocate buffer for capability check.\n");
+ return false;
+ }
+
+ ret = ally_gamepad_send_receive_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to check capability 0x%02x: %d\n", check_cmd, ret);
+ return false;
+ }
+
+ return buf[1] == HID_ALLY_FEATURE_CODE_PAGE && buf[2] == check_cmd &&
+ buf[4] == 0x01;
+}
+
+static int ally_detect_capabilities(struct hid_device *hdev, struct ally_handheld *ally,
+ struct ally_config *cfg)
+{
+ if (!hdev || !cfg || !ally)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ cfg->is_ally_x = (hdev->product == USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X);
+
+ cfg->xbox_controller_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_XBOX_SUPPORT);
+ cfg->user_cal_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_USER_CAL_SUPPORT);
+ cfg->turbo_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_TURBO_SUPPORT);
+ cfg->resp_curve_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_RESP_CURVE_SUPPORT);
+ cfg->dir_to_btn_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_DIR_TO_BTN_SUPPORT);
+ cfg->gyro_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_GYRO_TO_JOYSTICK);
+ cfg->anti_deadzone_support =
+ ally_check_capability(hdev, ally, CMD_CHECK_ANTI_DEADZONE);
+ }
+
+ return 0;
+}
+
+static int ally_set_xbox_controller(struct hid_device *hdev,
+ struct ally_handheld *ally,
+ struct ally_config *cfg, bool enabled)
+{
+ u8 payload[] = { enabled ? 0x01 : 0x00 };
+ int ret;
+
+ if (!cfg || !cfg->xbox_controller_support)
+ return -ENODEV;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_XBOX_CONTROLLER, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_gamepad_send_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set Xbox controller mode: %d\n", ret);
+ return ret;
+ }
+
+ cfg->xbox_controller_enabled = enabled;
+ return 0;
+}
+
+/**
+ * ally_get_config() - Get the configuration of the Ally device
+ * @ally: ally handheld structure
+ *
+ * Fetch the configuration published by hid_asus_ally_probe() under
+ * ally_data_lock: the pointer is also cleared by hid_asus_ally_remove(),
+ * so reading it without the lock would race with interface removal.
+ *
+ * The returned configuration outlives the sysfs callbacks using it: it is
+ * devm-managed on the same device as the sysfs attributes, which are
+ * removed before the memory is released.
+ *
+ * Return: the ally config, or NULL if no configuration is published
+ */
+static struct ally_config *ally_get_config(struct ally_handheld *ally)
+{
+ struct ally_config *cfg;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ cfg = ally->config;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ return cfg;
+}
+
+static ssize_t xbox_controller_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->xbox_controller_support)
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%d\n", cfg->xbox_controller_enabled ? 1 : 0);
+ }
+}
+
+static ssize_t xbox_controller_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ bool enabled;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtobool(buf, &enabled);
+ if (ret)
+ return ret;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->xbox_controller_support)
+ return -ENODEV;
+
+ ret = ally_set_xbox_controller(hdev, ally, cfg, enabled);
+ if (ret < 0)
+ return ret;
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(xbox_controller);
+
+static struct attribute *ally_config_attrs[] = {
+ &dev_attr_xbox_controller.attr,
+ NULL
+};
+
+static const struct attribute_group ally_attr_groups[] = {
+ {
+ .attrs = ally_config_attrs,
+ },
+};
+
+/**
+ * ally_config_create() - Initialize configuration and create sysfs entries
+ * @hdev: HID device
+ * @ally: non-NULL ally device data with uninitialized config pointer
+ *
+ * Return: valid pointer on success, error pointer on failure
+ */
+static struct ally_config *ally_config_create(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ struct ally_config *cfg;
+ int ret, sysfs_i;
+
+ cfg = devm_kzalloc(&hdev->dev, sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&cfg->config_mutex);
+
+ ret = ally_detect_capabilities(hdev, ally, cfg);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to detect Ally capabilities: %d\n", ret);
+ goto ally_config_create_err;
+ }
+
+ for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_attr_groups); sysfs_i++) {
+ ret = devm_device_add_group(&hdev->dev, &ally_attr_groups[sysfs_i]);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
+ ally_attr_groups[sysfs_i].name, ret);
+ goto ally_config_create_sysfs_err;
+ }
+ }
+
+ cfg->gamepad_mode = 0x01;
+ cfg->left_deadzone = 10;
+ cfg->left_outer_threshold = 90;
+ cfg->right_deadzone = 10;
+ cfg->right_outer_threshold = 90;
+
+ /* So far the only hardware this is supported is the Ally 1 */
+ if (cfg->xbox_controller_support) {
+ ret = ally_set_xbox_controller(hdev, ally, cfg, true);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to set default Xbox controller mode: %d\n",
+ ret);
+ }
+
+ cfg->initialized = true;
+
+ return cfg;
+ally_config_create_sysfs_err:
+ally_config_create_err:
+ devm_kfree(&hdev->dev, cfg);
+ return ERR_PTR(ret);
+}
+
+/**
+ * ally_config_remove() - Clean up configuration resources
+ * @hdev: HID device
+ * @ally: Non-NULL Ally device data
+ */
+static void ally_config_remove(struct hid_device *hdev, struct ally_handheld *ally)
+{
+ struct ally_config *cfg = ally->config;
+
+ if (!cfg || !cfg->initialized)
+ return;
+}
+
/**
* ally_gamepad_check_ready() - Wait for the gamepad MCU to report ready
* @ally: ally handheld structure
@@ -812,16 +1093,6 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all
{
int ret;
- /*
- * This function assumes the asus-specific initialization
- * to have been performed already at this point.
- */
- ret = ally_gamepad_check_ready(ally, hdev);
- if (ret < 0) {
- hid_err(hdev, "ROG Ally device is not ready: %d\n", ret);
- return ret;
- }
-
/* Failure at this point is non-critical */
ret = ally_gamepad_send_packet(ally, hdev, ALLY_FORCE_FEEDBACK_OFF,
sizeof(ALLY_FORCE_FEEDBACK_OFF));
@@ -896,6 +1167,7 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
{
unsigned long flags;
int ret, ep = ally_get_endpoint_address(hdev);
+ struct ally_config *ally_cfg;
struct hid_input *hidinput;
if (ep < 0)
@@ -908,11 +1180,31 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
*/
switch (ep) {
case HID_ALLY_INTF_CFG_IN:
+ /*
+ * This function assumes the asus-specific initialization
+ * to have been performed already at this point.
+ */
+ ret = ally_gamepad_check_ready(&ally_drvdata, hdev);
+ if (ret < 0) {
+ hid_err(hdev, "ROG Ally device is not ready: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ ally_cfg = ally_config_create(hdev, &ally_drvdata);
+ if (IS_ERR(ally_cfg)) {
+ hid_err(hdev, "Failed to create Ally cfg: %ld\n",
+ PTR_ERR(ally_cfg));
+ return ERR_PTR(PTR_ERR(ally_cfg));
+ }
+
ret = hid_asus_ally_init(hdev, &ally_drvdata);
- if (ret < 0)
+ if (ret < 0) {
+ ally_config_remove(hdev, &ally_drvdata);
return ERR_PTR(ret);
+ }
spin_lock_irqsave(&ally_data_lock, flags);
+ ally_drvdata.config = ally_cfg;
ally_drvdata.cfg_hdev = hdev;
spin_unlock_irqrestore(&ally_data_lock, flags);
break;
@@ -976,6 +1268,12 @@ static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *
ally->keyboard_input = NULL;
ally->keyboard_hdev = NULL;
}
+
+ if (ally->cfg_hdev == hdev) {
+ ally_config_remove(hdev, ally);
+ ally->cfg_hdev = NULL;
+ ally->config = NULL;
+ }
spin_unlock_irqrestore(&ally_data_lock, flags);
}
@@ -994,6 +1292,16 @@ static int hid_asus_ally_reset_resume(struct hid_device *hdev, struct ally_handh
if (ep != HID_ALLY_INTF_CFG_IN)
return 0;
+ /*
+ * This function assumes the asus-specific initialization
+ * to have been performed already at this point.
+ */
+ ret = ally_gamepad_check_ready(ally, hdev);
+ if (ret < 0) {
+ hid_err(hdev, "ROG Ally device is not ready: %d\n", ret);
+ return ret;
+ }
+
ret = hid_asus_ally_init(hdev, ally);
if (ret < 0)
return ret;
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 03/12] HID: asus: add gamepad configuration
2026-08-19 19:58 ` [PATCH v2 03/12] HID: asus: add gamepad configuration Denis Benato
@ 2026-08-19 20:10 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:10 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The device state (e.g., Xbox controller mode) is not restored during `hid_asus_ally_reset_resume()` after a device reset, leading to desynchronization between the hardware state and the cached sysfs state.
- [Low] Passing implicitly NULL sysfs group name to %s format specifier in error message.
--
--- Patch [3]: [PATCH v2 03/12] HID: asus: add gamepad configuration ---
commit 9e908c106ea4b31ecf33a0f67456da287b68ca3a
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add gamepad configuration
Add the base configuration structures for the gamepad configuration,
detect capabilities and initialize the device in a known state.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 88b30726f6c67..900914d21ca46 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static const struct attribute_group ally_attr_groups[] = {
> + {
> + .attrs = ally_config_attrs,
> + },
> +};
> +
> +/**
> + * ally_config_create() - Initialize configuration and create sysfs entries
> + * @hdev: HID device
> + * @ally: non-NULL ally device data with uninitialized config pointer
> + *
> + * Return: valid pointer on success, error pointer on failure
> + */
> +static struct ally_config *ally_config_create(struct hid_device *hdev, struct ally_handheld *ally)
> +{
> + struct ally_config *cfg;
> + int ret, sysfs_i;
[ ... ]
> + for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_attr_groups); sysfs_i++) {
> + ret = devm_device_add_group(&hdev->dev, &ally_attr_groups[sysfs_i]);
> + if (ret < 0) {
> + hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
> + ally_attr_groups[sysfs_i].name, ret);
[Severity: Low]
Is this going to print (null) in the error message? The ally_attr_groups
definition doesn't seem to have a .name field set for the group.
> + goto ally_config_create_sysfs_err;
> + }
> + }
[ ... ]
> @@ -994,6 +1292,16 @@ static int hid_asus_ally_reset_resume(struct hid_device *hdev, struct ally_handh
> if (ep != HID_ALLY_INTF_CFG_IN)
> return 0;
>
> + /*
> + * This function assumes the asus-specific initialization
> + * to have been performed already at this point.
> + */
> + ret = ally_gamepad_check_ready(ally, hdev);
> + if (ret < 0) {
> + hid_err(hdev, "ROG Ally device is not ready: %d\n", ret);
> + return ret;
> + }
> +
> ret = hid_asus_ally_init(hdev, ally);
> if (ret < 0)
> return ret;
[Severity: Medium]
Does this code need to restore the device configuration state, such as the Xbox
controller mode, after a device reset?
Since hid_asus_ally_init() only disables force-feedback, it looks like a system
suspend and resume cycle could cause the hardware to revert to defaults, while
the cached ally->config still holds the previous settings.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 04/12] HID: asus: add vibration strength configuration
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (2 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 03/12] HID: asus: add gamepad configuration Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:12 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration Denis Benato
` (7 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ASUS ROG Ally handhelds support the vibration strength to be configured:
add sysfs attributes to allow userspace configure motors vibration
intensity.
Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 180 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 180 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 900914d21ca4..6b005afe01a2 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -206,6 +206,10 @@ struct ally_config {
u8 left_trigger_max;
u8 right_trigger_min;
u8 right_trigger_max;
+
+ /* Vibration settings */
+ u8 vibration_intensity_left;
+ u8 vibration_intensity_right;
};
struct ally_handheld {
@@ -771,15 +775,189 @@ static ssize_t xbox_controller_store(struct device *dev,
static DEVICE_ATTR_RW(xbox_controller);
+/**
+ * ally_set_vibration_intensity() - Set vibration intensity values
+ * @hdev: HID device
+ * @left: Left motor intensity (0-100)
+ * @right: Right motor intensity (0-100)
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_vibration_intensity(struct hid_device *hdev, u8 left, u8 right)
+{
+ const u8 data[] = { left, right };
+ int ret;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_VIBRATION_INTENSITY, data, sizeof(data));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set vibration intensity: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t left_vibration_intensity_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->vibration_intensity_left);
+}
+
+static ssize_t left_vibration_intensity_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_vibration_intensity(hdev, value,
+ cfg->vibration_intensity_right);
+ if (ret < 0)
+ return ret;
+
+ cfg->vibration_intensity_left = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_vibration_intensity_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0 100\n");
+}
+
+static ssize_t right_vibration_intensity_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->vibration_intensity_right);
+}
+
+static ssize_t right_vibration_intensity_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_vibration_intensity(hdev,
+ cfg->vibration_intensity_left, value);
+ if (ret < 0)
+ return ret;
+
+ cfg->vibration_intensity_right = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_vibration_intensity_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0 100\n");
+}
+
+static struct device_attribute dev_attr_left_vibration_intensity =
+ __ATTR(intensity, 0644, left_vibration_intensity_show, left_vibration_intensity_store);
+
+static struct device_attribute dev_attr_left_vibration_intensity_range =
+ __ATTR(intensity_range, 0444, left_vibration_intensity_range_show, NULL);
+
+static struct device_attribute dev_attr_right_vibration_intensity =
+ __ATTR(intensity, 0644, right_vibration_intensity_show, right_vibration_intensity_store);
+
+static struct device_attribute dev_attr_right_vibration_intensity_range =
+ __ATTR(intensity_range, 0444, right_vibration_intensity_range_show, NULL);
+
static struct attribute *ally_config_attrs[] = {
&dev_attr_xbox_controller.attr,
NULL
};
+static struct attribute *ally_left_vibration_attrs[] = {
+ &dev_attr_left_vibration_intensity.attr,
+ &dev_attr_left_vibration_intensity_range.attr,
+ NULL
+};
+
+static struct attribute *ally_right_vibration_attrs[] = {
+ &dev_attr_right_vibration_intensity.attr,
+ &dev_attr_right_vibration_intensity_range.attr,
+ NULL
+};
+
static const struct attribute_group ally_attr_groups[] = {
{
.attrs = ally_config_attrs,
},
+ {
+ .name = "left_vibration",
+ .attrs = ally_left_vibration_attrs,
+ },
+ {
+ .name = "right_vibration",
+ .attrs = ally_right_vibration_attrs,
+ },
};
/**
@@ -820,6 +998,8 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
cfg->left_outer_threshold = 90;
cfg->right_deadzone = 10;
cfg->right_outer_threshold = 90;
+ cfg->vibration_intensity_left = 100;
+ cfg->vibration_intensity_right = 100;
/* So far the only hardware this is supported is the Ally 1 */
if (cfg->xbox_controller_support) {
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 04/12] HID: asus: add vibration strength configuration
2026-08-19 19:58 ` [PATCH v2 04/12] HID: asus: add vibration strength configuration Denis Benato
@ 2026-08-19 20:12 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:12 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] `ally_set_vibration_intensity()` directly sends raw HID requests without acquiring the required `ally->intf_mutex`, leading to potential race conditions and device state corruption.
- [Low] If `devm_device_add_group()` fails for one of the newly added vibration sysfs groups, the previously registered groups are not rolled back if the overall device probe succeeds.
--
--- Patch [4]: [PATCH v2 04/12] HID: asus: add vibration strength configuration ---
commit fd873329418038e807b5d4833861817dc262fc2b
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: add vibration strength configuration
This patch adds support for configuring the vibration strength of ASUS ROG
Ally handhelds. It introduces sysfs attributes to allow userspace to control
the intensity of the left and right motors.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 900914d21ca46..6b005afe01a24 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -771,15 +775,189 @@ static ssize_t xbox_controller_store(struct device *dev,
>
> static DEVICE_ATTR_RW(xbox_controller);
>
> +/**
> + * ally_set_vibration_intensity() - Set vibration intensity values
> + * @hdev: HID device
> + * @left: Left motor intensity (0-100)
> + * @right: Right motor intensity (0-100)
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +static int ally_set_vibration_intensity(struct hid_device *hdev, u8 left, u8 right)
> +{
> + const u8 data[] = { left, right };
> + int ret;
> +
> + u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_VIBRATION_INTENSITY, data, sizeof(data));
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
[Severity: Medium]
Does this direct call to ally_dev_set_report() need to be protected by
ally->intf_mutex?
Other commands like CMD_SET_XBOX_CONTROLLER use ally_gamepad_send_packet()
which takes the mutex:
static int ally_gamepad_send_packet(...)
{
scoped_guard(mutex, &ally->intf_mutex)
return ally_dev_set_report(hdev, buf, len);
}
Without the lock, could concurrent sysfs writes to the vibration attributes
race with other hardware communication on the IN interface, potentially breaking
the strict SET/GET pairing expected by initialization routines?
> + if (ret < 0) {
> + hid_err(hdev, "Failed to set vibration intensity: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
[ ... ]
> static const struct attribute_group ally_attr_groups[] = {
> {
> .attrs = ally_config_attrs,
> },
> + {
> + .name = "left_vibration",
> + .attrs = ally_left_vibration_attrs,
> + },
> + {
> + .name = "right_vibration",
> + .attrs = ally_right_vibration_attrs,
> + },
> };
[Severity: Low]
If devm_device_add_group() fails for one of these newly added sysfs groups
during the registration loop in ally_config_create(), and the overall device
probe continues and succeeds, are the previously registered groups properly
cleaned up?
Since devres actions are only rolled back when the probe fails or the device
is unbound, it looks like a failure partway through the loop might leave
partially registered sysfs groups visible indefinitely.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (3 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 04/12] HID: asus: add vibration strength configuration Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:15 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 06/12] HID: asus: add triggers " Denis Benato
` (6 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG Ally devices supports configuring joysticks inner and outer range:
add sysfs attributes to allow userspace modifying the sensitivity
of those controllers.
Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 342 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 342 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 6b005afe01a2..d028697f61bf 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -929,6 +929,324 @@ static struct device_attribute dev_attr_right_vibration_intensity =
static struct device_attribute dev_attr_right_vibration_intensity_range =
__ATTR(intensity_range, 0444, right_vibration_intensity_range_show, NULL);
+/**
+ * ally_set_joystick_thresholds() - Generic function to set joystick ranges
+ * @hdev: HID device
+ * @cfg: ally config
+ * @left_it: inner threshold (deadzone) of the left stick (0-50)
+ * @left_ot: outer threshold of the left stick (70-100)
+ * @right_it: inner threshold (deadzone) of the right stick (0-50)
+ * @right_ot: outer threshold of the right stick (70-100)
+ *
+ * This function sends the command to set both inner and outer threshold
+ * for the left and right joysticks.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_joystick_thresholds(struct hid_device *hdev, struct ally_config *cfg,
+ u8 left_it, u8 left_ot, u8 right_it, u8 right_ot)
+{
+ u8 payload[] = { left_it, left_ot, right_it, right_ot };
+ int ret;
+
+ if (!cfg->xbox_controller_support)
+ return -ENODEV;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_JOYSTICK_DEADZONE, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set joystick ranges: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t left_joystick_inner_threshold_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->left_deadzone);
+}
+
+static ssize_t left_joystick_inner_threshold_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 50)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_joystick_thresholds(hdev, cfg,
+ value,
+ cfg->left_outer_threshold,
+ cfg->right_deadzone,
+ cfg->right_outer_threshold);
+ if (ret)
+ return ret;
+
+ cfg->left_deadzone = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_joystick_inner_threshold_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 50\n");
+}
+
+static ssize_t left_joystick_outer_threshold_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->left_outer_threshold);
+}
+
+static ssize_t left_joystick_outer_threshold_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value < 70 || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_joystick_thresholds(hdev, cfg,
+ cfg->left_deadzone,
+ value,
+ cfg->right_deadzone,
+ cfg->right_outer_threshold);
+ if (ret)
+ return ret;
+
+ cfg->left_outer_threshold = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_joystick_outer_threshold_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "70 100\n");
+}
+
+static ssize_t right_joystick_inner_threshold_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->right_deadzone);
+}
+
+static ssize_t right_joystick_inner_threshold_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 50)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_joystick_thresholds(hdev, cfg,
+ cfg->left_deadzone,
+ cfg->left_outer_threshold,
+ value,
+ cfg->right_outer_threshold);
+ if (ret)
+ return ret;
+
+ cfg->right_deadzone = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_joystick_inner_threshold_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 50\n");
+}
+
+static ssize_t right_joystick_outer_threshold_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->right_outer_threshold);
+}
+
+static ssize_t right_joystick_outer_threshold_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value < 70 || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_joystick_thresholds(hdev, cfg,
+ cfg->left_deadzone,
+ cfg->left_outer_threshold,
+ cfg->right_deadzone,
+ value);
+ if (ret)
+ return ret;
+
+ cfg->right_outer_threshold = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_joystick_outer_threshold_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "70 100\n");
+}
+
+static struct device_attribute dev_attr_left_joystick_inner_threshold =
+ __ATTR(inner_threshold, 0644, left_joystick_inner_threshold_show,
+ left_joystick_inner_threshold_store);
+
+static struct device_attribute dev_attr_left_joystick_inner_threshold_range =
+ __ATTR(inner_threshold_range, 0444, left_joystick_inner_threshold_range_show, NULL);
+
+static struct device_attribute dev_attr_left_joystick_outer_threshold =
+ __ATTR(outer_threshold, 0644, left_joystick_outer_threshold_show,
+ left_joystick_outer_threshold_store);
+
+static struct device_attribute dev_attr_left_joystick_outer_threshold_range =
+ __ATTR(outer_threshold_range, 0444, left_joystick_outer_threshold_range_show, NULL);
+
+static struct device_attribute dev_attr_right_joystick_inner_threshold =
+ __ATTR(inner_threshold, 0644, right_joystick_inner_threshold_show,
+ right_joystick_inner_threshold_store);
+
+static struct device_attribute dev_attr_right_joystick_inner_threshold_range =
+ __ATTR(inner_threshold_range, 0444, right_joystick_inner_threshold_range_show, NULL);
+
+static struct device_attribute dev_attr_right_joystick_outer_threshold =
+ __ATTR(outer_threshold, 0644, right_joystick_outer_threshold_show,
+ right_joystick_outer_threshold_store);
+
+static struct device_attribute dev_attr_right_joystick_outer_threshold_range =
+ __ATTR(outer_threshold_range, 0444, right_joystick_outer_threshold_range_show, NULL);
+
static struct attribute *ally_config_attrs[] = {
&dev_attr_xbox_controller.attr,
NULL
@@ -946,6 +1264,22 @@ static struct attribute *ally_right_vibration_attrs[] = {
NULL
};
+static struct attribute *left_joystick_axis_attrs[] = {
+ &dev_attr_left_joystick_inner_threshold.attr,
+ &dev_attr_left_joystick_outer_threshold.attr,
+ &dev_attr_left_joystick_inner_threshold_range.attr,
+ &dev_attr_left_joystick_outer_threshold_range.attr,
+ NULL
+};
+
+static struct attribute *right_joystick_axis_attrs[] = {
+ &dev_attr_right_joystick_inner_threshold.attr,
+ &dev_attr_right_joystick_outer_threshold.attr,
+ &dev_attr_right_joystick_inner_threshold_range.attr,
+ &dev_attr_right_joystick_outer_threshold_range.attr,
+ NULL
+};
+
static const struct attribute_group ally_attr_groups[] = {
{
.attrs = ally_config_attrs,
@@ -958,6 +1292,14 @@ static const struct attribute_group ally_attr_groups[] = {
.name = "right_vibration",
.attrs = ally_right_vibration_attrs,
},
+ {
+ .name = "left_joystick_axis",
+ .attrs = left_joystick_axis_attrs,
+ },
+ {
+ .name = "right_joystick_axis",
+ .attrs = right_joystick_axis_attrs,
+ },
};
/**
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration
2026-08-19 19:58 ` [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration Denis Benato
@ 2026-08-19 20:15 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:15 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `intf_mutex` lock in `ally_set_joystick_thresholds` allows concurrent HID requests to interleave and corrupt device state.
- [Medium] Joystick threshold sysfs 'show' functions do not check for hardware support, returning a dummy default value even when the feature is unsupported.
--
--- Patch [5]: [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration ---
commit 2e6189717a427b3132c324a798d81ea6224e0246
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add joysticks inner and outer range configuration
This commit adds sysfs attributes to configure the inner and outer ranges
for the ROG Ally joysticks, allowing userspace to modify sensitivity.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 6b005afe01a24..d028697f61bfd 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static int ally_set_joystick_thresholds(struct hid_device *hdev, struct ally_config *cfg,
> + u8 left_it, u8 left_ot, u8 right_it, u8 right_ot)
> +{
> + u8 payload[] = { left_it, left_ot, right_it, right_ot };
> + int ret;
> +
> + if (!cfg->xbox_controller_support)
> + return -ENODEV;
> +
> + u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_JOYSTICK_DEADZONE, payload, sizeof(payload));
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
[Severity: High]
Does this need to hold ally->intf_mutex when sending the report?
Other configuration functions like ally_set_xbox_controller() and
ally_set_gamepad_mode() use ally_gamepad_send_packet(), which safely
acquires the lock before calling ally_dev_set_report().
If called concurrently with a USB reset (where ally_gamepad_check_ready()
holds intf_mutex but not config_mutex), could this interleave between the
set and get report phases and corrupt the device state?
[ ... ]
> +static ssize_t left_joystick_inner_threshold_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> + struct ally_handheld *const ally = drvdata->rog_ally;
> + struct ally_config *cfg;
> +
> + if (!ally)
> + return -ENODEV;
> +
> + cfg = ally_get_config(ally);
> + if (!cfg)
> + return -ENODEV;
> +
> + scoped_guard(mutex, &cfg->config_mutex)
> + return sysfs_emit(buf, "%u\n", cfg->left_deadzone);
> +}
[Severity: Medium]
Should this check cfg->xbox_controller_support before emitting the value?
The corresponding store paths return -ENODEV via
ally_set_joystick_thresholds() if the hardware doesn't support the feature,
but the show paths seem to return the default initialized values regardless
of hardware support.
This appears to apply to all the joystick threshold show attributes added in
this patch.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=5
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 06/12] HID: asus: add triggers inner and outer range configuration
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (4 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 05/12] HID: asus: add joysticks inner and outer range configuration Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 19:58 ` [PATCH v2 07/12] HID: asus: add joysticks anti-deadzone configuration Denis Benato
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG Ally devices allows configuring inner and outer ranges for triggers
buttons on the back: allow userspace to configure the sesitivity by
exposing sysfs attributes.
Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 345 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 345 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d028697f61bf..23a0cec2de15 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1247,6 +1247,323 @@ static struct device_attribute dev_attr_right_joystick_outer_threshold =
static struct device_attribute dev_attr_right_joystick_outer_threshold_range =
__ATTR(outer_threshold_range, 0444, right_joystick_outer_threshold_range_show, NULL);
+/**
+ * ally_set_trigger_ranges() - Generic function to set triggers ranges
+ * @hdev: HID device
+ * @cfg: ally config
+ * @left_it: lower limit of the left trigger range (0-50)
+ * @left_ot: upper limit of the left trigger range (70-100)
+ * @right_it: lower limit of the right trigger range (0-50)
+ * @right_ot: upper limit of the right trigger range (70-100)
+ *
+ * This function sends the command to set both inner and outer threshold
+ * for the left and right triggers.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_trigger_ranges(struct hid_device *hdev, struct ally_config *cfg,
+ u8 left_it, u8 left_ot, u8 right_it, u8 right_ot)
+{
+ const u8 payload[] = { left_it, left_ot, right_it, right_ot };
+ int ret;
+
+ if (!cfg->xbox_controller_support)
+ return -ENODEV;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_TRIGGER_RANGE, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set trigger ranges: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t left_trigger_range_lower_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->left_trigger_min);
+}
+
+static ssize_t left_trigger_range_lower_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 50)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_trigger_ranges(hdev, cfg,
+ value,
+ cfg->left_trigger_max,
+ cfg->right_trigger_min,
+ cfg->right_trigger_max);
+ if (ret)
+ return ret;
+
+ cfg->left_trigger_min = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_trigger_range_lower_limit_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 50\n");
+}
+
+static ssize_t right_trigger_range_upper_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->right_trigger_max);
+}
+
+static ssize_t right_trigger_range_upper_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value < 70 || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_trigger_ranges(hdev, cfg,
+ cfg->left_trigger_min,
+ cfg->left_trigger_max,
+ cfg->right_trigger_min,
+ value);
+ if (ret)
+ return ret;
+
+ cfg->right_trigger_max = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_trigger_range_upper_limit_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "70 100\n");
+}
+
+static ssize_t right_trigger_range_lower_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->right_trigger_min);
+}
+
+static ssize_t right_trigger_range_lower_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 50)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_trigger_ranges(hdev, cfg,
+ cfg->left_trigger_min,
+ cfg->left_trigger_max,
+ value,
+ cfg->right_trigger_max);
+ if (ret)
+ return ret;
+
+ cfg->right_trigger_min = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_trigger_range_lower_limit_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 50\n");
+}
+
+static ssize_t left_trigger_range_upper_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex)
+ return sysfs_emit(buf, "%u\n", cfg->left_trigger_max);
+}
+
+static ssize_t left_trigger_range_upper_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value < 70 || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_trigger_ranges(hdev, cfg,
+ cfg->left_trigger_min,
+ value,
+ cfg->right_trigger_min,
+ cfg->right_trigger_max);
+ if (ret)
+ return ret;
+
+ cfg->left_trigger_max = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_trigger_range_upper_limit_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "70 100\n");
+}
+
+static struct device_attribute dev_attr_left_trigger_range_lower_limit =
+ __ATTR(range_lower_limit, 0644, left_trigger_range_lower_limit_show,
+ left_trigger_range_lower_limit_store);
+
+static struct device_attribute dev_attr_left_trigger_range_lower_limit_range =
+ __ATTR(range_lower_limit_range, 0444, left_trigger_range_lower_limit_range_show, NULL);
+
+static struct device_attribute dev_attr_left_trigger_range_upper_limit =
+ __ATTR(range_upper_limit, 0644, left_trigger_range_upper_limit_show,
+ left_trigger_range_upper_limit_store);
+
+static struct device_attribute dev_attr_left_trigger_range_upper_limit_range =
+ __ATTR(range_upper_limit_range, 0444, left_trigger_range_upper_limit_range_show, NULL);
+
+static struct device_attribute dev_attr_right_trigger_range_lower_limit =
+ __ATTR(range_lower_limit, 0644, right_trigger_range_lower_limit_show,
+ right_trigger_range_lower_limit_store);
+
+static struct device_attribute dev_attr_right_trigger_range_lower_limit_range =
+ __ATTR(range_lower_limit_range, 0444, right_trigger_range_lower_limit_range_show, NULL);
+
+static struct device_attribute dev_attr_right_trigger_range_upper_limit =
+ __ATTR(range_upper_limit, 0644, right_trigger_range_upper_limit_show,
+ right_trigger_range_upper_limit_store);
+
+static struct device_attribute dev_attr_right_trigger_range_upper_limit_range =
+ __ATTR(range_upper_limit_range, 0444, right_trigger_range_upper_limit_range_show, NULL);
+
static struct attribute *ally_config_attrs[] = {
&dev_attr_xbox_controller.attr,
NULL
@@ -1280,6 +1597,22 @@ static struct attribute *right_joystick_axis_attrs[] = {
NULL
};
+static struct attribute *left_trigger_attrs[] = {
+ &dev_attr_left_trigger_range_lower_limit.attr,
+ &dev_attr_left_trigger_range_upper_limit.attr,
+ &dev_attr_left_trigger_range_lower_limit_range.attr,
+ &dev_attr_left_trigger_range_upper_limit_range.attr,
+ NULL
+};
+
+static struct attribute *right_trigger_attrs[] = {
+ &dev_attr_right_trigger_range_lower_limit.attr,
+ &dev_attr_right_trigger_range_upper_limit.attr,
+ &dev_attr_right_trigger_range_lower_limit_range.attr,
+ &dev_attr_right_trigger_range_upper_limit_range.attr,
+ NULL
+};
+
static const struct attribute_group ally_attr_groups[] = {
{
.attrs = ally_config_attrs,
@@ -1300,6 +1633,14 @@ static const struct attribute_group ally_attr_groups[] = {
.name = "right_joystick_axis",
.attrs = right_joystick_axis_attrs,
},
+ {
+ .name = "left_trigger",
+ .attrs = left_trigger_attrs,
+ },
+ {
+ .name = "right_trigger",
+ .attrs = right_trigger_attrs,
+ },
};
/**
@@ -1340,6 +1681,10 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
cfg->left_outer_threshold = 90;
cfg->right_deadzone = 10;
cfg->right_outer_threshold = 90;
+ cfg->left_trigger_min = 0;
+ cfg->left_trigger_max = 100;
+ cfg->right_trigger_min = 0;
+ cfg->right_trigger_max = 100;
cfg->vibration_intensity_left = 100;
cfg->vibration_intensity_right = 100;
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 07/12] HID: asus: add joysticks anti-deadzone configuration
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (5 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 06/12] HID: asus: add triggers " Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 19:58 ` [PATCH v2 08/12] HID: asus: add support for response curve Denis Benato
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG Ally devices allow configuring the anti-deadzone parameter for
the resistive joysticks devices as over time those develops drift,
therefore allow userspace to configure the anti-deadzone by exposing
relevant sysfs attributes.
Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 189 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 189 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 23a0cec2de15..050d979f4315 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1247,6 +1247,191 @@ static struct device_attribute dev_attr_right_joystick_outer_threshold =
static struct device_attribute dev_attr_right_joystick_outer_threshold_range =
__ATTR(outer_threshold_range, 0444, right_joystick_outer_threshold_range_show, NULL);
+/**
+ * ally_set_anti_deadzone() - Set anti-deadzone values for joysticks
+ * @hdev: HID device
+ * @cfg: ally config
+ * @left_adz: left joystick anti-deadzone value (0-100)
+ * @right_adz: right joystick anti-deadzone value (0-100)
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_anti_deadzone(struct hid_device *hdev, struct ally_config *cfg,
+ u8 left_adz, u8 right_adz)
+{
+ const u8 payload[] = { left_adz, right_adz };
+ int ret;
+
+ if (!cfg->anti_deadzone_support) {
+ hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
+ return -EOPNOTSUPP;
+ }
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_ANTI_DEADZONE, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set anti-deadzone values: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t left_joystick_anti_deadzone_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->anti_deadzone_support) {
+ hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
+ return -EOPNOTSUPP;
+ }
+
+ return sysfs_emit(buf, "%u\n", cfg->left_anti_deadzone);
+ }
+}
+
+static ssize_t left_joystick_anti_deadzone_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->anti_deadzone_support) {
+ hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
+ return -EOPNOTSUPP;
+ }
+
+ ret = ally_set_anti_deadzone(hdev, cfg, value, cfg->right_anti_deadzone);
+ if (ret)
+ return ret;
+
+ cfg->left_anti_deadzone = value;
+ }
+
+ return count;
+}
+
+static ssize_t left_joystick_anti_deadzone_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 100\n");
+}
+
+static ssize_t right_joystick_anti_deadzone_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->anti_deadzone_support) {
+ hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
+ return -EOPNOTSUPP;
+ }
+
+ return sysfs_emit(buf, "%u\n", cfg->right_anti_deadzone);
+ }
+}
+
+static ssize_t right_joystick_anti_deadzone_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 value;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret || value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->anti_deadzone_support) {
+ hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
+ return -EOPNOTSUPP;
+ }
+
+ ret = ally_set_anti_deadzone(hdev, cfg, cfg->left_anti_deadzone, value);
+ if (ret)
+ return ret;
+
+ cfg->right_anti_deadzone = value;
+ }
+
+ return count;
+}
+
+static ssize_t right_joystick_anti_deadzone_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0 100\n");
+}
+
+static struct device_attribute dev_attr_left_joystick_anti_deadzone =
+ __ATTR(anti_deadzone, 0644, left_joystick_anti_deadzone_show,
+ left_joystick_anti_deadzone_store);
+
+static struct device_attribute dev_attr_left_joystick_anti_deadzone_range =
+ __ATTR(anti_deadzone_range, 0444, left_joystick_anti_deadzone_range_show, NULL);
+
+static struct device_attribute dev_attr_right_joystick_anti_deadzone =
+ __ATTR(anti_deadzone, 0644, right_joystick_anti_deadzone_show,
+ right_joystick_anti_deadzone_store);
+
+static struct device_attribute dev_attr_right_joystick_anti_deadzone_range =
+ __ATTR(anti_deadzone_range, 0444, right_joystick_anti_deadzone_range_show, NULL);
+
/**
* ally_set_trigger_ranges() - Generic function to set triggers ranges
* @hdev: HID device
@@ -1586,6 +1771,8 @@ static struct attribute *left_joystick_axis_attrs[] = {
&dev_attr_left_joystick_outer_threshold.attr,
&dev_attr_left_joystick_inner_threshold_range.attr,
&dev_attr_left_joystick_outer_threshold_range.attr,
+ &dev_attr_left_joystick_anti_deadzone.attr,
+ &dev_attr_left_joystick_anti_deadzone_range.attr,
NULL
};
@@ -1594,6 +1781,8 @@ static struct attribute *right_joystick_axis_attrs[] = {
&dev_attr_right_joystick_outer_threshold.attr,
&dev_attr_right_joystick_inner_threshold_range.attr,
&dev_attr_right_joystick_outer_threshold_range.attr,
+ &dev_attr_right_joystick_anti_deadzone.attr,
+ &dev_attr_right_joystick_anti_deadzone_range.attr,
NULL
};
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 08/12] HID: asus: add support for response curve
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (6 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 07/12] HID: asus: add joysticks anti-deadzone configuration Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:17 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 09/12] HID: asus: add support to force feedback Denis Benato
` (3 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG ally devices allows configuring the response curve of both joysticks,
therefore add the ability of userspace to modify the response curve by
exposing relevant sysfs attributes.
Assisted-by: opencode:glm-5.2
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 445 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 445 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 050d979f4315..6f43b7317168 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -134,6 +134,14 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define TRKID_SGN ((TRKID_MAX + 1) >> 1)
+#define ALLY_DEVICE_ATTR_WO(_name, _sysfs_name) \
+ struct device_attribute dev_attr_##_name = \
+ __ATTR(_sysfs_name, 0200, NULL, _name##_store)
+
+#define ALLY_DEVICE_ATTR_RW(_name, _sysfs_name) \
+ struct device_attribute dev_attr_##_name = \
+ __ATTR(_sysfs_name, 0644, _name##_show, _name##_store)
+
enum asus_work_action_type {
FN_LOCK_SYNC,
BRIGHTNESS_SET,
@@ -178,6 +186,18 @@ struct asus_touchpad_info {
int report_size;
};
+struct ally_joystick_resp_curve_param {
+ u8 move;
+ u8 resp;
+} __packed;
+
+struct ally_joystick_resp_curve {
+ struct ally_joystick_resp_curve_param entry_1;
+ struct ally_joystick_resp_curve_param entry_2;
+ struct ally_joystick_resp_curve_param entry_3;
+ struct ally_joystick_resp_curve_param entry_4;
+} __packed;
+
struct ally_config {
/* Must be locked if the data is being changed */
struct mutex config_mutex;
@@ -210,6 +230,9 @@ struct ally_config {
/* Vibration settings */
u8 vibration_intensity_left;
u8 vibration_intensity_right;
+
+ struct ally_joystick_resp_curve left_curve;
+ struct ally_joystick_resp_curve right_curve;
};
struct ally_handheld {
@@ -1749,6 +1772,391 @@ static struct device_attribute dev_attr_right_trigger_range_upper_limit =
static struct device_attribute dev_attr_right_trigger_range_upper_limit_range =
__ATTR(range_upper_limit_range, 0444, right_trigger_range_upper_limit_range_show, NULL);
+enum ally_joystick_side {
+ JOYSTICK_LEFT = 0,
+ JOYSTICK_RIGHT,
+};
+
+/**
+ * ally_set_joystick_resp_curve() - Set joystick response curve parameters
+ * @hdev: HID device
+ * @side: which joystick side (0=left, 1=right)
+ * @curve: response curve parameter structure
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_joystick_resp_curve(struct hid_device *hdev, enum ally_joystick_side side,
+ struct ally_joystick_resp_curve *curve)
+{
+ const u8 payload[] = { side,
+ curve->entry_1.move, curve->entry_1.resp,
+ curve->entry_2.move, curve->entry_2.resp,
+ curve->entry_3.move, curve->entry_3.resp,
+ curve->entry_4.move, curve->entry_4.resp
+ };
+ int ret;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_RESP_CURVE, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_dev_set_report(hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int response_curve_apply(struct hid_device *hdev,
+ struct ally_config *cfg, bool is_left)
+{
+ struct ally_joystick_resp_curve curve;
+ int ret;
+
+ /*
+ * Snapshot under the lock so a concurrent sysfs write cannot change an
+ * entry between the monotonicity check and the packet being built.
+ */
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->resp_curve_support)
+ return -EOPNOTSUPP;
+
+ curve = is_left ? cfg->left_curve : cfg->right_curve;
+ }
+
+ if (!(curve.entry_1.move < curve.entry_2.move &&
+ curve.entry_2.move < curve.entry_3.move &&
+ curve.entry_3.move < curve.entry_4.move))
+ return -EINVAL;
+
+ ret = ally_set_joystick_resp_curve(hdev,
+ is_left ? JOYSTICK_LEFT : JOYSTICK_RIGHT,
+ &curve);
+ if (ret) {
+ hid_err(hdev, "Failed to set joystick response curve: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t left_response_curve_apply_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ bool apply;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtobool(buf, &apply);
+ if (ret)
+ return ret;
+
+ if (!apply)
+ return count;
+
+ ret = response_curve_apply(hdev, cfg, true);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static ssize_t right_response_curve_apply_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *const ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ bool apply;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = kstrtobool(buf, &apply);
+ if (ret)
+ return ret;
+
+ if (!apply)
+ return count;
+
+ ret = response_curve_apply(hdev, cfg, false);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static ALLY_DEVICE_ATTR_WO(left_response_curve_apply, response_curve_apply);
+static ALLY_DEVICE_ATTR_WO(right_response_curve_apply, response_curve_apply);
+
+static ssize_t response_curve_pct_show(struct device *dev,
+ char *buf,
+ struct ally_config *cfg, bool is_left,
+ unsigned int idx)
+{
+ struct ally_joystick_resp_curve *curve;
+ int ret = -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ curve = is_left ? &cfg->left_curve : &cfg->right_curve;
+
+ switch (idx) {
+ case 1:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_1.resp);
+ break;
+ case 2:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_2.resp);
+ break;
+ case 3:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_3.resp);
+ break;
+ case 4:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_4.resp);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static ssize_t response_curve_move_show(struct device *dev,
+ char *buf,
+ struct ally_config *cfg, bool is_left,
+ unsigned int idx)
+{
+ struct ally_joystick_resp_curve *curve;
+ int ret = -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ curve = is_left ? &cfg->left_curve : &cfg->right_curve;
+
+ switch (idx) {
+ case 1:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_1.move);
+ break;
+ case 2:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_2.move);
+ break;
+ case 3:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_3.move);
+ break;
+ case 4:
+ ret = sysfs_emit(buf, "%u\n", curve->entry_4.move);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static ssize_t response_curve_pct_store(struct device *dev,
+ const char *buf, size_t count,
+ struct ally_config *cfg, bool is_left,
+ unsigned int idx)
+{
+ struct ally_joystick_resp_curve *curve;
+ u8 value;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret)
+ return ret;
+
+ if (value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->resp_curve_support)
+ return -EOPNOTSUPP;
+
+ curve = is_left ? &cfg->left_curve : &cfg->right_curve;
+
+ switch (idx) {
+ case 1:
+ curve->entry_1.resp = value;
+ break;
+ case 2:
+ curve->entry_2.resp = value;
+ break;
+ case 3:
+ curve->entry_3.resp = value;
+ break;
+ case 4:
+ curve->entry_4.resp = value;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return count;
+}
+
+static ssize_t response_curve_move_store(struct device *dev,
+ const char *buf, size_t count,
+ struct ally_config *cfg, bool is_left,
+ unsigned int idx)
+{
+ struct ally_joystick_resp_curve *curve;
+ u8 value;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret)
+ return ret;
+
+ if (value > 100)
+ return -EINVAL;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ if (!cfg->resp_curve_support)
+ return -EOPNOTSUPP;
+
+ curve = is_left ? &cfg->left_curve : &cfg->right_curve;
+
+ switch (idx) {
+ case 1:
+ curve->entry_1.move = value;
+ break;
+ case 2:
+ curve->entry_2.move = value;
+ break;
+ case 3:
+ curve->entry_3.move = value;
+ break;
+ case 4:
+ curve->entry_4.move = value;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return count;
+}
+
+#define DEFINE_JS_CURVE_PCT_FOPS(region, side) \
+ static ssize_t side##_response_curve_pct_##region##_show( \
+ struct device *dev, struct device_attribute *attr, \
+ char *buf) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ struct ally_config *cfg; \
+ \
+ if (!ally) \
+ return -ENODEV; \
+ \
+ cfg = ally_get_config(ally); \
+ if (!cfg) \
+ return -ENODEV; \
+ \
+ return response_curve_pct_show(dev, buf, cfg, \
+ side##_is_left, region);\
+ } \
+ \
+ static ssize_t side##_response_curve_pct_##region##_store( \
+ struct device *dev, struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ struct ally_config *cfg; \
+ \
+ if (!ally) \
+ return -ENODEV; \
+ \
+ cfg = ally_get_config(ally); \
+ if (!cfg) \
+ return -ENODEV; \
+ \
+ return response_curve_pct_store(dev, buf, count, cfg, \
+ side##_is_left, region);\
+ }
+
+#define DEFINE_JS_CURVE_MOVE_FOPS(region, side) \
+ static ssize_t side##_response_curve_move_##region##_show( \
+ struct device *dev, struct device_attribute *attr, \
+ char *buf) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ struct ally_config *cfg; \
+ \
+ if (!ally) \
+ return -ENODEV; \
+ \
+ cfg = ally_get_config(ally); \
+ if (!cfg) \
+ return -ENODEV; \
+ \
+ return response_curve_move_show(dev, buf, cfg, \
+ side##_is_left, region);\
+ } \
+ \
+ static ssize_t side##_response_curve_move_##region##_store( \
+ struct device *dev, struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct hid_device *hdev = to_hid_device(dev); \
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev); \
+ struct ally_handheld *ally = drvdata->rog_ally; \
+ struct ally_config *cfg; \
+ \
+ if (!ally) \
+ return -ENODEV; \
+ \
+ cfg = ally_get_config(ally); \
+ if (!cfg) \
+ return -ENODEV; \
+ \
+ return response_curve_move_store(dev, buf, count, cfg, \
+ side##_is_left, region);\
+ }
+
+#define DEFINE_JS_CURVE_ATTRS(region, side) \
+ DEFINE_JS_CURVE_PCT_FOPS(region, side) \
+ DEFINE_JS_CURVE_MOVE_FOPS(region, side) \
+ static ALLY_DEVICE_ATTR_RW(side##_response_curve_pct_##region, \
+ response_curve_pct_##region); \
+ static ALLY_DEVICE_ATTR_RW(side##_response_curve_move_##region, \
+ response_curve_move_##region)
+
+/* Helper defines for "is_left" parameter in DEFINE_JS_CURVE_ATTRS macros */
+#define left_is_left true
+#define right_is_left false
+
+DEFINE_JS_CURVE_ATTRS(1, left);
+DEFINE_JS_CURVE_ATTRS(2, left);
+DEFINE_JS_CURVE_ATTRS(3, left);
+DEFINE_JS_CURVE_ATTRS(4, left);
+
+DEFINE_JS_CURVE_ATTRS(1, right);
+DEFINE_JS_CURVE_ATTRS(2, right);
+DEFINE_JS_CURVE_ATTRS(3, right);
+DEFINE_JS_CURVE_ATTRS(4, right);
+
static struct attribute *ally_config_attrs[] = {
&dev_attr_xbox_controller.attr,
NULL
@@ -1773,6 +2181,15 @@ static struct attribute *left_joystick_axis_attrs[] = {
&dev_attr_left_joystick_outer_threshold_range.attr,
&dev_attr_left_joystick_anti_deadzone.attr,
&dev_attr_left_joystick_anti_deadzone_range.attr,
+ &dev_attr_left_response_curve_pct_1.attr,
+ &dev_attr_left_response_curve_pct_2.attr,
+ &dev_attr_left_response_curve_pct_3.attr,
+ &dev_attr_left_response_curve_pct_4.attr,
+ &dev_attr_left_response_curve_move_1.attr,
+ &dev_attr_left_response_curve_move_2.attr,
+ &dev_attr_left_response_curve_move_3.attr,
+ &dev_attr_left_response_curve_move_4.attr,
+ &dev_attr_left_response_curve_apply.attr,
NULL
};
@@ -1783,6 +2200,15 @@ static struct attribute *right_joystick_axis_attrs[] = {
&dev_attr_right_joystick_outer_threshold_range.attr,
&dev_attr_right_joystick_anti_deadzone.attr,
&dev_attr_right_joystick_anti_deadzone_range.attr,
+ &dev_attr_right_response_curve_pct_1.attr,
+ &dev_attr_right_response_curve_pct_2.attr,
+ &dev_attr_right_response_curve_pct_3.attr,
+ &dev_attr_right_response_curve_pct_4.attr,
+ &dev_attr_right_response_curve_move_1.attr,
+ &dev_attr_right_response_curve_move_2.attr,
+ &dev_attr_right_response_curve_move_3.attr,
+ &dev_attr_right_response_curve_move_4.attr,
+ &dev_attr_right_response_curve_apply.attr,
NULL
};
@@ -1877,6 +2303,25 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
cfg->vibration_intensity_left = 100;
cfg->vibration_intensity_right = 100;
+ /* Initialize default response curve values (linear) */
+ cfg->left_curve.entry_1.move = 0;
+ cfg->left_curve.entry_1.resp = 0;
+ cfg->left_curve.entry_2.move = 33;
+ cfg->left_curve.entry_2.resp = 33;
+ cfg->left_curve.entry_3.move = 66;
+ cfg->left_curve.entry_3.resp = 66;
+ cfg->left_curve.entry_4.move = 100;
+ cfg->left_curve.entry_4.resp = 100;
+
+ cfg->right_curve.entry_1.move = 0;
+ cfg->right_curve.entry_1.resp = 0;
+ cfg->right_curve.entry_2.move = 33;
+ cfg->right_curve.entry_2.resp = 33;
+ cfg->right_curve.entry_3.move = 66;
+ cfg->right_curve.entry_3.resp = 66;
+ cfg->right_curve.entry_4.move = 100;
+ cfg->right_curve.entry_4.resp = 100;
+
/* So far the only hardware this is supported is the Ally 1 */
if (cfg->xbox_controller_support) {
ret = ally_set_xbox_controller(hdev, ally, cfg, true);
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 08/12] HID: asus: add support for response curve
2026-08-19 19:58 ` [PATCH v2 08/12] HID: asus: add support for response curve Denis Benato
@ 2026-08-19 20:17 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:17 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inconsistent capability check for response curve sysfs read operations.
--
--- Patch [8]: [PATCH v2 08/12] HID: asus: add support for response curve ---
commit 64bba5ccd81bdede00727b014f07c871c53e11d2
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for response curve
This commit adds the ability for userspace to modify the response curve of
both joysticks on ROG ally devices by exposing relevant sysfs attributes.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 050d979f4315d..6f43b7317168f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static ssize_t response_curve_pct_show(struct device *dev,
> + char *buf,
> + struct ally_config *cfg, bool is_left,
> + unsigned int idx)
> +{
> + struct ally_joystick_resp_curve *curve;
> + int ret = -EINVAL;
> +
> + scoped_guard(mutex, &cfg->config_mutex) {
> + curve = is_left ? &cfg->left_curve : &cfg->right_curve;
> +
> + switch (idx) {
[Severity: Medium]
Should there be a capability check here before returning values?
The corresponding store operations check for !cfg->resp_curve_support and
return -EOPNOTSUPP, but this show function unconditionally returns the locally
initialized defaults (0, 33, 66, 100) even if the device doesn't support
response curves. Can this cause userspace feature-detection logic to
erroneously believe response curves are supported until a write fails?
> + case 1:
> + ret = sysfs_emit(buf, "%u\n", curve->entry_1.resp);
> + break;
> + case 2:
> + ret = sysfs_emit(buf, "%u\n", curve->entry_2.resp);
> + break;
[ ... ]
> +static ssize_t response_curve_move_show(struct device *dev,
> + char *buf,
> + struct ally_config *cfg, bool is_left,
> + unsigned int idx)
> +{
> + struct ally_joystick_resp_curve *curve;
> + int ret = -EINVAL;
> +
> + scoped_guard(mutex, &cfg->config_mutex) {
> + curve = is_left ? &cfg->left_curve : &cfg->right_curve;
> +
> + switch (idx) {
[Severity: Medium]
Similarly, should this show function also check !cfg->resp_curve_support
before returning values?
> + case 1:
> + ret = sysfs_emit(buf, "%u\n", curve->entry_1.move);
> + break;
> + case 2:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=8
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 09/12] HID: asus: add support to force feedback
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (7 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 08/12] HID: asus: add support for response curve Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:12 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 10/12] HID: asus: add support for gamepad mode Denis Benato
` (2 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
Unlike ROG ally the X version and following ones uses DInput protocol
and the force feedback needs to be implemented as its protocol is
vendor-specific, therefore add support for FF_RUMBLE with magnitude
scaling on a work-queue based approach to avoid using possibly
sleeping calls in atomic context.
Assisted-by: opencode:glm-5.2
Assisted-by: VSCode:gpt-5.3-codex
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Khamunetri Clark <khamunetriclark@gmail.com>
Signed-off-by: Luke Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 198 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 191 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 6f43b7317168..ca3050fea843 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -235,6 +235,23 @@ struct ally_config {
struct ally_joystick_resp_curve right_curve;
};
+/* XInput force-feedback report (output report 0x0d, gamepad interface) */
+struct ff_data {
+ u8 enable;
+ u8 magnitude_left;
+ u8 magnitude_right;
+ u8 magnitude_strong;
+ u8 magnitude_weak;
+ u8 pulse_sustain_10ms;
+ u8 pulse_release_10ms;
+ u8 loop_count;
+} __packed;
+
+struct ff_report {
+ u8 report_id;
+ struct ff_data ff;
+} __packed;
+
struct ally_handheld {
/* All read/write to IN interfaces must lock */
struct mutex intf_mutex;
@@ -243,6 +260,13 @@ struct ally_handheld {
struct input_dev *ally_x_input;
struct hid_device *ally_x_hdev;
+ struct ff_report ff_packet;
+ struct work_struct ff_work;
+ /* Serializes ff_packet and update_ff between play_effect and ff_work */
+ spinlock_t ff_lock;
+ bool ff_work_initialized;
+ bool update_ff;
+
struct hid_device *keyboard_hdev;
struct input_dev *keyboard_input;
@@ -357,9 +381,13 @@ enum ally_command_codes {
CMD_SET_ANTI_DEADZONE = 0x18,
};
+/* XInput rumble magnitudes use the hardware's 0..100 intensity range. */
+#define ALLY_FF_MAX_INTENSITY 100
+
static const u8 ALLY_FORCE_FEEDBACK_OFF[] = {
0x0D, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xEB
};
+static_assert(sizeof(struct ff_report) == sizeof(ALLY_FORCE_FEEDBACK_OFF));
/*
* The ROG Ally device presents multiple USB interfaces (keyboard, mouse, gamepad,
@@ -368,6 +396,7 @@ static const u8 ALLY_FORCE_FEEDBACK_OFF[] = {
* ally_handheld structure to share state across these separate HID interfaces.
*/
static void ally_resume_work_fn(struct work_struct *work);
+static void ally_x_ff_work_fn(struct work_struct *work);
/*
* Changes to ally_drvdata must lock: the raw_event callbacks, which may
@@ -383,6 +412,13 @@ static struct ally_handheld ally_drvdata = {
*/
.resume_work = __DELAYED_WORK_INITIALIZER(ally_drvdata.resume_work,
ally_resume_work_fn, 0),
+ /*
+ * Initialised statically for the same reason as resume_work: remove()
+ * drains the work whichever of the interfaces probed or failed to
+ * probe, so it must be safe to cancel unconditionally.
+ */
+ .ff_work = __WORK_INITIALIZER(ally_drvdata.ff_work, ally_x_ff_work_fn),
+ .ff_lock = __SPIN_LOCK_UNLOCKED(ally_drvdata.ff_lock),
};
static const u8 asus_report_id_init[] = {
@@ -2519,6 +2555,86 @@ static bool ally_x_raw_event(struct input_dev *input, struct hid_device *hdev,
return true;
}
+/**
+ * ally_x_send_ff_report() - Send a force-feedback report to the gamepad
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ * @buf: buffer containing the report to send
+ * @len: length of the report
+ *
+ * The gamepad interface consumes force-feedback packets as output reports,
+ * unlike the config interface which expects feature reports: a rumble packet
+ * sent as HID_REQ_SET_REPORT would be rejected by the hardware.
+ *
+ * Return: count of data transferred, negative if error
+ */
+static int ally_x_send_ff_report(struct ally_handheld *ally,
+ struct hid_device *hdev,
+ const u8 *buf, size_t len)
+{
+ u8 *dmabuf __free(kfree) = kmemdup(buf, len, GFP_KERNEL);
+
+ if (!dmabuf)
+ return -ENOMEM;
+
+ scoped_guard(mutex, &ally->intf_mutex)
+ return hid_hw_output_report(hdev, dmabuf, len);
+}
+
+static int ally_x_send_ff_off(struct ally_handheld *ally, struct hid_device *hdev)
+{
+ return ally_x_send_ff_report(ally, hdev, ALLY_FORCE_FEEDBACK_OFF,
+ sizeof(ALLY_FORCE_FEEDBACK_OFF));
+}
+
+static void ally_x_ff_work_fn(struct work_struct *work)
+{
+ struct ally_handheld *ally =
+ container_of(work, struct ally_handheld, ff_work);
+ struct hid_device *hdev = NULL;
+ struct ff_report report;
+ bool update = false;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &ally->ff_lock) {
+ if (ally->update_ff) {
+ report = ally->ff_packet;
+ ally->update_ff = false;
+ update = true;
+ hdev = ally->ally_x_hdev;
+ }
+ }
+
+ if (!update || !hdev)
+ return;
+
+ ret = ally_x_send_ff_report(ally, hdev, (u8 *)&report, sizeof(report));
+ if (ret < 0)
+ hid_err(hdev, "Failed to send force-feedback: %d\n", ret);
+}
+
+static int ally_x_play_effect(struct input_dev *idev, void *data,
+ struct ff_effect *effect)
+{
+ struct ally_handheld *ally = &ally_drvdata;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ scoped_guard(spinlock_irqsave, &ally->ff_lock) {
+ ally->ff_packet.ff.magnitude_strong =
+ effect->u.rumble.strong_magnitude * ALLY_FF_MAX_INTENSITY / 65535;
+ ally->ff_packet.ff.magnitude_weak =
+ effect->u.rumble.weak_magnitude * ALLY_FF_MAX_INTENSITY / 65535;
+ ally->update_ff = true;
+
+ if (ally->ff_work_initialized)
+ schedule_work(&ally->ff_work);
+ }
+
+ return 0;
+}
+
static struct input_dev *ally_x_alloc_input_dev(struct hid_device *hdev)
{
struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
@@ -2574,6 +2690,21 @@ static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *all
input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY);
input_set_capability(input, EV_KEY, BTN_TRIGGER_HAPPY1);
+ memcpy(&ally->ff_packet, ALLY_FORCE_FEEDBACK_OFF, sizeof(ally->ff_packet));
+ ally->ff_work_initialized = true;
+
+ input_set_capability(input, EV_FF, FF_RUMBLE);
+
+ /*
+ * A registered device advertising FF_RUMBLE without the memless core
+ * behind it would crash input_ff_upload(): fail the whole setup instead.
+ */
+ ret = input_ff_create_memless(input, NULL, ally_x_play_effect);
+ if (ret) {
+ hid_err(hdev, "Failed to create force-feedback: %d\n", ret);
+ goto ally_x_setup_input_err;
+ }
+
ret = input_register_device(input);
if (ret) {
hid_err(hdev, "Failed to register Ally X gamepad device: %d\n", ret);
@@ -2592,13 +2723,28 @@ static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *all
static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *ally)
{
- int ret;
+ struct hid_device *x_hdev;
+ unsigned long flags;
- /* Failure at this point is non-critical */
- ret = ally_gamepad_send_packet(ally, hdev, ALLY_FORCE_FEEDBACK_OFF,
- sizeof(ALLY_FORCE_FEEDBACK_OFF));
- if (ret < 0)
- hid_err(hdev, "Ally failed to init force-feedback off: %d\n", ret);
+ /*
+ * The force-feedback "off" packet belongs to the gamepad interface,
+ * which consumes it as an output report: the config interface probed
+ * here would reject it. The gamepad probe path sends the packet itself,
+ * so this only matters when the gamepad interface is already bound,
+ * e.g. after a reset resume that re-initialized the MCU.
+ */
+ spin_lock_irqsave(&ally_data_lock, flags);
+ x_hdev = ally->ally_x_hdev;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ if (x_hdev) {
+ /* Failure at this point is non-critical */
+ int ret = ally_x_send_ff_off(ally, x_hdev);
+
+ if (ret < 0)
+ hid_err(hdev, "Ally failed to init force-feedback off: %d\n",
+ ret);
+ }
return 0;
}
@@ -2717,6 +2863,14 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
return ERR_PTR(ret);
}
+ /*
+ * Make sure rumble starts disabled: this is the interface that
+ * owns the force-feedback output report. Failure is non-critical.
+ */
+ ret = ally_x_send_ff_off(&ally_drvdata, hdev);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to disable force-feedback: %d\n", ret);
+
spin_lock_irqsave(&ally_data_lock, flags);
ally_drvdata.ally_x_hdev = hdev;
spin_unlock_irqrestore(&ally_data_lock, flags);
@@ -2741,6 +2895,7 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *ally)
{
unsigned long flags;
+ bool owns_xpad;
if (!ally)
return;
@@ -2754,7 +2909,36 @@ static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *
cancel_delayed_work_sync(&ally->resume_work);
spin_lock_irqsave(&ally_data_lock, flags);
- if (ally->ally_x_hdev == hdev) {
+ owns_xpad = ally->ally_x_hdev == hdev;
+ spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ if (owns_xpad) {
+ /*
+ * Stop queueing force-feedback work before the gamepad
+ * pointers are cleared: play_effect() tests the flag under the
+ * same lock, so no work can be queued past the cancel below.
+ */
+ scoped_guard(spinlock_irqsave, &ally->ff_lock)
+ ally->ff_work_initialized = false;
+
+ /*
+ * cancel_work_sync() may sleep: keep it out of ally_data_lock,
+ * but run it before the pointers are cleared so in-flight work
+ * cannot outlive the interface it sends through.
+ */
+ cancel_work_sync(&ally->ff_work);
+
+ /*
+ * The input core can no longer stop effects through the
+ * disabled work: quiesce any rumble still playing ourselves,
+ * or the device would keep vibrating after being unbound.
+ */
+ if (ally_x_send_ff_off(ally, hdev) < 0)
+ hid_warn(hdev, "Failed to stop force-feedback\n");
+ }
+
+ spin_lock_irqsave(&ally_data_lock, flags);
+ if (owns_xpad) {
ally->ally_x_input = NULL;
ally->ally_x_hdev = NULL;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 09/12] HID: asus: add support to force feedback
2026-08-19 19:58 ` [PATCH v2 09/12] HID: asus: add support to force feedback Denis Benato
@ 2026-08-19 20:12 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:12 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Concurrent unbind of the gamepad interface causes a Use-After-Free of its `hid_device` in `hid_asus_ally_init()`.
- [Medium] Data race and lifecycle violation on `ally_x_hdev` causes dropped force-feedback events and undefined behavior.
--
--- Patch [9]: [PATCH v2 09/12] HID: asus: add support to force feedback ---
commit 009ee410a607fb5f11138c63d43a972421d99051
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: add support to force feedback
Unlike ROG ally the X version and following ones uses DInput protocol
and the force feedback needs to be implemented as its protocol is
vendor-specific, therefore add support for FF_RUMBLE with magnitude
scaling on a work-queue based approach to avoid using possibly
sleeping calls in atomic context.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 6f43b7317168f..ca3050fea843b 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -2519,6 +2555,86 @@ static bool ally_x_raw_event(struct input_dev *input, struct hid_device *hdev,
[ ... ]
> +static void ally_x_ff_work_fn(struct work_struct *work)
> +{
> + struct ally_handheld *ally =
> + container_of(work, struct ally_handheld, ff_work);
> + struct hid_device *hdev = NULL;
> + struct ff_report report;
> + bool update = false;
> + int ret;
> +
> + scoped_guard(spinlock_irqsave, &ally->ff_lock) {
> + if (ally->update_ff) {
> + report = ally->ff_packet;
> + ally->update_ff = false;
> + update = true;
> + hdev = ally->ally_x_hdev;
[Severity: Medium]
Will this read of ally_x_hdev race with the assignment in
hid_asus_ally_probe()?
Here in ally_x_ff_work_fn(), the hdev pointer is read under the ff_lock.
However, during probe, ally_x_hdev is assigned under a different lock
(ally_data_lock):
hid_asus_ally_probe() {
...
spin_lock_irqsave(&ally_data_lock, flags);
ally_drvdata.ally_x_hdev = hdev;
spin_unlock_irqrestore(&ally_data_lock, flags);
...
}
> + }
> + }
[ ... ]
> @@ -2574,6 +2690,21 @@ static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *all
[ ... ]
> + ret = input_ff_create_memless(input, NULL, ally_x_play_effect);
> + if (ret) {
> + hid_err(hdev, "Failed to create force-feedback: %d\n", ret);
> + goto ally_x_setup_input_err;
> + }
> +
> ret = input_register_device(input);
[Severity: Medium]
Is it possible for userspace to immediately send force-feedback events as
soon as this registers, before ally_x_hdev is initialized?
If input_register_device() returns successfully, the device is exposed to
userspace. If a client immediately sends an effect, ally_x_play_effect() can
schedule ally_x_ff_work_fn().
Since ally_x_hdev is only assigned later in hid_asus_ally_probe() after
setup completes, could ally_x_ff_work_fn() see a NULL ally_x_hdev and drop
the initial effects?
> if (ret) {
> hid_err(hdev, "Failed to register Ally X gamepad device: %d\n", ret);
[ ... ]
> @@ -2592,13 +2723,28 @@ static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *all
>
> static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *ally)
> {
> - int ret;
> + struct hid_device *x_hdev;
> + unsigned long flags;
>
> - /* Failure at this point is non-critical */
> - ret = ally_gamepad_send_packet(ally, hdev, ALLY_FORCE_FEEDBACK_OFF,
> - sizeof(ALLY_FORCE_FEEDBACK_OFF));
> - if (ret < 0)
> - hid_err(hdev, "Ally failed to init force-feedback off: %d\n", ret);
> + /*
> + * The force-feedback "off" packet belongs to the gamepad interface,
> + * which consumes it as an output report: the config interface probed
> + * here would reject it. The gamepad probe path sends the packet itself,
> + * so this only matters when the gamepad interface is already bound,
> + * e.g. after a reset resume that re-initialized the MCU.
> + */
> + spin_lock_irqsave(&ally_data_lock, flags);
> + x_hdev = ally->ally_x_hdev;
> + spin_unlock_irqrestore(&ally_data_lock, flags);
> +
> + if (x_hdev) {
> + /* Failure at this point is non-critical */
> + int ret = ally_x_send_ff_off(ally, x_hdev);
[Severity: High]
Can this use the hid_device after it has been freed?
The ally_x_hdev pointer is read and cached under ally_data_lock, but it
is used after the lock is dropped without acquiring a reference via
get_device().
If userspace concurrently unbinds the gamepad interface via sysfs, the
gamepad's remove callback (hid_asus_ally_remove) will clear
ally->ally_x_hdev and return, allowing the HID core to destroy and free
the hid_device structure.
Since hid_asus_ally_init() operates outside the lock without a reference,
would this result in ally_x_send_ff_off() dereferencing a freed pointer?
> +
> + if (ret < 0)
> + hid_err(hdev, "Ally failed to init force-feedback off: %d\n",
> + ret);
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=9
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 10/12] HID: asus: add support for gamepad mode
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (8 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 09/12] HID: asus: add support to force feedback Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 19:58 ` [PATCH v2 11/12] HID: asus: add support for turbo buttons Denis Benato
2026-08-19 19:58 ` [PATCH v2 12/12] HID: asus: add support for btn remapping Denis Benato
11 siblings, 0 replies; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG Ally devices can emulate either a mouse+keyboard (desktop mode)
or an gamepad device (xbox360 controller in ROG ally and a custom
DInput device on newer models): add support for switching the current
controller mode.
Assisted-by: opencode:glm-5.2
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 191 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 188 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index ca3050fea843..6be25cc022cb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -384,6 +384,20 @@ enum ally_command_codes {
/* XInput rumble magnitudes use the hardware's 0..100 intensity range. */
#define ALLY_FF_MAX_INTENSITY 100
+enum ally_gamepad_mode_index {
+ ALLY_GAMEPAD_MODE_GAMEPAD = 0x01,
+ ALLY_GAMEPAD_MODE_KEYBOARD = 0x02,
+};
+
+static const char *const ally_gamepad_mode_text[] = {
+ "gamepad", "desktop"
+};
+
+static const u8 ally_gamepad_mode[] = {
+ ALLY_GAMEPAD_MODE_GAMEPAD,
+ ALLY_GAMEPAD_MODE_KEYBOARD
+};
+
static const u8 ALLY_FORCE_FEEDBACK_OFF[] = {
0x0D, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xEB
};
@@ -834,6 +848,152 @@ static ssize_t xbox_controller_store(struct device *dev,
static DEVICE_ATTR_RW(xbox_controller);
+/**
+ * ally_set_gamepad_mode() - Set the gamepad operating mode
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ * @mode: gamepad mode to set
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device *hdev, u8 mode)
+{
+ u8 payload[] = { mode };
+ int ret;
+
+ if (mode < ALLY_GAMEPAD_MODE_GAMEPAD ||
+ mode > ALLY_GAMEPAD_MODE_KEYBOARD) {
+ hid_err(hdev, "Invalid gamepad mode: %u\n", mode);
+ return -EINVAL;
+ }
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_GAMEPAD_MODE, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_gamepad_send_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set gamepad mode: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 mode_byte;
+ int i;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ mode_byte = cfg->gamepad_mode;
+
+ for (i = 0; i < ARRAY_SIZE(ally_gamepad_mode); i++) {
+ if (ally_gamepad_mode[i] == mode_byte)
+ return sysfs_emit(buf, "%s\n", ally_gamepad_mode_text[i]);
+ }
+ }
+
+ return sysfs_emit(buf, "unsupported\n");
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct ally_config *cfg;
+ u8 mode_byte;
+ int mode;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ mode = sysfs_match_string(ally_gamepad_mode_text, buf);
+ if (mode < 0) {
+ hid_err(hdev, "Unknown gamepad mode\n");
+ return mode;
+ }
+
+ /* Convert the index of the text mode array to the byte
+ * that will be accepted by the ally MCU.
+ */
+ mode_byte = ally_gamepad_mode[mode];
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ ret = ally_set_gamepad_mode(ally, hdev, mode_byte);
+ if (ret < 0)
+ return ret;
+
+ cfg->gamepad_mode = mode_byte;
+ }
+
+ hid_dbg(hdev, "Set gamepad mode to %s\n", ally_gamepad_mode_text[mode]);
+
+ return count;
+}
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ int i;
+ ssize_t len = 0;
+
+ for (i = 0; i < ARRAY_SIZE(ally_gamepad_mode_text); i++) {
+ if (!ally_gamepad_mode_text[i] || ally_gamepad_mode_text[i][0] == '\0')
+ continue;
+ len += sysfs_emit_at(buf, len, "%s ", ally_gamepad_mode_text[i]);
+ }
+
+ /* Replace the last space with a newline */
+ if (len > 0)
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static DEVICE_ATTR_RW(gamepad_mode);
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+/**
+ * ally_apply_gamepad_mode() - Re-send the recorded gamepad mode to the hardware
+ * @hdev: HID device
+ * @ally: ally handheld structure
+ * @cfg: ally config
+ *
+ * Re-send the gamepad mode currently recorded in the configuration to the
+ * hardware: during probe that is the default mode, while a reset resume
+ * restores whatever the user last wrote instead of overwriting it. The
+ * shared state is accessed under the lock, as sysfs is already live.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_apply_gamepad_mode(struct hid_device *hdev,
+ struct ally_handheld *ally,
+ struct ally_config *cfg)
+{
+ scoped_guard(mutex, &cfg->config_mutex)
+ return ally_set_gamepad_mode(ally, hdev, cfg->gamepad_mode);
+}
+
/**
* ally_set_vibration_intensity() - Set vibration intensity values
* @hdev: HID device
@@ -2195,6 +2355,8 @@ DEFINE_JS_CURVE_ATTRS(4, right);
static struct attribute *ally_config_attrs[] = {
&dev_attr_xbox_controller.attr,
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
NULL
};
@@ -2327,7 +2489,7 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
}
}
- cfg->gamepad_mode = 0x01;
+ cfg->gamepad_mode = ALLY_GAMEPAD_MODE_GAMEPAD;
cfg->left_deadzone = 10;
cfg->left_outer_threshold = 90;
cfg->right_deadzone = 10;
@@ -2366,6 +2528,14 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
ret);
}
+ /*
+ * Set the default gamepad mode now that the MCU is confirmed ready:
+ * failure is non-critical, the mode can still be changed from sysfs.
+ */
+ ret = ally_apply_gamepad_mode(hdev, ally, cfg);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to set default gamepad mode: %d\n", ret);
+
cfg->initialized = true;
return cfg;
@@ -2723,8 +2893,10 @@ static int ally_x_setup_input(struct hid_device *hdev, struct ally_handheld *all
static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *ally)
{
+ struct ally_config *cfg;
struct hid_device *x_hdev;
unsigned long flags;
+ int ret;
/*
* The force-feedback "off" packet belongs to the gamepad interface,
@@ -2739,13 +2911,26 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all
if (x_hdev) {
/* Failure at this point is non-critical */
- int ret = ally_x_send_ff_off(ally, x_hdev);
-
+ ret = ally_x_send_ff_off(ally, x_hdev);
if (ret < 0)
hid_err(hdev, "Ally failed to init force-feedback off: %d\n",
ret);
}
+ cfg = ally_get_config(ally);
+ if (cfg) {
+ /*
+ * The MCU was just reset: restore the mode the configuration
+ * records instead of forcing a default, so a user-configured
+ * mode survives suspend/resume cycles. During probe this runs
+ * before the config is published, and ally_config_create()
+ * applies the default itself.
+ */
+ ret = ally_apply_gamepad_mode(hdev, ally, cfg);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to restore gamepad mode: %d\n", ret);
+ }
+
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 11/12] HID: asus: add support for turbo buttons
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (9 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 10/12] HID: asus: add support for gamepad mode Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:23 ` sashiko-bot
2026-08-19 19:58 ` [PATCH v2 12/12] HID: asus: add support for btn remapping Denis Benato
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
ROG ally devices supports a feature called turbo buttons that allows
the user to quickly toggle button state by keeping a button pressed,
therefore allow userspace to configure the feature by exposing
relevant sysfs attributes.
Assisted-by: opencode:glm-5.2
Assisted-by: Claude:claude-fable-5
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Khamunetri Clark <khamunetriclark@gmail.com>
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
drivers/hid/hid-asus.c | 414 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 414 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 6be25cc022cb..d6ab906b7e78 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -198,6 +198,99 @@ struct ally_joystick_resp_curve {
struct ally_joystick_resp_curve_param entry_4;
} __packed;
+/* Button identifiers for the turbo attribute system */
+enum ally_button_id {
+ ALLY_BTN_A,
+ ALLY_BTN_B,
+ ALLY_BTN_X,
+ ALLY_BTN_Y,
+ ALLY_BTN_LB,
+ ALLY_BTN_RB,
+ ALLY_BTN_DU,
+ ALLY_BTN_DD,
+ ALLY_BTN_DL,
+ ALLY_BTN_DR,
+ ALLY_BTN_J0B,
+ ALLY_BTN_J1B,
+ ALLY_BTN_MENU,
+ ALLY_BTN_VIEW,
+ ALLY_BTN_M1,
+ ALLY_BTN_M2,
+ ALLY_BTN_MAX
+};
+
+/* Names for the button directories in sysfs */
+static const char *const ally_button_names[ALLY_BTN_MAX] = {
+ [ALLY_BTN_A] = "btn_a",
+ [ALLY_BTN_B] = "btn_b",
+ [ALLY_BTN_X] = "btn_x",
+ [ALLY_BTN_Y] = "btn_y",
+ [ALLY_BTN_LB] = "btn_lb",
+ [ALLY_BTN_RB] = "btn_rb",
+ [ALLY_BTN_DU] = "dpad_up",
+ [ALLY_BTN_DD] = "dpad_down",
+ [ALLY_BTN_DL] = "dpad_left",
+ [ALLY_BTN_DR] = "dpad_right",
+ [ALLY_BTN_J0B] = "btn_l3",
+ [ALLY_BTN_J1B] = "btn_r3",
+ [ALLY_BTN_MENU] = "btn_menu",
+ [ALLY_BTN_VIEW] = "btn_view",
+ [ALLY_BTN_M1] = "btn_m1",
+ [ALLY_BTN_M2] = "btn_m2",
+};
+
+/*
+ * Button turbo parameters structure
+ * Each button can have:
+ * - turbo: Turbo press interval in multiples of 50ms (0 = disabled, 1-20 = 50ms-1000ms)
+ * - toggle: Toggle interval (0 = disabled)
+ */
+struct ally_btn_turbo_params {
+ u8 turbo;
+ u8 toggle;
+} __packed;
+
+#define ALLY_TURBO_PERIOD_MIN 0
+#define ALLY_TURBO_PERIOD_MAX 20
+#define ALLY_TOGGLE_PERIOD_MIN 0
+#define ALLY_TOGGLE_PERIOD_MAX 255
+
+/* Collection of all button turbo settings */
+struct ally_turbo_config {
+ struct ally_btn_turbo_params btn_du;
+ struct ally_btn_turbo_params btn_dd;
+ struct ally_btn_turbo_params btn_dl;
+ struct ally_btn_turbo_params btn_dr;
+ struct ally_btn_turbo_params btn_j0b;
+ struct ally_btn_turbo_params btn_j1b;
+ struct ally_btn_turbo_params btn_lb;
+ struct ally_btn_turbo_params btn_rb;
+ struct ally_btn_turbo_params btn_a;
+ struct ally_btn_turbo_params btn_b;
+ struct ally_btn_turbo_params btn_x;
+ struct ally_btn_turbo_params btn_y;
+ struct ally_btn_turbo_params btn_view;
+ struct ally_btn_turbo_params btn_menu;
+ struct ally_btn_turbo_params btn_m2;
+ struct ally_btn_turbo_params btn_m1;
+};
+
+struct ally_btn_sysfs_entry {
+ struct attribute_group group;
+ struct attribute *attrs[5]; /* turbo_period + toggle_period + ranges + NULL */
+ /*
+ * The entries are created while hid_asus_ally_probe() is still running,
+ * before asus_probe() publishes drvdata->rog_ally: carrying the shared
+ * state pointer here avoids dereferencing that not-yet-assigned field.
+ */
+ struct ally_handheld *ally;
+ struct ally_config *cfg;
+ struct hid_device *hdev;
+ enum ally_button_id btn;
+ struct device_attribute attr_turbo_period;
+ struct device_attribute attr_toggle_period;
+};
+
struct ally_config {
/* Must be locked if the data is being changed */
struct mutex config_mutex;
@@ -231,6 +324,9 @@ struct ally_config {
u8 vibration_intensity_left;
u8 vibration_intensity_right;
+ struct ally_turbo_config turbo;
+ struct ally_btn_sysfs_entry *button_entries;
+
struct ally_joystick_resp_curve left_curve;
struct ally_joystick_resp_curve right_curve;
};
@@ -2456,6 +2552,311 @@ static const struct attribute_group ally_attr_groups[] = {
},
};
+/**
+ * ally_set_turbo_params() - Set turbo parameters for all buttons
+ * @ally: ally handheld structure
+ * @hdev: HID device
+ * @cfg: ally config structure
+ *
+ * The payload mirrors the whole per-button turbo configuration, so the
+ * caller must hold cfg->config_mutex: building it locklessly could send a
+ * mix of parameters from concurrent sysfs writes to the hardware.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_set_turbo_params(struct ally_handheld *ally,
+ struct hid_device *hdev,
+ struct ally_config *cfg)
+{
+ struct ally_turbo_config *turbo = &cfg->turbo;
+ const u8 payload[] = {
+ turbo->btn_du.turbo,
+ turbo->btn_du.toggle,
+ turbo->btn_dd.turbo,
+ turbo->btn_dd.toggle,
+ turbo->btn_dl.turbo,
+ turbo->btn_dl.toggle,
+ turbo->btn_dr.turbo,
+ turbo->btn_dr.toggle,
+ turbo->btn_j0b.turbo,
+ turbo->btn_j0b.toggle,
+ turbo->btn_j1b.turbo,
+ turbo->btn_j1b.toggle,
+ turbo->btn_lb.turbo,
+ turbo->btn_lb.toggle,
+ turbo->btn_rb.turbo,
+ turbo->btn_rb.toggle,
+ turbo->btn_a.turbo,
+ turbo->btn_a.toggle,
+ turbo->btn_b.turbo,
+ turbo->btn_b.toggle,
+ turbo->btn_x.turbo,
+ turbo->btn_x.toggle,
+ turbo->btn_y.turbo,
+ turbo->btn_y.toggle,
+ turbo->btn_view.turbo,
+ turbo->btn_view.toggle,
+ turbo->btn_menu.turbo,
+ turbo->btn_menu.toggle,
+ turbo->btn_m2.turbo,
+ turbo->btn_m2.toggle,
+ turbo->btn_m1.turbo,
+ turbo->btn_m1.toggle,
+ };
+ int ret;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_TURBO_PARAMS, payload, sizeof(payload));
+ if (!buf)
+ return -ENOMEM;
+
+ ret = ally_gamepad_send_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to set turbo parameters: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct ally_btn_turbo_params *ally_btn_get_turbo_params(struct ally_config *cfg,
+ enum ally_button_id btn)
+{
+ switch (btn) {
+ case ALLY_BTN_DU: return &cfg->turbo.btn_du;
+ case ALLY_BTN_DD: return &cfg->turbo.btn_dd;
+ case ALLY_BTN_DL: return &cfg->turbo.btn_dl;
+ case ALLY_BTN_DR: return &cfg->turbo.btn_dr;
+ case ALLY_BTN_J0B: return &cfg->turbo.btn_j0b;
+ case ALLY_BTN_J1B: return &cfg->turbo.btn_j1b;
+ case ALLY_BTN_LB: return &cfg->turbo.btn_lb;
+ case ALLY_BTN_RB: return &cfg->turbo.btn_rb;
+ case ALLY_BTN_A: return &cfg->turbo.btn_a;
+ case ALLY_BTN_B: return &cfg->turbo.btn_b;
+ case ALLY_BTN_X: return &cfg->turbo.btn_x;
+ case ALLY_BTN_Y: return &cfg->turbo.btn_y;
+ case ALLY_BTN_VIEW: return &cfg->turbo.btn_view;
+ case ALLY_BTN_MENU: return &cfg->turbo.btn_menu;
+ case ALLY_BTN_M2: return &cfg->turbo.btn_m2;
+ case ALLY_BTN_M1: return &cfg->turbo.btn_m1;
+ default: return NULL;
+ }
+}
+
+static ssize_t btn_turbo_period_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
+ attr_turbo_period);
+ struct ally_btn_turbo_params *params = ally_btn_get_turbo_params(entry->cfg,
+ entry->btn);
+
+ if (!params)
+ return -ENODEV;
+
+ scoped_guard(mutex, &entry->cfg->config_mutex)
+ return sysfs_emit(buf, "%hhu\n", params->turbo);
+}
+
+static ssize_t btn_turbo_period_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
+ attr_turbo_period);
+ struct ally_btn_turbo_params *params;
+ u8 value;
+ int ret;
+
+ if (!entry->cfg->turbo_support)
+ return -EOPNOTSUPP;
+
+ params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
+ if (!params)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret)
+ return ret;
+
+ if (value < ALLY_TURBO_PERIOD_MIN || value > ALLY_TURBO_PERIOD_MAX)
+ return -EINVAL;
+
+ scoped_guard(mutex, &entry->cfg->config_mutex) {
+ params->turbo = value;
+ ret = ally_set_turbo_params(entry->ally, entry->hdev, entry->cfg);
+ if (ret)
+ return ret;
+ }
+
+ return count;
+}
+
+static ssize_t btn_toggle_period_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
+ attr_toggle_period);
+ struct ally_btn_turbo_params *params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
+
+ if (!params)
+ return -ENODEV;
+
+ scoped_guard(mutex, &entry->cfg->config_mutex)
+ return sysfs_emit(buf, "%hhu\n", params->toggle);
+}
+
+static ssize_t btn_toggle_period_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
+ attr_toggle_period);
+ struct ally_btn_turbo_params *params;
+ u8 value;
+ int ret;
+
+ if (!entry->cfg->turbo_support)
+ return -EOPNOTSUPP;
+
+ params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
+ if (!params)
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &value);
+ if (ret)
+ return ret;
+
+ if (value < ALLY_TOGGLE_PERIOD_MIN || value > ALLY_TOGGLE_PERIOD_MAX)
+ return -EINVAL;
+
+ scoped_guard(mutex, &entry->cfg->config_mutex) {
+ params->toggle = value;
+ ret = ally_set_turbo_params(entry->ally, entry->hdev, entry->cfg);
+ if (ret)
+ return ret;
+ }
+
+ return count;
+}
+
+static ssize_t btn_turbo_period_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0 20\n");
+}
+
+static ssize_t btn_toggle_period_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0 255\n");
+}
+
+static struct device_attribute dev_attr_btn_turbo_period_range =
+ __ATTR(turbo_period_range, 0444, btn_turbo_period_range_show, NULL);
+
+static struct device_attribute dev_attr_btn_toggle_period_range =
+ __ATTR(toggle_period_range, 0444, btn_toggle_period_range_show, NULL);
+
+static void ally_btn_turbo_init_attrs(struct ally_btn_sysfs_entry *entry)
+{
+ sysfs_attr_init(&entry->attr_turbo_period.attr);
+ entry->attr_turbo_period.attr.name = "turbo_period";
+ entry->attr_turbo_period.attr.mode = 0644;
+ entry->attr_turbo_period.show = btn_turbo_period_show;
+ entry->attr_turbo_period.store = btn_turbo_period_store;
+
+ sysfs_attr_init(&entry->attr_toggle_period.attr);
+ entry->attr_toggle_period.attr.name = "toggle_period";
+ entry->attr_toggle_period.attr.mode = 0644;
+ entry->attr_toggle_period.show = btn_toggle_period_show;
+ entry->attr_toggle_period.store = btn_toggle_period_store;
+ entry->attrs[0] = &entry->attr_turbo_period.attr;
+ entry->attrs[1] = &entry->attr_toggle_period.attr;
+ entry->attrs[2] = &dev_attr_btn_turbo_period_range.attr;
+ entry->attrs[3] = &dev_attr_btn_toggle_period_range.attr;
+ entry->attrs[4] = NULL;
+}
+
+/**
+ * ally_create_button_attributes() - Create turbo button attributes
+ * @hdev: HID device
+ * @ally: ally handheld structure
+ * @cfg: ally config structure
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int ally_create_button_attributes(struct hid_device *hdev,
+ struct ally_handheld *ally,
+ struct ally_config *cfg)
+{
+ struct ally_btn_sysfs_entry *entries;
+ int i, ret;
+
+ if (!cfg->turbo_support)
+ return 0;
+
+ entries = devm_kcalloc(&hdev->dev, ALLY_BTN_MAX, sizeof(*entries), GFP_KERNEL);
+ if (!entries)
+ return -ENOMEM;
+
+ cfg->button_entries = entries;
+
+ for (i = 0; i < ALLY_BTN_MAX; i++) {
+ entries[i].ally = ally;
+ entries[i].cfg = cfg;
+ entries[i].hdev = hdev;
+ entries[i].btn = i;
+
+ if (!ally_btn_get_turbo_params(cfg, i)) {
+ hid_err(hdev, "Invalid button id %d for turbo attributes\n", i);
+ ret = -EINVAL;
+ goto err_cleanup;
+ }
+
+ ally_btn_turbo_init_attrs(&entries[i]);
+
+ entries[i].group.name = ally_button_names[i];
+ entries[i].group.attrs = entries[i].attrs;
+
+ ret = sysfs_create_group(&hdev->dev.kobj, &entries[i].group);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to create sysfs group for %s: %d\n",
+ ally_button_names[i], ret);
+ goto err_cleanup;
+ }
+ }
+
+ return 0;
+
+err_cleanup:
+ /* Only groups [0, i) were registered; the failure happened at i. */
+ while (--i >= 0)
+ sysfs_remove_group(&hdev->dev.kobj, &entries[i].group);
+
+ cfg->button_entries = NULL;
+
+ return ret;
+}
+
+/**
+ * ally_remove_button_attributes() - Remove turbo button attributes
+ * @hdev: HID device
+ * @cfg: ally config structure
+ */
+static void ally_remove_button_attributes(struct hid_device *hdev, struct ally_config *cfg)
+{
+ struct ally_btn_sysfs_entry *entries;
+ int i;
+
+ if (!cfg || !cfg->button_entries)
+ return;
+
+ entries = cfg->button_entries;
+
+ for (i = 0; i < ALLY_BTN_MAX; i++)
+ sysfs_remove_group(&hdev->dev.kobj, &entries[i].group);
+
+ cfg->button_entries = NULL;
+}
+
/**
* ally_config_create() - Initialize configuration and create sysfs entries
* @hdev: HID device
@@ -2489,6 +2890,14 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
}
}
+ if (cfg->turbo_support) {
+ ret = ally_create_button_attributes(hdev, ally, cfg);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to create button attributes: %d\n", ret);
+ goto ally_config_create_sysfs_err;
+ }
+ }
+
cfg->gamepad_mode = ALLY_GAMEPAD_MODE_GAMEPAD;
cfg->left_deadzone = 10;
cfg->left_outer_threshold = 90;
@@ -2540,6 +2949,8 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
return cfg;
ally_config_create_sysfs_err:
+ if (cfg->turbo_support && cfg->button_entries)
+ ally_remove_button_attributes(hdev, cfg);
ally_config_create_err:
devm_kfree(&hdev->dev, cfg);
return ERR_PTR(ret);
@@ -2556,6 +2967,9 @@ static void ally_config_remove(struct hid_device *hdev, struct ally_handheld *al
if (!cfg || !cfg->initialized)
return;
+
+ if (cfg->turbo_support && cfg->button_entries)
+ ally_remove_button_attributes(hdev, cfg);
}
/**
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 11/12] HID: asus: add support for turbo buttons
2026-08-19 19:58 ` [PATCH v2 11/12] HID: asus: add support for turbo buttons Denis Benato
@ 2026-08-19 20:23 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:23 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Sysfs attribute leak on probe error path leads to a Use-After-Free when accessed.
- [High] Calling `sysfs_remove_group()` while holding a spinlock with interrupts disabled causes a scheduling-while-atomic kernel panic.
- [Medium] State desynchronization on write failure: the local cached turbo configuration is permanently modified before the hardware acknowledges the change.
- [Low] Missing trailing return statement after `scoped_guard` causes `-Wreturn-type` compiler warnings.
--
--- Patch [11]: [PATCH v2 11/12] HID: asus: add support for turbo buttons ---
commit 1c542d232085f0c0b17c85a45a979b053ef8c90e
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for turbo buttons
ROG ally devices supports a feature called turbo buttons that allows
the user to quickly toggle button state by keeping a button pressed,
therefore allow userspace to configure the feature by exposing
relevant sysfs attributes.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 6be25cc022cba..d6ab906b7e782 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static ssize_t btn_turbo_period_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
> + attr_turbo_period);
> + struct ally_btn_turbo_params *params = ally_btn_get_turbo_params(entry->cfg,
> + entry->btn);
> +
> + if (!params)
> + return -ENODEV;
> +
> + scoped_guard(mutex, &entry->cfg->config_mutex)
> + return sysfs_emit(buf, "%hhu\n", params->turbo);
> +}
[Severity: Low]
Does this code trigger a -Wreturn-type compiler warning?
The function ends inside a scoped_guard() block without an explicit trailing
return statement. Since scoped_guard() is implemented as a macro wrapping a for
loop, the compiler's flow analysis cannot definitively prove the loop executes,
which leads to a warning that control reaches the end of a non-void function.
> +
> +static ssize_t btn_turbo_period_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
> + attr_turbo_period);
> + struct ally_btn_turbo_params *params;
> + u8 value;
> + int ret;
> +
> + if (!entry->cfg->turbo_support)
> + return -EOPNOTSUPP;
> +
> + params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
> + if (!params)
> + return -ENODEV;
> +
> + ret = kstrtou8(buf, 10, &value);
> + if (ret)
> + return ret;
> +
> + if (value < ALLY_TURBO_PERIOD_MIN || value > ALLY_TURBO_PERIOD_MAX)
> + return -EINVAL;
> +
> + scoped_guard(mutex, &entry->cfg->config_mutex) {
> + params->turbo = value;
> + ret = ally_set_turbo_params(entry->ally, entry->hdev, entry->cfg);
> + if (ret)
> + return ret;
> + }
> +
> + return count;
> +}
[Severity: Medium]
Are we permanently desynchronizing local state if the hardware transmission fails?
By updating params->turbo before confirming the hardware acknowledged the change
via ally_set_turbo_params(), the local cache becomes modified. If the USB/HID
transmission fails, the function returns an error without rolling back
params->turbo.
A subsequent successful sysfs write to another button will then silently flush
this invalid cached state to the hardware.
> +
> +static ssize_t btn_toggle_period_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
> + attr_toggle_period);
> + struct ally_btn_turbo_params *params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
> +
> + if (!params)
> + return -ENODEV;
> +
> + scoped_guard(mutex, &entry->cfg->config_mutex)
> + return sysfs_emit(buf, "%hhu\n", params->toggle);
> +}
[Severity: Low]
Does this code trigger a -Wreturn-type compiler warning?
The function ends inside a scoped_guard() block without an explicit trailing
return statement. Since scoped_guard() is implemented as a macro wrapping a for
loop, the compiler's flow analysis cannot definitively prove the loop executes,
which leads to a warning that control reaches the end of a non-void function.
> +
> +static ssize_t btn_toggle_period_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ally_btn_sysfs_entry *entry = container_of(attr, struct ally_btn_sysfs_entry,
> + attr_toggle_period);
> + struct ally_btn_turbo_params *params;
> + u8 value;
> + int ret;
> +
> + if (!entry->cfg->turbo_support)
> + return -EOPNOTSUPP;
> +
> + params = ally_btn_get_turbo_params(entry->cfg, entry->btn);
> + if (!params)
> + return -ENODEV;
> +
> + ret = kstrtou8(buf, 10, &value);
> + if (ret)
> + return ret;
> +
> + if (value < ALLY_TOGGLE_PERIOD_MIN || value > ALLY_TOGGLE_PERIOD_MAX)
> + return -EINVAL;
> +
> + scoped_guard(mutex, &entry->cfg->config_mutex) {
> + params->toggle = value;
> + ret = ally_set_turbo_params(entry->ally, entry->hdev, entry->cfg);
> + if (ret)
> + return ret;
> + }
> +
> + return count;
> +}
[Severity: Medium]
Are we permanently desynchronizing local state if the hardware transmission fails?
By updating params->toggle before confirming the hardware acknowledged the change
via ally_set_turbo_params(), the local cache becomes modified. If the USB/HID
transmission fails, the function returns an error without rolling back
params->toggle.
A subsequent successful sysfs write to another button will then silently flush
this invalid cached state to the hardware.
[ ... ]
> @@ -2556,6 +2967,9 @@ static void ally_config_remove(struct hid_device *hdev, struct ally_handheld *al
>
> if (!cfg || !cfg->initialized)
> return;
> +
> + if (cfg->turbo_support && cfg->button_entries)
> + ally_remove_button_attributes(hdev, cfg);
> }
[Severity: High]
Does this cause a scheduling-while-atomic kernel panic during device unbind?
Looking at hid_asus_ally_remove(), it acquires ally_data_lock via
spin_lock_irqsave() and disables interrupts before calling ally_config_remove():
drivers/hid/hid-asus.c:hid_asus_ally_remove() {
...
spin_lock_irqsave(&ally_data_lock, flags);
...
if (ally->cfg_hdev == hdev) {
ally_config_remove(hdev, ally);
...
}
This patch modifies ally_config_remove() to call ally_remove_button_attributes()
which calls sysfs_remove_group(). Because sysfs_remove_group() blocks on
kernfs_rwsem, sleeping inside this atomic context will predictably trigger a
panic when the device is unbound or the module is removed.
[Severity: Critical]
Does this result in a Use-After-Free and sysfs attribute leak if probe fails?
When hid_asus_ally_setup() calls hid_asus_ally_init(), an error causes
ally_config_remove() to be called:
drivers/hid/hid-asus.c:hid_asus_ally_setup() {
...
ret = hid_asus_ally_init(hdev, &ally_drvdata);
if (ret < 0) {
ally_config_remove(hdev, &ally_drvdata);
return ERR_PTR(ret);
}
spin_lock_irqsave(&ally_data_lock, flags);
ally_drvdata.config = ally_cfg;
...
}
However, ally_drvdata.config is not assigned until after this check. When
ally_config_remove() runs, ally_drvdata.config is NULL, causing it to return
early without calling ally_remove_button_attributes().
The sysfs groups created by ally_create_button_attributes() are leaked and
remain accessible. When devres subsequently frees the backing memory
(ally_cfg and entries), any unprivileged user reading the leaked sysfs files
like btn_turbo_period_show() will dereference freed memory, triggering a UAF.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=11
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 12/12] HID: asus: add support for btn remapping
2026-08-19 19:58 [PATCH v2 00/12] HID: asus: add support for ROG Ally handhelds Denis Benato
` (10 preceding siblings ...)
2026-08-19 19:58 ` [PATCH v2 11/12] HID: asus: add support for turbo buttons Denis Benato
@ 2026-08-19 19:58 ` Denis Benato
2026-08-19 20:22 ` sashiko-bot
11 siblings, 1 reply; 21+ messages in thread
From: Denis Benato @ 2026-08-19 19:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schwartz, Denis Benato, Jonathan LoBue, Khamunetri Clark,
Derek J. Clark, Denis Benato
Add support for buttons remapping: the ability to assign different
button events to each one of the physical button on the device.
Each button exposes a remap_index sysfs attribute listing the valid
remap target names for the current gamepad mode: M1 and M2 can be
remapped to anything at any time, while other buttons may only be
remapped to gamepad codes in gamepad mode and keyboard/mouse/media
codes in desktop mode.
When the gamepad mode is changed the firmware automatically restores
its internal defaults, so the software cache is refreshed to reflect
the hardware state.
Assisted-by: opencode:glm-5.2
Assisted-by: Claude:claude-fable-5
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
drivers/hid/hid-asus.c | 1016 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 979 insertions(+), 37 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d6ab906b7e78..06dceb77d108 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -275,9 +275,12 @@ struct ally_turbo_config {
struct ally_btn_turbo_params btn_m1;
};
+struct button_remap_attr;
+struct ally_button_mapping;
+
struct ally_btn_sysfs_entry {
struct attribute_group group;
- struct attribute *attrs[5]; /* turbo_period + toggle_period + ranges + NULL */
+ struct attribute *attrs[8]; /* turbo + ranges + remap + macro + remap_index + NULL */
/*
* The entries are created while hid_asus_ally_probe() is still running,
* before asus_probe() publishes drvdata->rog_ally: carrying the shared
@@ -289,6 +292,9 @@ struct ally_btn_sysfs_entry {
enum ally_button_id btn;
struct device_attribute attr_turbo_period;
struct device_attribute attr_toggle_period;
+ struct button_remap_attr *remap_attr;
+ struct button_remap_attr *macro_attr;
+ struct device_attribute attr_remap_index;
};
struct ally_config {
@@ -326,6 +332,8 @@ struct ally_config {
struct ally_turbo_config turbo;
struct ally_btn_sysfs_entry *button_entries;
+ /* Button mappings for each gamepad mode, indexed by gamepad_mode */
+ struct ally_button_mapping *button_mappings;
struct ally_joystick_resp_curve left_curve;
struct ally_joystick_resp_curve right_curve;
@@ -976,6 +984,325 @@ static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device *
return 0;
}
+enum btn_map_type {
+ BTN_TYPE_NONE = 0,
+ BTN_TYPE_PAD = 0x01,
+ BTN_TYPE_KB = 0x02,
+ BTN_TYPE_MOUSE = 0x03,
+ BTN_TYPE_MEDIA = 0x05,
+};
+
+struct btn_code_map {
+ unsigned char type;
+ unsigned char value;
+ const char *name;
+};
+
+static const struct btn_code_map ally_btn_codes[] = {
+ { BTN_TYPE_NONE, 0x00, "NONE" },
+ /* Gamepad button codes */
+ { BTN_TYPE_PAD, 0x01, "PAD_A" },
+ { BTN_TYPE_PAD, 0x02, "PAD_B" },
+ { BTN_TYPE_PAD, 0x03, "PAD_X" },
+ { BTN_TYPE_PAD, 0x04, "PAD_Y" },
+ { BTN_TYPE_PAD, 0x05, "PAD_LB" },
+ { BTN_TYPE_PAD, 0x06, "PAD_RB" },
+ { BTN_TYPE_PAD, 0x07, "PAD_LS" },
+ { BTN_TYPE_PAD, 0x08, "PAD_RS" },
+ { BTN_TYPE_PAD, 0x09, "PAD_DPAD_UP" },
+ { BTN_TYPE_PAD, 0x0A, "PAD_DPAD_DOWN" },
+ { BTN_TYPE_PAD, 0x0B, "PAD_DPAD_LEFT" },
+ { BTN_TYPE_PAD, 0x0C, "PAD_DPAD_RIGHT" },
+ { BTN_TYPE_PAD, 0x0D, "PAD_LT" },
+ { BTN_TYPE_PAD, 0x0E, "PAD_RT" },
+ { BTN_TYPE_PAD, 0x11, "PAD_VIEW" },
+ { BTN_TYPE_PAD, 0x12, "PAD_MENU" },
+ { BTN_TYPE_PAD, 0x13, "PAD_XBOX" },
+
+ /* Keyboard button codes */
+ { BTN_TYPE_KB, 0x8E, "FN_M2" },
+ { BTN_TYPE_KB, 0x8F, "FN_M1" },
+ { BTN_TYPE_KB, 0x76, "KEY_ESC" },
+ { BTN_TYPE_KB, 0x05, "KEY_F1" },
+ { BTN_TYPE_KB, 0x06, "KEY_F2" },
+ { BTN_TYPE_KB, 0x04, "KEY_F3" },
+ { BTN_TYPE_KB, 0x0C, "KEY_F4" },
+ { BTN_TYPE_KB, 0x03, "KEY_F5" },
+ { BTN_TYPE_KB, 0x0B, "KEY_F6" },
+ { BTN_TYPE_KB, 0x80, "KEY_F7" },
+ { BTN_TYPE_KB, 0x0A, "KEY_F8" },
+ { BTN_TYPE_KB, 0x01, "KEY_F9" },
+ { BTN_TYPE_KB, 0x09, "KEY_F10" },
+ { BTN_TYPE_KB, 0x78, "KEY_F11" },
+ { BTN_TYPE_KB, 0x07, "KEY_F12" },
+ { BTN_TYPE_KB, 0x08, "KEY_F13" },
+ { BTN_TYPE_KB, 0x10, "KEY_F14" },
+ { BTN_TYPE_KB, 0x18, "KEY_F15" },
+ { BTN_TYPE_KB, 0x0E, "KEY_GRAVE" }, // backtick
+ { BTN_TYPE_KB, 0x16, "KEY_1" },
+ { BTN_TYPE_KB, 0x1E, "KEY_2" },
+ { BTN_TYPE_KB, 0x26, "KEY_3" },
+ { BTN_TYPE_KB, 0x25, "KEY_4" },
+ { BTN_TYPE_KB, 0x2E, "KEY_5" },
+ { BTN_TYPE_KB, 0x36, "KEY_6" },
+ { BTN_TYPE_KB, 0x3D, "KEY_7" },
+ { BTN_TYPE_KB, 0x3E, "KEY_8" },
+ { BTN_TYPE_KB, 0x46, "KEY_9" },
+ { BTN_TYPE_KB, 0x45, "KEY_0" },
+ { BTN_TYPE_KB, 0x4E, "KEY_MINUS" }, // hypen
+ { BTN_TYPE_KB, 0x55, "KEY_EQUAL" },
+ { BTN_TYPE_KB, 0x66, "KEY_BACKSPACE" },
+ { BTN_TYPE_KB, 0x0D, "KEY_TAB" },
+ { BTN_TYPE_KB, 0x15, "KEY_Q" },
+ { BTN_TYPE_KB, 0x1D, "KEY_W" },
+ { BTN_TYPE_KB, 0x24, "KEY_E" },
+ { BTN_TYPE_KB, 0x2D, "KEY_R" },
+ { BTN_TYPE_KB, 0x2C, "KEY_T" },
+ { BTN_TYPE_KB, 0x35, "KEY_Y" },
+ { BTN_TYPE_KB, 0x3C, "KEY_U" },
+ { BTN_TYPE_KB, 0x43, "KEY_I" },
+ { BTN_TYPE_KB, 0x44, "KEY_O" },
+ { BTN_TYPE_KB, 0x4D, "KEY_P" },
+ { BTN_TYPE_KB, 0x54, "KEY_LEFTBRACE" },
+ { BTN_TYPE_KB, 0x5B, "KEY_RIGHTBRACE" },
+ { BTN_TYPE_KB, 0x5D, "KEY_BACKSLASH" },
+ { BTN_TYPE_KB, 0x58, "KEY_CAPSLOCK" },
+ { BTN_TYPE_KB, 0x1C, "KEY_A" },
+ { BTN_TYPE_KB, 0x1B, "KEY_S" },
+ { BTN_TYPE_KB, 0x23, "KEY_D" },
+ { BTN_TYPE_KB, 0x2B, "KEY_F" },
+ { BTN_TYPE_KB, 0x34, "KEY_G" },
+ { BTN_TYPE_KB, 0x33, "KEY_H" },
+ { BTN_TYPE_KB, 0x3B, "KEY_J" },
+ { BTN_TYPE_KB, 0x42, "KEY_K" },
+ { BTN_TYPE_KB, 0x4B, "KEY_L" },
+ { BTN_TYPE_KB, 0x4C, "KEY_SEMICOLON" },
+ { BTN_TYPE_KB, 0x52, "KEY_APOSTROPHE" },
+ { BTN_TYPE_KB, 0x5A, "KEY_ENTER" },
+ { BTN_TYPE_KB, 0x88, "KEY_LEFTSHIFT" },
+ { BTN_TYPE_KB, 0x1A, "KEY_Z" },
+ { BTN_TYPE_KB, 0x22, "KEY_X" },
+ { BTN_TYPE_KB, 0x21, "KEY_C" },
+ { BTN_TYPE_KB, 0x2A, "KEY_V" },
+ { BTN_TYPE_KB, 0x32, "KEY_B" },
+ { BTN_TYPE_KB, 0x31, "KEY_N" },
+ { BTN_TYPE_KB, 0x3A, "KEY_M" },
+ { BTN_TYPE_KB, 0x41, "KEY_COMMA" },
+ { BTN_TYPE_KB, 0x49, "KEY_DOT" },
+ { BTN_TYPE_KB, 0x4A, "KEY_SLASH" },
+ { BTN_TYPE_KB, 0x89, "KEY_RIGHTSHIFT" },
+ { BTN_TYPE_KB, 0x82, "KEY_LEFTMETA" },
+ { BTN_TYPE_KB, 0x8A, "KEY_LEFTALT" },
+ { BTN_TYPE_KB, 0x29, "KEY_SPACE" },
+ { BTN_TYPE_KB, 0x8B, "KEY_RIGHTALT" },
+ { BTN_TYPE_KB, 0x84, "KEY_COMPOSE" },
+ { BTN_TYPE_KB, 0x8D, "KEY_RIGHTCTRL" },
+ { BTN_TYPE_KB, 0xC3, "KEY_SYSRQ" },
+ { BTN_TYPE_KB, 0x7E, "KEY_SCROLLLOCK" },
+ { BTN_TYPE_KB, 0x91, "KEY_PAUSE" },
+ { BTN_TYPE_KB, 0xC2, "KEY_INSERT" },
+ { BTN_TYPE_KB, 0x94, "KEY_HOME" },
+ { BTN_TYPE_KB, 0x96, "KEY_PAGEUP" },
+ { BTN_TYPE_KB, 0xC0, "KEY_DELETE" },
+ { BTN_TYPE_KB, 0x95, "KEY_END" },
+ { BTN_TYPE_KB, 0x97, "KEY_PAGEDOWN" },
+ { BTN_TYPE_KB, 0x98, "KEY_UP" },
+ { BTN_TYPE_KB, 0x99, "KEY_DOWN" },
+ { BTN_TYPE_KB, 0x9A, "KEY_LEFT" },
+ { BTN_TYPE_KB, 0x9B, "KEY_RIGHT" },
+
+ /* Numpad button codes */
+ { BTN_TYPE_KB, 0x77, "KEY_NUMLOCK" },
+ { BTN_TYPE_KB, 0x90, "KEY_KPSLASH" },
+ { BTN_TYPE_KB, 0x7C, "KEY_KPASTERISK" },
+ { BTN_TYPE_KB, 0x7B, "KEY_KPMINUS" },
+ { BTN_TYPE_KB, 0x70, "KEY_KP0" },
+ { BTN_TYPE_KB, 0x69, "KEY_KP1" },
+ { BTN_TYPE_KB, 0x72, "KEY_KP2" },
+ { BTN_TYPE_KB, 0x7A, "KEY_KP3" },
+ { BTN_TYPE_KB, 0x6B, "KEY_KP4" },
+ { BTN_TYPE_KB, 0x73, "KEY_KP5" },
+ { BTN_TYPE_KB, 0x74, "KEY_KP6" },
+ { BTN_TYPE_KB, 0x6C, "KEY_KP7" },
+ { BTN_TYPE_KB, 0x75, "KEY_KP8" },
+ { BTN_TYPE_KB, 0x7D, "KEY_KP9" },
+ { BTN_TYPE_KB, 0x79, "KEY_KPPLUS" },
+ { BTN_TYPE_KB, 0x81, "KEY_KPENTER" },
+ { BTN_TYPE_KB, 0x71, "KEY_KPDOT" },
+
+ /* Mouse button codes */
+ { BTN_TYPE_MOUSE, 0x01, "BTN_LEFT" },
+ { BTN_TYPE_MOUSE, 0x02, "BTN_RIGHT" },
+ { BTN_TYPE_MOUSE, 0x03, "BTN_MIDDLE" },
+ { BTN_TYPE_MOUSE, 0x04, "REL_WHEEL_HI_RES_UP" },
+ { BTN_TYPE_MOUSE, 0x05, "REL_WHEEL_HI_RES_DOWN" },
+
+ /* Media button codes */
+ { BTN_TYPE_MEDIA, 0x16, "MEDIA_SCREENSHOT" },
+ { BTN_TYPE_MEDIA, 0x19, "MEDIA_SHOW_KEYBOARD" },
+ { BTN_TYPE_MEDIA, 0x1C, "MEDIA_SHOW_DESKTOP" },
+ { BTN_TYPE_MEDIA, 0x1E, "MEDIA_START_RECORDING" },
+ { BTN_TYPE_MEDIA, 0x01, "MEDIA_MIC_OFF" },
+ { BTN_TYPE_MEDIA, 0x02, "MEDIA_VOL_DOWN" },
+ { BTN_TYPE_MEDIA, 0x03, "MEDIA_VOL_UP" },
+};
+
+static const size_t keymap_len = ARRAY_SIZE(ally_btn_codes);
+
+/* Button pair indexes for mapping commands */
+enum btn_pair_index {
+ BTN_PAIR_DPAD_UPDOWN = 0x01,
+ BTN_PAIR_DPAD_LEFTRIGHT = 0x02,
+ BTN_PAIR_STICK_LR = 0x03,
+ BTN_PAIR_BUMPER_LR = 0x04,
+ BTN_PAIR_AB = 0x05,
+ BTN_PAIR_XY = 0x06,
+ BTN_PAIR_VIEW_MENU = 0x07,
+ BTN_PAIR_M1M2 = 0x08,
+ BTN_PAIR_TRIGGER_LR = 0x09,
+};
+
+struct button_map {
+ const struct btn_code_map *remap;
+ const struct btn_code_map *macro;
+};
+
+struct button_pair_map {
+ enum btn_pair_index pair_index;
+ struct button_map first;
+ struct button_map second;
+};
+
+/* Store button mapping per gamepad mode */
+struct ally_button_mapping {
+ struct button_pair_map button_pairs[9]; /* 9 button pairs */
+};
+
+static void ally_set_default_gamepad_mapping(struct ally_button_mapping *mappings)
+{
+ struct ally_button_mapping *map = &mappings[ALLY_GAMEPAD_MODE_GAMEPAD];
+ int i;
+
+ /* Set all pair indexes and initialize to NONE */
+ for (i = 0; i < 9; i++) {
+ map->button_pairs[i].pair_index = i + 1;
+ map->button_pairs[i].first.remap =
+ &ally_btn_codes[0];
+ map->button_pairs[i].first.macro =
+ &ally_btn_codes[0];
+ map->button_pairs[i].second.remap =
+ &ally_btn_codes[0];
+ map->button_pairs[i].second.macro =
+ &ally_btn_codes[0];
+ }
+
+ /* Set direct mappings using array indices */
+ map->button_pairs[BTN_PAIR_AB - 1].first.remap =
+ &ally_btn_codes[1]; /* PAD_A */
+ map->button_pairs[BTN_PAIR_AB - 1].second.remap =
+ &ally_btn_codes[2]; /* PAD_B */
+
+ map->button_pairs[BTN_PAIR_XY - 1].first.remap =
+ &ally_btn_codes[3]; /* PAD_X */
+ map->button_pairs[BTN_PAIR_XY - 1].second.remap =
+ &ally_btn_codes[4]; /* PAD_Y */
+
+ map->button_pairs[BTN_PAIR_BUMPER_LR - 1].first.remap =
+ &ally_btn_codes[5]; /* PAD_LB */
+ map->button_pairs[BTN_PAIR_BUMPER_LR - 1].second.remap =
+ &ally_btn_codes[6]; /* PAD_RB */
+
+ map->button_pairs[BTN_PAIR_STICK_LR - 1].first.remap =
+ &ally_btn_codes[7]; /* PAD_LS */
+ map->button_pairs[BTN_PAIR_STICK_LR - 1].second.remap =
+ &ally_btn_codes[8]; /* PAD_RS */
+
+ map->button_pairs[BTN_PAIR_DPAD_UPDOWN - 1].first.remap =
+ &ally_btn_codes[9]; /* PAD_DPAD_UP */
+ map->button_pairs[BTN_PAIR_DPAD_UPDOWN - 1].second.remap =
+ &ally_btn_codes[10]; /* PAD_DPAD_DOWN */
+
+ map->button_pairs[BTN_PAIR_DPAD_LEFTRIGHT - 1].first.remap =
+ &ally_btn_codes[11]; /* PAD_DPAD_LEFT */
+ map->button_pairs[BTN_PAIR_DPAD_LEFTRIGHT - 1].second.remap =
+ &ally_btn_codes[12]; /* PAD_DPAD_RIGHT */
+
+ map->button_pairs[BTN_PAIR_TRIGGER_LR - 1].first.remap =
+ &ally_btn_codes[13]; /* PAD_LT */
+ map->button_pairs[BTN_PAIR_TRIGGER_LR - 1].second.remap =
+ &ally_btn_codes[14]; /* PAD_RT */
+
+ map->button_pairs[BTN_PAIR_VIEW_MENU - 1].first.remap =
+ &ally_btn_codes[15]; /* PAD_VIEW */
+ map->button_pairs[BTN_PAIR_VIEW_MENU - 1].second.remap =
+ &ally_btn_codes[16]; /* PAD_MENU */
+
+ map->button_pairs[BTN_PAIR_M1M2 - 1].first.remap =
+ &ally_btn_codes[18]; /* FN_M2 */
+ map->button_pairs[BTN_PAIR_M1M2 - 1].second.remap =
+ &ally_btn_codes[19]; /* FN_M1 */
+}
+
+static void ally_set_default_keyboard_mapping(struct ally_button_mapping *mappings)
+{
+ struct ally_button_mapping *map = &mappings[ALLY_GAMEPAD_MODE_KEYBOARD];
+ int i;
+
+ /* Set all pair indexes and initialize to NONE */
+ for (i = 0; i < 9; i++) {
+ map->button_pairs[i].pair_index = i + 1;
+ map->button_pairs[i].first.remap =
+ &ally_btn_codes[0];
+ map->button_pairs[i].first.macro =
+ &ally_btn_codes[0];
+ map->button_pairs[i].second.remap =
+ &ally_btn_codes[0];
+ map->button_pairs[i].second.macro =
+ &ally_btn_codes[0];
+ }
+
+ /*
+ * Desktop mode defaults: keyboard and mouse mappings.
+ * Buttons not listed here (dpad, view, menu) remain NONE.
+ */
+
+ /* btn_a => KB_SPACE, btn_b => KB_E */
+ map->button_pairs[BTN_PAIR_AB - 1].first.remap =
+ &ally_btn_codes[91]; /* KB_SPACE */
+ map->button_pairs[BTN_PAIR_AB - 1].second.remap =
+ &ally_btn_codes[53]; /* KB_E */
+
+ /* btn_x => KB_R, btn_y => KB_F */
+ map->button_pairs[BTN_PAIR_XY - 1].first.remap =
+ &ally_btn_codes[54]; /* KB_R */
+ map->button_pairs[BTN_PAIR_XY - 1].second.remap =
+ &ally_btn_codes[68]; /* KB_F */
+
+ /* LB => MOUSE_WHEEL_UP, RB => MOUSE_WHEEL_DOWN */
+ map->button_pairs[BTN_PAIR_BUMPER_LR - 1].first.remap =
+ &ally_btn_codes[128]; /* MOUSE_WHEEL_UP */
+ map->button_pairs[BTN_PAIR_BUMPER_LR - 1].second.remap =
+ &ally_btn_codes[129]; /* MOUSE_WHEEL_DOWN */
+
+ /* left stick click => MOUSE_LCLICK, right stick click => KB_LSHIFT */
+ map->button_pairs[BTN_PAIR_STICK_LR - 1].first.remap =
+ &ally_btn_codes[125]; /* MOUSE_LCLICK */
+ map->button_pairs[BTN_PAIR_STICK_LR - 1].second.remap =
+ &ally_btn_codes[77]; /* KB_LSHIFT */
+
+ /* LT => MOUSE_LCLICK, RT => MOUSE_RCLICK */
+ map->button_pairs[BTN_PAIR_TRIGGER_LR - 1].first.remap =
+ &ally_btn_codes[125]; /* MOUSE_LCLICK */
+ map->button_pairs[BTN_PAIR_TRIGGER_LR - 1].second.remap =
+ &ally_btn_codes[126]; /* MOUSE_RCLICK */
+
+ /* M2 => FN_M2, M1 => FN_M1 */
+ map->button_pairs[BTN_PAIR_M1M2 - 1].first.remap =
+ &ally_btn_codes[18]; /* FN_M2 */
+ map->button_pairs[BTN_PAIR_M1M2 - 1].second.remap =
+ &ally_btn_codes[19]; /* FN_M1 */
+}
+
static ssize_t gamepad_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
{
struct hid_device *hdev = to_hid_device(dev);
@@ -1039,6 +1366,24 @@ static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *a
return ret;
cfg->gamepad_mode = mode_byte;
+
+ /*
+ * When the gamepad mode changes, the hardware automatically
+ * resets all button mappings to its internal defaults for the
+ * new mode.
+ *
+ * Refresh the software cache so that subsequent
+ * reads from sysfs reflect what the firmware is actually
+ * using.
+ */
+ if (cfg->button_mappings) {
+ struct ally_button_mapping *mappings = cfg->button_mappings;
+
+ if (mode_byte == ALLY_GAMEPAD_MODE_GAMEPAD)
+ ally_set_default_gamepad_mapping(mappings);
+ else if (mode_byte == ALLY_GAMEPAD_MODE_KEYBOARD)
+ ally_set_default_keyboard_mapping(mappings);
+ }
}
hid_dbg(hdev, "Set gamepad mode to %s\n", ally_gamepad_mode_text[mode]);
@@ -2775,6 +3120,515 @@ static void ally_btn_turbo_init_attrs(struct ally_btn_sysfs_entry *entry)
entry->attrs[4] = NULL;
}
+/**
+ * find_button_by_name() - Find a button code map by its name
+ * @name: name of the button code to look up
+ *
+ * Return: the matching button code map, or NULL if not found
+ */
+static const struct btn_code_map *find_button_by_name(const char *name)
+{
+ int i;
+
+ for (i = 0; i < keymap_len; i++) {
+ if (strcmp(ally_btn_codes[i].name, name) == 0)
+ return &ally_btn_codes[i];
+ }
+
+ return NULL;
+}
+
+/**
+ * ally_remap_code_valid() - Check if a remap target is valid for a button
+ * @btn: button to remap
+ * @gamepad_mode: gamepad mode to validate the remap against
+ * @code: remap target to check
+ *
+ * M1 and M2 can be remapped to anything at any time.
+ * All other buttons may only be remapped to gamepad buttons (BTN_TYPE_PAD)
+ * when in gamepad mode, and to keyboard/mouse/media codes when in desktop
+ * (keyboard) mode. BTN_TYPE_NONE (unmapped) is always allowed.
+ *
+ * Return: true if the remap is valid, false otherwise
+ */
+static bool ally_remap_code_valid(enum ally_button_id btn, u8 gamepad_mode,
+ const struct btn_code_map *code)
+{
+ if (!code)
+ return false;
+
+ /* M1 and M2 can be remapped to everything */
+ if (btn == ALLY_BTN_M1 || btn == ALLY_BTN_M2)
+ return true;
+
+ /* NONE is always valid */
+ if (code->type == BTN_TYPE_NONE)
+ return true;
+
+ if (gamepad_mode == ALLY_GAMEPAD_MODE_GAMEPAD)
+ return code->type == BTN_TYPE_PAD;
+
+ /* Desktop / keyboard mode: allow keyboard, mouse and media */
+ return code->type == BTN_TYPE_KB ||
+ code->type == BTN_TYPE_MOUSE ||
+ code->type == BTN_TYPE_MEDIA;
+}
+
+/**
+ * ally_set_button_mapping() - Set the button mapping for a button pair
+ * @hdev: HID device
+ * @ally: ally handheld structure
+ * @mapping: button pair mapping to send to the device
+ *
+ * Return: count of data transferred, negative if error
+ */
+static int ally_set_button_mapping(struct hid_device *hdev, struct ally_handheld *ally,
+ struct button_pair_map *mapping)
+{
+ /* The MCU mapping block is four consecutive 11-byte entries starting at
+ * buf[5]: first remap 5-15, first macro 16-26, second remap 27-37,
+ * second macro 38-48 (see hid-asus-ally __btn_pair_to_pkt, BTN_CODE_LEN).
+ */
+ u8 macro_bytes[11] = {0};
+ u8 btn_bytes[11] = {0};
+
+ if (!mapping)
+ return -EINVAL;
+
+ u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_MAPPING, NULL, 0);
+ if (!buf)
+ return -ENOMEM;
+
+ /* This packet is slightly different from the other
+ * as before the packet length there is an extra byte
+ * which is the pair index.
+ */
+ buf[3] = mapping->pair_index;
+ buf[4] = 0x2C; /* Length */
+
+ /* First button mapping */
+ buf[5] = mapping->first.remap->type;
+ /* Fill in bytes 6-14 with button code */
+ if (mapping->first.remap->type) {
+ memset(btn_bytes, 0, sizeof(btn_bytes));
+ btn_bytes[0] = mapping->first.remap->type;
+
+ /* Value byte position depends on type: pad=1, kb=2, media=3,
+ * mouse=4 (see hid-asus-ally BTN_CODE definitions).
+ */
+ switch (mapping->first.remap->type) {
+ case BTN_TYPE_NONE:
+ break;
+ case BTN_TYPE_PAD:
+ btn_bytes[1] = mapping->first.remap->value;
+ break;
+ case BTN_TYPE_KB:
+ btn_bytes[2] = mapping->first.remap->value;
+ break;
+ case BTN_TYPE_MEDIA:
+ btn_bytes[3] = mapping->first.remap->value;
+ break;
+ case BTN_TYPE_MOUSE:
+ btn_bytes[4] = mapping->first.remap->value;
+ break;
+ }
+ memcpy(&buf[5], btn_bytes, 11);
+ }
+
+ /* Macro mapping for first button if any */
+ buf[16] = mapping->first.macro->type;
+ if (mapping->first.macro->type) {
+ memset(macro_bytes, 0, sizeof(macro_bytes));
+ macro_bytes[0] = mapping->first.macro->type;
+
+ switch (mapping->first.macro->type) {
+ case BTN_TYPE_NONE:
+ break;
+ case BTN_TYPE_PAD:
+ macro_bytes[1] = mapping->first.macro->value;
+ break;
+ case BTN_TYPE_KB:
+ macro_bytes[2] = mapping->first.macro->value;
+ break;
+ case BTN_TYPE_MEDIA:
+ macro_bytes[3] = mapping->first.macro->value;
+ break;
+ case BTN_TYPE_MOUSE:
+ macro_bytes[4] = mapping->first.macro->value;
+ break;
+ }
+ memcpy(&buf[16], macro_bytes, 11);
+ }
+
+ /* Second button mapping */
+ buf[27] = mapping->second.remap->type;
+ /* Fill in bytes 28-36 with button code */
+ if (mapping->second.remap->type) {
+ memset(btn_bytes, 0, sizeof(btn_bytes));
+ btn_bytes[0] = mapping->second.remap->type;
+
+ switch (mapping->second.remap->type) {
+ case BTN_TYPE_NONE:
+ break;
+ case BTN_TYPE_PAD:
+ btn_bytes[1] = mapping->second.remap->value;
+ break;
+ case BTN_TYPE_KB:
+ btn_bytes[2] = mapping->second.remap->value;
+ break;
+ case BTN_TYPE_MEDIA:
+ btn_bytes[3] = mapping->second.remap->value;
+ break;
+ case BTN_TYPE_MOUSE:
+ btn_bytes[4] = mapping->second.remap->value;
+ break;
+ }
+ memcpy(&buf[27], btn_bytes, 11);
+ }
+
+ /* Macro mapping for second button if any */
+ buf[38] = mapping->second.macro->type;
+ if (mapping->second.macro->type) {
+ memset(macro_bytes, 0, sizeof(macro_bytes));
+ macro_bytes[0] = mapping->second.macro->type;
+
+ switch (mapping->second.macro->type) {
+ case BTN_TYPE_NONE:
+ break;
+ case BTN_TYPE_PAD:
+ macro_bytes[1] = mapping->second.macro->value;
+ break;
+ case BTN_TYPE_KB:
+ macro_bytes[2] = mapping->second.macro->value;
+ break;
+ case BTN_TYPE_MEDIA:
+ macro_bytes[3] = mapping->second.macro->value;
+ break;
+ case BTN_TYPE_MOUSE:
+ macro_bytes[4] = mapping->second.macro->value;
+ break;
+ }
+ memcpy(&buf[38], macro_bytes, 11);
+ }
+
+ return ally_gamepad_send_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
+}
+
+/* Button remap attribute structure */
+struct button_remap_attr {
+ struct device_attribute dev_attr;
+ enum ally_button_id button_id;
+ bool is_macro;
+};
+
+#define to_button_remap_attr(x) container_of(x, struct button_remap_attr, dev_attr)
+
+/**
+ * get_button_pair_info() - Get the pair index and position of a button
+ * @button_id: button to look up
+ * @pair_idx: output pointer for the pair index of the button
+ * @is_first: output pointer for the position of the button within the pair
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+static int get_button_pair_info(enum ally_button_id button_id,
+ enum btn_pair_index *pair_idx,
+ bool *is_first)
+{
+ switch (button_id) {
+ case ALLY_BTN_DU:
+ *pair_idx = BTN_PAIR_DPAD_UPDOWN;
+ *is_first = true;
+ break;
+ case ALLY_BTN_DD:
+ *pair_idx = BTN_PAIR_DPAD_UPDOWN;
+ *is_first = false;
+ break;
+ case ALLY_BTN_DL:
+ *pair_idx = BTN_PAIR_DPAD_LEFTRIGHT;
+ *is_first = true;
+ break;
+ case ALLY_BTN_DR:
+ *pair_idx = BTN_PAIR_DPAD_LEFTRIGHT;
+ *is_first = false;
+ break;
+ case ALLY_BTN_J0B:
+ *pair_idx = BTN_PAIR_STICK_LR;
+ *is_first = true;
+ break;
+ case ALLY_BTN_J1B:
+ *pair_idx = BTN_PAIR_STICK_LR;
+ *is_first = false;
+ break;
+ case ALLY_BTN_LB:
+ *pair_idx = BTN_PAIR_BUMPER_LR;
+ *is_first = true;
+ break;
+ case ALLY_BTN_RB:
+ *pair_idx = BTN_PAIR_BUMPER_LR;
+ *is_first = false;
+ break;
+ case ALLY_BTN_A:
+ *pair_idx = BTN_PAIR_AB;
+ *is_first = true;
+ break;
+ case ALLY_BTN_B:
+ *pair_idx = BTN_PAIR_AB;
+ *is_first = false;
+ break;
+ case ALLY_BTN_X:
+ *pair_idx = BTN_PAIR_XY;
+ *is_first = true;
+ break;
+ case ALLY_BTN_Y:
+ *pair_idx = BTN_PAIR_XY;
+ *is_first = false;
+ break;
+ case ALLY_BTN_VIEW:
+ *pair_idx = BTN_PAIR_VIEW_MENU;
+ *is_first = true;
+ break;
+ case ALLY_BTN_MENU:
+ *pair_idx = BTN_PAIR_VIEW_MENU;
+ *is_first = false;
+ break;
+ case ALLY_BTN_M1:
+ *pair_idx = BTN_PAIR_M1M2;
+ *is_first = false;
+ break;
+ case ALLY_BTN_M2:
+ *pair_idx = BTN_PAIR_M1M2;
+ *is_first = true;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static ssize_t button_remap_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct button_remap_attr *btn_attr = to_button_remap_attr(attr);
+ const struct btn_code_map *code;
+ struct ally_config *cfg;
+ enum ally_button_id button_id = btn_attr->button_id;
+ enum btn_pair_index pair_idx;
+ struct button_pair_map *pair;
+ struct button_map *btn_map;
+ bool is_first;
+ int ret;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ ret = get_button_pair_info(button_id, &pair_idx, &is_first);
+ if (ret < 0)
+ return ret;
+
+ guard(mutex)(&cfg->config_mutex);
+
+ /*
+ * button_mappings is unpublished by the remove path while holding
+ * this same lock, so the check cannot race with the teardown.
+ */
+ if (!cfg->button_mappings)
+ return -ENODEV;
+
+ pair = &cfg->button_mappings[cfg->gamepad_mode]
+ .button_pairs[pair_idx - 1];
+ btn_map = is_first ? &pair->first : &pair->second;
+ code = btn_attr->is_macro ? btn_map->macro : btn_map->remap;
+
+ if (code->type == BTN_TYPE_NONE)
+ return sysfs_emit(buf, "NONE\n");
+
+ return sysfs_emit(buf, "%s\n", code->name);
+}
+
+static ssize_t button_remap_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct ally_handheld *ally = drvdata->rog_ally;
+ struct button_remap_attr *btn_attr = to_button_remap_attr(attr);
+ enum ally_button_id button_id = btn_attr->button_id;
+ struct ally_button_mapping *mode_map;
+ const struct btn_code_map *code;
+ const struct btn_code_map *old_code;
+ enum btn_pair_index pair_idx;
+ struct button_pair_map *pair;
+ struct button_map *btn_map;
+ struct ally_config *cfg;
+ char btn_name[32];
+ bool is_first;
+ int ret, i;
+
+ if (!ally)
+ return -ENODEV;
+
+ cfg = ally_get_config(ally);
+ if (!cfg)
+ return -ENODEV;
+
+ if (sscanf(buf, "%31s", btn_name) != 1)
+ return -EINVAL;
+
+ /* Handle "NONE" specially */
+ if (strcmp(btn_name, "NONE") == 0) {
+ code = &ally_btn_codes[0]; /* NONE entry */
+ } else {
+ code = find_button_by_name(btn_name);
+ if (!code)
+ return -EINVAL;
+ }
+
+ ret = get_button_pair_info(button_id, &pair_idx, &is_first);
+ if (ret < 0)
+ return ret;
+
+ scoped_guard(mutex, &cfg->config_mutex) {
+ /*
+ * button_mappings is unpublished by the remove path while
+ * holding this same lock, so the check cannot race with the
+ * teardown.
+ */
+ if (!cfg->button_mappings)
+ return -ENODEV;
+
+ /*
+ * Validate the remap target while holding the lock: the
+ * gamepad mode can change concurrently, and the check must
+ * be done against the very mode the mapping is written to.
+ */
+ if (!ally_remap_code_valid(button_id, cfg->gamepad_mode, code))
+ return -EINVAL;
+
+ /* Access the mapping for current gamepad mode */
+ pair = &cfg->button_mappings[cfg->gamepad_mode]
+ .button_pairs[pair_idx - 1];
+ btn_map = is_first ? &pair->first : &pair->second;
+
+ if (btn_attr->is_macro) {
+ old_code = btn_map->macro;
+ btn_map->macro = code;
+ } else {
+ old_code = btn_map->remap;
+ btn_map->remap = code;
+ }
+
+ /* Update pair index */
+ pair->pair_index = pair_idx;
+
+ /*
+ * Send mapping to device with the caveat that first
+ * generation devices require individual button pair updates.
+ */
+ if (cfg->is_ally_x) {
+ ret = ally_set_button_mapping(hdev, ally, pair);
+ } else {
+ mode_map = &cfg->button_mappings[cfg->gamepad_mode];
+ ret = 0;
+ for (i = 0; i < 9; i++) {
+ mode_map->button_pairs[i].pair_index = i + 1;
+ ret = ally_set_button_mapping(hdev, ally,
+ &mode_map->button_pairs[i]);
+ if (ret < 0)
+ break;
+ }
+ }
+
+ /*
+ * The hardware rejected the update: restore the previous
+ * mapping so that the software cache keeps describing the
+ * state the device is actually in.
+ */
+ if (ret < 0) {
+ if (btn_attr->is_macro)
+ btn_map->macro = old_code;
+ else
+ btn_map->remap = old_code;
+
+ hid_err(hdev, "Failed to set button mapping: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return count;
+}
+
+/**
+ * button_remap_attr_create() - Create a button remap attribute
+ * @button_id: button the attribute is for
+ * @is_macro: whether the attribute controls the macro mapping
+ *
+ * Return: the newly created attribute, or NULL on allocation failure
+ */
+static struct button_remap_attr *button_remap_attr_create(enum ally_button_id button_id,
+ bool is_macro)
+{
+ struct button_remap_attr *attr __free(kfree) = kzalloc_obj(*attr);
+ if (!attr)
+ return NULL;
+
+ attr->button_id = button_id;
+ attr->is_macro = is_macro;
+ sysfs_attr_init(&attr->dev_attr.attr);
+ attr->dev_attr.attr.name = is_macro ? "macro" : "remap";
+ attr->dev_attr.attr.mode = 0644;
+ attr->dev_attr.show = button_remap_show;
+ attr->dev_attr.store = button_remap_store;
+
+ return no_free_ptr(attr);
+}
+
+/**
+ * btn_remap_index_show() - Show the list of valid remap targets for a button
+ * @dev: device the attribute belongs to
+ * @attr: attribute being read
+ * @buf: buffer to write the list into
+ *
+ * M1/M2 list everything; other buttons list only the codes valid for the
+ * active mode (gamepad buttons in gamepad mode, keyboard / mouse / media
+ * in desktop mode).
+ *
+ * Return: number of characters written
+ */
+static ssize_t btn_remap_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ally_btn_sysfs_entry *entry = container_of(attr,
+ struct ally_btn_sysfs_entry, attr_remap_index);
+ enum ally_button_id btn = entry->btn;
+ u8 mode;
+ ssize_t len = 0;
+ int i;
+
+ scoped_guard(mutex, &entry->cfg->config_mutex)
+ mode = entry->cfg->gamepad_mode;
+
+ for (i = 0; i < keymap_len; i++) {
+ if (!ally_remap_code_valid(btn, mode, &ally_btn_codes[i]))
+ continue;
+ len += sysfs_emit_at(buf, len, "%s ", ally_btn_codes[i].name);
+ }
+
+ if (len > 0)
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
/**
* ally_create_button_attributes() - Create turbo button attributes
* @hdev: HID device
@@ -2788,16 +3642,23 @@ static int ally_create_button_attributes(struct hid_device *hdev,
struct ally_config *cfg)
{
struct ally_btn_sysfs_entry *entries;
+ struct ally_button_mapping *mappings;
int i, ret;
- if (!cfg->turbo_support)
- return 0;
-
entries = devm_kcalloc(&hdev->dev, ALLY_BTN_MAX, sizeof(*entries), GFP_KERNEL);
if (!entries)
return -ENOMEM;
+ /* Allocate mappings for each gamepad mode (1-based indexing) */
+ mappings = devm_kcalloc(&hdev->dev, ALLY_GAMEPAD_MODE_KEYBOARD + 1,
+ sizeof(*mappings), GFP_KERNEL);
+ if (!mappings)
+ return -ENOMEM;
+
cfg->button_entries = entries;
+ cfg->button_mappings = mappings;
+ ally_set_default_gamepad_mapping(mappings);
+ ally_set_default_keyboard_mapping(mappings);
for (i = 0; i < ALLY_BTN_MAX; i++) {
entries[i].ally = ally;
@@ -2805,13 +3666,53 @@ static int ally_create_button_attributes(struct hid_device *hdev,
entries[i].hdev = hdev;
entries[i].btn = i;
- if (!ally_btn_get_turbo_params(cfg, i)) {
- hid_err(hdev, "Invalid button id %d for turbo attributes\n", i);
- ret = -EINVAL;
+ if (cfg->turbo_support) {
+ if (!ally_btn_get_turbo_params(cfg, i)) {
+ hid_err(hdev, "Invalid button id %d for turbo attributes\n", i);
+ ret = -EINVAL;
+ goto err_cleanup;
+ }
+
+ ally_btn_turbo_init_attrs(&entries[i]);
+ }
+
+ entries[i].remap_attr = button_remap_attr_create(i, false);
+ if (!entries[i].remap_attr) {
+ ret = -ENOMEM;
goto err_cleanup;
}
- ally_btn_turbo_init_attrs(&entries[i]);
+ entries[i].macro_attr = button_remap_attr_create(i, true);
+ if (!entries[i].macro_attr) {
+ ret = -ENOMEM;
+ goto err_cleanup;
+ }
+
+ /* Initialize the remap_index attribute */
+ sysfs_attr_init(&entries[i].attr_remap_index.attr);
+ entries[i].attr_remap_index.attr.name = "remap_index";
+ entries[i].attr_remap_index.attr.mode = 0444;
+ entries[i].attr_remap_index.show = btn_remap_index_show;
+ entries[i].attr_remap_index.store = NULL;
+
+ /* Set up attributes array based on what's supported */
+ if (cfg->turbo_support) {
+ entries[i].attrs[4] =
+ &entries[i].remap_attr->dev_attr.attr;
+ entries[i].attrs[5] =
+ &entries[i].macro_attr->dev_attr.attr;
+ entries[i].attrs[6] =
+ &entries[i].attr_remap_index.attr;
+ entries[i].attrs[7] = NULL;
+ } else {
+ entries[i].attrs[0] =
+ &entries[i].remap_attr->dev_attr.attr;
+ entries[i].attrs[1] =
+ &entries[i].macro_attr->dev_attr.attr;
+ entries[i].attrs[2] =
+ &entries[i].attr_remap_index.attr;
+ entries[i].attrs[3] = NULL;
+ }
entries[i].group.name = ally_button_names[i];
entries[i].group.attrs = entries[i].attrs;
@@ -2831,7 +3732,19 @@ static int ally_create_button_attributes(struct hid_device *hdev,
while (--i >= 0)
sysfs_remove_group(&hdev->dev.kobj, &entries[i].group);
+ for (i = 0; i < ALLY_BTN_MAX; i++) {
+ kfree(entries[i].remap_attr);
+ kfree(entries[i].macro_attr);
+ }
+
+ /*
+ * The arrays are devm-managed on the same device as the sysfs
+ * attributes, so they are released together with them: only
+ * unpublish the pointers here, or a concurrent sysfs callback
+ * would dereference freed memory.
+ */
cfg->button_entries = NULL;
+ cfg->button_mappings = NULL;
return ret;
}
@@ -2851,10 +3764,23 @@ static void ally_remove_button_attributes(struct hid_device *hdev, struct ally_c
entries = cfg->button_entries;
- for (i = 0; i < ALLY_BTN_MAX; i++)
+ for (i = 0; i < ALLY_BTN_MAX; i++) {
sysfs_remove_group(&hdev->dev.kobj, &entries[i].group);
+ kfree(entries[i].remap_attr);
+ kfree(entries[i].macro_attr);
+ }
- cfg->button_entries = NULL;
+ /*
+ * The arrays are devm-managed on the same device as the sysfs
+ * attributes, so they are released together with them. Only
+ * unpublish the pointers here, under the lock, so that a sysfs
+ * callback which is still holding config_mutex observes the
+ * teardown instead of racing against a manual free.
+ */
+ scoped_guard(mutex, &cfg->config_mutex) {
+ cfg->button_entries = NULL;
+ cfg->button_mappings = NULL;
+ }
}
/**
@@ -2881,23 +3807,12 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
goto ally_config_create_err;
}
- for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_attr_groups); sysfs_i++) {
- ret = devm_device_add_group(&hdev->dev, &ally_attr_groups[sysfs_i]);
- if (ret < 0) {
- hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
- ally_attr_groups[sysfs_i].name, ret);
- goto ally_config_create_sysfs_err;
- }
- }
-
- if (cfg->turbo_support) {
- ret = ally_create_button_attributes(hdev, ally, cfg);
- if (ret < 0) {
- hid_err(hdev, "Failed to create button attributes: %d\n", ret);
- goto ally_config_create_sysfs_err;
- }
- }
-
+ /*
+ * Initialize the software state before any sysfs attribute is
+ * created: gamepad_mode in particular indexes button_mappings,
+ * so leaving it at zero would make an early access dereference
+ * the uninitialized first element of that array.
+ */
cfg->gamepad_mode = ALLY_GAMEPAD_MODE_GAMEPAD;
cfg->left_deadzone = 10;
cfg->left_outer_threshold = 90;
@@ -2929,6 +3844,21 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
cfg->right_curve.entry_4.move = 100;
cfg->right_curve.entry_4.resp = 100;
+ for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_attr_groups); sysfs_i++) {
+ ret = devm_device_add_group(&hdev->dev, &ally_attr_groups[sysfs_i]);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
+ ally_attr_groups[sysfs_i].name, ret);
+ goto ally_config_create_sysfs_err;
+ }
+ }
+
+ ret = ally_create_button_attributes(hdev, ally, cfg);
+ if (ret < 0) {
+ hid_err(hdev, "Failed to create button attributes: %d\n", ret);
+ goto ally_config_create_sysfs_err;
+ }
+
/* So far the only hardware this is supported is the Ally 1 */
if (cfg->xbox_controller_support) {
ret = ally_set_xbox_controller(hdev, ally, cfg, true);
@@ -2949,7 +3879,7 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
return cfg;
ally_config_create_sysfs_err:
- if (cfg->turbo_support && cfg->button_entries)
+ if (cfg->button_entries)
ally_remove_button_attributes(hdev, cfg);
ally_config_create_err:
devm_kfree(&hdev->dev, cfg);
@@ -2959,16 +3889,17 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
/**
* ally_config_remove() - Clean up configuration resources
* @hdev: HID device
- * @ally: Non-NULL Ally device data
+ * @cfg: ally config to clean up, may be NULL
+ *
+ * Must be called in process context: it removes sysfs groups and takes
+ * sleeping locks.
*/
-static void ally_config_remove(struct hid_device *hdev, struct ally_handheld *ally)
+static void ally_config_remove(struct hid_device *hdev, struct ally_config *cfg)
{
- struct ally_config *cfg = ally->config;
-
if (!cfg || !cfg->initialized)
return;
- if (cfg->turbo_support && cfg->button_entries)
+ if (cfg->button_entries)
ally_remove_button_attributes(hdev, cfg);
}
@@ -3445,7 +4376,7 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
ret = hid_asus_ally_init(hdev, &ally_drvdata);
if (ret < 0) {
- ally_config_remove(hdev, &ally_drvdata);
+ ally_config_remove(hdev, ally_cfg);
return ERR_PTR(ret);
}
@@ -3493,8 +4424,9 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *ally)
{
+ struct ally_config *cfg = NULL;
unsigned long flags;
- bool owns_xpad;
+ bool owns_xpad, owns_cfg;
if (!ally)
return;
@@ -3553,12 +4485,22 @@ static void hid_asus_ally_remove(struct hid_device *hdev, struct ally_handheld *
ally->keyboard_hdev = NULL;
}
- if (ally->cfg_hdev == hdev) {
- ally_config_remove(hdev, ally);
+ owns_cfg = ally->cfg_hdev == hdev;
+ if (owns_cfg) {
+ cfg = ally->config;
ally->cfg_hdev = NULL;
ally->config = NULL;
}
spin_unlock_irqrestore(&ally_data_lock, flags);
+
+ /*
+ * The config teardown removes sysfs groups and takes sleeping locks:
+ * it must not run under ally_data_lock. The config pointer has been
+ * unpublished above, so no new sysfs callback can find it while the
+ * groups are being removed.
+ */
+ if (owns_cfg)
+ ally_config_remove(hdev, cfg);
}
static int hid_asus_ally_reset_resume(struct hid_device *hdev, struct ally_handheld *ally)
--
2.47.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 12/12] HID: asus: add support for btn remapping
2026-08-19 19:58 ` [PATCH v2 12/12] HID: asus: add support for btn remapping Denis Benato
@ 2026-08-19 20:22 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-19 20:22 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Software mapping cache rollback logic introduces state desync when a sequential hardware update fails on first-generation devices.
- [Low] Default button mapping initialization uses highly brittle hardcoded array indices that will silently break if the button code map array changes.
--
--- Patch [12]: [PATCH v2 12/12] HID: asus: add support for btn remapping ---
commit a8628b197357f3dab376cf274ae63c642d78d4ff
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for btn remapping
This commit adds support for button remapping on ROG Ally devices, exposing
sysfs attributes to allow configuring gamepad and desktop mode button codes
for individual physical buttons.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d6ab906b7e782..06dceb77d1086 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -976,6 +984,325 @@ static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device *
> return 0;
> }
>
> +enum btn_map_type {
> + BTN_TYPE_NONE = 0,
> + BTN_TYPE_PAD = 0x01,
> + BTN_TYPE_KB = 0x02,
> + BTN_TYPE_MOUSE = 0x03,
> + BTN_TYPE_MEDIA = 0x05,
> +};
> +
> +struct btn_code_map {
> + unsigned char type;
> + unsigned char value;
> + const char *name;
> +};
> +
> +static const struct btn_code_map ally_btn_codes[] = {
> + { BTN_TYPE_NONE, 0x00, "NONE" },
> + /* Gamepad button codes */
> + { BTN_TYPE_PAD, 0x01, "PAD_A" },
[ ... ]
> + /* Media button codes */
> + { BTN_TYPE_MEDIA, 0x16, "MEDIA_SCREENSHOT" },
> + { BTN_TYPE_MEDIA, 0x19, "MEDIA_SHOW_KEYBOARD" },
> + { BTN_TYPE_MEDIA, 0x1C, "MEDIA_SHOW_DESKTOP" },
> + { BTN_TYPE_MEDIA, 0x1E, "MEDIA_START_RECORDING" },
> + { BTN_TYPE_MEDIA, 0x01, "MEDIA_MIC_OFF" },
> + { BTN_TYPE_MEDIA, 0x02, "MEDIA_VOL_DOWN" },
> + { BTN_TYPE_MEDIA, 0x03, "MEDIA_VOL_UP" },
> +};
> +
> +static const size_t keymap_len = ARRAY_SIZE(ally_btn_codes);
[ ... ]
> +static void ally_set_default_keyboard_mapping(struct ally_button_mapping *mappings)
> +{
> + struct ally_button_mapping *map = &mappings[ALLY_GAMEPAD_MODE_KEYBOARD];
> + int i;
> +
> + /* Set all pair indexes and initialize to NONE */
> + for (i = 0; i < 9; i++) {
> + map->button_pairs[i].pair_index = i + 1;
> + map->button_pairs[i].first.remap =
> + &ally_btn_codes[0];
> + map->button_pairs[i].first.macro =
> + &ally_btn_codes[0];
> + map->button_pairs[i].second.remap =
> + &ally_btn_codes[0];
> + map->button_pairs[i].second.macro =
> + &ally_btn_codes[0];
> + }
> +
> + /*
> + * Desktop mode defaults: keyboard and mouse mappings.
> + * Buttons not listed here (dpad, view, menu) remain NONE.
> + */
> +
> + /* btn_a => KB_SPACE, btn_b => KB_E */
[Severity: Low]
This isn't a bug, but does hardcoding these array indices create a risk of
breaking the mappings if the ally_btn_codes array is modified in the future?
If new codes are added, removed, or reordered, explicit numbers like 91 and 53
will silently point to the wrong button codes, causing the default mappings
to silently regress.
> + map->button_pairs[BTN_PAIR_AB - 1].first.remap =
> + &ally_btn_codes[91]; /* KB_SPACE */
> + map->button_pairs[BTN_PAIR_AB - 1].second.remap =
> + &ally_btn_codes[53]; /* KB_E */
> +
> + /* btn_x => KB_R, btn_y => KB_F */
> + map->button_pairs[BTN_PAIR_XY - 1].first.remap =
> + &ally_btn_codes[54]; /* KB_R */
> + map->button_pairs[BTN_PAIR_XY - 1].second.remap =
> + &ally_btn_codes[68]; /* KB_F */
[ ... ]
> @@ -2775,6 +3120,515 @@ static void ally_btn_turbo_init_attrs(struct ally_btn_sysfs_entry *entry)
> entry->attrs[4] = NULL;
> }
[ ... ]
> +static ssize_t button_remap_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
[ ... ]
> + /*
> + * Send mapping to device with the caveat that first
> + * generation devices require individual button pair updates.
> + */
> + if (cfg->is_ally_x) {
> + ret = ally_set_button_mapping(hdev, ally, pair);
> + } else {
> + mode_map = &cfg->button_mappings[cfg->gamepad_mode];
> + ret = 0;
> + for (i = 0; i < 9; i++) {
> + mode_map->button_pairs[i].pair_index = i + 1;
> + ret = ally_set_button_mapping(hdev, ally,
> + &mode_map->button_pairs[i]);
> + if (ret < 0)
> + break;
> + }
> + }
> +
> + /*
> + * The hardware rejected the update: restore the previous
> + * mapping so that the software cache keeps describing the
> + * state the device is actually in.
> + */
[Severity: Medium]
Could this rollback logic cause a desynchronization between the software cache
and the hardware state?
When iterating over the 9 button pairs on older devices, if a USB transfer
fails on an iteration after the target pair was already successfully written
to the device (e.g. failure at iteration 5 when the target was pair 2), the
loop breaks and this rollback path triggers.
This unrolls old_code in the software cache, but the hardware has already
accepted and applied the new mapping, causing the sysfs cache to incorrectly
report the previous state.
> + if (ret < 0) {
> + if (btn_attr->is_macro)
> + btn_map->macro = old_code;
> + else
> + btn_map->remap = old_code;
> +
> + hid_err(hdev, "Failed to set button mapping: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + return count;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819195835.1649657-1-denis.benato@linux.dev?part=12
^ permalink raw reply [flat|nested] 21+ messages in thread