Re: [PATCH] USB: serial: ftdi_sio: Fix serial port stall after resume

[Helge Deller <deller@gmx.de>]

On 10/8/20 5:21 PM, Johan Hovold wrote:
> On Tue, Sep 29, 2020 at 09:33:27PM +0200, Helge Deller wrote:
>> With a 4-port serial USB HUB with FT232BM chips the serial ports stop
>> working after a software suspend/resume cycle.
>> Rewriting the latency timer during the resume phase fixes it.
>
> Hmm. This sounds weird. Why would the latency timer make such a
> difference?

Actually, I don't know.
My assumption is, that by setting the latency timer, the USB
transmissions somehow starts again. Maybe the integrated HUB is involved as well?
I'm no expert on this driver, the only thing I did is to compare
what is being configured when the device is detected and initialized
and when what happens when it's waked up.
Setting the latency timer seemed to me to be the only difference,
and it actually worked then.

But I agree, it seems weird. If you have other ideas, I'm happy
to test.

>> Cc: stable@vger.kernel.org
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index 9823bb424abd..8ee6cf6215c1 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -1092,6 +1092,7 @@ static u32 ftdi_232bm_baud_base_to_divisor(int baud, int base);
>>  static u32 ftdi_232bm_baud_to_divisor(int baud);
>>  static u32 ftdi_2232h_baud_base_to_divisor(int baud, int base);
>>  static u32 ftdi_2232h_baud_to_divisor(int baud);
>> +static int ftdi_reset_resume(struct usb_serial *serial);
>>
>>  static struct usb_serial_driver ftdi_sio_device = {
>>  	.driver = {
>> @@ -1122,6 +1123,7 @@ static struct usb_serial_driver ftdi_sio_device = {
>>  	.set_termios =		ftdi_set_termios,
>>  	.break_ctl =		ftdi_break_ctl,
>>  	.tx_empty =		ftdi_tx_empty,
>> +	.reset_resume =		ftdi_reset_resume,
>>  };
>>
>>  static struct usb_serial_driver * const serial_drivers[] = {
>> @@ -2379,6 +2381,16 @@ static int ftdi_stmclite_probe(struct usb_serial *serial)
>>  	return 0;
>>  }
>>
>> +static int ftdi_reset_resume(struct usb_serial *serial)
>> +{
>> +	struct usb_serial_port *port = serial->port[0];
>> +
>> +	if (tty_port_initialized(&port->port))
>> +		write_latency_timer(port);
>
> Why are you only doing this for open ports?

I more or less copied it from another driver....

>> +
>> +	return usb_serial_generic_resume(serial);
>> +}
>
> And if the device has been reset there may need to reconfigured the
> termios settings for open ports.
>
> Could you expand a bit on what the problem is here?

My testcase is pretty simple:
1. I use e.g. "minicom -D /dev/ttyUSB2". Serial connection works.
2. I exit minicom.
3. I suspend the workstation: "systemctl suspend"
4. I wake up the machine and wait a few seconds.
5. I start "minicom -D /dev/ttyUSB2" again. No transfers on the serial port.

With my patch the minicom serial communications does work.
Another way to wake up the connection is to rmmod the driver and insmod it again.

Helge