diff mbox series

[1/2] serial: imx: Introduce timeout when waiting on transmitter empty

Message ID 76cf9ce9cbf9dcdf78bc00ce7a919db1776ebce1.1712309058.git.esben@geanix.com
State Superseded
Headers show
Series [1/2] serial: imx: Introduce timeout when waiting on transmitter empty | expand

Commit Message

Esben Haabendal April 5, 2024, 9:25 a.m. UTC
By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
deadlock.

In case of the timeout, there is not much we can do, so we simply ignore
the transmitter state and optimistically try to continue.

Signed-off-by: Esben Haabendal <esben@geanix.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/tty/serial/imx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Esben Haabendal April 8, 2024, 8:56 a.m. UTC | #1
Fabio Estevam <festevam@gmail.com> writes:

> On Fri, Apr 5, 2024 at 6:25 AM Esben Haabendal <esben@geanix.com> wrote:
>>
>> By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
>
> s/potentital/potential

Thanks, fixing.

> Could you elaborate on this deadlock? Have you seen it in practice?

I cannot say for sure if I have seen it. But in some cases, that is
exactly what you would see. Nothing.

If it would occur during shutdown, the console would simply stop/block,
and you would see nothing.

> Should a Fixes tag be added?

Which commit should I add to that tag? The polling without timeout dates
back to at least 2.6.12-rc2.

/Esben
Esben Haabendal April 8, 2024, 8:57 a.m. UTC | #2
Marc Kleine-Budde <mkl@pengutronix.de> writes:

> On 05.04.2024 19:22:29, Esben Haabendal wrote:
>> Marc Kleine-Budde <mkl@pengutronix.de> writes:
>> 
>> > On 05.04.2024 11:25:13, Esben Haabendal wrote:
>> >> By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
>> >> deadlock.
>> >> 
>> >> In case of the timeout, there is not much we can do, so we simply ignore
>> >> the transmitter state and optimistically try to continue.
>> >> 
>> >> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> >> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> >
>> > Where's the cover letter and patch 2/2? Have a look at b4 [1], it's a
>> > great tool to help you with sending git patch series.
>> 
>> It is left out on purpose.
>> 
>> This patch is a stand-alone patch as it is. The other part of the series
>> you are talking about is not going to mainline for now. It needs still
>> quite some work, and will only go in after all the other printk stuff.
>> 
>> I hope we can merge this patch as it to mainline now, instead of piling
>> up more than necessary in the rt tree.
>
> Ok, then send it as patch 1/1.

Sure. Sorry about that.

/Esben
diff mbox series

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e14813250616..09c1678ddfd4 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -26,6 +26,7 @@ 
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/dma-mapping.h>
 
 #include <asm/irq.h>
@@ -2010,7 +2011,7 @@  imx_uart_console_write(struct console *co, const char *s, unsigned int count)
 	struct imx_port *sport = imx_uart_ports[co->index];
 	struct imx_port_ucrs old_ucr;
 	unsigned long flags;
-	unsigned int ucr1;
+	unsigned int ucr1, usr2;
 	int locked = 1;
 
 	if (sport->port.sysrq)
@@ -2041,8 +2042,8 @@  imx_uart_console_write(struct console *co, const char *s, unsigned int count)
 	 *	Finally, wait for transmitter to become empty
 	 *	and restore UCR1/2/3
 	 */
-	while (!(imx_uart_readl(sport, USR2) & USR2_TXDC));
-
+	read_poll_timeout_atomic(imx_uart_readl, usr2, usr2 & USR2_TXDC,
+				 0, USEC_PER_SEC, false, sport, USR2);
 	imx_uart_ucrs_restore(sport, &old_ucr);
 
 	if (locked)