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(-)
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:
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.
linux-stable-mirror@lists.linaro.org