Thread (13 messages) flat view 13 messages, 4 authors, 2015-02-25

Re: [PATCH 2/3] HID: i2c-hid: Read wMaxInputLength of input report instead of bufsize

From: Benjamin Tissoires <hidden>
Date: 2015-02-23 15:26:51
Also in: lkml

On Feb 23 2015 or thereabouts, Mika Westerberg wrote:
Wacom digitizer (WCOM0008) on Lenovo Thinkpad 10 seems to allow reading of
subsequent reports (or part of them) if we read more than wMaxInputLength
of data at the time. This data is not always aligned so that the next
report would start right after another.

For example we might get following sequence:

 i2c_hid i2c-WCOM0008:00: input: 0a 00 02 21 20 17 ad 22 11 03
 i2c_hid i2c-WCOM0008:00: input: ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03 0a 00 02 21 20 17
                                 ad 22 11 03
 i2c_hid i2c-WCOM0008:00: input: 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17 ad 22 11 03 0a 00
                                 02 21 20 17
 i2c_hid i2c-WCOM0008:00: i2c_hid_get_input: incomplete report (76/8450)

The bufsize is 76 and wMaxInputLength is 10. In above example the first
read gets right amount of data. The second and third reads get full bufsize
(76 bytes) but the report is missing a start already. This causes the
driver to reject the report because we got less than was expected by the
report length (0x2102 = 8450).

If we read only wMaxInputLength at the time this does not happen at all and
the digitizer works fine.

Based on this change the driver to read wMaxInputLength bytes instead of
bufsize if the value looks sane.

Reported-by: Sébastien Bourdeauducq <redacted>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
We can actually drop this one. Jiri already applied a patch sent last
week which does the exact same thing:
https://patchwork.kernel.org/patch/5857521/ (should appear shortly in
Jiri's tree I guess).

But thanks for confirming that it was really needed for other devices.

Cheers,
Benjamin
quoted hunk ↗ jump to hunk
 drivers/hid/i2c-hid/i2c-hid.c | 8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 8f1dfc5c5d9c..404ccde49acd 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -369,9 +369,17 @@ static int i2c_hid_hwreset(struct i2c_client *client)
 
 static void i2c_hid_get_input(struct i2c_hid *ihid)
 {
+	unsigned max_input_length = le16_to_cpu(ihid->hdesc.wMaxInputLength);
 	int ret, ret_size;
 	int size = ihid->bufsize;
 
+	/*
+	 * Take the input length from HID descriptor if it is available and
+	 * looks reasonable.
+	 */
+	if (max_input_length > 0)
+		size = min_t(unsigned, ihid->bufsize, max_input_length);
+
 	ret = i2c_master_recv(ihid->client, ihid->inbuf, size);
 	if (ret != size) {
 		if (ret < 0)
-- 
2.1.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help