Re: [PATCH v4 RESEND] hid-ft260: Add serial driver

[Michael Zaidman <michael.zaidman@gmail.com>]

On Tue, Jan 02, 2024 at 11:42:37PM +0200, Michael Zaidman wrote:
> On Thu, Dec 28, 2023 at 12:50:08PM +0100, Christina Quast wrote:
> > Hi everyone!
> > 
> > On 12/18/23 10:31, Christina Quast wrote:
> > > Adds the serial driver for FT260 USB HID devices, providing direct and
> > > simplified access to UART functionality without the need for FT260 HID
> > > report format knowledge.
> > > 
> > > This chip implements an UART and I2C interface, but only the latter was
> > > previously supported with a kernel driver. For the UART interface, only
> > > FTDI example code using hidraw from userspace was available.
> > > 
> > > This commit adds a serial interface /dev/ttyFTx (FT as in FT260), which
> > > implements tty serial driver ops, facilitating baudrate configuration,
> > > data transmission and reception, termios settings.
> > > 
> > > Signed-off-by: Daniel Beer <daniel.beer@igorinstitute.com>
> > > Signed-off-by: Christina Quast <contact@christina-quast.de>
> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Reviewed-by: David Lamparter <equinox@diac24.net>
> > 
> > Is this already queued for 6.8?
> > 
> > Cheers and happy new year!
> > 
> > Christina
> > 
> 
> Thanks for this work!
> 
> I am adding Jiri Kosina, the maintainer of the HID subsystem.
> 
> The FTDI FT260 chip implements three functionalities: USB to I2C, UART,
> and GPIO bridges through two USB HID class interfaces.
> 
> I use the https://github.com/MichaelZaidman/hid-ft260 repository for FT260
> driver development. The I2C support has been upstreamed since 5.13. The code
> is mature enough and is I2C performance-tuned.
> 
> The initial GPIO support was developed and committed into the mainline
> https://patchwork.kernel.org/project/linux-input/cover/20230211115752.26276-1-michael.zaidman@gmail.com/.
> 
> Two versions of the initial UART support were around for some time, and
> now they are unified in this commit.
> 
> I am going to test it with I2C traffic to exclude a suspect of possible
> impact on the I2C performance and will provide feedback in a week or two.
> 
> --Michael
> 

Hi Christina and Daniel,

I tested the patch and fixed several issues. I did it in the uart branch
of the https://github.com/MichaelZaidman/hid-ft260/tree/uart repository.

4a8430c hid-ft260: uart: protect write_buf concurrent access
bec5f5b hid-ft260: uart: remove dead code
9a8e145 hid-ft260: fix i2c regression
fa8b63f hid-ft260: fix Wformat warning
1b1a2c6 hid-ft260: fix incompatible-pointer-types error
60bf153 hid-ft260: Add serial driver
16b6536 hid-ft260: driver code from mainline

Then, I briefly measured the performance impact on I2C traffic by simultaneously
writing into UART at 921600 baud rate and into EEPROM via I2C at 100KHz. The I2C
speed was limited by EEPROM, slightly degrading compared to the I2C-only case.
 
Also, please see my comments inline in the original e-mail with the patch.

--Michael