diff mbox series

[for,v5.5,2/2] Input: rmi_f54: read from FIFO in 32 byte blocks

Message ID 20200115124819.3191024-3-hverkuil-cisco@xs4all.nl (mailing list archive)
State Mainlined
Commit c15f8ba6dc1f6a8330cd89374a21388a5d91f92c
Headers show
Series input/rmi4 regression fix | expand

Commit Message

Hans Verkuil Jan. 15, 2020, 12:48 p.m. UTC 
The F54 Report Data is apparently read through a fifo and for
the smbus protocol that means that between reading a block of 32
bytes the rmiaddr shouldn't be incremented. However, changing
that causes other non-fifo reads to fail and so that change was
reverted.

This patch changes just the F54 function and it now reads 32 bytes
at a time from the fifo, using the F54_FIFO_OFFSET to update the
start address that is used when reading from the fifo.

This has only been tested with smbus, not with i2c or spi. But I
suspect that the same is needed there since I think similar
problems will occur there when reading more than 256 bytes.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reported-by: Timo Kaufmann <timokau@zoho.com>
Fixes: a284e11c371e ("Input: synaptics-rmi4 - don't increment rmiaddr for SMBus transfers")
Cc: stable@vger.kernel.org
---
 drivers/input/rmi4/rmi_f54.c | 43 ++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 16 deletions(-)

Comments

Dmitry Torokhov Jan. 17, 2020, 4:15 a.m. UTC | #1 
Hi Hans,

On Wed, Jan 15, 2020 at 01:48:19PM +0100, Hans Verkuil wrote:
> The F54 Report Data is apparently read through a fifo and for
> the smbus protocol that means that between reading a block of 32
> bytes the rmiaddr shouldn't be incremented. However, changing
> that causes other non-fifo reads to fail and so that change was
> reverted.
> 
> This patch changes just the F54 function and it now reads 32 bytes
> at a time from the fifo, using the F54_FIFO_OFFSET to update the
> start address that is used when reading from the fifo.
> 
> This has only been tested with smbus, not with i2c or spi. But I
> suspect that the same is needed there since I think similar
> problems will occur there when reading more than 256 bytes.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Reported-by: Timo Kaufmann <timokau@zoho.com>
> Fixes: a284e11c371e ("Input: synaptics-rmi4 - don't increment rmiaddr for SMBus transfers")
> Cc: stable@vger.kernel.org

As you mentioned this one is not urgent so I dropped the stable
designation (you may forward to stable once it cooks in 5.5 for a bit)
and also dropped fixes as it does not fixes this particular commit but
something that was done before.

Otherwise applied.

Thanks.
diff mbox series

Patch

diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 0bc01cfc2b51..6b23e679606e 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -24,6 +24,12 @@ 
 #define F54_NUM_TX_OFFSET       1
 #define F54_NUM_RX_OFFSET       0
 
+/*
+ * The smbus protocol can read only 32 bytes max at a time.
+ * But this should be fine for i2c/spi as well.
+ */
+#define F54_REPORT_DATA_SIZE	32
+
 /* F54 commands */
 #define F54_GET_REPORT          1
 #define F54_FORCE_CAL           2
@@ -526,6 +532,7 @@  static void rmi_f54_work(struct work_struct *work)
 	int report_size;
 	u8 command;
 	int error;
+	int i;
 
 	report_size = rmi_f54_get_report_size(f54);
 	if (report_size == 0) {
@@ -558,23 +565,27 @@  static void rmi_f54_work(struct work_struct *work)
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "Get report command completed, reading data\n");
 
-	fifo[0] = 0;
-	fifo[1] = 0;
-	error = rmi_write_block(fn->rmi_dev,
-				fn->fd.data_base_addr + F54_FIFO_OFFSET,
-				fifo, sizeof(fifo));
-	if (error) {
-		dev_err(&fn->dev, "Failed to set fifo start offset\n");
-		goto abort;
-	}
+	for (i = 0; i < report_size; i += F54_REPORT_DATA_SIZE) {
+		int size = min(F54_REPORT_DATA_SIZE, report_size - i);
+
+		fifo[0] = i & 0xff;
+		fifo[1] = i >> 8;
+		error = rmi_write_block(fn->rmi_dev,
+					fn->fd.data_base_addr + F54_FIFO_OFFSET,
+					fifo, sizeof(fifo));
+		if (error) {
+			dev_err(&fn->dev, "Failed to set fifo start offset\n");
+			goto abort;
+		}
 
-	error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr +
-			       F54_REPORT_DATA_OFFSET, f54->report_data,
-			       report_size);
-	if (error) {
-		dev_err(&fn->dev, "%s: read [%d bytes] returned %d\n",
-			__func__, report_size, error);
-		goto abort;
+		error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr +
+				       F54_REPORT_DATA_OFFSET,
+				       f54->report_data + i, size);
+		if (error) {
+			dev_err(&fn->dev, "%s: read [%d bytes] returned %d\n",
+				__func__, size, error);
+			goto abort;
+		}
 	}
 
 abort: