All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID: Accept Digitizers as input devices
@ 2022-08-04 15:18 Torge Matthies
  2022-08-04 18:00 ` José Expósito
  0 siblings, 1 reply; 9+ messages in thread
From: Torge Matthies @ 2022-08-04 15:18 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input
  Cc: Nikolai Kondrashov, José Expósito, Torge Matthies

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

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


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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-08-04 15:18 [PATCH] HID: Accept Digitizers as input devices Torge Matthies
@ 2022-08-04 18:00 ` José Expósito
  2022-08-11 15:27   ` Benjamin Tissoires
  0 siblings, 1 reply; 9+ messages in thread
From: José Expósito @ 2022-08-04 18:00 UTC (permalink / raw)
  To: Torge Matthies
  Cc: Jiri Kosina, Benjamin Tissoires, linux-input, Nikolai Kondrashov

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.

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
> 

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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-08-04 18:00 ` José Expósito
@ 2022-08-11 15:27   ` Benjamin Tissoires
  2022-10-26 22:42     ` Alexander Zhang
  0 siblings, 1 reply; 9+ messages in thread
From: Benjamin Tissoires @ 2022-08-11 15:27 UTC (permalink / raw)
  To: José Expósito
  Cc: Torge Matthies, Jiri Kosina, open list:HID CORE LAYER,
	Nikolai Kondrashov

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.

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


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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-08-11 15:27   ` Benjamin Tissoires
@ 2022-10-26 22:42     ` Alexander Zhang
  2022-10-27 12:06       ` Thorsten Leemhuis
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Zhang @ 2022-10-26 22:42 UTC (permalink / raw)
  To: Benjamin Tissoires, José Expósito
  Cc: Torge Matthies, Jiri Kosina, open list:HID CORE LAYER,
	Nikolai Kondrashov

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

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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-10-26 22:42     ` Alexander Zhang
@ 2022-10-27 12:06       ` Thorsten Leemhuis
  2022-10-27 20:42         ` José Expósito
  2022-10-28  3:16         ` Alexander Zhang
  0 siblings, 2 replies; 9+ messages in thread
From: Thorsten Leemhuis @ 2022-10-27 12:06 UTC (permalink / raw)
  To: Alexander Zhang, Benjamin Tissoires, José Expósito
  Cc: Torge Matthies, Jiri Kosina, open list:HID CORE LAYER,
	Nikolai Kondrashov, regressions

Hi, this is your Linux kernel regression tracker. CCing the regression
mailing list, as it should be in the loop for all regressions, as
explained here:
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html

On 27.10.22 00:42, Alexander Zhang wrote:
> 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:
>>> 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?

What's the latest version you tested? This is not my area of expertise,
but I noticed there was a patch with a fix for f7d8e387d9ae that went
into 6.0.3:
https://lore.kernel.org/all/20221019083311.156155236@linuxfoundation.org/

Maybe it helps, but maybe I'm just confusing everything with this mail.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-10-27 12:06       ` Thorsten Leemhuis
@ 2022-10-27 20:42         ` José Expósito
  2022-10-28  3:16         ` Alexander Zhang
  1 sibling, 0 replies; 9+ messages in thread
From: José Expósito @ 2022-10-27 20:42 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Alexander Zhang, Benjamin Tissoires, Torge Matthies, Jiri Kosina,
	open list:HID CORE LAYER, Nikolai Kondrashov, regressions

Hi Thorsten,

On Thu, Oct 27, 2022 at 02:06:43PM +0200, Thorsten Leemhuis wrote:
> Hi, this is your Linux kernel regression tracker. CCing the regression
> mailing list, as it should be in the loop for all regressions, as
> explained here:
> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
> 
> [...]
>  
> What's the latest version you tested? This is not my area of expertise,
> but I noticed there was a patch with a fix for f7d8e387d9ae that went
> into 6.0.3:
> https://lore.kernel.org/all/20221019083311.156155236@linuxfoundation.org/
> 
> Maybe it helps, but maybe I'm just confusing everything with this mail.

Tablets have different nodes for their pad (the tablet per se) and the
pen. That patch fixed a missing prefix in the device name. For example,
"Tablet Name" becomes "Tablet Name Pen" after the fix. So, it doesn't
affect the problem Torge is trying to fix.

Bejamin, are there any updates on the broken automated regression tests
you mentioned? Do you think it is worth adding HID_QUIRK_HIDINPUT_FORCE
in the meantime?

Best wishes,
Jose

> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I deal with a lot of
> reports and sometimes miss something important when writing mails like
> this. If that's the case here, don't hesitate to tell me in a public
> reply, it's in everyone's interest to set the public record straight.

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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-10-27 12:06       ` Thorsten Leemhuis
  2022-10-27 20:42         ` José Expósito
@ 2022-10-28  3:16         ` Alexander Zhang
  2022-11-10 17:48           ` José Expósito
  1 sibling, 1 reply; 9+ messages in thread
From: Alexander Zhang @ 2022-10-28  3:16 UTC (permalink / raw)
  To: Thorsten Leemhuis, Benjamin Tissoires, José Expósito
  Cc: Torge Matthies, Jiri Kosina, open list:HID CORE LAYER,
	Nikolai Kondrashov, regressions

Hi Thorsten,

On 10/27/22 5:06 AM, Thorsten Leemhuis wrote:
> Hi, this is your Linux kernel regression tracker. CCing the regression
> mailing list, as it should be in the loop for all regressions, as
> explained here:
> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
> 
> On 27.10.22 00:42, Alexander Zhang wrote:
>> 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:
>>>> 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?
> 
> What's the latest version you tested? This is not my area of expertise,
> but I noticed there was a patch with a fix for f7d8e387d9ae that went
> into 6.0.3:
> https://lore.kernel.org/all/20221019083311.156155236@linuxfoundation.org/
> 
> Maybe it helps, but maybe I'm just confusing everything with this mail.

I've tested up to commit 98555239e4c3 which was the latest mainline 
version yesterday and it was still broken.

Best,
Alex

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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-10-28  3:16         ` Alexander Zhang
@ 2022-11-10 17:48           ` José Expósito
  2022-11-12  2:44             ` Alexander Zhang
  0 siblings, 1 reply; 9+ messages in thread
From: José Expósito @ 2022-11-10 17:48 UTC (permalink / raw)
  To: Alexander Zhang
  Cc: Thorsten Leemhuis, Benjamin Tissoires, Torge Matthies,
	Jiri Kosina, open list:HID CORE LAYER, Nikolai Kondrashov,
	regressions

Hi!

> > On 27.10.22 00:42, Alexander Zhang wrote:
> > > 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:
> > > > > 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.

Since it seems like this patch is kind of stuck, I sent you a different
patch [1] that aims to fix the same issue but using a more conservative
approach.

Could you test it and confirm that it fixes your problem, please?

Hopefully, we'd manage to get it merged as the risk of doing so would
be smaller.

Jose

[1] https://lore.kernel.org/linux-input/20221110174056.393697-1-jose.exposito89@gmail.com/T/


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

* Re: [PATCH] HID: Accept Digitizers as input devices
  2022-11-10 17:48           ` José Expósito
@ 2022-11-12  2:44             ` Alexander Zhang
  0 siblings, 0 replies; 9+ messages in thread
From: Alexander Zhang @ 2022-11-12  2:44 UTC (permalink / raw)
  To: José Expósito
  Cc: Thorsten Leemhuis, Benjamin Tissoires, Torge Matthies,
	Jiri Kosina, open list:HID CORE LAYER, Nikolai Kondrashov,
	regressions

Hi José,

On 11/10/22 9:48 AM, José Expósito wrote:
> Hi!
> 
>>> On 27.10.22 00:42, Alexander Zhang wrote:
>>>> 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:
>>>>>> 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.
> 
> Since it seems like this patch is kind of stuck, I sent you a different
> patch [1] that aims to fix the same issue but using a more conservative
> approach.
> 
> Could you test it and confirm that it fixes your problem, please?

I tested the patch on commit 4bbf3422df78 and it fixes the issue. 
(Sending this again to make sure it reaches everyone.)

Thank you,
Alexander Zhang

> Hopefully, we'd manage to get it merged as the risk of doing so would
> be smaller.
> 
> Jose
> 
> [1] https://lore.kernel.org/linux-input/20221110174056.393697-1-jose.exposito89@gmail.com/T/
> 

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

end of thread, other threads:[~2022-11-12  2:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 15:18 [PATCH] HID: Accept Digitizers as input devices Torge Matthies
2022-08-04 18:00 ` José Expósito
2022-08-11 15:27   ` Benjamin Tissoires
2022-10-26 22:42     ` Alexander Zhang
2022-10-27 12:06       ` Thorsten Leemhuis
2022-10-27 20:42         ` José Expósito
2022-10-28  3:16         ` Alexander Zhang
2022-11-10 17:48           ` José Expósito
2022-11-12  2:44             ` Alexander Zhang

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.