Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 3 Apr 2023 18:47:32 +0200
From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
To:     Torsha Banerjee <torsha@chromium.org>
Cc:     u-boot@lists.denx.de, Jing <jingyliang@chromium.org>,
        Sean <seobrien@chromium.org>, Jora Jacobi <jora@google.com>,
        Harry Cutts <hcutts@chromium.org>,
        Torsha Banerjee <torsha@google.com>,
        Jiri Kosina <jikos@kernel.org>, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Series-to: LKML <linux-kernel@vger.kernel.org>
Message-ID: <20230403164646.zwqluchagef34bei@mail.corp.redhat.com>
References: <20230331224536.1685149-1-torsha@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230331224536.1685149-1-torsha@google.com>
Precedence: bulk

Hi Torsha,

Well, your commit title is just wrong, it should not start by
"Series-to: LKML ", which seems very much like a badly configured client
;)

On Mar 31 2023, Torsha Banerjee wrote:
> From: Jora Jacobi <jora@google.com>

This is much better than v1: we got the from and a matching S-o-B :)

So thanks for that.

> 
> Restore missing cursor for digitizer devices
> 
> A previously committed patch to remove cursors for HID Digitizer-Pen
> devices also removed the cursors for some tablets which have incorrect HID
> descriptors. These devices should enumerate with Usage ID "Digitizer"
> instead of Usage ID "Pen".
> 
> Patch which introduced the issue: commit 8473a93d1ba5
> ("HID: input: Set INPUT_PROP_-property for HID_UP_DIGITIZERS")
> 
> Changes-
> Add HID quirk HID_QUIRK_DEVICE_IS_DIGITIZER
> Quirk will force INPUT_PROP_POINTER for HID Digitizers
> Apply quirk to Huion tablets
> Apply quirk to UGEE/XP-Pen tablets based on device ID
> 
> Tested with Huion H640P and H430P. Connected the digitizer to the
> Chromebook and confirmed with a drawing program that the cursor appears and
> moves when the digitizer's stylus is hovering over the surface of the
> digitizer.

Would you mind re-testing the issue against the branch for-6.4/core of
hid.git[0]

IIRC UCLogic tablets where having an issue after Commit f7d8e387d9ae
("HID: uclogic: Switch to Digitizer usage for styluses") and it was
manually fixed in 3405a4beaaa8 ("HID: uclogic: Add HID_QUIRK_HIDINPUT_FORCE
quirk")

One proposed solution was to use a0f5276716c8 ("HID: Recognize "Digitizer"
as a valid input application"), and given that it was slightly broader,
I was initially reluctant to take it.

Given that Huion tablets tend to use hid-uclogic, I hope that it's
already fixed, and we don't even need to do more work to have your problem
resolved.

Cheers,
Benjamin


[0] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.4/core

> 
> Signed-off-by: Jora Jacobi <jora@google.com>
> Reviewed-by: Harry Cutts <hcutts@chromium.org>
> Signed-off-by: Torsha Banerjee <torsha@google.com>
> ---
> 
>  drivers/hid/hid-input.c  | 3 ++-
>  drivers/hid/hid-quirks.c | 9 +++++++++
>  include/linux/hid.h      | 1 +
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 7fc967964dd8..f0c67baddc8d 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -927,7 +927,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>  		break;
>  
>  	case HID_UP_DIGITIZER:
> -		if ((field->application & 0xff) == 0x01) /* Digitizer */
> +		if (((field->application & 0xff) == 0x01) ||
> +			(device->quirks & HID_QUIRK_DEVICE_IS_DIGITIZER)) /* Digitizer */
>  			__set_bit(INPUT_PROP_POINTER, input->propbit);
>  		else if ((field->application & 0xff) == 0x02) /* Pen */
>  			__set_bit(INPUT_PROP_DIRECT, input->propbit);
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 66e64350f138..094fe4c4f3b3 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -102,6 +102,8 @@ static const struct hid_device_id hid_quirks[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0941), HID_QUIRK_ALWAYS_POLL },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0641), HID_QUIRK_ALWAYS_POLL },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_1f4a), HID_QUIRK_ALWAYS_POLL },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_HS64), HID_QUIRK_DEVICE_IS_DIGITIZER },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET), HID_QUIRK_DEVICE_IS_DIGITIZER },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_IDEACOM, USB_DEVICE_ID_IDEACOM_IDC6680), HID_QUIRK_MULTI_INPUT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_INNOMEDIA, USB_DEVICE_ID_INNEX_GENESIS_ATARI), HID_QUIRK_MULTI_INPUT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X), HID_QUIRK_MULTI_INPUT },
> @@ -1298,6 +1300,13 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
>  		quirks = hid_gets_squirk(hdev);
>  	mutex_unlock(&dquirks_lock);
>  
> +	/*
> +	 * UGEE/XP-Pen HID Pen devices which have 0x0-0x9 as the low nibble
> +	 * of the device ID are actually digitizers, not HID Pen devices
> +	 */
> +	if (hdev->vendor == USB_VENDOR_ID_UGEE && (hdev->product & 0x0F) <= 0x09)
> +		quirks |= HID_QUIRK_DEVICE_IS_DIGITIZER;
> +
>  	return quirks;
>  }
>  EXPORT_SYMBOL_GPL(hid_lookup_quirk);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 1ea8c7a3570b..1653359002b3 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -354,6 +354,7 @@ struct hid_item {
>  #define HID_QUIRK_INPUT_PER_APP			BIT(11)
>  #define HID_QUIRK_X_INVERT			BIT(12)
>  #define HID_QUIRK_Y_INVERT			BIT(13)
> +#define HID_QUIRK_DEVICE_IS_DIGITIZER           BIT(14)
>  #define HID_QUIRK_SKIP_OUTPUT_REPORTS		BIT(16)
>  #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID		BIT(17)
>  #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP	BIT(18)
> -- 
> 2.40.0.348.gf938b09366-goog
>