linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Henrik Rydberg" <rydberg@euromail.se>
To: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Cc: "Jiri Kosina" <jkosina@suse.cz>, "Sean Young" <sean@mess.org>,
	linux-input <linux-input@vger.kernel.org>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Stéphane Chatty" <chatty@enac.fr>
Subject: Re: BUG: hid-multitouch causes 10 second delay and error
Date: Wed, 2 Nov 2011 11:12:38 +0100	[thread overview]
Message-ID: <20111102101238.GA2249@polaris.bitmath.org> (raw)
In-Reply-To: <CAN+gG=FOJv=J9=DF4HKsZT5HcLBkBM4rAWE7peOGLo4XZJ4GNg@mail.gmail.com>

Hi Benjamin,

> > Everyone with an umatched hid device, even completely unrelated to
> > touch, will be surprised to find the hid-multitouch module loaded.
> >
> 
> Well, this is a problem that can not be easily solved: IIRC, we can
> not force the load of an external driver from within the kernel.
> The best solution would be to merge hid-input and hid-multitouch.

Yes, or actually adding a dynamic mechanism.  With hid, it would
clearly be beneficial to be able to load modules based on the result
of the report parsing.

> Indeed both systems aim at handling generic devices.However, I'd
> rather not doing it now as we are not as "good" as the Win 7 driver
> (i.e. there are some fallback modes that allow every devices to be
> handled even if they don't send clean hid reports).

What's wrong with having a generic handling in addition to the
specific device list in hid-multitouch?

> And, even if hid-multitouch is loaded, only 2 or 3 lines of codes will
> be executed to reject the driver in mt_probe, which won't be very time
> consuming for end user.

The time to load the module will hit _every_ user, which is worse than
having the code merged with hid-input. Not to mention the annoyance,
it is simply unacceptable.

> For your second point:
> >> > 2. It did not work after removing the tested device from the hid core
> >> > whitelist; the device quirk seemed to get lost in the process.
> 
> Didn't you forget to remove the following line?
> 
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1230,7 +1230,6 @@ int hid_connect(struct hid_device *hdev,
> unsigned int connect_mask)
>                hdev->claimed |= HID_CLAIMED_INPUT;
>        if (hdev->quirks & HID_QUIRK_MULTITOUCH) {
>                /* this device should be handled by hid-multitouch, skip it */
> -               hdev->quirks &= ~HID_QUIRK_MULTITOUCH;
>                return -ENODEV;
>        }
> 
>

Gah, how did that end up there? Yes, I missed that line in my testing,
which explains it (although I won't test again right now).

> If not, that's maybe that you encountered the only case that is not
> correctly handled:
> if you register hid-multitouch before hid, then it will be the first
> driver tested, and hid-input won't set the quirk correctly.
> BTW, it's not a big deal, because if systems do have this behavior, we
> can easily put the device from the user space by using
> /sys/module/hid_multitouch/drivers/hid\:hid-multitouch/new_id

This is also hackish. I _do_ understand the benefits of what we are
aiming at here, but we are piling up crap in the kernel.

To summarize, the idea looked good at first glance, but I think it
creates unacceptable dependencies between modules.

To try again later on, at the very least one should move the essence
of hid-multitouch to something like hid-input-mt, and have hid-input
either include it, select it or depend dynamically on it, in a
try_module_get fashion. The hid_multitouch would then simply contain
the device white list, leaving the general case to hid-input.

Henrik

      reply	other threads:[~2011-11-02 10:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-26 21:37 BUG: hid-multitouch causes 10 second delay and error Sean Young
2011-10-27  9:25 ` Benjamin Tissoires
2011-10-27 11:54   ` Benjamin Tissoires
2011-10-27 20:35     ` Sean Young
2011-10-28 11:16     ` Henrik Rydberg
2011-10-28 13:19       ` Benjamin Tissoires
2011-10-28 14:00         ` Henrik Rydberg
2011-10-28 17:14           ` Jiri Kosina
2011-11-01 14:17             ` Henrik Rydberg
2011-11-01 14:27               ` Jiri Kosina
2011-11-01 15:33                 ` Henrik Rydberg
2011-11-02  8:23                   ` Benjamin Tissoires
2011-11-02 10:12                     ` Henrik Rydberg [this message]

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=20111102101238.GA2249@polaris.bitmath.org \
    --to=rydberg@euromail.se \
    --cc=benjamin.tissoires@gmail.com \
    --cc=chatty@enac.fr \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=sean@mess.org \
    /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).