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

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

On Wed, Jan 17, 2024 at 10:43:28PM +0200, Michael Zaidman wrote:
> On Wed, Jan 17, 2024 at 10:43:31AM +1300, Daniel Beer wrote:
> > On Tue, Jan 16, 2024 at 11:34:32PM +0200, Michael Zaidman wrote:
> > > > +/* The FT260 has a "power saving mode" that causes the device to switch
> > > > + * to a 30 kHz oscillator if there's no activity for 5 seconds.
> > > > + * Unfortunately this mode can only be disabled by reprogramming
> > > > + * internal fuses, which requires an additional programming voltage.
> > > > + *
> > > > + * One effect of this mode is to cause data loss on a fast UART that
> > > > + * transmits after being idle for longer than 5 seconds. We work around
> > > > + * this by sending a dummy report at least once per 4 seconds if the
> > > > + * UART is in use.
> > > > + */
> > > 
> > > For I2C, we addressed a similar issue in
> > > https://lore.kernel.org/all/20221105211151.7094-8-michael.zaidman@gmail.com/
> 
> Link to the correct patch
> https://lore.kernel.org/all/20221105211151.7094-11-michael.zaidman@gmail.com/
> 
> > > commit. But we did it per IO synchronously when the distance between this and
> > > the previous IO exceeded 5 seconds. In this way, the chip can still sleep
> > > between the IOs. On the contrary, the suggested workaround prevents the chip
> > > from entering the power saving mode during active TTY sessions regardless of
> > > the traffic intensity on the UART bus.
> > > 
> > > I cannot reproduce the issue with 1K Tx bursts at 921600 baud rate sent every
> > > 10 seconds with the disabled chip wakeup workaround.
> > > 
> > > Can you guide me on how to reproduce the data loss you observed?
> > 
> > Hi Michael,
> > 
> > This was my comment originally. It's been a long time (at least a year),
> > but from memory I had an FT260 attached to a UART console on an MCU dev
> > kit, which would print messages at 115200.
> > 
> > If the MCU sat idle for more than 5 seconds and then printed a message,
> > the first few characters of the line would be missing in picocom. If the
> > MCU kept busy, printing more frequently than once every 5 seconds, the
> > problem did not occur.
> > 
> > Cheers,
> > Daniel
> > 
> 
> Hi Daniel,
> 
> Thanks for the clarification. It was not clear from the issue description
> in the commit whether it happens on the ft260 Tx or Rx line, and I assumed
> it is Tx. Also, the periodic dummy report workaround is not active in the
> submitted patch. I reproduced the issue on the Rx line. And confirm the
> workaround works as expected when enabled.
> 
> May I suggest modifying the description to clarify that the data loss
> happens on the Rx line and state that the current dummy report period is
> 4.8 seconds?
> 
> Also, please enable the reschedule_work flag in the ft260_uart_probe
> routine to activate the periodic dummy reports.
> 
> --Michael
>

Hi Daniel and Christina,

My concern with the implemented workaround is that sending a dummy report
every 4.8 seconds eliminates the chip's power save mode functionality.

I did more testing and figured out that the baud rates 4800 and below work
fine with power saving mode and do not require this workaround.

So, I modified the code, making it dependent on the baud rate in this commit:
https://github.com/MichaelZaidman/hid-ft260/commit/b5a2ad68c7cebbaaba0aa1675ae376f2895e19aa


Another improvement is not to activate the wakeup workaround if power
saving mode is not enabled in EEPROM.
https://github.com/MichaelZaidman/hid-ft260/commit/0a41f3f3a4c664edc3bb90718807f2e62fe6d375

For UART testing, I sent files via picocom opened on ttyUSB0 (ch340 USB
dongle device) connected to ft260 UART, receiving and displaying the
trafic in screen manager utility.

--Michael