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

[sashiko-bot@kernel.org]

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 <lee@kernel.org>

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.

> 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);
> -	}
>  }

[ ... ]

> 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