Hi Stephan,

Not sure if you saw my comment regarding your patch [1],
so I'll just repeat it properly inline here:

[1]: https://patchwork.kernel.org/patch/11178515/#22927311

On Sun, Oct 20, 2019 at 11:28:55PM +0300, Andi Shyti wrote:

quoted

The exchange of data to and from the mms114 touchscreen never
exceeds 256 bytes. In the worst case it goes up to 80 bytes in
the interrupt handler while reading the events.

i2c_smbus_read_i2c_block_data() is actually limited to
I2C_SMBUS_BLOCK_MAX = 32.

oh sorry, I don't know how I slipped on this.

But this means that the i2c in the kernel is wrong (or outdated),
smbus specifies 256 bytes of data[*]. I might have relied on the
specification more than the code.

I guess SMBUS needs some update.

quoted

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index a5ab774da4cc..170dcb5312b9 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c

@@ -204,14 +204,15 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 	}
 	mutex_unlock(&input_dev->mutex);
 
-	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
+	packet_size = i2c_smbus_read_byte_data(data->client,
+					       MMS114_PACKET_SIZE);
 	if (packet_size <= 0)
 		goto out;
 
 	touch_size = packet_size / MMS114_PACKET_NUM;
 
-	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
-			(u8 *)touch);
+	error = i2c_smbus_read_i2c_block_data(data->client, MMS114_INFORMATION,
+					      packet_size, (u8 *)touch);

... and here we try to read up to 80 bytes, as you mentioned.

i2c_smbus_read_i2c_block_data() will silently fall back to reading only
32 bytes. Therefore, if we try to read more than 32 bytes here we will
later read uninitialized data.

With this change, if you use more than 4 fingers you can easily trigger
a situation where one of the fingers gets "stuck", together with:
  mms114 4-0048: Wrong touch type (0)

yes, it can be that for this there might be some delays or miss
reads.

So we still need the custom functions here, or maybe avoid the problem
by using regmap instead.

This was what Dmitry was proposing almost a couple of years ago,
but then I stopped working on this.

quoted

+		error = i2c_smbus_read_i2c_block_data(data->client,
+						      MMS152_FW_REV, 3, buf);
 		if (error)

i2c_smbus_read_i2c_block_data() returns the number of read bytes,
therefore this check will always fail.

It should be: if (error < 0)

quoted

+		error = i2c_smbus_read_i2c_block_data(data->client,
+						      MMS114_TSP_REV, 6, buf);
 		if (error)

As above.

Yes, an oversight in these two cases. Thanks!

Well, I guess some more rework needs to be done in this patch...
at the end Dmitry was right :)

Thanks,
Andi

[*] http://www.smbus.org/specs/