Re: [PATCH v6 09/13] HID: asus: add support for response curve
From: sashiko-bot@kernel.org
Date: 2026-09-08 18:13:50
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Suspend/resume silently drops the active response curve or incorrectly applies an unapplied staged curve.
--
commit 67958cd2657dbe5bf7b4f12a20b0fbd79fa19fbf
Author: Denis Benato [off-list ref]
HID: asus: add support for response curve
This commit adds support for configuring the response curve of both
joysticks on ROG ally devices by exposing relevant sysfs attributes.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index 4ed438121cc97..f338c0eed54b3 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
+static ssize_t response_curve_move_store(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;
+
+ 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;[Severity: Medium] Since this directly mutates the active configuration structure (cfg->left_curve or cfg->right_curve), does this cause issues with suspend and resume if a user has written uncommitted changes? If userspace stages partial sysfs modifications to response_curve_move_X without triggering the apply attribute, and a suspend/resume cycle occurs, could the resume path be impacted? Could we store the staged curve separately from the active curve to prevent uncommitted edits from interfering with the system resume path?
+ break; + case 2: + curve->entry_2.move = value;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2421,6 +2879,31 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all ret); } + if (cfg->resp_curve_support) { + /* + * sysfs stages the entries without validating them: do not + * push a curve to the MCU until userspace has applied one + * that is valid. + */ + if (ally_resp_curve_is_valid(&cfg->left_curve)) { + ret = ally_set_joystick_resp_curve(ally, hdev, JOYSTICK_LEFT, + &cfg->left_curve);
[Severity: Medium] Will this restore an uncommitted partially edited state if it happens to be valid, or silently drop the previously valid state if the uncommitted edits made cfg->left_curve invalid?
+ if (ret < 0)
+ hid_warn(hdev,
+ "Failed to restore left response curve: %d\n",
+ ret);
+ }
+
+ if (ally_resp_curve_is_valid(&cfg->right_curve)) {
+ ret = ally_set_joystick_resp_curve(ally, hdev, JOYSTICK_RIGHT,
+ &cfg->right_curve);
+ if (ret < 0)
+ hid_warn(hdev,
+ "Failed to restore right response curve: %d\n",
+ ret);
+ }
+ }
+
return 0;
}-- Sashiko AI review · https://sashiko.dev/#/patchset/20260908174433.74260-1-denis.benato@linux.dev?part=9