linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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>
Date: Mon, 3 Apr 2023 18:47:32 +0200	[thread overview]
Message-ID: <20230403164646.zwqluchagef34bei@mail.corp.redhat.com> (raw)
In-Reply-To: <20230331224536.1685149-1-torsha@google.com>

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
> 


      reply	other threads:[~2023-04-03 16:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31 22:45 [PATCH] Series-to: LKML <linux-kernel@vger.kernel.org> Torsha Banerjee
2023-04-03 16:47 ` Benjamin Tissoires [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230403164646.zwqluchagef34bei@mail.corp.redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=hcutts@chromium.org \
    --cc=jikos@kernel.org \
    --cc=jingyliang@chromium.org \
    --cc=jora@google.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seobrien@chromium.org \
    --cc=torsha@chromium.org \
    --cc=torsha@google.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).