| Message ID | 20220524192422.13967-1-champagne.guillaume.c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | HID: ft260: fix multi packet i2c transactions | expand |
On Tue, May 24, 2022 at 03:24:22PM -0400, Guillaume Champagne wrote: > Only trigger START and STOP conditions for the first and last HID > packets when i2c writes are split in multiple packets. Otherwise, slave > i2c devices receive each packet as standalone i2c transactions. Since > i2c slave devices clear their internal state on STOP, this breaks auto > increment of the register address written to. > > Concretely, SCL is now held low between processing of HID packets so i2c > slave devices know to keep increment the same register address when the > next bytes arrive. > > Co-developed-by: Mathieu Gallichand <mathieu.gallichand@sonatest.com> > Signed-off-by: Mathieu Gallichand <mathieu.gallichand@sonatest.com> > Signed-off-by: Guillaume Champagne <champagne.guillaume.c@gmail.com> > --- > drivers/hid/hid-ft260.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c > index 79505c64dbfe..9c5912a21ccb 100644 > --- a/drivers/hid/hid-ft260.c > +++ b/drivers/hid/hid-ft260.c > @@ -390,6 +390,8 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, > struct hid_device *hdev = dev->hdev; > struct ft260_i2c_write_request_report *rep = > (struct ft260_i2c_write_request_report *)dev->write_buf; > + bool multi_packet = data_len > FT260_WR_DATA_MAX; > + u8 packet_flag = multi_packet ? flag & FT260_FLAG_START_REPEATED : flag; Please take a look at the https://patchwork.kernel.org/project/linux-input/patch/20220525074757.7519-4-michael.zaidman@gmail.com/ that addresses the same issue but does it more efficiently by adding one conditional statement per ft260_i2c_write call in the main path vs. three in this commit. It comes in the patch set with other performance improvements published on my GitHub https://github.com/MichaelZaidman/hid-ft260 several months ago. I would greatly appreciate it if you could test and feedback on the patch set content. Thanks, Michael
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 79505c64dbfe..9c5912a21ccb 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -390,6 +390,8 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, struct hid_device *hdev = dev->hdev; struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + bool multi_packet = data_len > FT260_WR_DATA_MAX; + u8 packet_flag = multi_packet ? flag & FT260_FLAG_START_REPEATED : flag; do { if (data_len <= FT260_WR_DATA_MAX) @@ -400,7 +402,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, rep->report = FT260_I2C_DATA_REPORT_ID(len); rep->address = addr; rep->length = len; - rep->flag = flag; + rep->flag = packet_flag; memcpy(rep->data, &data[idx], len); @@ -418,6 +420,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, data_len -= len; idx += len; + if (multi_packet) { + if (data_len <= FT260_WR_DATA_MAX) + packet_flag = flag & FT260_FLAG_STOP; + else + packet_flag = FT260_FLAG_NONE; + } } while (data_len > 0); return 0;