UART on MPC83xx in irq loop

UART on MPC83xx in irq loop

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Hello,

I have a system based on MPC8313ERDB here that in some situations gets
stuck in an irq loop. I have a reproducer here that works reliably. A
workaround is:

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 45b8a59d937c..5ab32b6ba701 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2009,6 +2009,14 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 
 	status = serial_port_in(port, UART_LSR);
 
+	/*
+	 * Sometimes a "Character time-out" (IID3:0 == 0xc) happens on MPC8313,
+	 * but LSR doesn't report "Data ready". To clear the former the receive
+	 * buffer must be read. It's unclear if the char read is valid or not.
+	 */
+	if ((iir & UART_IIR_ID) == UART_IIR_RX_TIMEOUT)
+		status |= UART_LSR_DR;
+
 	/*
 	 * If port is stopped and there are no error conditions in the
 	 * FIFO, then don't drain the FIFO, as this may lead to TTY buffer

I havn't debugged that further than written in the comment but I wonder
if this is a known issue (didn't find it in the errata though) and/or if
someone with hardware knowledge could confirm this to be a hardware
fault.

Without feedback from NXP I'd look in more detail into that to for
example find out the timing and so maybe more hints about the hardware
and a better SW workaround/fix.

Any input is very welcome.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: UART on MPC83xx in irq loop

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Mon, 17 Oct 2022, Uwe Kleine-König wrote:

> Hello,
> 
> I have a system based on MPC8313ERDB here that in some situations gets
> stuck in an irq loop. I have a reproducer here that works reliably. A
> workaround is:
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 45b8a59d937c..5ab32b6ba701 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2009,6 +2009,14 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>  
>  	status = serial_port_in(port, UART_LSR);
>  
> +	/*
> +	 * Sometimes a "Character time-out" (IID3:0 == 0xc) happens on MPC8313,
> +	 * but LSR doesn't report "Data ready". To clear the former the receive
> +	 * buffer must be read. It's unclear if the char read is valid or not.
> +	 */
> +	if ((iir & UART_IIR_ID) == UART_IIR_RX_TIMEOUT)
> +		status |= UART_LSR_DR;
> +
>  	/*
>  	 * If port is stopped and there are no error conditions in the
>  	 * FIFO, then don't drain the FIFO, as this may lead to TTY buffer
> 
> I havn't debugged that further than written in the comment but I wonder
> if this is a known issue (didn't find it in the errata though) and/or if
> someone with hardware knowledge could confirm this to be a hardware
> fault.
> 
> Without feedback from NXP I'd look in more detail into that to for
> example find out the timing and so maybe more hints about the hardware
> and a better SW workaround/fix.
> 
> Any input is very welcome.

I find it bit odd you seem to assume w/o any justification that the data 
would be valid (that workaround will read one byte and consider it 
valid, no?). According to the comments & workarounds to the same problem
(just look for IIR_RX_TIMEOUT and you'll find a few), they all do dummy 
reads rather than assume the data is valid.

-- 
 i.

Re: UART on MPC83xx in irq loop

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Mon, Oct 17, 2022 at 03:27:19PM +0300, Ilpo Järvinen wrote:
> On Mon, 17 Oct 2022, Uwe Kleine-König wrote:
> 
> > Hello,
> > 
> > I have a system based on MPC8313ERDB here that in some situations gets
> > stuck in an irq loop. I have a reproducer here that works reliably. A
> > workaround is:
> > 
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 45b8a59d937c..5ab32b6ba701 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -2009,6 +2009,14 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> >  
> >  	status = serial_port_in(port, UART_LSR);
> >  
> > +	/*
> > +	 * Sometimes a "Character time-out" (IID3:0 == 0xc) happens on MPC8313,
> > +	 * but LSR doesn't report "Data ready". To clear the former the receive
> > +	 * buffer must be read. It's unclear if the char read is valid or not.
> > +	 */
> > +	if ((iir & UART_IIR_ID) == UART_IIR_RX_TIMEOUT)
> > +		status |= UART_LSR_DR;
> > +
> >  	/*
> >  	 * If port is stopped and there are no error conditions in the
> >  	 * FIFO, then don't drain the FIFO, as this may lead to TTY buffer
> > 
> > I havn't debugged that further than written in the comment but I wonder
> > if this is a known issue (didn't find it in the errata though) and/or if
> > someone with hardware knowledge could confirm this to be a hardware
> > fault.
> > 
> > Without feedback from NXP I'd look in more detail into that to for
> > example find out the timing and so maybe more hints about the hardware
> > and a better SW workaround/fix.
> > 
> > Any input is very welcome.
> 
> I find it bit odd you seem to assume w/o any justification that the data 
> would be valid (that workaround will read one byte and consider it 
> valid, no?). According to the comments & workarounds to the same problem
> (just look for IIR_RX_TIMEOUT and you'll find a few), they all do dummy 
> reads rather than assume the data is valid.

AFAICT the irq doesn't stop being pending without the read. But yes,
there are some details to research/explain, for example what is read
from the data register and if this byte is valid or not.

Also without status having the DR (or BI) bit set, UART_IIR_RX_TIMEOUT
doesn't kick in?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: UART on MPC83xx in irq loop

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

On Mon, 17 Oct 2022, Uwe Kleine-König wrote:

> On Mon, Oct 17, 2022 at 03:27:19PM +0300, Ilpo Järvinen wrote:
> > On Mon, 17 Oct 2022, Uwe Kleine-König wrote:
> > 
> > > Hello,
> > > 
> > > I have a system based on MPC8313ERDB here that in some situations gets
> > > stuck in an irq loop. I have a reproducer here that works reliably. A
> > > workaround is:
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > > index 45b8a59d937c..5ab32b6ba701 100644
> > > --- a/drivers/tty/serial/8250/8250_port.c
> > > +++ b/drivers/tty/serial/8250/8250_port.c
> > > @@ -2009,6 +2009,14 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> > >  
> > >  	status = serial_port_in(port, UART_LSR);
> > >  
> > > +	/*
> > > +	 * Sometimes a "Character time-out" (IID3:0 == 0xc) happens on MPC8313,
> > > +	 * but LSR doesn't report "Data ready". To clear the former the receive
> > > +	 * buffer must be read. It's unclear if the char read is valid or not.
> > > +	 */
> > > +	if ((iir & UART_IIR_ID) == UART_IIR_RX_TIMEOUT)
> > > +		status |= UART_LSR_DR;
> > > +
> > >  	/*
> > >  	 * If port is stopped and there are no error conditions in the
> > >  	 * FIFO, then don't drain the FIFO, as this may lead to TTY buffer
> > > 
> > > I havn't debugged that further than written in the comment but I wonder
> > > if this is a known issue (didn't find it in the errata though) and/or if
> > > someone with hardware knowledge could confirm this to be a hardware
> > > fault.
> > > 
> > > Without feedback from NXP I'd look in more detail into that to for
> > > example find out the timing and so maybe more hints about the hardware
> > > and a better SW workaround/fix.
> > > 
> > > Any input is very welcome.
> > 
> > I find it bit odd you seem to assume w/o any justification that the data 
> > would be valid (that workaround will read one byte and consider it 
> > valid, no?). According to the comments & workarounds to the same problem
> > (just look for IIR_RX_TIMEOUT and you'll find a few), they all do dummy 
> > reads rather than assume the data is valid.
> 
> AFAICT the irq doesn't stop being pending without the read.

At least the comment in 8250_dw says it has similar irq loop behavior if
nothing is read when IIR_RX_TIMEOUT is seen w/o DR. The others 
IIR_RX_TIMEOUT comments don't state it so it could be either way (but if 
I'd hazard a guess w/o history dig, they'll probably irq loop as well).

> But yes,
> there are some details to research/explain, for example what is read
> from the data register and if this byte is valid or not.
>
> Also without status having the DR (or BI) bit set, UART_IIR_RX_TIMEOUT
> doesn't kick in?!

I didn't understand what you were trying to ask/note here.

-- 
 i.