[PATCH 4/6] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm

Ilpo Järvinen posted 6 patches 2 weeks ago
There is a newer version of this series
[PATCH 4/6] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
Posted by Ilpo Järvinen 2 weeks ago
INTC10EE UART can end up into an interrupt storm where it reports
IIR_NO_INT (0x1). If the storm happens during active UART operation, it
is promptly stopped by IIR value change due to Rx or Tx events.
However, when there is no activity, either due to idle serial line or
due to specific circumstances such as during shutdown that writes
IER=0, there is nothing to stop the storm.

During shutdown the storm is particularly problematic because
serial8250_do_shutdown() calls synchronize_irq() that will hang in
waiting for the storm to finish which never happens.

This problem can also result in triggering a warning:

  irq 45: nobody cared (try booting with the "irqpoll" option)
  [...snip...]
  handlers:
    serial8250_interrupt
  Disabling IRQ #45

Normal means to reset interrupt status by reading LSR, MSR, USR, or RX
register do not result in the UART deasserting the IRQ.

Add a quirk to INTC10EE UARTs to enable Tx interrupts if UART's Tx is
currently empty and inactive. Rework IIR_NO_INT to keep track of the
number of consecutive IIR_NO_INT, and on third one perform the quirk.
Enabling Tx interrupts should change IIR value from IIR_NO_INT to
IIR_THRI which has been observed to stop the storm.

Reported-by: "Bandal, Shankar" <shankar.bandal@intel.com>
Tested-by: "Bandal, Shankar" <shankar.bandal@intel.com>
Tested-by: "Murthy, Shanth" <shanth.murthy@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 60 ++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 7cee89f14179..a40c0851f39c 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -60,6 +60,7 @@
 #define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
 #define DW_UART_QUIRK_APMC0D08		BIT(4)
 #define DW_UART_QUIRK_CPR_VALUE		BIT(5)
+#define DW_UART_QUIRK_IER_KICK		BIT(6)
 
 struct dw8250_platform_data {
 	u8 usr_reg;
@@ -81,6 +82,8 @@ struct dw8250_data {
 
 	unsigned int		skip_autocfg:1;
 	unsigned int		uart_16550_compatible:1;
+
+	u64			no_int_count;
 };
 
 static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
@@ -308,6 +311,29 @@ static u32 dw8250_serial_in32be(struct uart_port *p, unsigned int offset)
        return dw8250_modify_msr(p, offset, value);
 }
 
+/*
+ * INTC10EE UART can IRQ storm while reporting IIR_NO_INT. Inducing IIR value
+ * change has been observed to break the storm.
+ *
+ * If Tx is empty (THRE asserted), we use here IER_THRI to cause IIR_NO_INT ->
+ * IIR_THRI transition.
+ */
+static void dw8250_quirk_ier_kick(struct uart_port *p)
+{
+	struct uart_8250_port *up = up_to_u8250p(p);
+	u32 lsr;
+
+	if (up->ier & UART_IER_THRI)
+		return;
+
+	lsr = serial_lsr_in(up);
+	if (!(lsr & UART_LSR_THRE))
+		return;
+
+	serial_out(up, UART_IER, up->ier | UART_IER_THRI);
+	serial_in(up, UART_LCR);	/* safe, no side-effects */
+	serial_out(up, UART_IER, up->ier);
+}
 
 static int dw8250_handle_irq(struct uart_port *p)
 {
@@ -318,18 +344,29 @@ static int dw8250_handle_irq(struct uart_port *p)
 	unsigned int quirks = d->pdata->quirks;
 	unsigned int status;
 
+	guard(uart_port_lock_irqsave)(p);
+
 	switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
 	case UART_IIR_NO_INT:
+		if (d->uart_16550_compatible || up->dma)
+			return 0;
+
+		d->no_int_count++;
+		if (d->no_int_count > 2 && quirks & DW_UART_QUIRK_IER_KICK)
+			dw8250_quirk_ier_kick(p);
+
 		return 0;
 
 	case UART_IIR_BUSY:
 		/* Clear the USR */
 		serial_port_in(p, d->pdata->usr_reg);
 
+		d->no_int_count = 0;
+
 		return 1;
 	}
 
-	guard(uart_port_lock_irqsave)(p);
+	d->no_int_count = 0;
 
 	/*
 	 * There are ways to get Designware-based UARTs into a state where
@@ -562,6 +599,14 @@ static void dw8250_reset_control_assert(void *data)
 	reset_control_assert(data);
 }
 
+static void dw8250_shutdown(struct uart_port *port)
+{
+	struct dw8250_data *d = to_dw8250_data(port->private_data);
+
+	serial8250_do_shutdown(port);
+	d->no_int_count = 0;
+}
+
 static int dw8250_probe(struct platform_device *pdev)
 {
 	struct uart_8250_port uart = {}, *up = &uart;
@@ -685,10 +730,12 @@ static int dw8250_probe(struct platform_device *pdev)
 		dw8250_quirks(p, data);
 
 	/* If the Busy Functionality is not implemented, don't handle it */
-	if (data->uart_16550_compatible)
+	if (data->uart_16550_compatible) {
 		p->handle_irq = NULL;
-	else if (data->pdata)
+	} else if (data->pdata) {
 		p->handle_irq = dw8250_handle_irq;
+		p->shutdown = dw8250_shutdown;
+	}
 
 	dw8250_setup_dma_filter(p, data);
 
@@ -815,6 +862,11 @@ static const struct dw8250_platform_data dw8250_skip_set_rate_data = {
 	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
 };
 
+static const struct dw8250_platform_data dw8250_intc10ee = {
+	.usr_reg = DW_UART_USR,
+	.quirks = DW_UART_QUIRK_IER_KICK,
+};
+
 static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
 	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
@@ -844,7 +896,7 @@ static const struct acpi_device_id dw8250_acpi_match[] = {
 	{ "INT33C5", (kernel_ulong_t)&dw8250_dw_apb },
 	{ "INT3434", (kernel_ulong_t)&dw8250_dw_apb },
 	{ "INT3435", (kernel_ulong_t)&dw8250_dw_apb },
-	{ "INTC10EE", (kernel_ulong_t)&dw8250_dw_apb },
+	{ "INTC10EE", (kernel_ulong_t)&dw8250_intc10ee },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
-- 
2.39.5

Re: [PATCH 4/6] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
Posted by Andy Shevchenko 2 weeks ago
On Fri, Jan 23, 2026 at 07:27:37PM +0200, Ilpo Järvinen wrote:
> INTC10EE UART can end up into an interrupt storm where it reports
> IIR_NO_INT (0x1). If the storm happens during active UART operation, it
> is promptly stopped by IIR value change due to Rx or Tx events.
> However, when there is no activity, either due to idle serial line or
> due to specific circumstances such as during shutdown that writes
> IER=0, there is nothing to stop the storm.
> 
> During shutdown the storm is particularly problematic because
> serial8250_do_shutdown() calls synchronize_irq() that will hang in
> waiting for the storm to finish which never happens.
> 
> This problem can also result in triggering a warning:
> 
>   irq 45: nobody cared (try booting with the "irqpoll" option)
>   [...snip...]
>   handlers:
>     serial8250_interrupt
>   Disabling IRQ #45
> 
> Normal means to reset interrupt status by reading LSR, MSR, USR, or RX
> register do not result in the UART deasserting the IRQ.
> 
> Add a quirk to INTC10EE UARTs to enable Tx interrupts if UART's Tx is
> currently empty and inactive. Rework IIR_NO_INT to keep track of the
> number of consecutive IIR_NO_INT, and on third one perform the quirk.
> Enabling Tx interrupts should change IIR value from IIR_NO_INT to
> IIR_THRI which has been observed to stop the storm.

...

> +	u64			no_int_count;

Why so big?

...

> +		d->no_int_count++;
> +		if (d->no_int_count > 2 && quirks & DW_UART_QUIRK_IER_KICK)
> +			dw8250_quirk_ier_kick(p);

Usual way is to use modulo. And perhaps use 4 for the sake of avoiding
division:

		if (d->no_int_count == 3 && quirks & DW_UART_QUIRK_IER_KICK)
			dw8250_quirk_ier_kick(p);

		d->no_int_count = (d->no_int_count + 1) % 4;

where 4 may be defined with meaningful name. With that u8 is more than enough.

>  		return 0;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 4/6] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
Posted by Ilpo Järvinen 1 week, 3 days ago
On Sat, 24 Jan 2026, Andy Shevchenko wrote:

> On Fri, Jan 23, 2026 at 07:27:37PM +0200, Ilpo Järvinen wrote:
> > INTC10EE UART can end up into an interrupt storm where it reports
> > IIR_NO_INT (0x1). If the storm happens during active UART operation, it
> > is promptly stopped by IIR value change due to Rx or Tx events.
> > However, when there is no activity, either due to idle serial line or
> > due to specific circumstances such as during shutdown that writes
> > IER=0, there is nothing to stop the storm.
> > 
> > During shutdown the storm is particularly problematic because
> > serial8250_do_shutdown() calls synchronize_irq() that will hang in
> > waiting for the storm to finish which never happens.
> > 
> > This problem can also result in triggering a warning:
> > 
> >   irq 45: nobody cared (try booting with the "irqpoll" option)
> >   [...snip...]
> >   handlers:
> >     serial8250_interrupt
> >   Disabling IRQ #45
> > 
> > Normal means to reset interrupt status by reading LSR, MSR, USR, or RX
> > register do not result in the UART deasserting the IRQ.
> > 
> > Add a quirk to INTC10EE UARTs to enable Tx interrupts if UART's Tx is
> > currently empty and inactive. Rework IIR_NO_INT to keep track of the
> > number of consecutive IIR_NO_INT, and on third one perform the quirk.
> > Enabling Tx interrupts should change IIR value from IIR_NO_INT to
> > IIR_THRI which has been observed to stop the storm.
> 
> ...
> 
> > +	u64			no_int_count;
> 
> Why so big?
> 
> ...

No particular reason, it's a leftover from debugging this issue.

> > +		d->no_int_count++;
> > +		if (d->no_int_count > 2 && quirks & DW_UART_QUIRK_IER_KICK)
> > +			dw8250_quirk_ier_kick(p);
> 
> Usual way is to use modulo. And perhaps use 4 for the sake of avoiding
> division:
> 
> 		if (d->no_int_count == 3 && quirks & DW_UART_QUIRK_IER_KICK)
> 			dw8250_quirk_ier_kick(p);
> 
> 		d->no_int_count = (d->no_int_count + 1) % 4;

This doesn't look equivalent code as it only fires on 4th NO_INT, but I 
guess the difference doesn't matter that much so changing to your 
suggestion so that the kick will only occurs on fourth NO_INT interrupt.

-- 
 i.

> where 4 may be defined with meaningful name. With that u8 is more than enough.
>
> >  		return 0;
> 
> 
Re: [PATCH 4/6] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
Posted by Andy Shevchenko 1 week, 3 days ago
On Tue, Jan 27, 2026 at 03:01:46PM +0200, Ilpo Järvinen wrote:
> On Sat, 24 Jan 2026, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 07:27:37PM +0200, Ilpo Järvinen wrote:

...

> > > +		d->no_int_count++;
> > > +		if (d->no_int_count > 2 && quirks & DW_UART_QUIRK_IER_KICK)
> > > +			dw8250_quirk_ier_kick(p);
> > 
> > Usual way is to use modulo. And perhaps use 4 for the sake of avoiding
> > division:
> > 
> > 		if (d->no_int_count == 3 && quirks & DW_UART_QUIRK_IER_KICK)
> > 			dw8250_quirk_ier_kick(p);
> > 
> > 		d->no_int_count = (d->no_int_count + 1) % 4;
> 
> This doesn't look equivalent code as it only fires on 4th NO_INT,

Correct, I forgot to clarify this in my original reply. Yes, bumping to
power-of-two for simplicity, but as you noticed it bumps also the loop to
"every 4th". (I was under impression that I wrote it somewhere else in
the reply, but now I see it's not the case.)

> but I guess the difference doesn't matter that much so changing to your
> suggestion so that the kick will only occurs on fourth NO_INT interrupt.

> > where 4 may be defined with meaningful name. With that u8 is more than enough.


-- 
With Best Regards,
Andy Shevchenko