All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: core: fix tcdrain() with CTS enabled
@ 2022-02-28  5:49 Tomasz Moń
  0 siblings, 0 replies; only message in thread
From: Tomasz Moń @ 2022-02-28  5:49 UTC (permalink / raw)
  To: linux-serial
  Cc: Greg Kroah-Hartman, Jiri Slaby, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Krzysztof Drobiński, Tomasz Moń

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>
---
RFC did not receive any comments. The only reason for sending RFC first
was the fact that it touches the comment that dates back to pre-git era.

RFC: https://lore.kernel.org/linux-serial/20220203142337.1993024-1-tomasz.mon@camlingroup.com/
---
 drivers/tty/serial/serial_core.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0db90be4c3bc..0592e1267642 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1607,17 +1607,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;
 
@@ -1633,7 +1635,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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2022-02-28  5:49 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28  5:49 [PATCH] serial: core: fix tcdrain() with CTS enabled Tomasz Moń

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.