Linux Serial subsystem development
 help / color / mirror / Atom feed
From: "Ionut Nechita (Wind River)" <ionut.nechita@windriver.com>
To: ilpo.jarvinen@linux.intel.com, gregkh@linuxfoundation.org
Cc: andriy.shevchenko@linux.intel.com, linux-serial@vger.kernel.org,
	"Bandal, Shankar" <shankar.bandal@intel.com>,
	"Murthy, Shanth" <shanth.murthy@intel.com>,
	stable <stable@kernel.org>,
	Ionut Nechita <ionut.nechita@windriver.com>
Subject: [PATCH 6.12.y 6/8] serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
Date: Sun, 10 May 2026 16:40:09 +0300	[thread overview]
Message-ID: <20260510134011.618215-7-ionut.nechita@windriver.com> (raw)
In-Reply-To: <20260510134011.618215-1-ionut.nechita@windriver.com>

From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

commit 883c5a2bc934c165c4491d1ef7da0ac4e9765077 upstream.

dw8250_handle_irq() takes port's lock multiple times with no good
reason to release it in between and calls serial8250_handle_irq()
that also takes port's lock.

Take port's lock only once in dw8250_handle_irq() and use
serial8250_handle_irq_locked() to avoid releasing port's lock in
between.

As IIR_NO_INT check in serial8250_handle_irq() was outside of port's
lock, it has to be done already in dw8250_handle_irq().

DW UART can, in addition to IIR_NO_INT, report BUSY_DETECT (0x7) which
collided with the IIR_NO_INT (0x1) check in serial8250_handle_irq()
(because & is used instead of ==) meaning that no other work is done by
serial8250_handle_irq() during an BUSY_DETECT interrupt.

This allows reorganizing code in dw8250_handle_irq() to do both
IIR_NO_INT and BUSY_DETECT handling right at the start simplifying
the logic.

Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Cc: stable <stable@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-5-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y - replace guard(uart_port_lock_irqsave)(p)
 in dw8250_handle_irq() with explicit uart_port_lock_irqsave/
 unlock_irqrestore around the post-switch body; the cleanup-based
 uart_port_lock_irqsave guard class is not present in 6.12.y.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
 drivers/tty/serial/8250/8250_dw.c | 36 +++++++++++++++++++------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 05e45b63e5f5..1076b72c120b 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -9,6 +9,9 @@
  * LCR is written whilst busy.  If it is, then a busy detect interrupt is
  * raised, the LCR needs to be rewritten and the uart status register read.
  */
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -40,6 +43,8 @@
 #define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */
 
 /* DesignWare specific register fields */
+#define DW_UART_IIR_IID			GENMASK(3, 0)
+
 #define DW_UART_MCR_SIRE		BIT(6)
 
 /* Renesas specific register fields */
@@ -314,6 +319,19 @@ static int dw8250_handle_irq(struct uart_port *p)
 	unsigned int status;
 	unsigned long flags;
 
+	switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
+	case UART_IIR_NO_INT:
+		return 0;
+
+	case UART_IIR_BUSY:
+		/* Clear the USR */
+		serial_port_in(p, d->pdata->usr_reg);
+
+		return 1;
+	}
+
+	uart_port_lock_irqsave(p, &flags);
+
 	/*
 	 * There are ways to get Designware-based UARTs into a state where
 	 * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
@@ -325,20 +343,15 @@ static int dw8250_handle_irq(struct uart_port *p)
 	 * so we limit the workaround only to non-DMA mode.
 	 */
 	if (!up->dma && rx_timeout) {
-		uart_port_lock_irqsave(p, &flags);
 		status = serial_lsr_in(up);
 
 		if (!(status & (UART_LSR_DR | UART_LSR_BI)))
 			serial_port_in(p, UART_RX);
-
-		uart_port_unlock_irqrestore(p, flags);
 	}
 
 	/* Manually stop the Rx DMA transfer when acting as flow controller */
 	if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running && rx_timeout) {
-		uart_port_lock_irqsave(p, &flags);
 		status = serial_lsr_in(up);
-		uart_port_unlock_irqrestore(p, flags);
 
 		if (status & (UART_LSR_DR | UART_LSR_BI)) {
 			dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
@@ -346,17 +359,11 @@ static int dw8250_handle_irq(struct uart_port *p)
 		}
 	}
 
-	if (serial8250_handle_irq(p, iir))
-		return 1;
-
-	if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
-		/* Clear the USR */
-		serial_port_in(p, d->pdata->usr_reg);
+	serial8250_handle_irq_locked(p, iir);
 
-		return 1;
-	}
+	uart_port_unlock_irqrestore(p, flags);
 
-	return 0;
+	return 1;
 }
 
 static void dw8250_clk_work_cb(struct work_struct *work)
@@ -859,6 +866,7 @@ static struct platform_driver dw8250_platform_driver = {
 
 module_platform_driver(dw8250_platform_driver);
 
+MODULE_IMPORT_NS("SERIAL_8250");
 MODULE_AUTHOR("Jamie Iles");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Synopsys DesignWare 8250 serial port driver");
-- 
2.54.0


  parent reply	other threads:[~2026-05-10 13:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10 13:40 [PATCH 6.12.y 0/8] serial: 8250_dw: backport BUSY deassert series Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 1/8] serial: 8250: use serial_port_in/out() helpers Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 2/8] serial: 8250_dw: Comment possible corner cases in serial_out() implementation Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 3/8] serial: 8250: Protect LCR write in shutdown Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 4/8] serial: 8250_dw: Avoid unnecessary LCR writes Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 5/8] serial: 8250: Add serial8250_handle_irq_locked() Ionut Nechita (Wind River)
2026-05-10 13:40 ` Ionut Nechita (Wind River) [this message]
2026-05-10 13:40 ` [PATCH 6.12.y 7/8] serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm Ionut Nechita (Wind River)
2026-05-10 13:40 ` [PATCH 6.12.y 8/8] serial: 8250_dw: Ensure BUSY is deasserted Ionut Nechita (Wind River)
2026-05-11 10:26 ` [PATCH 6.12.y 0/8] serial: 8250_dw: backport BUSY deassert series Ilpo Järvinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260510134011.618215-7-ionut.nechita@windriver.com \
    --to=ionut.nechita@windriver.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=shankar.bandal@intel.com \
    --cc=shanth.murthy@intel.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox