Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Michael Zaidman <michael.zaidman@gmail.com>
To:     jikos@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com,
        Enrik.Berkhan@inka.de, Michael Zaidman <michael.zaidman@gmail.com>
Subject: [PATCH v3 09/12] HID: ft260: missed NACK from big i2c read
Date:   Sun, 30 Oct 2022 22:34:00 +0200
Message-Id: <20221030203403.4637-10-michael.zaidman@gmail.com>
In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com>
References: <20221030203403.4637-1-michael.zaidman@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

The FT260 controller does not return NACK when performing a big
read (of multiple hid reports size) from a non-existing device
or from the device responding with NACK when it is not ready
to serve the request. However, it responds correctly with NACK
to a read of up to a single hid report size.

To overcome this issue, we split the muli-report read request
into a read of a single HID report of 60 bytes size and a
multi-report read.

Big read of 256 bytes with first read of 60 bytes:

$ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x50 -S

[  +5.633280] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2
[  +0.000006] ft260_i2c_write: rep 0xd0 addr 0x50 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0
[  +0.013205] ft260_xfer_status: bus_status 0x20, clock 100
[  +0.000007] ft260_i2c_read: rep 0xc2 addr 0x50 len 255 rlen 60 flag 0x3
[  +0.010932] ft260_raw_event: i2c resp: rep 0xde len 60
[  +0.004733] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000006] ft260_i2c_read: rep 0xc2 addr 0x50 len 195 rlen 128 flag 0x0
[  +0.012572] ft260_raw_event: i2c resp: rep 0xde len 60
[  +0.005789] ft260_raw_event: i2c resp: rep 0xde len 60
[  +0.003189] ft260_raw_event: i2c resp: rep 0xd1 len 8
[  +0.004092] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000010] ft260_i2c_read: rep 0xc2 addr 0x50 len 67 rlen 67 flag 0x4
[  +0.011688] ft260_raw_event: i2c resp: rep 0xde len 60
[  +0.004700] ft260_raw_event: i2c resp: rep 0xd1 len 7
[  +0.004858] ft260_xfer_status: bus_status 0x20, clock 100

Read from non-existing device at address 8. The first 60 read responded
with NACK.

$ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x8 -S
[Oct19 15:37] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2
[  +0.000007] ft260_i2c_write: rep 0xd0 addr 0x8 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0
[  +0.022820] ft260_xfer_status: bus_status 0x20, clock 100
[  +0.000007] ft260_i2c_read: rep 0xc2 addr 0x8 len 255 rlen 60 flag 0x3
[  +0.010658] ft260_raw_event: i2c resp: rep 0xde len 60
[  +0.005965] ft260_xfer_status: bus_status 0x46, clock 100  <-- NACK
[  +0.000009] ft260 0003:0403:6030.0004: i2c bus error: 0x46
[  +0.007784] ft260_i2c_reset: done

Co-developed-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index d186aa5a8e73..40fae81386e3 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -464,6 +464,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
 			  u16 len, u8 flag)
 {
 	u16 rd_len;
+	u16 rd_data_max = 60;
 	int timeout, ret = 0;
 	struct ft260_i2c_read_request_report rep;
 	struct hid_device *hdev = dev->hdev;
@@ -473,12 +474,13 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
 	else
 		flag = FT260_FLAG_START;
 	do {
-		if (len <= FT260_RD_DATA_MAX) {
+		if (len <= rd_data_max) {
 			rd_len = len;
 			flag |= FT260_FLAG_STOP;
 		} else {
-			rd_len = FT260_RD_DATA_MAX;
+			rd_len = rd_data_max;
 		}
+		rd_data_max = FT260_RD_DATA_MAX;
 
 		rep.report = FT260_I2C_READ_REQ;
 		rep.length = cpu_to_le16(rd_len);
-- 
2.34.1