From: Jiri Kosina <jkosina@suse.cz>
To: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Cc: Henrik Rydberg <rydberg@euromail.se>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Benjamin Tissoires <Benjamin_Tissoires@logitech.com>,
Stephane Chatty <chatty@enac.fr>
Subject: Re: [PATCH v3 5/6] hid-multitouch: Switch to device groups
Date: Mon, 30 Apr 2012 18:42:02 +0200 (CEST) [thread overview]
Message-ID: <alpine.LNX.2.00.1204301837050.1856@pobox.suse.cz> (raw)
In-Reply-To: <CAN+gG=EWHi18kb1q2xQg8mfS9o6BHVLVdU+KYUcWdM4L3gJybA@mail.gmail.com>
On Mon, 30 Apr 2012, Benjamin Tissoires wrote:
> though I was not able to test it with the devices we have, I remarked
> a small regression in this patch: the detection of the serial protocol
> is not handled anymore. I was indeed relying on the fact that the
> parameter "id" in mt_probe was null to know that the device was not
> already in the list of known devices. I think that this small pitfall
> can be assessed in a separate commit later (after more testing) to
> keep the patch clear.
>
> By looking at the whole code of the patch set, Acked-by: Benjamin
> Tissoires <benjamin.tissoires@gmail.com>
>
> I'll tell you by the end of the week if anything goes wrong with our
> multitouch devices, but I don't see any reasons why it could fail.
Thanks for the review.
I will still wait for the mt_probe() serial protocol fix, as I'd like to
be certain that what I push into for-next doesn't introduce any known
regressions for which there is not a fix floating around yet.
One super-minor thing I have noticed when reviewing this patchset is that
we have never put a upper bound on HID_DG_CONTACTMAX as presented by the
device. In an extreme potential case of (likely broken) device this might
result in memory corruption in mt_probe(), as the argument of kzalloc()
when allocating the slots will overflow.
But this is not a regressions and has always been there, so something to
fix separately.
--
Jiri Kosina
SUSE Labs
next prev parent reply other threads:[~2012-04-30 16:42 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-23 10:07 [PATCH v3 0/6] hid: Introduce device groups Henrik Rydberg
2012-04-23 10:07 ` [PATCH v2 1/6] hid: Add device group to modalias Henrik Rydberg
2012-04-23 10:07 ` [PATCH 2/6] hid: Scan the device for group info before adding it Henrik Rydberg
2012-04-24 12:31 ` Jiri Kosina
2012-04-24 12:41 ` Henrik Rydberg
2012-04-23 10:07 ` [PATCH v2 3/6] hid: Allow bus wildcard matching Henrik Rydberg
2012-04-23 10:07 ` [PATCH v3 4/6] hid: Create a generic device group Henrik Rydberg
2012-04-23 21:26 ` Gustavo Padovan
2012-04-23 22:31 ` Henrik Rydberg
2012-04-23 10:07 ` [PATCH v3 5/6] hid-multitouch: Switch to device groups Henrik Rydberg
2012-04-30 14:58 ` Benjamin Tissoires
2012-04-30 14:58 ` Benjamin Tissoires
2012-04-30 16:42 ` Jiri Kosina [this message]
2012-04-30 18:30 ` Henrik Rydberg
2012-04-30 18:51 ` Benjamin Tissoires
2012-04-30 18:51 ` Benjamin Tissoires
2012-05-01 6:35 ` Henrik Rydberg
2012-04-23 10:07 ` [PATCH 6/6] hid: Create a common generic driver Henrik Rydberg
2012-04-23 10:51 ` [PATCH v3 0/6] hid: Introduce device groups Benjamin Tissoires
2012-04-23 10:51 ` Benjamin Tissoires
2012-04-23 15:21 ` Benjamin Tissoires
2012-04-23 15:21 ` Benjamin Tissoires
2012-04-30 8:35 ` Jiri Kosina
2012-04-30 8:44 ` Jiri Kosina
2012-04-30 11:53 ` Henrik Rydberg
2012-04-30 11:55 ` Jiri Kosina
2012-05-01 12:23 ` Jiri Kosina
2012-05-01 11:03 ` Jiri Kosina
2012-05-02 5:46 ` Henrik Rydberg
2012-05-02 8:33 ` Benjamin Tissoires
2012-05-02 8:45 ` Jiri Kosina
2012-05-02 10:55 ` Benjamin Tissoires
2012-05-03 10:06 ` Benjamin Tissoires
2012-05-03 12:23 ` Henrik Rydberg
2012-05-03 12:45 ` Benjamin Tissoires
2012-05-03 13:19 ` Henrik Rydberg
2012-05-03 13:37 ` Benjamin Tissoires
2012-05-03 13:54 ` Henrik Rydberg
2012-05-03 14:04 ` Benjamin Tissoires
2012-05-03 14:16 ` Henrik Rydberg
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=alpine.LNX.2.00.1204301837050.1856@pobox.suse.cz \
--to=jkosina@suse.cz \
--cc=Benjamin_Tissoires@logitech.com \
--cc=benjamin.tissoires@gmail.com \
--cc=chatty@enac.fr \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rydberg@euromail.se \
/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 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.