linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "José Expósito" <jose.exposito89@gmail.com>
To: Alexander Zhang <alex@alexyzhang.dev>
Cc: Thorsten Leemhuis <regressions@leemhuis.info>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Torge Matthies <openglfreak@googlemail.com>,
	Jiri Kosina <jikos@kernel.org>,
	"open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
	Nikolai Kondrashov <spbnick@gmail.com>,
	"regressions@lists.linux.dev" <regressions@lists.linux.dev>
Subject: Re: [PATCH] HID: Accept Digitizers as input devices
Date: Thu, 10 Nov 2022 18:48:08 +0100	[thread overview]
Message-ID: <20221110174808.GA396731@elementary> (raw)
In-Reply-To: <b1536115-1d4b-67dd-197e-4b6ed8e5cbc6@alexyzhang.dev>

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/


  reply	other threads:[~2022-11-10 17:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2022-11-12  2:44             ` Alexander Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221110174808.GA396731@elementary \
    --to=jose.exposito89@gmail.com \
    --cc=alex@alexyzhang.dev \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=openglfreak@googlemail.com \
    --cc=regressions@leemhuis.info \
    --cc=regressions@lists.linux.dev \
    --cc=spbnick@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).