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

On Mon, Jul 20, 2026 at 5:54 AM Lee Jones [off-list ref] wrote:

Is Jason back in this week?

Note that all of the Sashiko review comments are pre-existing issues.

I'm back now :)

quoted

Input subsystem guidelines require that device capabilities are advertised
before the input device is registered.  The Wacom driver was violating
this by advertising the SW_MUTE_DEVICE capability post-registration in
wacom_set_shared_values() (and duplicating it in device-specific setup
cases).

Resolve this by moving the SW_MUTE_DEVICE capability setup to
wacom_setup_touch_input_capabilities() for all touch devices that support
it, excluding TabletPC devices.

This paragraph is no longer accurate. The TabletPC-specific checks
were removed. It would be more correct to say something more like "for
all HID_GENERIC touch devices that support it" given the new logic.

quoted

Additionally, replace the lookup-dependent
'wacom_wac->shared->touch->product' references with 'hdev->product'
inside wacom_setup_touch_input_capabilities() as 'hdev' is already
available and represents the touch device itself.

Fixes: d2ec58aee8b1 ("HID: wacom: generic: support generic touch switch")
Signed-off-by: Lee Jones <lee@kernel.org>
---

v4 -> v5: New patch used to split out SW_MUTE_DEVICE as per Jason's request
v5 -> v6: Unconditionally advertise SW_MUTE_DEVICE on generic touch devices

 drivers/hid/wacom_sys.c |  6 ------
 drivers/hid/wacom_wac.c | 19 +++++++++++--------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..6c2a3ddfe366 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);
-     }
 }

 static int wacom_parse_and_register(struct wacom *wacom, bool wireless)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a29bf051ada7..32c4fd03479e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c

@@ -3953,6 +3953,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)
 {
+     struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+     struct hid_device *hdev = wacom->hdev;
      struct wacom_features *features = &wacom_wac->features;

      if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))

@@ -3963,9 +3965,12 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
      else
              __set_bit(INPUT_PROP_POINTER, input_dev->propbit);

-     if (features->type == HID_GENERIC)
+     if (features->type == HID_GENERIC) {
+             input_dev->evbit[0] |= BIT_MASK(EV_SW);
+             __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
              /* setup has already been done */
              return 0;
+     }

I'm still concerned about this not working properly. I don't see how
you are preserving the desired behavior where SW_MUTE_DEVICE is only
set on devices where `has_mute_touch_switch` is true (e.g. when a
WACOM_HID_WD_TOUCHONOFF usage is discovered in the HID descriptor). It
seems like this change will result in *all* HID_GENERIC touch devices
gaining SW_MUTE_DEVICE, which is not correct.

For devices that have SW_MUTE_DEVICE on the touch interface, I think
it would be sufficient to simply add a `has_mute_touch_switch` check
here, since the HID descriptor will have already been parsed by this
point. **But** I think the trickier part is handling the case where
SW_MUTE_DEVICE for the touch appears on a different interface,
endpoint, or (potentially) sibling USB device... (The SW_MUTE_DEVICE
usage is usually found in the "pad" collection, which is appears in
the HID descriptor for the pen interface/endpoint/device)

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

quoted

      input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
      __set_bit(BTN_TOUCH, input_dev->keybit);

@@ -3997,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
              input_dev->evbit[0] |= BIT_MASK(EV_SW);
              __set_bit(SW_MUTE_DEVICE, input_dev->swbit);

-             if (wacom_wac->shared->touch->product == 0x361) {
+             if (hdev->product == 0x361) {
                      input_set_abs_params(input_dev, ABS_MT_POSITION_X,
                                           0, 12440, 4, 0);
                      input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
                                           0, 8640, 4, 0);
-             }
-             else if (wacom_wac->shared->touch->product == 0x360) {
+             } else if (hdev->product == 0x360) {
                      input_set_abs_params(input_dev, ABS_MT_POSITION_X,
                                           0, 8960, 4, 0);
                      input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
                                           0, 5920, 4, 0);
-             }
-             else if (wacom_wac->shared->touch->product == 0x393) {
+             } else if (hdev->product == 0x393) {
                      input_set_abs_params(input_dev, ABS_MT_POSITION_X,
                                           0, 6400, 4, 0);
                      input_set_abs_params(input_dev, ABS_MT_POSITION_Y,

@@ -4039,8 +4042,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
              fallthrough;

      case WACOM_27QHDT:
-             if (wacom_wac->shared->touch->product == 0x32C ||
-                 wacom_wac->shared->touch->product == 0xF6) {
+             if (hdev->product == 0x32C ||
+                 hdev->product == 0xF6) {
                      input_dev->evbit[0] |= BIT_MASK(EV_SW);
                      __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
                      wacom_wac->has_mute_touch_switch = true;
--

2.55.0.141.g00534a21ce-goog

--
Lee Jones