On Wed, 24 May 2023, Uwe Kleine-König wrote:
> At least on MPC83xx (but I suspect also on other implementations) it can
> happen that after irqs are disabled but before the FIFOs are cleared a
> character is received. Resetting the FIFO throws away that character,
> but doesn't clear the IIR.RDI event that signals that there is read data
> available.
>
> Read from RX to clear IIR.RDI and throw away the result.
>
> This fixes a infinite loop after the above described race happened: The
> irq handler (here: fsl8250_handle_irq()) triggers, but LSR.DR isn't set,
> so RX isn't read and the irq handler returns just to be called again.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_port.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index fe8d79c4ae95..8b47ec000922 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -556,6 +556,18 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
> serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
> UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
> serial_out(p, UART_FCR, 0);
> +
> + /*
> + * When doing rs485 on MPC8313 it can happen that in the short
> + * window when switching from RX to TX there is a character in
> + * the RX FIFO which isn't processed any more and then discarded
> + * in here by clearing the FIFO. In that case IIR signals an RX
> + * timeout once irqs are reenabled (i.e. in
> + * serial8250_em485_stop_tx()) but LSR shows no pending event.
> + * The RX timeout event can only be cleared by reading RX. Do
> + * this here before reenabling the FIFO and irqs.
> + */
> + serial_port_in(&p->port, UART_RX);
> }
> }
This solution has too wide impact, I think. It should be made driver
specific. Can't you read IIR to see if the event indication is there
before doing this UART_RX read? Maybe add an UART specific function for
fifo clearing/reset.
I've long wondered this related thing:
Does anyone have idea why serial8250_clear_and_reinit_fifos() and
serial8250_clear_fifos() are separate, what is the benefit of not setting
FCR back to up->fcr? That is that intermediate FCR <= 0 really required
for the FIFO reset sequence or is it just an artifact of how the code is
structured into those two functions.
It might make sense to drop serial8250_clear_and_reinit_fifos() and
change serial8250_clear_fifos() into something like this (depending on
the answers):
static void serial8250_clear_fifos(struct uart_8250_port *p, bool disable)
{
if (p->capabilities & UART_CAP_FIFO) {
serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO);
serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
serial_out(p, UART_FCR, disable ? 0 : p->fcr);
}
}
--
i.
