summaryrefslogtreecommitdiff
path: root/drivers/hid/hid-ft260.c
AgeCommit message (Collapse)Author
2022-11-11HID: ft260: fix 'cast to restricted' kernel CI bot warningsMichael Zaidman
Fix 'cast to restricted' sparse warnings reported by kernel test robot in https://lore.kernel.org/all/202211021607.ssjymlKi-lkp@intel.com/ Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: missed NACK from busy deviceMichael Zaidman
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. Co-developed-by: Germain Hebert <germain.hebert@ca.abb.com> Signed-off-by: Germain Hebert <germain.hebert@ca.abb.com> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: fix a NULL pointer dereference in ft260_i2c_writeMichael Zaidman
The zero-length passed into the ft260_i2c_write() triggered the NULL pointer dereference in the debug message on data[0] access. Since the controller does not support a write of zero length, let's not allow it. Before: $ sudo i2ctransfer -y 13 w0@0x51 Killed After: $ sudo i2ctransfer -y 13 w0@0x51 Error: Sending messages failed: Invalid argument Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: wake up device from power saving modeMichael Zaidman
The FT260 can enter a power saving mode after being idle for longer than 5 seconds. When being woken up from power saving mode by an I2C write request, a possible NACK is not correctly reported by the controller. As a workaround, the driver will issue an I2C status report two times in ft260_xfer_status() after the chip has been idle for more than 5s. 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> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: missed NACK from big i2c readMichael Zaidman
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> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: remove SMBus Quick command supportMichael Zaidman
The i2cdetect uses the SMBus Quick command by default to scan devices on the I2C bus. The FT260 implements an I2C bus controller. The SMBus is derived from I2C, but there are several differences between the specifications of the two buses in the areas of timing, protocols, operation modes, and electrical characteristics. One of the differences is that the I2C devices allow the slave not to ACK its slave address, but SMBus requires it to always ACK it as a mechanism to detect a detachable device’s presence on the bus. Since FT260 is the I2C bus controller, it does not acknowledge the SMBus Quick write command, which sends a single bit to the device at the place of the RD/WR bit. The ft260 driver attempted to mimic the SMBus Quick Write functionality by writing a single byte as the SMBus Byte Write command does. Usually, one byte in the SMBus Quick Write will be fine. However, it may cause problems with devices with a control register at offset 0, like i2c muxes, for example, when scanned with the i2cdetect utility. The i2cdetect with the "-r" option uses the SMBus Read Byte command, which is a reasonable workaround. To prevent the I2C bus from locking at write-only devices (most notably clock chips at address 0x69), use the "-r" option in conjunction with scanning range parameters. This patch removes the SMBus Quick command support. $ sudo i2cdetect -y 13 Warning: Can't use SMBus Quick Write command, will skip some addresses 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 10: 20: 30: -- -- -- -- -- -- -- -- 40: 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: 70: $ sudo i2cdetect -y -r 13 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- Reported-by: Vince Asbridge <VAsbridge@sanblaze.com> Reported-by: Stephen Shirron <SShirron@sanblaze.com> Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: skip unexpected HID input reportsMichael Zaidman
The FT260 is not supposed to generate unexpected HID reports. However, in theory, the unsolicited HID Input reports can be issued by a specially crafted malicious USB device masquerading as FT260 when the attacker has physical access to the USB port. In this case, the read_buf pointer points to the final data portion of the previous I2C Read transfer, and the memcpy invoked in the ft260_raw_event() will try copying the content of the unexpected report into the wrong location. This commit sets the Read buffer pointer to NULL on the I2C Read transaction completion and checks it in the ft260_raw_event() to detect and skip the unsolicited Input report. Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: do not populate /dev/hidraw deviceMichael Zaidman
Do not populate the /dev/hidraw on ft260 interfaces when the hid-ft260 driver is loaded. $ sudo insmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 $ sudo rmmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 /dev/hidraw1 /dev/hidraw2 Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: improve i2c large reads performanceMichael Zaidman
The patch increases the read buffer size to 180 bytes. It reduces the number of ft260_i2c_read() calls by three, improving the big reads performance. $ sudo i2ctransfer -y -f 13 w2@0x51 0x0 0x0 r180 Before: [ +4.071878] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001097] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000175] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000004] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 60 flag 0x3 [ +0.008579] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000208] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 120 rlen 60 flag 0x0 [ +0.008794] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000181] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 rlen 60 flag 0x4 [ +0.008817] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000223] ft260_xfer_status: bus_status 0x20, clock 100 After: [ +11.611642] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.008001] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 180 flag 0x7 [ +0.008994] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007987] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007992] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000206] ft260_xfer_status: bus_status 0x20, clock 100 Suggested-by: Enrik Berkhan <Enrik.Berkhan@inka.de> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: support i2c reads greater than HID report sizeMichael Zaidman
A random i2c read operation in EEPROM devices is implemented as a dummy write operation, followed by a current address read operation. The dummy write operation is used to load the target byte or word address (a.k.a offset) into the offset counter, from which the subsequent read operation then reads. To support longer than one HID report size random read, the ft260 driver issues multiple pairs of i2c write offset + read data transactions of HID report size so that the EEPROM device sees many i2c random read requests from different offsets. Two issues with the current implementation: - This approach suffers from extra overhead caused by writing offset requests. - Necessity to handle offset per HID report in big-endian representation as EEPROM devices expect. The current implementation does not do it and correctly handles the reads up to 60 bytes only. This patch addresses both issues by implementing more efficient approach. It issues a single i2c read request of up to the EEPROM page size and then waits for the data to arrive in multiple HID reports. For example, to read the 256 bytes from a 24LC512 chip, which has 128 bytes page size, the old method performs six ft260_i2c_write_read transactions while the new - two only. Before: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 40803 85 256 2 128 Kernel log of a single 128 bytes read request: [ +2.376308] ft260_i2c_write_read: read_off 0x0 left_len 128 len 60 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.000707] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000173] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008660] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000156] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x3c left_len 68 len 60 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x3c [ +0.001034] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008614] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x78 left_len 8 len 8 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x78 [ +0.000987] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000192] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 [ +0.002614] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000200] ft260_xfer_status: bus_status 0x20, clock 100 After: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 43990 85 256 2 128 Kernel log of a single 128 bytes read request: [ +1.464346] ft260_i2c_write_read: off 0x0 rlen 128 wlen 2 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001653] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000188] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 128 rlen 60 flag 0x3 [ +0.008609] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000157] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 68 rlen 60 flag 0x0 [ +0.008840] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 rlen 8 flag 0x4 [ +0.002794] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000201] ft260_xfer_status: bus_status 0x20, clock 100 Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Tested-by: Guillaume Champagne <champagne.guillaume.c@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: support i2c writes larger than HID report sizeMichael Zaidman
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. 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_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) ------------------------------------------------------------------- 71260 86 256 2 128 Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Tested-by: Guillaume Champagne <champagne.guillaume.c@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: improve i2c write performanceMichael Zaidman
The patch improves the I2C write performance by 20 - 30 percent by revising the sleep time in the ft260_hid_output_report_check_status() in the following ways: 1. Reduce the wait time and start to poll earlier. Sending a large amount of data at a low I2C clock rate saturates the internal FT260 buffer and causes hiccups in status readiness, as shown below in the log fragment. Aligning the status check wait time to the worst case significantly reduces the write performance. [Oct22 10:28] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.005296] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013460] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003244] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000190] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.015324] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003491] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000202] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.016047] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.002768] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000150] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011389] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003467] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000191] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000172] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000131] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000241] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000190] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000196] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011314] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003334] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000227] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000204] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000198] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000147] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011060] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 32 -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) ------------------------------------------------------------------- 40510 80 256 8 32 After: $ sudo ./i2cperf -f 2 -o 2 -s 32 -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) ------------------------------------------------------------------- 52584 80 256 8 32 2. Do not sleep if the estimated I2C transfer time is below 2 ms since the first xfer status query frequently takes around 1.5 ms, and the following status queries take about 200us on average. So we usually return from the routine after the first 1 - 3 status checks. [Oct22 11:14] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.004270] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013889] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.000856] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000138] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013352] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001501] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000177] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.014477] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001377] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013197] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 16 -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) ------------------------------------------------------------------- 28826 73 256 16 16 After: $ sudo ./i2cperf -f 2 -o 2 -s 16 -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) ------------------------------------------------------------------- 45138 73 256 16 16 Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Tested-by: Guillaume Champagne <champagne.guillaume.c@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2022-11-11HID: ft260: ft260_xfer_status routine cleanupMichael Zaidman
After clarifying with FTDI's support, it turned out that the error condition (bit 1) in byte 1 of the i2c status HID report is a status bit reflecting all error conditions. When bits 2, 3, or 4 are raised to 1, bit 1 is set to 1 also. Since the ft260_xfer_status routine tests the error condition bit and exits in the case of an error, the program flow never reaches the conditional expressions for 2, 3, and 4 bits when any of them indicates an error state. Though these expressions are never evaluated to true, they are checked several times per IO, increasing the ft260_xfer_status polling cycle duration. The patch removes the conditional expressions for 2, 3, and 4 bits in byte 1 of the i2c status HID report. Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Tested-by: Guillaume Champagne <champagne.guillaume.c@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-12-02HID: check for valid USB device for many HID driversGreg Kroah-Hartman
Many HID drivers assume that the HID device assigned to them is a USB device as that was the only way HID devices used to be able to be created in Linux. However, with the additional ways that HID devices can be created for many different bus types, that is no longer true, so properly check that we have a USB device associated with the HID device before allowing a driver that makes this assumption to claim it. Cc: Jiri Kosina <jikos@kernel.org> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cc: Michael Zaidman <michael.zaidman@gmail.com> Cc: Stefan Achatz <erazor_de@users.sourceforge.net> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> Cc: linux-input@vger.kernel.org Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> [bentiss: amended for thrustmater.c hunk to apply] Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Link: https://lore.kernel.org/r/20211201183503.2373082-3-gregkh@linuxfoundation.org
2021-11-19HID: ft260: fix i2c probing for hwmon devicesMichael Zaidman
The below scenario causes the kernel NULL pointer dereference failure: 1. sudo insmod hid-ft260.ko 2. sudo modprobe lm75 3. unplug USB hid-ft260 4. plug USB hid-ft260 [ +0.000006] Call Trace: [ +0.000004] __i2c_smbus_xfer.part.0+0xd1/0x310 [ +0.000007] ? ft260_smbus_write+0x140/0x140 [hid_ft260] [ +0.000005] __i2c_smbus_xfer+0x2b/0x80 [ +0.000004] i2c_smbus_xfer+0x61/0xf0 [ +0.000005] i2c_default_probe+0xf9/0x130 [ +0.000004] i2c_detect_address+0x84/0x160 [ +0.000004] ? kmem_cache_alloc_trace+0xf6/0x200 [ +0.000009] ? i2c_detect.isra.0+0x69/0x130 [ +0.000005] i2c_detect.isra.0+0xbf/0x130 [ +0.000004] ? __process_new_driver+0x30/0x30 [ +0.000004] __process_new_adapter+0x18/0x20 [ +0.000004] bus_for_each_drv+0x84/0xd0 [ +0.000003] i2c_register_adapter+0x1e4/0x400 [ +0.000005] i2c_add_adapter+0x5c/0x80 [ +0.000004] ft260_probe.cold+0x222/0x2e2 [hid_ft260] [ +0.000006] hid_device_probe+0x10e/0x170 [hid] [ +0.000009] really_probe+0xff/0x460 [ +0.000004] driver_probe_device+0xe9/0x160 [ +0.000003] __device_attach_driver+0x71/0xd0 [ +0.000004] ? driver_allows_async_probing+0x50/0x50 [ +0.000004] bus_for_each_drv+0x84/0xd0 [ +0.000002] __device_attach+0xde/0x1e0 [ +0.000004] device_initial_probe+0x13/0x20 [ +0.000004] bus_probe_device+0x8f/0xa0 [ +0.000003] device_add+0x333/0x5f0 It happened when i2c core probed for the devices associated with the lm75 driver by invoking 2c_detect()-->..-->ft260_smbus_write() from within the ft260_probe before setting the adapter data with i2c_set_adapdata(). Moving the i2c_set_adapdata() before i2c_add_adapter() fixed the failure. Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Germain Hebert <germain.hebert@ca.abb.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-07-29HID: ft260: fix device removal due to USB disconnectMichael Zaidman
This commit fixes a functional regression introduced by the commit 82f09a637dd3 ("HID: ft260: improve error handling of ft260_hid_feature_report_get()") when upon USB disconnect, the FTDI FT260 i2c device is still available within the /dev folder. In my company's product, where the host USB to FT260 USB connection is hard-wired in the PCB, the issue is not reproducible. To reproduce it, I used the VirtualBox Ubuntu 20.04 VM and the UMFT260EV1A development module for the FTDI FT260 chip: Plug the UMFT260EV1A module into a USB port and attach it to VM. The VM shows 2 i2c devices under the /dev: michael@michael-VirtualBox:~$ ls /dev/i2c-* /dev/i2c-0 /dev/i2c-1 The i2c-0 is not related to the FTDI FT260: michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-0/name SMBus PIIX4 adapter at 4100 The i2c-1 is created by hid-ft260.ko: michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-1/name FT260 usb-i2c bridge on hidraw1 Now, detach the FTDI FT260 USB device from VM. We expect the /dev/i2c-1 to disappear, but it's still here: michael@michael-VirtualBox:~$ ls /dev/i2c-* /dev/i2c-0 /dev/i2c-1 And the kernel log shows: [ +0.001202] usb 2-2: USB disconnect, device number 3 [ +0.000109] ft260 0003:0403:6030.0002: failed to retrieve system status [ +0.000316] ft260 0003:0403:6030.0003: failed to retrieve system status It happens because the commit 82f09a637dd3 changed the ft260_get_system_config() return logic. This caused the ft260_is_interface_enabled() to exit with error upon the FT260 device USB disconnect, which in turn, aborted the ft260_remove() before deleting the FT260 i2c device and cleaning its sysfs stuff. This commit restores the FT260 USB removal functionality and improves the ft260_is_interface_enabled() code to handle correctly all chip modes defined by the device interface configuration pins DCNF0 and DCNF1. Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Acked-by: Aaron Jones (FTDI-UK) <aaron.jones@ftdichip.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-07-27HID: ft260: fix format type warning in ft260_word_show()Michael Zaidman
Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver") Fix warning reported by static analysis when built with W=1 for arm64 by clang version 13.0.0 >> drivers/hid/hid-ft260.c:794:44: warning: format specifies type 'short' but the argument has type 'int' [-Wformat] return scnprintf(buf, PAGE_SIZE, "%hi\n", le16_to_cpu(*field)); ~~~ ^~~~~~~~~~~~~~~~~~~ %i include/linux/byteorder/generic.h:91:21: note: expanded from macro 'le16_to_cpu' #define le16_to_cpu __le16_to_cpu ^ include/uapi/linux/byteorder/big_endian.h:36:26: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/uapi/linux/swab.h:105:2: note: expanded from macro '__swab16' (__builtin_constant_p((__u16)(x)) ? \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Any sprintf style use of %h or %hi for a sub-int sized value isn't useful since integer promotion is done on the value anyway. So, use %d instead. https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/ Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Suggested-by: Joe Perches <joe@perches.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-05-27HID: ft260: improve error handling of ft260_hid_feature_report_get()Michael Zaidman
The ft260_hid_feature_report_get() checks if the return size matches the requested size. But the function can also fail with at least -ENOMEM. Add the < 0 checks. In ft260_hid_feature_report_get(), do not do the memcpy to the caller's buffer if there is an error. Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver") Signed-off-by: Tom Rix <trix@redhat.com> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-05-05HID: ft260: check data size in ft260_smbus_write()Michael Zaidman
The SMbus block transaction limits the number of bytes transferred to 32, but nothing prevents a user from specifying via ioctl a larger data size than the ft260 can handle in a single transfer. i2cdev_ioctl_smbus() --> i2c_smbus_xfer --> __i2c_smbus_xfer --> ft260_smbus_xfer --> ft260_smbus_write This patch adds data size checking in the ft260_smbus_write(). Fixes: 98189a0adfa0 ("HID: ft260: add usb hid to i2c host bridge driver") Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-03-19HID: ft260: fix an error message in ft260_i2c_write_read()Dan Carpenter
The "len" variable is uninitialize. Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2021-03-16HID: ft260: add usb hid to i2c host bridge driverMichael Zaidman
The FTDI FT260 chip implements USB to I2C/UART bridges through two USB HID class interfaces. The first - for I2C, and the second for UART. Each interface is independent, and the kernel detects it as a separate USB hidraw device. This commit adds I2C host adapter support. Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com> Tested-by: Aaron Jones (FTDI-UK) <aaron.jones@ftdichip.com Signed-off-by: Jiri Kosina <jkosina@suse.cz>
generated by cgit (git 2.34.1) at 2024-06-28 00:33:58 +0000