| Message ID | 20220203142337.1993024-1-tomasz.mon@camlingroup.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] serial: core: fix tcdrain() with CTS enabled | expand |
On Thu, Feb 03, 2022 at 03:23:37PM +0100, Tomasz Moń wrote: > Do not set timeout to twice the approximate amount of time to send the > entire FIFO if CTS is enabled. If the caller requested no timeout, e.g. > when userspace program called tcdrain(), then wait without any timeout. > > Premature return from tcdrain() was observed on imx based system which > has 32 character long transmitter FIFO with hardware CTS handling. > > Simple userspace application that reproduces problem has to: > * Open tty device, enable hardware flow control (CRTSCTS) > * Write data, e.g. 26 bytes > * Call tcdrain() to wait for the transmitter > * Close tty device > > The other side of serial connection has to: > * Receive some data, e.g. 10 bytes > * Set RTS output (CTS input from sender perspective) inactive for > at least twice the port timeout > * Try to receive remaining data > > Without this patch, userspace application will finish without any error > while the other side of connection will never receive remaining data. > > Signed-off-by: Tomasz Moń <tomasz.mon@camlingroup.com> > --- > drivers/tty/serial/serial_core.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) Note, I do not apply "RFC" patches as obviously the submitter does not think they are good enough to be merged :) thanks, greg k-h
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index ba4baa756d51..4687c9d586df 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1605,17 +1605,19 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout) if (timeout && timeout < char_time) char_time = timeout; - /* - * If the transmitter hasn't cleared in twice the approximate - * amount of time to send the entire FIFO, it probably won't - * ever clear. This assumes the UART isn't doing flow - * control, which is currently the case. Hence, if it ever - * takes longer than port->timeout, this is probably due to a - * UART bug of some kind. So, we clamp the timeout parameter at - * 2*port->timeout. - */ - if (timeout == 0 || timeout > 2 * port->timeout) - timeout = 2 * port->timeout; + if (!uart_cts_enabled(port)) { + /* + * If the transmitter hasn't cleared in twice the approximate + * amount of time to send the entire FIFO, it probably won't + * ever clear. This assumes the UART isn't doing flow + * control, which is currently the case. Hence, if it ever + * takes longer than port->timeout, this is probably due to a + * UART bug of some kind. So, we clamp the timeout parameter at + * 2*port->timeout. + */ + if (timeout == 0 || timeout > 2 * port->timeout) + timeout = 2 * port->timeout; + } expire = jiffies + timeout; @@ -1631,7 +1633,7 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout) msleep_interruptible(jiffies_to_msecs(char_time)); if (signal_pending(current)) break; - if (time_after(jiffies, expire)) + if (timeout && time_after(jiffies, expire)) break; } uart_port_deref(port);
Do not set timeout to twice the approximate amount of time to send the entire FIFO if CTS is enabled. If the caller requested no timeout, e.g. when userspace program called tcdrain(), then wait without any timeout. Premature return from tcdrain() was observed on imx based system which has 32 character long transmitter FIFO with hardware CTS handling. Simple userspace application that reproduces problem has to: * Open tty device, enable hardware flow control (CRTSCTS) * Write data, e.g. 26 bytes * Call tcdrain() to wait for the transmitter * Close tty device The other side of serial connection has to: * Receive some data, e.g. 10 bytes * Set RTS output (CTS input from sender perspective) inactive for at least twice the port timeout * Try to receive remaining data Without this patch, userspace application will finish without any error while the other side of connection will never receive remaining data. Signed-off-by: Tomasz Moń <tomasz.mon@camlingroup.com> --- drivers/tty/serial/serial_core.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)