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

[Alexander Zhang <alex@alexyzhang.dev>]

Hello,

On 8/11/22 8:27 AM, Benjamin Tissoires wrote:
> On Thu, Aug 4, 2022 at 8:00 PM José Expósito <jose.exposito89@gmail.com> 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@googlemail.com>
>>> ---
>>> 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.

My XP-Pen Star G640 hasn't been working since commit f7d8e387d9ae and 
this patch fixes it. Is there anything I can do to help get this issue 
resolved? Should this be reported as a regression?

Best,
Alex

> 
> 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