MichaelZaidman / hid-ft260

FTDI FT260 Linux kernel driver
GNU General Public License v2.0
20 stars 6 forks source link

HID: ft260: missed NACK from busy device #6

Closed MichaelZaidman closed 1 year ago

MichaelZaidman commented 1 year ago

HID: ft260: missed NACK from busy device

When writing into a slow device like an EEPROM chip, the controller may exit the busy state before the device releases the bus. In this case, the ft260_xfer_status returns success before the data transfer completion.

The patch fixes it by returning from the ft260_xfer_status() with the "-EAGAIN" on both controller and bus busy status when appropriate.

It does not apply to the i2c combined transactions when after the write IO, the controller keeps the bus busy until the read IO and then between reading IOs to ensure an atomic operation.

Signed-off-by: Germain Hebert germain.hebert@ca.abb.com Signed-off-by: Michael Zaidman michael.zaidman@gmail.com

Tested with 24LC32A EEPROM by writing 2K in 32-byte chunks of data.

400KHz clock

Before:

$ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0x7ff 13 0x51 -S

   Fill block with increment via i2ctransfer by chunks
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  119303          80             2048          64          32

[  +0.000002] ft260_i2c_write: device wakeup
[  +0.000001] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001265] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.014412] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001585] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.011660] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001379] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.013519] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001462] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.018298] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001678] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.015957] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.001059] ft260_xfer_status: bus_status 0x40, clock 400

After:

$ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0x7ff 13 0x51 -S

  Fill block with increment via i2ctransfer by chunks
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  47764           80             2048          64          32

[  +0.011518] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x7
[  +0.001297] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000223] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000172] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000225] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000171] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000176] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000225] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000184] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000230] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000150] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000183] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000225] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000171] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000132] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000283] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000171] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000219] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000171] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.010912] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x7

100KHz clock

Before:

$ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0x7ff 13 0x51 -S

  Fill block with increment via i2ctransfer by chunks
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  54528           80             2048          64          32

[  +0.000001] ft260_i2c_write: device wakeup
[  +0.000002] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.002690] ft260_hid_output_report_check_status: wait 1920 usec, len 38
[  +0.000204] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000202] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000207] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000127] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000194] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000195] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000152] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.012992] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.002711] ft260_hid_output_report_check_status: wait 1920 usec, len 38
[  +0.000185] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.013646] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.003190] ft260_hid_output_report_check_status: wait 1920 usec, len 38
[  +0.000194] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.017842] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.002957] ft260_hid_output_report_check_status: wait 1920 usec, len 38
[  +0.000198] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000197] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000162] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000202] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000228] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000249] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000177] ft260_xfer_status: bus_status 0x40, clock 100

After:

$ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0x7ff 13 0x51 -S

  Fill block with increment via i2ctransfer by chunks
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  40324           80             2048          64          32

[  +0.000002] ft260_i2c_write: device wakeup
[  +0.000001] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0
[  +0.002929] ft260_hid_output_report_check_status: wait 1920 usec, len 38
[  +0.000248] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000248] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000239] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000172] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000202] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000185] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000191] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000229] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000183] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000191] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000183] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000128] ft260_xfer_status: bus_status 0x20, clock 100
[  +0.016610] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 wlen 34 flag 0x6 d[0] 0x0

SMBus one byte at a time writes into 24LC32A EEPROM

100KHz clock

$ sudo ./i2cperf -f 3 -o 2 -s 32 -r 0-0x7f 13 0x51 -S

  Fill block with increment via i2cset byte by byte
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  2972            21             128           128         1
=================================================================================

[  +0.012933] ft260_smbus_xfer: smbus size 8
[  +0.000004] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001682] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000182] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000179] ft260_xfer_status: bus_status 0x20, clock 100
[  +0.014000] ft260_smbus_xfer: smbus size 8
[  +0.000004] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001606] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000201] ft260_xfer_status: bus_status 0x40, clock 100
[  +0.000191] ft260_xfer_status: bus_status 0x20, clock 100
[  +0.015356] ft260_smbus_xfer: smbus size 8
[  +0.000004] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001208] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000203] ft260_xfer_status: bus_status 0x41, clock 100
[  +0.000197] ft260_xfer_status: bus_status 0x20, clock 100

400KHz clock

sudo bash -c 'echo 400 > /sys/bus/usb/devices/3-3.1:1.0/0003:0403:6030.0008/clock'

swuser@comex-usb01:~/sw/i2cperf$ sudo ./i2cperf -f 3 -o 2 -s 32 -r 0-0x7f 13 0x51 -S

  Fill block with increment via i2cset byte by byte
  -------------------------------------------------------------------
  data rate(bps)  efficiency(%)  data size(B)  total IOs   IO size(B)
  -------------------------------------------------------------------
  3475            21             128           128         1

[  +0.000005] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001553] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000199] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.011834] ft260_smbus_xfer: smbus size 8
[  +0.000003] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001061] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.012525] ft260_smbus_xfer: smbus size 8
[  +0.000004] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001391] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000181] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.010613] ft260_smbus_xfer: smbus size 8
[  +0.000003] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001215] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000189] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.010903] ft260_smbus_xfer: smbus size 8
[  +0.000004] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.000899] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000191] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.011259] ft260_smbus_xfer: smbus size 8
[  +0.000005] ft260_smbus_write: rep 0xd1 addr 0x51 cmd 0x0 datlen 3 replen 7
[  +0.001524] ft260_xfer_status: bus_status 0x40, clock 400
[  +0.000229] ft260_xfer_status: bus_status 0x20, clock 400
[  +0.011098] ft260_smbus_xfer: smbus size 8
MichaelZaidman commented 1 year ago

This commit breaks big reads and needs to be revised.

MichaelZaidman commented 1 year ago

@GermainHebert, please review the commit before I merge this PR. Thanks.

GermainHebert commented 1 year ago
MichaelZaidman commented 1 year ago

Fix NACK issue, didn't find any regression

@GermainHebert, thanks for testing!