Re: [PATCH v2 03/12] HID: asus: add gamepad configuration
From: sashiko-bot@kernel.org
Date: 2026-08-19 20:10:51
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 [off-list ref] HID: asus: add gamepad configuration Add the base configuration structures for the gamepad configuration, detect capabilities and initialize the device in a known state.
quoted hunk ↗ jump to hunk
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; + } + }
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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