2023-03-31 23:08:39

by Torsha Banerjee

[permalink] [raw]
Subject: [PATCH] Series-to: LKML <[email protected]>

From: Jora Jacobi <[email protected]>

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.

Signed-off-by: Jora Jacobi <[email protected]>
Reviewed-by: Harry Cutts <[email protected]>
Signed-off-by: Torsha Banerjee <[email protected]>
---

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


2023-04-03 17:13:43

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH] Series-to: LKML <[email protected]>

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 <[email protected]>

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 <[email protected]>
> Reviewed-by: Harry Cutts <[email protected]>
> Signed-off-by: Torsha Banerjee <[email protected]>
> ---
>
> 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
>