linux-kernel - [PATCH] serial: imx: also enable Transmit Complete interrupt in rs232 mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231120132256.136625-1-rasmus.villemoes@prevas.dk>
Date:   Mon, 20 Nov 2023 14:22:56 +0100
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>
Cc:     Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: [PATCH] serial: imx: also enable Transmit Complete interrupt in rs232 mode

Currently, if one switches to rs232 mode, writes something to the
device, and then switches to rs485 mode, the imx_port's ->tx_state is
left as SEND. This then prevents a subsequent write in rs485 mode from
properly asserting the rts pin (i.e. enabling the transceiver),
because imx_uart_start_rx() does not enter the "if (sport->tx_state ==
OFF)" branch. Hence nothing is actually transmitted.

The problem is that in rs232 mode, ->tx_state never gets set to OFF,
due to

	usr2 = imx_uart_readl(sport, USR2);
	if (!(usr2 & USR2_TXDC)) {
		/* The shifter is still busy, so retry once TC triggers */
		return;
	}

in imx_uart_stop_tx(), and TC never triggers because the Transmit
Complete interrupt is not enabled for rs232.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@...vas.dk>
---
I'm not sure this is the best fix.

At first I considered doing something much more targeted, but
definitely also more hacky: In imx_uart_rs485_config(), if switching
on rs485 mode, simply add "sport->tx_state = OFF;".

If someone has a better suggestion, I'm all ears.

 drivers/tty/serial/imx.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 7341d060f85c..ffee157e13cd 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -708,25 +708,19 @@ static void imx_uart_start_tx(struct uart_port *port)
 		    || sport->tx_state == WAIT_AFTER_RTS) {
 
 			hrtimer_try_to_cancel(&sport->trigger_stop_tx);
-
-			/*
-			 * Enable transmitter and shifter empty irq only if DMA
-			 * is off.  In the DMA case this is done in the
-			 * tx-callback.
-			 */
-			if (!sport->dma_is_enabled) {
-				u32 ucr4 = imx_uart_readl(sport, UCR4);
-				ucr4 |= UCR4_TCEN;
-				imx_uart_writel(sport, ucr4, UCR4);
-			}
-
-			sport->tx_state = SEND;
 		}
-	} else {
-		sport->tx_state = SEND;
 	}
 
+	sport->tx_state = SEND;
+
+	/*
+	 * Enable transmitter and shifter empty irq only if DMA is
+	 * off.  In the DMA case this is done in the tx-callback.
+	 */
 	if (!sport->dma_is_enabled) {
+		u32 ucr4 = imx_uart_readl(sport, UCR4);
+		ucr4 |= UCR4_TCEN;
+		imx_uart_writel(sport, ucr4, UCR4);
 		ucr1 = imx_uart_readl(sport, UCR1);
 		imx_uart_writel(sport, ucr1 | UCR1_TRDYEN, UCR1);
 	}
-- 
2.40.1.1.g1c60b9335d