From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A32243B14D5; Mon, 10 Aug 2026 10:56:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786359399; cv=none; b=JiBKTL11nVV9gSUy6QwfqppIQZ4CRQn+ACx82Z69okUFbUD3GE6N3vqbKFYc3WWj8rTvefI9XzYYB9j15qc9OFh3ysaoi6eJZ26DxyJaE6PZst/1bzUfw3IMqYFDQuVQvuBEAzRUYk/JV3qMMhCnDJLqpkHzMLHa/PmQJ5Zoj7k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786359399; c=relaxed/simple; bh=FcC+8qNn4CkKVg90lcKylua5Eiy7LX9NPCyvVFukhT0=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GinAFwIwgvMSMotJSlPWuBHNm29XXVGLyu7W/a+krgiL62YjU1ng5vV9biO6pKfi8XLIvI+ASFuSjDibYSL5hf1/9cALXEH9VdYoPa35MjiRUfjhXZaB19Dl92DKJj4W0WjPGNFmmc7rWqpdO9Ni13CBfmUWlE9++mEhAQTW9ec= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TZcxY0aC; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TZcxY0aC" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B96A1F000E9; Mon, 10 Aug 2026 10:56:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1786359398; bh=zdNfmbEO60BNxQLDtjP0gZGUv3ksIWau//PSgd/XITs=; h=Date:From:To:Subject:References:In-Reply-To; b=TZcxY0aCp4n2oDKk3egdnLCOEDkeLdQbGkZ28wk8Q/vYVnA95eqp9kIGCIpXguKJI cg0S+TTEvq/5byFiVjxk2+zJ72WPqWBtoDdINoZSWsJH0n5ML2Hji2RR3ZZs2IPEHO 8NtojiZCuUwCHWFUYbU7fR2dsakfho73Ye+GrmxvtxnvmrslNnb9dHqbcZ6tokB7rq PK9rqH24auTyDwknFhN+DRS9EPoP9naUBrlZrHQLamKSKtGOTBAreMXn9uSaw5cP8N VbrKAfJXUTuedlM/TjcnEOZsKaY9sVVAUbuh+An9GtIN23Y1josAwnQNqJ0hHJORsd IBLggfde60Eng== Date: Mon, 10 Aug 2026 11:56:33 +0100 From: Lee Jones To: Ping Cheng , Jason Gerecke , Jiri Kosina , Benjamin Tissoires , Aaron Skomra , Peter Hutterer , Dmitry Torokhov , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Message-ID: <20260810105633.GS2869284@google.com> References: <20260804103209.1496683-1-lee@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260804103209.1496683-1-lee@kernel.org> Jason, Can you confirm that you've seen this new version please? [intentional top-post] On Tue, 04 Aug 2026, Lee Jones wrote: > 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, including composite USB generic touch devices. > > 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 > --- > > 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 > v6 -> v7: Only advertise SW_MUTE_DEVICE on composite USB generic touch devices > > drivers/hid/wacom_sys.c | 23 +++++++++++++++++------ > drivers/hid/wacom_wac.c | 19 +++++++++++-------- > 2 files changed, 28 insertions(+), 14 deletions(-) > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c > index 0eafa483b7f7..92b73ed0028f 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) > @@ -2414,6 +2408,23 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless) > wacom_retrieve_hid_descriptor(hdev, features); > wacom_setup_device_quirks(wacom); > > + if (features->type == HID_GENERIC && > + (features->device_type & WACOM_DEVICETYPE_TOUCH)) { > + if (wacom->usbdev && wacom->usbdev->actconfig && > + wacom->usbdev->actconfig->desc.bNumInterfaces > 1) { > + /* > + * Heuristic: Composite USB devices (like tablets with > + * pen/pad + touch) likely have a touch mute switch. > + * We flag it here to advertise the capability before > + * registration. We also set is_soft_touch_switch to > + * default touch to ON in case there is no physical > + * switch. > + */ > + wacom_wac->has_mute_touch_switch = true; > + wacom_wac->is_soft_touch_switch = true; > + } > + } > + > if (features->device_type == WACOM_DEVICETYPE_NONE && > features->type != WIRELESS) { > error = features->type == HID_GENERIC ? -ENODEV : 0; > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c > index a29bf051ada7..afc82fcbb20b 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) { > + if (wacom_wac->has_mute_touch_switch) > + input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE); > /* setup has already been done */ > return 0; > + } > > 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.571.g244d577d93-goog > -- Lee Jones