All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
@ 2022-02-03 18:40 Sergio Costas
  2022-02-04  8:03 ` Benjamin Tissoires
  0 siblings, 1 reply; 6+ messages in thread
From: Sergio Costas @ 2022-02-03 18:40 UTC (permalink / raw)
  To: linux-input, jikos

 From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
From: Sergio Costas <rastersoft@gmail.com>
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.

With this new quirk enabled for it, the UGTABLET WP5540
digitizer tablet works like a charm.

Signed-off-by: Sergio Costas <rastersoft@gmail.com>
---
  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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
  2022-02-03 18:40 [PATCH] HID:Add support for UGTABLET WP5540 with new quirk Sergio Costas
@ 2022-02-04  8:03 ` Benjamin Tissoires
  2022-02-04  9:08   ` Sergio Costas
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Tissoires @ 2022-02-04  8:03 UTC (permalink / raw)
  To: Sergio Costas; +Cc: open list:HID CORE LAYER, Jiri Kosina

Hi Sergio,

On Thu, Feb 3, 2022 at 7:40 PM Sergio Costas <rastersoft@gmail.com> wrote:
>
>  From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
> From: Sergio Costas <rastersoft@gmail.com>
> 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@gmail.com>
> ---
>   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
>
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
  2022-02-04  8:03 ` Benjamin Tissoires
@ 2022-02-04  9:08   ` Sergio Costas
  2022-02-04  9:45     ` Benjamin Tissoires
  0 siblings, 1 reply; 6+ messages in thread
From: Sergio Costas @ 2022-02-04  9:08 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER, Jiri Kosina

Hi Benjamin:

You are absolutely right. Here is the new patch. It works perfectly too, 
and is much simpler.

 From 139638a264bdbc47e4f55c484c58867e2dbd4ddc Mon Sep 17 00:00:00 2001
From: Sergio Costas <rastersoft@gmail.com>
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@gmail.com> wrote:
>>   From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
>> From: Sergio Costas <rastersoft@gmail.com>
>> 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@gmail.com>
>> ---
>>    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@gmail.com                https://www.rastersoft.com


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
  2022-02-04  9:08   ` Sergio Costas
@ 2022-02-04  9:45     ` Benjamin Tissoires
  2022-02-04 10:09       ` Sergio Costas
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Tissoires @ 2022-02-04  9:45 UTC (permalink / raw)
  To: Sergio Costas; +Cc: open list:HID CORE LAYER, Jiri Kosina

On Fri, Feb 4, 2022 at 10:08 AM Sergio Costas <rastersoft@gmail.com> 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@gmail.com>
> 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@gmail.com> wrote:
> >>   From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
> >> From: Sergio Costas <rastersoft@gmail.com>
> >> 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@gmail.com>
> >> ---
> >>    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@gmail.com                https://www.rastersoft.com
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
  2022-02-04  9:45     ` Benjamin Tissoires
@ 2022-02-04 10:09       ` Sergio Costas
  2022-02-05 11:37         ` Benjamin Tissoires
  0 siblings, 1 reply; 6+ messages in thread
From: Sergio Costas @ 2022-02-04 10:09 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER, Jiri Kosina

Hi Benjamin:

Ops! My fault (I'm still a newcomer to the kernel). Here it is.


From 7caa5076eb61a667a3ccc1ecae377b270760c464 Mon Sep 17 00:00:00 2001
From: Sergio Costas <rastersoft@gmail.com>
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.

Signed-off-by: Sergio Costas <rastersoft@gmail.com>
---
  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 10:45, Benjamin Tissoires escribió:
> On Fri, Feb 4, 2022 at 10:08 AM Sergio Costas <rastersoft@gmail.com> 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@gmail.com>
>> 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@gmail.com> wrote:
>>>>    From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
>>>> From: Sergio Costas <rastersoft@gmail.com>
>>>> 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@gmail.com>
>>>> ---
>>>>     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@gmail.com                https://www.rastersoft.com
>>
-- 
Nos leemos
		         RASTER    (Linux user #228804)
rastersoft@gmail.com                https://www.rastersoft.com


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID:Add support for UGTABLET WP5540 with new quirk
  2022-02-04 10:09       ` Sergio Costas
@ 2022-02-05 11:37         ` Benjamin Tissoires
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2022-02-05 11:37 UTC (permalink / raw)
  To: Sergio Costas; +Cc: open list:HID CORE LAYER, Jiri Kosina

Hi Sergio,

On Fri, Feb 4, 2022 at 11:09 AM Sergio Costas <rastersoft@gmail.com> wrote:
>
> Hi Benjamin:
>
> Ops! My fault (I'm still a newcomer to the kernel). Here it is.

Ok, thanks. A few notes for your next submission:
- never ever use the gmail web UI to send your patches. It completely
mangled the lines and I had to manually apply it, change the commit
author/date to look like it was yours
- only use git send-email to send a patch (like the first version I think)
- if you need to add some blurb to your patch, after your
Signed-off-by, add on the next line '---' (without the quotes), then a
new line and put whatever you want there. This will be ignored by git
am when applying the patch. FYI, I often add those on my local tree so
I keep the changes in the versions in the tree, git format-patch will
keep those annotations, but they will be stripped out when applied :)

Anyway, I have now pushed your change to for-5.17/upstream-fixes and
added the stable@vger.kernel.org marker, meaning that when the patch
lands into Linus' tree, it will be automatically backported to older
kernels.

Cheers,
Benjamin

>
>
> From 7caa5076eb61a667a3ccc1ecae377b270760c464 Mon Sep 17 00:00:00 2001
> From: Sergio Costas <rastersoft@gmail.com>
> 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.
>
> Signed-off-by: Sergio Costas <rastersoft@gmail.com>
> ---
>   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 10:45, Benjamin Tissoires escribió:
> > On Fri, Feb 4, 2022 at 10:08 AM Sergio Costas <rastersoft@gmail.com> 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@gmail.com>
> >> 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@gmail.com> wrote:
> >>>>    From 82e513c85f4bdc027e5b907000a3bbaf74314bcb Mon Sep 17 00:00:00 2001
> >>>> From: Sergio Costas <rastersoft@gmail.com>
> >>>> 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@gmail.com>
> >>>> ---
> >>>>     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@gmail.com                https://www.rastersoft.com
> >>
> --
> Nos leemos
>                          RASTER    (Linux user #228804)
> rastersoft@gmail.com                https://www.rastersoft.com
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-05 11:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 18:40 [PATCH] HID:Add support for UGTABLET WP5540 with new quirk Sergio Costas
2022-02-04  8:03 ` Benjamin Tissoires
2022-02-04  9:08   ` Sergio Costas
2022-02-04  9:45     ` Benjamin Tissoires
2022-02-04 10:09       ` Sergio Costas
2022-02-05 11:37         ` Benjamin Tissoires

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.