Re: [PATCH] HID: Accept Digitizers as input devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 4, 2022 at 8:00 PM José Expósito <jose.exposito89@xxxxxxxxx> wrote:
>
> Hi Torge,
>
> Thanks for the patch.
>
> On Thu, Aug 04, 2022 at 05:18:32PM +0200, Torge Matthies wrote:
> > Commit f7d8e387d9ae ("HID: uclogic: Switch to Digitizer usage for
> > styluses") broke input from my XP-Pen Star G640. This is because the
> > "Digitizer" usage is not recognized as a valid usage for input devices.
> >
> > This patch changes the IS_INPUT_APPLICATION macro so that the "Digitizer"
> > (HID_DG_DIGITIZER) usage is recognized as an input device usage.
> >
> > Fixes: f7d8e387d9ae ("HID: uclogic: Switch to Digitizer usage for styluses")
> > Signed-off-by: Torge Matthies <openglfreak@xxxxxxxxxxxxxx>
> > ---
> > This patch could be risky, because any digitizer devices that were
> > previously not treated as input devices are now used for input.
> > Alternatively the linked commit could be reverted, but that would
> > re-introduce the problem detailed in its commit message.
> >
> >  include/linux/hid.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I hesitated about this when I sent the patch you mentioned. In the end,
> I didn't include any fix because the digitizer use was tested for 2
> years in DIGImend, so I (wrongly) assumed that it was safe enough.
>
> However, my initial thought was to add in uclogic_probe():
>
>         hdev->quirks |= HID_QUIRK_HIDINPUT_FORCE;
> +       hdev->quirks |= HID_QUIRK_HIDINPUT_FORCE;
>
> Let's see if we can hear more opinions, but if you are worried about
> affecting other drivers, that could be a good solution.

Sadly, my automated regression tests are broken for a while and I
haven't checked if that patch is introducing errors in hid-multitouch.

FWIW, this part has always been painful because some tablets were not
using the correct usages. And so that's why we are ending up in that
weird situation.

Anyway, just to mention that any code that touches this part should be
tested against the hid regression tests suite[0], because that's the
only way to find out if the change is affecting other devices.

Cheers,
Benjamin

[0] https://gitlab.freedesktop.org/libevdev/hid-tools

>
> Best wishes,
> Jose
>
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index 4363a63b9775..07803e144d98 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -883,7 +883,7 @@ static inline bool hid_is_usb(struct hid_device *hdev)
> >  /* We ignore a few input applications that are not widely used */
> >  #define IS_INPUT_APPLICATION(a) \
> >               (((a >= HID_UP_GENDESK) && (a <= HID_GD_MULTIAXIS)) \
> > -             || ((a >= HID_DG_PEN) && (a <= HID_DG_WHITEBOARD)) \
> > +             || ((a >= HID_DG_DIGITIZER) && (a <= HID_DG_WHITEBOARD)) \
> >               || (a == HID_GD_SYSTEM_CONTROL) || (a == HID_CP_CONSUMER_CONTROL) \
> >               || (a == HID_GD_WIRELESS_RADIO_CTLS))
> >
> > --
> > 2.37.1
> >
>





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux