Thread (21 messages) flat view 21 messages, 2 authors, 4d ago

Re: [PATCH v6 13/13] HID: asus: add support for btn remapping

From: sashiko-bot@kernel.org
Date: 2026-09-08 18:07:21

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Button remapping attributes are never exposed for devices lacking turbo support because `ally_create_button_attributes` is incorrectly gated by `if (cfg->turbo_support)` in the caller.
--

commit f62bd64b66b8edafa21a4dcfaec466f614d7bae9
Author: Denis Benato [off-list ref]
Subject: HID: asus: add support for btn remapping

Adds support for button remapping to the ROG Ally, exposing remap_index,
remap, and macro sysfs attributes for physical buttons on the device.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 5cdbcc2bd1bc7..f8655c1ab7359 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -2932,15 +3842,12 @@ static void ally_btn_turbo_init_attrs(struct ally_btn_sysfs_entry *entry)
  * 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_handheld *ally,
+					  struct ally_config *cfg)
 {
 	struct ally_btn_sysfs_entry *entries;
 	int i, ret;
 
-	if (!cfg->turbo_support)
-		return 0;
-
[ ... ]
+		/* 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;
+		}
[Severity: Medium]
Since ally_create_button_attributes() is now designed to create remap
attributes even when the device lacks turbo support, should the caller
ally_config_create() also be updated to match?

Currently, ally_config_create() still completely gates calling this function
on cfg->turbo_support:

	if (cfg->turbo_support) {
		ret = ally_create_button_attributes(hdev, ally, cfg);
		if (ret < 0) {
			...
		}
	}

Does this result in a regression where the new remap attributes are
completely inaccessible for devices without turbo support?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908174433.74260-1-denis.benato@linux.dev?part=13
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help