New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847561 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

hid-multitouch omits the last touch usage when completing a slot under certain scenarios

Project Member Reported by benchan@chromium.org, May 29 2018

Issue description

According to [1] and also seemingly agreed by [2], the Scan Time usage (0x0D 0x56) of a touch device is a report level usage, not a contact level usage.

Commit 29cc309d8bf19 ("HID: hid-multitouch: forward MSC_TIMESTAMP") [3] added handling of the Scan Time usage (0x0D 0x56) in the hid-multitouch kernel driver. However, it includes HID_DG_SCANTIME when calculating `td->last_slot_field', which may lead to mt_complete_slot() being prematurely called in certain cases (e.g. when each touch input report includes more than one contact and the Scan Time usage appears before any contact logical collection).

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#windows-precision-touchpad-input-reports
[2] https://patchwork.kernel.org/patch/1742181/
[3] https://patchwork.kernel.org/patch/9914039/
 
Cc: sheckylin@chromium.org drinkcat@chromium.org dtor@chromium.org adlr@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, May 30 2018

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2018

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/151f31d1bbbc58456ec8ced5db36b775f31a5dc2

commit 151f31d1bbbc58456ec8ced5db36b775f31a5dc2
Author: Ben Chan <benchan@chromium.org>
Date: Thu May 31 10:50:22 2018

BACKPORT: HID: multitouch: fix calculation of last slot field in multi-touch reports

According to [1] and also seemingly agreed by [2], the Scan Time usage
(0x0D 0x56) is a report level usage, not a contact level usage.

However, the hid-multitouch driver currently includes HID_DG_SCANTIME
when calculating `td->last_slot_field', which may lead to
mt_complete_slot() being prematurely called in certain cases (e.g. when
each touch input report includes more than one contact and the Scan Time
usage appears before any contact logical collection).

This patch fixes the issue by skipping mt_store_field() on
HID_DG_SCANTIME, similar to how HID_DG_CONTACTCOUNT and
HID_DG_CONTACTMAX.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#windows-precision-touchpad-input-reports
[2] https://patchwork.kernel.org/patch/1742181/

Fixes: 29cc309d8bf19 ("HID: hid-multitouch: forward MSC_TIMESTAMP")
Signed-off-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

(backported from commit abb36fe691b28f2a64926b61448d6b9610ed879a in
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
for-4.18/multitouch branch)

BUG= chromium:847561 
TEST=Verified with the latest eve touchpad firmware, which includes the
     Scan Time usage in touch input reports.

Change-Id: Id4ea83fce008b5e1b3bb06a84f9f7ef69f9e8283
Signed-off-by: Ben Chan <benchan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1077590
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/151f31d1bbbc58456ec8ced5db36b775f31a5dc2/drivers/hid/hid-multitouch.c

Project Member

Comment 4 by bugdroid1@chromium.org, May 31 2018

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/176bf8e3de91ba004f684b63d7a49eacf7e29db4

commit 176bf8e3de91ba004f684b63d7a49eacf7e29db4
Author: Ben Chan <benchan@chromium.org>
Date: Thu May 31 19:26:14 2018

BACKPORT: HID: multitouch: fix calculation of last slot field in multi-touch reports

According to [1] and also seemingly agreed by [2], the Scan Time usage
(0x0D 0x56) is a report level usage, not a contact level usage.

However, the hid-multitouch driver currently includes HID_DG_SCANTIME
when calculating `td->last_slot_field', which may lead to
mt_complete_slot() being prematurely called in certain cases (e.g. when
each touch input report includes more than one contact and the Scan Time
usage appears before any contact logical collection).

This patch fixes the issue by skipping mt_store_field() on
HID_DG_SCANTIME, similar to how HID_DG_CONTACTCOUNT and
HID_DG_CONTACTMAX.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#windows-precision-touchpad-input-reports
[2] https://patchwork.kernel.org/patch/1742181/

Fixes: 29cc309d8bf19 ("HID: hid-multitouch: forward MSC_TIMESTAMP")
Signed-off-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

(backported from commit abb36fe691b28f2a64926b61448d6b9610ed879a in
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
for-4.18/multitouch branch)

BUG= chromium:847561 
TEST=Verified with the latest eve touchpad firmware, which includes the
     Scan Time usage in touch input reports.

Change-Id: Id4ea83fce008b5e1b3bb06a84f9f7ef69f9e8283
Signed-off-by: Ben Chan <benchan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1077011
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/176bf8e3de91ba004f684b63d7a49eacf7e29db4/drivers/hid/hid-multitouch.c

Status: Fixed (was: Started)

Sign in to add a comment