diff mbox series

[v2,4/7] serial: imx: set_termios(): preserve RTS state

Message ID 1561558293-7683-5-git-send-email-sorganov@gmail.com (mailing list archive)
State New, archived
Headers show
Series serial: imx: fix RTS and RTS/CTS handling | expand

Commit Message

Sergey Organov June 26, 2019, 2:11 p.m. UTC 
imx_set_termios() cleared RTS on every call, now fixed.

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Uwe Kleine-König June 27, 2019, 5:40 a.m. UTC | #1 
On Wed, Jun 26, 2019 at 05:11:30PM +0300, Sergey Organov wrote:
> imx_set_termios() cleared RTS on every call, now fixed.

Is this a real problem, or something you noticed by looking at the code?
I think I already asked that in a previous round, if so this should at
least be explained in more detail in the commit log. Also please note
that this is about the UCR2_CTS flag. (It is, isn't it? I don't
understand it after staring at the code for a while.)

Assuming this is a real fix, it would be great if this patch came first
in the series (i.e. before the cleanups) and would be more straight
forward to understand.

Best regards
Uwe
Sergey Organov June 27, 2019, 6:15 a.m. UTC | #2 
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> On Wed, Jun 26, 2019 at 05:11:30PM +0300, Sergey Organov wrote:
>> imx_set_termios() cleared RTS on every call, now fixed.
>
> Is this a real problem, or something you noticed by looking at the code?
> I think I already asked that in a previous round, if so this should at
> least be explained in more detail in the commit log.

Yes, it was real observed problem. Every call to set_termios from user
space (through tcsetattr() function) cleared RTS.

> Also please note that this is about the UCR2_CTS flag. (It is, isn't
> it? I don't understand it after staring at the code for a while.)

"CTS" in iMX terms means what everybody else calls "RTS"! Please notice
how they are used in the entire driver, e.g.:

		if (mctrl & TIOCM_RTS) {
			ucr2 |= UCR2_CTS;


> Assuming this is a real fix, it would be great if this patch came first
> in the series (i.e. before the cleanups) and would be more straight
> forward to understand.

I rather believe that pre-cleanups actually make the fix more
straightforward to understand.

Thanks!

-- Sergey
diff mbox series

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 17e2322..e0f5365 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1563,7 +1563,14 @@  imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	ucr2 = UCR2_SRST | UCR2_IRTS;
+	/*
+	 * Read current UCR2 and save it for future use, then clear all the bits
+	 * except those we will or may need to preserve.
+	 */
+	old_ucr2 = imx_uart_readl(sport, UCR2);
+	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);
+
+	ucr2 |= UCR2_SRST | UCR2_IRTS;
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 |= UCR2_WS;
 
@@ -1632,7 +1639,6 @@  imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	imx_uart_writel(sport,
 			old_ucr1 & ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN),
 			UCR1);
-	old_ucr2 = imx_uart_readl(sport, UCR2);
 	imx_uart_writel(sport, old_ucr2 & ~UCR2_ATEN, UCR2);
 
 	while (!(imx_uart_readl(sport, USR2) & USR2_TXDC))
@@ -1640,7 +1646,6 @@  imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* then, disable everything */
 	imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN | UCR2_ATEN), UCR2);
-	old_ucr2 &= (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
 
 	/* custom-baudrate handling */
 	div = sport->port.uartclk / (baud * 16);
@@ -1678,8 +1683,7 @@  imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	imx_uart_writel(sport, old_ucr1, UCR1);
 
-	/* set the parity, stop bits and data size */
-	imx_uart_writel(sport, ucr2 | old_ucr2, UCR2);
+	imx_uart_writel(sport, ucr2, UCR2);
 
 	if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
 		imx_uart_enable_ms(&sport->port);