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
prev parent 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).