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>
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>
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
This commit fixes a functional regression introduced by the commit 82f09a637d
("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 82f09a637d 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>
Fixes: 6a82582d9f ("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>
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: 6a82582d9f ("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>
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>
The "len" variable is uninitialize.
Fixes: 6a82582d9f ("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>
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>
