[PATCH 1/2] serial: 8250: 8250_omap.c: Add support for handling UART error conditions

Moteen Shah posted 2 patches 4 weeks ago
[PATCH 1/2] serial: 8250: 8250_omap.c: Add support for handling UART error conditions
Posted by Moteen Shah 4 weeks ago
The DMA IRQ handler does not accounts for the overrun(OE) or any other
errors being reported by the IP before triggering a DMA transaction which
leads to the interrupts not being handled resulting into an IRQ storm.

The way to handle OE is to:
1. Reset the RX FIFO.
2. Read the UART_RESUME register, which clears the internal flag

Earlier, the driver issued DMA transations even in case of OE which shouldn't
be done according to the OE handling mechanism mentioned above, as we are
resetting the FIFO's, refer section: "12.1.6.4.8.1.3.6 Overrun During
Receive" [0].

[0] https://www.ti.com/lit/pdf/spruiu1

Signed-off-by: Moteen Shah <m-shah@ti.com>
---
 drivers/tty/serial/8250/8250_omap.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 9e49ef48b851..e26bae0a6488 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -100,6 +100,9 @@
 #define OMAP_UART_REV_52 0x0502
 #define OMAP_UART_REV_63 0x0603
 
+/* Resume register */
+#define UART_OMAP_RESUME		0x0B
+
 /* Interrupt Enable Register 2 */
 #define UART_OMAP_IER2			0x1B
 #define UART_OMAP_IER2_RHR_IT_DIS	BIT(2)
@@ -119,7 +122,6 @@
 /* Timeout low and High */
 #define UART_OMAP_TO_L                 0x26
 #define UART_OMAP_TO_H                 0x27
-
 struct omap8250_priv {
 	void __iomem *membase;
 	int line;
@@ -1256,6 +1258,20 @@ static u16 omap_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status
 	return status;
 }
 
+static void am654_8250_handle_uart_errors(struct uart_8250_port *up, u8 iir, u16 status)
+{
+	if (status & UART_LSR_OE) {
+		serial8250_clear_and_reinit_fifos(up);
+		serial_in(up, UART_LSR);
+		serial_in(up, UART_OMAP_RESUME);
+	} else {
+		if (status & (UART_LSR_FE | UART_LSR_PE | UART_LSR_BI))
+			serial_in(up, UART_RX);
+		if (iir & UART_IIR_XOFF)
+			serial_in(up, UART_IIR);
+	}
+}
+
 static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 				     u16 status)
 {
@@ -1266,7 +1282,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 	 * Queue a new transfer if FIFO has data.
 	 */
 	if ((status & (UART_LSR_DR | UART_LSR_BI)) &&
-	    (up->ier & UART_IER_RDI)) {
+	    (up->ier & UART_IER_RDI) && !(status & UART_LSR_OE)) {
+		am654_8250_handle_uart_errors(up, iir, status);
 		omap_8250_rx_dma(up);
 		serial_out(up, UART_OMAP_EFR2, UART_OMAP_EFR2_TIMEOUT_BEHAVE);
 	} else if ((iir & 0x3f) == UART_IIR_RX_TIMEOUT) {
@@ -1282,6 +1299,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 		serial_out(up, UART_OMAP_EFR2, 0x0);
 		up->ier |= UART_IER_RLSI | UART_IER_RDI;
 		serial_out(up, UART_IER, up->ier);
+	} else {
+		am654_8250_handle_uart_errors(up, iir, status);
 	}
 }
 
-- 
2.34.1
Re: [PATCH 1/2] serial: 8250: 8250_omap.c: Add support for handling UART error conditions
Posted by Kumar, Udit 2 weeks, 6 days ago
please check subject of patch

On 1/12/2026 1:48 PM, Moteen Shah wrote:
> The DMA IRQ handler does not accounts for the overrun(OE) or any other
accounts to account
> errors being reported by the IP before triggering a DMA transaction which
transaction, which
> leads to the interrupts not being handled resulting into an IRQ storm.
>
> The way to handle OE is to:
> 1. Reset the RX FIFO.
> 2. Read the UART_RESUME register, which clears the internal flag
>
> Earlier, the driver issued DMA transations even in case of OE which shouldn't

transations to transactions

> be done according to the OE handling mechanism mentioned above, as we are
> resetting the FIFO's, refer section: "12.1.6.4.8.1.3.6 Overrun During

FIFO's to FIFO

> Receive" [0].
>
> [0] https://www.ti.com/lit/pdf/spruiu1
>
> Signed-off-by: Moteen Shah <m-shah@ti.com>
> ---
>   drivers/tty/serial/8250/8250_omap.c | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 9e49ef48b851..e26bae0a6488 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -100,6 +100,9 @@
>   #define OMAP_UART_REV_52 0x0502
>   #define OMAP_UART_REV_63 0x0603
>   
> +/* Resume register */
> +#define UART_OMAP_RESUME		0x0B
> +
>   /* Interrupt Enable Register 2 */
>   #define UART_OMAP_IER2			0x1B
>   #define UART_OMAP_IER2_RHR_IT_DIS	BIT(2)
> @@ -119,7 +122,6 @@
>   /* Timeout low and High */
>   #define UART_OMAP_TO_L                 0x26
>   #define UART_OMAP_TO_H                 0x27
> -
>   struct omap8250_priv {
>   	void __iomem *membase;
>   	int line;
> @@ -1256,6 +1258,20 @@ static u16 omap_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status
>   	return status;
>   }
>   
> +static void am654_8250_handle_uart_errors(struct uart_8250_port *up, u8 iir, u16 status)
> +{
> +	if (status & UART_LSR_OE) {
> +		serial8250_clear_and_reinit_fifos(up);
> +		serial_in(up, UART_LSR);
> +		serial_in(up, UART_OMAP_RESUME);
> +	} else {
> +		if (status & (UART_LSR_FE | UART_LSR_PE | UART_LSR_BI))
> +			serial_in(up, UART_RX);
> +		if (iir & UART_IIR_XOFF)
> +			serial_in(up, UART_IIR);
> +	}
> +}
> +
>   static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
>   				     u16 status)
>   {
> @@ -1266,7 +1282,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
>   	 * Queue a new transfer if FIFO has data.
>   	 */
>   	if ((status & (UART_LSR_DR | UART_LSR_BI)) &&
> -	    (up->ier & UART_IER_RDI)) {
> +	    (up->ier & UART_IER_RDI) && !(status & UART_LSR_OE)) {
> +		am654_8250_handle_uart_errors(up, iir, status);

may be you can think of splitting error handing into 2 functions ie 
handle BI or so and OE error.

when you call above function am654_8250_handle_uart_errors, first 
condition will be always false.


>   		omap_8250_rx_dma(up);
>   		serial_out(up, UART_OMAP_EFR2, UART_OMAP_EFR2_TIMEOUT_BEHAVE);
>   	} else if ((iir & 0x3f) == UART_IIR_RX_TIMEOUT) {
> @@ -1282,6 +1299,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
>   		serial_out(up, UART_OMAP_EFR2, 0x0);
>   		up->ier |= UART_IER_RLSI | UART_IER_RDI;
>   		serial_out(up, UART_IER, up->ier);
> +	} else {
> +		am654_8250_handle_uart_errors(up, iir, status);

I assume , you will hit this else only case of OE ? , if yes then do you 
think, call to error handling should be conditional ?


>   	}
>   }
>