Le mer. 25 mai 2022 à 03:48, Michael Zaidman <michael.zaidman@xxxxxxxxx> a écrit : > > To support longer than one HID report size write, the driver splits a single > i2c message data payload into multiple i2c messages of HID report size. > However, it does not replicate the offset bytes within the EEPROM chip in > every consequent HID report because it is not and should not be aware of > the EEPROM type. It breaks the i2c write message integrity and causes the > EEPROM device not to acknowledge the second HID report keeping the i2c bus > busy until the ft260 controller reports failure. > I tested this whole patchset and it resolves the issue I raised https://patchwork.kernel.org/project/linux-input/patch/20220524192422.13967-1-champagne.guillaume.c@xxxxxxxxx/, thanks. > This patch preserves the i2c write message integrity by manipulating the > i2c flag bits across multiple HID reports to be seen by the EEPROM device > as a single i2c write transfer. > > Before: > > $ sudo ./i2cperf -f 2 -o 2 -s 64 -r 0-0xff 13 0x51 -S > Error: Sending messages failed: Input/output error > > [ +3.667741] ft260_i2c_write: rep 0xde addr 0x51 off 0 len 60 d[0] 0x0 > [ +0.007330] ft260_hid_output_report_check_status: wait 6400 usec, len 64 > [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 > [ +0.000001] ft260_i2c_write: rep 0xd1 addr 0x51 off 60 len 6 d[0] 0x0 > [ +0.002337] ft260_hid_output_report_check_status: wait 1000 usec, len 10 > [ +0.000157] ft260_xfer_status: bus_status 0x2e, clock 100 > [ +0.000241] ft260_i2c_reset: done > [ +0.000003] ft260 0003:0403:6030.000E: ft260_i2c_write: failed to start transfer, ret -5 > > After: > > $ sudo ./i2cperf -f 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S > > Fill block with increment via i2ctransfer by chunks > ------------------------------------------------------------------- > data rate(bps) efficiency(%) data size(B) total IOs IO size(B) > ------------------------------------------------------------------- > 58986 86 256 2 128 > > Signed-off-by: Michael Zaidman <michael.zaidman@xxxxxxxxx> > --- > drivers/hid/hid-ft260.c | 45 ++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 18 deletions(-) > > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c > index 44106cadd746..bfda5b191a3a 100644 > --- a/drivers/hid/hid-ft260.c > +++ b/drivers/hid/hid-ft260.c > @@ -378,41 +378,50 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, > } > > static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, > - int data_len, u8 flag) > + int len, u8 flag) > { > - int len, ret, idx = 0; > + int ret, wr_len, idx = 0; > + bool first = true; > struct hid_device *hdev = dev->hdev; > struct ft260_i2c_write_request_report *rep = > (struct ft260_i2c_write_request_report *)dev->write_buf; > > do { > - if (data_len <= FT260_WR_DATA_MAX) > - len = data_len; > - else > - len = FT260_WR_DATA_MAX; > + rep->flag = 0; > + if (first) { > + rep->flag = FT260_FLAG_START; I feel like multi packet transactions must still honor flag sent to ft20_i2c_write. This adds a START even if ft260_i2c_write is called with FT260_FLAG_START_REPEATED or FT260_FLAG_NONE. > + first = false; > + } > + > + if (len <= FT260_WR_DATA_MAX) { > + wr_len = len; > + if (flag == FT260_FLAG_START_STOP) > + rep->flag |= FT260_FLAG_STOP; > + } else { > + wr_len = FT260_WR_DATA_MAX; > + } > > - rep->report = FT260_I2C_DATA_REPORT_ID(len); > + rep->report = FT260_I2C_DATA_REPORT_ID(wr_len); > rep->address = addr; > - rep->length = len; > - rep->flag = flag; > + rep->length = wr_len; > > - memcpy(rep->data, &data[idx], len); > + memcpy(rep->data, &data[idx], wr_len); > > - ft260_dbg("rep %#02x addr %#02x off %d len %d d[0] %#02x\n", > - rep->report, addr, idx, len, data[0]); > + ft260_dbg("rep %#02x addr %#02x off %d len %d wlen %d flag %#x d[0] %#02x\n", > + rep->report, addr, idx, len, wr_len, > + rep->flag, data[0]); > > ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, > - len + 4); > + wr_len + 4); > if (ret < 0) { > - hid_err(hdev, "%s: failed to start transfer, ret %d\n", > - __func__, ret); > + hid_err(hdev, "%s: failed with %d\n", __func__, ret); > return ret; > } > > - data_len -= len; > - idx += len; > + len -= wr_len; > + idx += wr_len; > > - } while (data_len > 0); > + } while (len > 0); > > return 0; > } > -- > 2.25.1 >
