On Fri, Feb 4, 2022 at 10:08 AM Sergio Costas <rastersoft@xxxxxxxxx> wrote:
>
> Hi Benjamin:
>
> You are absolutely right. Here is the new patch. It works perfectly too,
> and is much simpler.
Great, thanks.
Can you please resubmit it with your Signed-of-by line?
I should be able to apply it to the for-5.17 tree and add the stable@
tag but I can not do that without this tiny line :)
Cheers,
Benjamin
>
> From 139638a264bdbc47e4f55c484c58867e2dbd4ddc Mon Sep 17 00:00:00 2001
> From: Sergio Costas <rastersoft@xxxxxxxxx>
> Date: Fri, 4 Feb 2022 10:01:17 +0100
> Subject: [PATCH] HID:Add support for UGTABLET WP5540
>
> This patch adds support for the UGTABLET WP5540 digitizer tablet
> devices. Without it, the pen moves the cursor, but neither the
> buttons nor the tap sensor in the tip do work.
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-quirks.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 85975031389b..78bd3ddda442 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1370,6 +1370,7 @@
> #define USB_VENDOR_ID_UGTIZER 0x2179
> #define USB_DEVICE_ID_UGTIZER_TABLET_GP0610 0x0053
> #define USB_DEVICE_ID_UGTIZER_TABLET_GT5040 0x0077
> +#define USB_DEVICE_ID_UGTIZER_TABLET_WP5540 0x0004
>
> #define USB_VENDOR_ID_VIEWSONIC 0x0543
> #define USB_DEVICE_ID_VIEWSONIC_PD1011 0xe621
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 9af1dc8ae3a2..c066ba901867 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -187,6 +187,7 @@ static const struct hid_device_id hid_quirks[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_TURBOX,
> USB_DEVICE_ID_TURBOX_KEYBOARD), HID_QUIRK_NOGET },
> { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
> USB_DEVICE_ID_UCLOGIC_TABLET_KNA5), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
> USB_DEVICE_ID_UCLOGIC_TABLET_TWA60), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_UGTIZER,
> USB_DEVICE_ID_UGTIZER_TABLET_WP5540), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> USB_DEVICE_ID_WALTOP_MEDIA_TABLET_10_6_INCH), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET), HID_QUIRK_MULTI_INPUT },
> --
> 2.34.1
>
> El 4/2/22 a las 9:03, Benjamin Tissoires escribió:
> > Hi Sergio,
> >
> > On Thu, Feb 3, 2022 at 7:40 PM Sergio Costas <rastersoft@xxxxxxxxx> wrote:
> >> From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
> >> From: Sergio Costas <rastersoft@xxxxxxxxx>
> >> Date: Thu, 3 Feb 2022 14:15:07 +0100
> >> Subject: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
> >>
> >> This patch adds a new quirk to HID, allowing to configure the
> >> usage rules in reverse order. This is needed to enable the use
> >> of the UGTABLET WP5540 USB digitizer tablet.
> >>
> >> This tablet defines three HID collections: one for a digitizer
> >> device and two for mouse devices, the first one with relative
> >> coordinates, and the second one with absolute coordinates.
> >> The problem is that both mouse devices share the "button 1",
> >> "button 2" and "button 3" usages, but the Linux HID driver
> >> doesn't support that, so those usages remain unassigned for
> >> the second mouse because they are considered "duplicated".
> >> Unfortunately, the second mouse is, really, the pencil,
> >> which means that the movements are detected and managed,
> >> but not the pencil touch or any button press, because those
> >> usages are already assigned to the first mouse (I suspect
> >> that the chip used in this tablet is designed to work both
> >> with pencils and with uncorded mouses, allowing dual devices
> >> where both a pencil and a mouse can be used over the same
> >> rug).
> >>
> >> The result is that the user can move the cursor with the pencil,
> >> but can't "paint" things because the "touch" action doesn't
> >> work.
> >>
> >> Currently there is already a quirk that, at first glance, could
> >> seem as the solution to this problem. That quirk is
> >> HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE. Unfortunately, the
> >> result is that the "pencil touch" and "press button" actions
> >> are translated into "previous page", "next page"... and so on,
> >> which is not a desirable option. Although those events could
> >> be reasigned in user space using xmodmap, it requires actions
> >> from the user, and also could interfere with other devices,
> >> like multibutton mouses.
> >>
> >> I also tried to remove the duplicate test, but the system
> >> hangs as soon as it receives an event from the tablet.
> >>
> >> This patch aims to fix this by configuring the entries in
> >> reverse order, thus giving priority to the pencil over the
> >> mouse. This makes sense today, because it is more common to
> >> use optical mouses instead of tablet-based ones, and also
> >> ensures that devices marketed only with the pencil but
> >> using this chip do work out-of-the-box.
> >>
> >> To do this, the patch adds a new quirk:
> >> HID_QUIRK_REVERSE_CONFIGURE_USAGES, which can be enabled for
> >> an specific device, and it does assign the actions in reverse
> >> order, thus reversing the priority. This quirk can still be
> >> combined with HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE, so the
> >> events from the physical mouse could still be received, but
> >> giving priority to the digitizer, which makes more sense.
> > This seems extremely over-engineered. Have you tried the quirk
> > HID_QUIRK_MULTI_INPUT?
> > This quirk should create one independent input node per input report,
> > and I suspect your tablet is using different report IDs for the 2
> > mices.
> >
> > Cheers,
> > Benjamin
> >
> >> With this new quirk enabled for it, the UGTABLET WP5540
> >> digitizer tablet works like a charm.
> >>
> >> Signed-off-by: Sergio Costas <rastersoft@xxxxxxxxx>
> >> ---
> >> drivers/hid/hid-ids.h | 1 +
> >> drivers/hid/hid-input.c | 101 +++++++++++++++++++++++++++------------
> >> drivers/hid/hid-quirks.c | 1 +
> >> include/linux/hid.h | 1 +
> >> 4 files changed, 73 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> >> index 85975031389b..78bd3ddda442 100644
> >> --- a/drivers/hid/hid-ids.h
> >> +++ b/drivers/hid/hid-ids.h
> >> @@ -1370,6 +1370,7 @@
> >> #define USB_VENDOR_ID_UGTIZER 0x2179
> >> #define USB_DEVICE_ID_UGTIZER_TABLET_GP0610 0x0053
> >> #define USB_DEVICE_ID_UGTIZER_TABLET_GT5040 0x0077
> >> +#define USB_DEVICE_ID_UGTIZER_TABLET_WP5540 0x0004
> >>
> >> #define USB_VENDOR_ID_VIEWSONIC 0x0543
> >> #define USB_DEVICE_ID_VIEWSONIC_PD1011 0xe621
> >> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> >> index 112901d2d8d2..8cf4f63581cf 100644
> >> --- a/drivers/hid/hid-input.c
> >> +++ b/drivers/hid/hid-input.c
> >> @@ -1938,6 +1938,56 @@ static inline void
> >> hidinput_configure_usages(struct hid_input *hidinput,
> >> report->field[i]->usage + j);
> >> }
> >>
> >> +static inline void hidinput_configure_usages_reversed(struct hid_input
> >> *hidinput,
> >> + struct hid_report *report)
> >> +{
> >> + int i, j;
> >> +
> >> + for (i = report->maxfield; i > 0 ; i--)
> >> + for (j = report->field[i-1]->maxusage; j > 0 ; j--)
> >> + hidinput_configure_usage(hidinput, report->field[i-1],
> >> + report->field[i-1]->usage + j - 1);
> >> +}
> >> +
> >> +static bool hidinput_configure_usage_list(struct hid_device *hid,
> >> + struct hid_report *report,
> >> + bool reverse)
> >> +{
> >> +
> >> + unsigned int application;
> >> + struct hid_input *hidinput = NULL;
> >> +
> >> + application = report->application;
> >> +
> >> + /*
> >> + * Find the previous hidinput report attached
> >> + * to this report id.
> >> + */
> >> + if (hid->quirks & HID_QUIRK_MULTI_INPUT)
> >> + hidinput = hidinput_match(report);
> >> + else if (hid->maxapplication > 1 &&
> >> + (hid->quirks & HID_QUIRK_INPUT_PER_APP))
> >> + hidinput = hidinput_match_application(report);
> >> +
> >> + if (!hidinput) {
> >> + hidinput = hidinput_allocate(hid, application);
> >> + if (!hidinput)
> >> + return true;
> >> + }
> >> +
> >> + if (reverse)
> >> + hidinput_configure_usages_reversed(hidinput, report);
> >> + else
> >> + hidinput_configure_usages(hidinput, report);
> >> +
> >> + if (hid->quirks & HID_QUIRK_MULTI_INPUT)
> >> + hidinput->report = report;
> >> +
> >> + list_add_tail(&report->hidinput_list,
> >> + &hidinput->reports);
> >> + return false;
> >> +}
> >> +
> >> /*
> >> * Register the input device; print a message.
> >> * Configure the input layer interface
> >> @@ -1949,8 +1999,7 @@ int hidinput_connect(struct hid_device *hid,
> >> unsigned int force)
> >> struct hid_driver *drv = hid->driver;
> >> struct hid_report *report;
> >> struct hid_input *next, *hidinput = NULL;
> >> - unsigned int application;
> >> - int i, k;
> >> + int i, k, l;
> >>
> >> INIT_LIST_HEAD(&hid->inputs);
> >> INIT_WORK(&hid->led_work, hidinput_led_worker);
> >> @@ -1972,41 +2021,31 @@ int hidinput_connect(struct hid_device *hid,
> >> unsigned int force)
> >>
> >> report_features(hid);
> >>
> >> - for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) {
> >> + for (l = HID_INPUT_REPORT; l <= HID_OUTPUT_REPORT; l++) {
> >> + if (hid->quirks & HID_QUIRK_REVERSE_CONFIGURE_USAGES)
> >> + k = HID_OUTPUT_REPORT - l + HID_INPUT_REPORT;
> >> + else
> >> + k = l;
> >> if (k == HID_OUTPUT_REPORT &&
> >> hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS)
> >> continue;
> >>
> >> - list_for_each_entry(report, &hid->report_enum[k].report_list,
> >> list) {
> >> -
> >> - if (!report->maxfield)
> >> - continue;
> >> -
> >> - application = report->application;
> >> -
> >> - /*
> >> - * Find the previous hidinput report attached
> >> - * to this report id.
> >> - */
> >> - if (hid->quirks & HID_QUIRK_MULTI_INPUT)
> >> - hidinput = hidinput_match(report);
> >> - else if (hid->maxapplication > 1 &&
> >> - (hid->quirks & HID_QUIRK_INPUT_PER_APP))
> >> - hidinput = hidinput_match_application(report);
> >> -
> >> - if (!hidinput) {
> >> - hidinput = hidinput_allocate(hid, application);
> >> - if (!hidinput)
> >> + if (hid->quirks & HID_QUIRK_REVERSE_CONFIGURE_USAGES) {
> >> + list_for_each_entry_reverse(report,
> >> + &hid->report_enum[k].report_list,
> >> + list) {
> >> + if (!report->maxfield)
> >> + continue;
> >> + if (hidinput_configure_usage_list(hid, report, true))
> >> + goto out_unwind;
> >> + }
> >> + } else {
> >> + list_for_each_entry(report,
> >> &hid->report_enum[k].report_list, list) {
> >> + if (!report->maxfield)
> >> + continue;
> >> + if (hidinput_configure_usage_list(hid, report, false))
> >> goto out_unwind;
> >> }
> >> -
> >> - hidinput_configure_usages(hidinput, report);
> >> -
> >> - if (hid->quirks & HID_QUIRK_MULTI_INPUT)
> >> - hidinput->report = report;
> >> -
> >> - list_add_tail(&report->hidinput_list,
> >> - &hidinput->reports);
> >> }
> >> }
> >>
> >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> >> index 9af1dc8ae3a2..a59510bc76a1 100644
> >> --- a/drivers/hid/hid-quirks.c
> >> +++ b/drivers/hid/hid-quirks.c
> >> @@ -187,6 +187,7 @@ static const struct hid_device_id hid_quirks[] = {
> >> { HID_USB_DEVICE(USB_VENDOR_ID_TURBOX,
> >> USB_DEVICE_ID_TURBOX_KEYBOARD), HID_QUIRK_NOGET },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
> >> USB_DEVICE_ID_UCLOGIC_TABLET_KNA5), HID_QUIRK_MULTI_INPUT },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC,
> >> USB_DEVICE_ID_UCLOGIC_TABLET_TWA60), HID_QUIRK_MULTI_INPUT },
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_UGTIZER,
> >> USB_DEVICE_ID_UGTIZER_TABLET_WP5540), HID_QUIRK_REVERSE_CONFIGURE_USAGES },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> >> USB_DEVICE_ID_WALTOP_MEDIA_TABLET_10_6_INCH), HID_QUIRK_MULTI_INPUT },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> >> USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH), HID_QUIRK_MULTI_INPUT },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP,
> >> USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET), HID_QUIRK_MULTI_INPUT },
> >> diff --git a/include/linux/hid.h b/include/linux/hid.h
> >> index 7487b0586fe6..61bf462ca658 100644
> >> --- a/include/linux/hid.h
> >> +++ b/include/linux/hid.h
> >> @@ -361,6 +361,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_REVERSE_CONFIGURE_USAGES 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.34.1
> >>
> >>
> >>
> --
> Nos leemos
> RASTER (Linux user #228804)
> rastersoft@xxxxxxxxx https://www.rastersoft.com
>
