Thread (8 messages) 8 messages, 2 authors, 11d ago

Re: [PATCH 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

From: sashiko-bot@kernel.org
Date: 2026-07-07 12:28:07

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the `SW_MUTE_DEVICE` capability assignment into `wacom_setup_touch_input_capabilities()` silently breaks the mute switch feature for generic Wacom devices due to interface probe ordering.
--
--- Patch [1]: [PATCH 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration ---
commit 36a40be02315e2e469201d6490c033d8bc04fa73
Author: Lee Jones [off-list ref]

HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

This commit moves the SW_MUTE_DEVICE capability advertisement into
wacom_setup_touch_input_capabilities() to ensure it is set prior to input
device registration, conforming to input subsystem guidelines. It also
removes the post-registration assignment in wacom_set_shared_values() and
simplifies some product ID checks.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f75..6c2a3ddfe366a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2359,12 +2359,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
 			wacom_wac->shared->is_touch_on = true;
 	}
 
-	if (wacom_wac->shared->has_mute_touch_switch &&
-	    wacom_wac->shared->touch_input) {
-		set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
-		input_set_capability(wacom_wac->shared->touch_input, EV_SW,
-				     SW_MUTE_DEVICE);
-	}
 }
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625d..a31325be50f8c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3951,6 +3951,8 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
 int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 					 struct wacom_wac *wacom_wac)
 {
[ ... ]
-	if (features->type == HID_GENERIC)
+	if (features->type == HID_GENERIC) {
+		if (wacom_wac->shared && wacom_wac->shared->has_mute_touch_switch) {
+			input_dev->evbit[0] |= BIT_MASK(EV_SW);
+			__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+		}
 		/* setup has already been done */
 		return 0;
+	}
[Severity: High]
Does moving this assignment inadvertently break the mute switch on composite
USB devices?

For multi-interface generic Wacom devices, the touch interface typically
probes before the pad interface. During the touch interface's initialization,
this checks wacom_wac->shared->has_mute_touch_switch.

Since the pad interface (which contains the mute switch usage) hasn't been
parsed yet, has_mute_touch_switch will still be false.

Because the fallback assignment in wacom_set_shared_values() was removed
above, will this result in the SW_MUTE_DEVICE capability never being
advertised to the touch device?

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