On 19. 05. 24, 21:31, Doug Brown wrote:
If a TX interrupt occurs and no new data gets loaded into the TX FIFO,
the UART will never fire another TX interrupt until the UART_IER_THRI
flag is toggled off and back on. If nothing ever calls stop_tx(), this
effectively results in transmissions getting hung up until another
unrelated UART IRQ occurs, such as an RX interrupt.
The driver used to do this correctly until the transition to
uart_port_tx_limited(). This didn't matter until the behavior of
__uart_port_tx changed in commit 7bfb915a597a ("serial: core: only stop
transmit when HW fifo is empty").
Fixes: d11cc8c3c4b6 ("tty: serial: use uart_port_tx_limited()")
Signed-off-by: Doug Brown <doug@xxxxxxxxxxxxx>
---
Note: I based this on v6.9 instead of tty-next since it's a fix; please
let me know if that was the wrong move and I would be happy to resubmit
it based on tty-next. The patch changes ever so slightly because of the
circ_buf -> kfifo transition. The only difference is it needs this
condition in the "if" statement instead:
kfifo_is_empty(&up->port.state->port.xmit_fifo)
This has been tested to work properly on tty-next as well.
drivers/tty/serial/pxa.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index e395ff29c1a2..8abb85bee87c 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -176,6 +176,14 @@ static void transmit_chars(struct uart_pxa_port *up)
{
u8 ch;
+ /* If there is nothing left to transmit, disable the TX interrupt.
+ * Otherwise we can get stuck waiting for another IRQ that never happens.
+ */
+ if (uart_circ_empty(&up->port.state->xmit)) {
+ serial_pxa_stop_tx(&up->port);
+ return;
+ }
This does not make sense. If the circ buf is empty,
uart_port_tx_limited() should stop the TX already. You simply revert to
the state before 7bfb915a597a, but on a per-driver basis.
IOW all drivers using the helper would have exactly this issue after
7bfb915a597a.
What driver was 7bfb915a597a about after all? The commit log of the
commit is hopeless (it's very vague) in this respect:
commit 7bfb915a597a301abb892f620fe5c283a9fdbd77
Author: Jonas Gorski <jonas.gorski@xxxxxxxxx>
Date: Sun Mar 3 16:08:07 2024 +0100
serial: core: only stop transmit when HW fifo is empty
If the circular buffer is empty, it just means we fit all characters to
send into the HW fifo, but not that the hardware finished transmitting
them.
So if we immediately call stop_tx() after that, this may abort any
pending characters in the HW fifo, and cause dropped characters on the
console.
Fix this by only stopping tx when the tx HW fifo is actually empty.
Fixes: 8275b48b2780 ("tty: serial: introduce transmit helpers")
(And it barely fixes 8275b48b2780 per se. 8275b48b2780 only moved the
processing to a single place, most/all of the drivers already did it
that way.)
/me confused.
Perhaps, it should be reverted and the affected driver should just pass
UART_TX_NOSTOP instead?
uart_port_tx_limited(&up->port, ch, up->port.fifosize / 2,
true,
serial_out(up, UART_TX, ch),
thanks,
--
js
suse labs
