* [PATCH v2 6.12.y 08/10] serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Bandal, Shankar, Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260513065028.112468-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>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 37 ++++++++++++++++++-------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 05e45b63e5f5..1460eaf2a3a3 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 */
@@ -312,7 +317,19 @@ static int dw8250_handle_irq(struct uart_port *p)
bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
unsigned int quirks = d->pdata->quirks;
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;
+ }
+
+ guard(uart_port_lock_irqsave)(p);
/*
* There are ways to get Designware-based UARTs into a state where
@@ -325,20 +342,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 +358,9 @@ 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;
- }
-
- return 0;
+ return 1;
}
static void dw8250_clk_work_cb(struct work_struct *work)
@@ -859,6 +863,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
^ permalink raw reply related
* [PATCH v2 6.12.y 06/10] serial: 8250_dw: Avoid unnecessary LCR writes
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Bandal, Shankar, Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 8002d6d6d0d8a36a7d6ca523b17a51cb0fa7c3c3 upstream.
When DW UART is configured with BUSY flag, LCR writes may not always
succeed which can make any LCR write complex and very expensive.
Performing write directly can trigger IRQ and the driver has to perform
complex and distruptive sequence while retrying the write.
Therefore, it's better to avoid doing LCR write that would not change
the value of the LCR register. Add LCR write avoidance code into the
8250_dw driver's .serial_out() functions.
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
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-3-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 88c55336d50f..05e45b63e5f5 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -181,6 +181,22 @@ static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
*/
}
+/*
+ * With BUSY, LCR writes can be very expensive (IRQ + complex retry logic).
+ * If the write does not change the value of the LCR register, skip it entirely.
+ */
+static bool dw8250_can_skip_reg_write(struct uart_port *p, unsigned int offset, u32 value)
+{
+ struct dw8250_data *d = to_dw8250_data(p->private_data);
+ u32 lcr;
+
+ if (offset != UART_LCR || d->uart_16550_compatible)
+ return false;
+
+ lcr = serial_port_in(p, offset);
+ return lcr == value;
+}
+
/* Returns once the transmitter is empty or we run out of retries */
static void dw8250_tx_wait_empty(struct uart_port *p)
{
@@ -207,12 +223,18 @@ static void dw8250_tx_wait_empty(struct uart_port *p)
static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
writeb(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
static void dw8250_serial_out38x(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
/* Allow the TX to drain before we reconfigure */
if (offset == UART_LCR)
dw8250_tx_wait_empty(p);
@@ -237,6 +259,9 @@ static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
value &= 0xff;
__raw_writeq(value, p->membase + (offset << p->regshift));
/* Read back to ensure register write ordering. */
@@ -248,6 +273,9 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
writel(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
@@ -261,6 +289,9 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
{
+ if (dw8250_can_skip_reg_write(p, offset, value))
+ return;
+
iowrite32be(value, p->membase + (offset << p->regshift));
dw8250_check_lcr(p, offset, value);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 07/10] serial: 8250: Add serial8250_handle_irq_locked()
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Bandal, Shankar, Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 8324a54f604da18f21070702a8ad82ab2062787b upstream.
8250_port exports serial8250_handle_irq() to HW specific 8250 drivers.
It takes port's lock within but a HW specific 8250 driver may want to
take port's lock itself, do something, and then call the generic
handler in 8250_port but to do that, the caller has to release port's
lock for no good reason.
Introduce serial8250_handle_irq_locked() which a HW specific driver can
call while already holding port's lock.
As this is new export, put it straight into a namespace (where all 8250
exports should eventually be moved).
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-4-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: adapt to 6.12.y - use bare-identifier form in
EXPORT_SYMBOL_NS_GPL(serial8250_handle_irq_locked, SERIAL_8250)
because 6.12.y EXPORT_SYMBOL_NS_GPL() applies __stringify(ns) to
the namespace argument; the upstream string-literal form fails the
.vmlinux.export.c link step with "expected ':' or ')' before
'SERIAL_8250'".]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_port.c | 24 ++++++++++++++++--------
include/linux/serial_8250.h | 1 +
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4252138ccc3d..d05254fb115a 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -18,6 +18,7 @@
#include <linux/irq.h>
#include <linux/console.h>
#include <linux/gpio/consumer.h>
+#include <linux/lockdep.h>
#include <linux/sysrq.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
@@ -1884,20 +1885,16 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
}
/*
- * This handles the interrupt from one port.
+ * Context: port's lock must be held by the caller.
*/
-int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
+void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
{
struct uart_8250_port *up = up_to_u8250p(port);
struct tty_port *tport = &port->state->port;
bool skip_rx = false;
- unsigned long flags;
u16 status;
- if (iir & UART_IIR_NO_INT)
- return 0;
-
- uart_port_lock_irqsave(port, &flags);
+ lockdep_assert_held_once(&port->lock);
status = serial_lsr_in(up);
@@ -1930,8 +1927,19 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
else if (!up->dma->tx_running)
__stop_tx(up);
}
+}
+EXPORT_SYMBOL_NS_GPL(serial8250_handle_irq_locked, SERIAL_8250);
+
+/*
+ * This handles the interrupt from one port.
+ */
+int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
+{
+ if (iir & UART_IIR_NO_INT)
+ return 0;
- uart_unlock_and_check_sysrq_irqrestore(port, flags);
+ guard(uart_port_lock_irqsave)(port);
+ serial8250_handle_irq_locked(port, iir);
return 1;
}
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index e0717c8393d7..7e0439ecd496 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -195,6 +195,7 @@ void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl);
void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot);
int fsl8250_handle_irq(struct uart_port *port);
+void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir);
int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
u16 serial8250_rx_chars(struct uart_8250_port *up, u16 lsr);
void serial8250_read_char(struct uart_8250_port *up, u16 lsr);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 04/10] serial: 8250: convert serial8250_do_shutdown() to scoped_guard()
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Jiri Slaby (SUSE), Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
This is a partial backport of upstream commit b339809edda1 ("serial:
8250: use guard()s"). Only the serial8250_do_shutdown() hunks of
drivers/tty/serial/8250/8250_port.c are taken; the rest of the
upstream commit is not needed for the BUSY deassert series.
This conversion is a textual prerequisite for applying
commit 59a33d83bbe6 ("serial: 8250: Protect LCR write in shutdown")
without conflicts, because that commit moves the LCR write into a
scoped_guard(uart_port_lock_irqsave) block that does not yet exist
in 6.12.y.
This depends on commit 0fd60b689b0d ("serial: introduce
uart_port_lock() guard()s") (also backported in this series) which
introduces DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, ...).
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250814072456.182853-10-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Ionut: partial backport - only the serial8250_do_shutdown() hunks
from b339809edda1, per Ilpo's guidance to take "8250_port.c shutdown
hunks" from that commit; the remaining hunks are not required for
the BUSY deassert series.]
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index b4c8388ea6fc..21fa7907c0d6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2461,7 +2461,6 @@ static int serial8250_startup(struct uart_port *port)
void serial8250_do_shutdown(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
serial8250_rpm_get(up);
/*
@@ -2469,26 +2468,26 @@ void serial8250_do_shutdown(struct uart_port *port)
*
* Synchronize UART_IER access against the console.
*/
- uart_port_lock_irqsave(port, &flags);
- up->ier = 0;
- serial_port_out(port, UART_IER, 0);
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port) {
+ up->ier = 0;
+ serial_port_out(port, UART_IER, 0);
+ }
synchronize_irq(port->irq);
if (up->dma)
serial8250_release_dma(up);
- uart_port_lock_irqsave(port, &flags);
- if (port->flags & UPF_FOURPORT) {
- /* reset interrupts on the AST Fourport board */
- inb((port->iobase & 0xfe0) | 0x1f);
- port->mctrl |= TIOCM_OUT1;
- } else
- port->mctrl &= ~TIOCM_OUT2;
+ scoped_guard(uart_port_lock_irqsave, port) {
+ if (port->flags & UPF_FOURPORT) {
+ /* reset interrupts on the AST Fourport board */
+ inb((port->iobase & 0xfe0) | 0x1f);
+ port->mctrl |= TIOCM_OUT1;
+ } else
+ port->mctrl &= ~TIOCM_OUT2;
- serial8250_set_mctrl(port, port->mctrl);
- uart_port_unlock_irqrestore(port, flags);
+ serial8250_set_mctrl(port, port->mctrl);
+ }
/*
* Disable break condition and FIFOs
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 05/10] serial: 8250: Protect LCR write in shutdown
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Bandal, Shankar, Murthy, Shanth, Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
commit 59a33d83bbe6d73d2071d7ae21590b29faed0503 upstream.
The 8250_dw driver needs to potentially perform very complex operations
during LCR writes because its BUSY handling prevents updates to LCR
while UART is BUSY (which is not fully under our control without those
complex operations). Thus, LCR writes should occur under port's lock.
Move LCR write under port's lock in serial8250_do_shutdown(). Also
split the LCR RMW so that the logic is on a separate line for clarity.
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
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-2-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_port.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 21fa7907c0d6..4252138ccc3d 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2461,6 +2461,7 @@ static int serial8250_startup(struct uart_port *port)
void serial8250_do_shutdown(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
+ u32 lcr;
serial8250_rpm_get(up);
/*
@@ -2487,13 +2488,13 @@ void serial8250_do_shutdown(struct uart_port *port)
port->mctrl &= ~TIOCM_OUT2;
serial8250_set_mctrl(port, port->mctrl);
+
+ /* Disable break condition */
+ lcr = serial_port_in(port, UART_LCR);
+ lcr &= ~UART_LCR_SBC;
+ serial_port_out(port, UART_LCR, lcr);
}
- /*
- * Disable break condition and FIFOs
- */
- serial_port_out(port, UART_LCR,
- serial_port_in(port, UART_LCR) & ~UART_LCR_SBC);
serial8250_clear_fifos(up);
#ifdef CONFIG_SERIAL_8250_RSA
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 00/10] serial: 8250_dw: backport BUSY deassert series
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Ionut Nechita
From: Ionut Nechita <ionut.nechita@windriver.com>
Hi Greg, Ilpo,
This is v2 of the 8250_dw BUSY deassert backport to 6.12.y,
addressing Ilpo's review feedback on v1.
v1 thread:
https://lore.kernel.org/linux-serial/20260510134011.618215-1-ionut.nechita@windriver.com/
v1 review (Ilpo, 2026-05-11):
https://lore.kernel.org/linux-serial/d4b4e4db-505f-4400-c1b9-fe589786456e@linux.intel.com/
Changes in v2:
- Keep guard()/scoped_guard() from upstream verbatim in patches
5, 7, 8 and 9 (no more explicit uart_port_lock_irqsave/
unlock_irqrestore + goto-out adaptations), per Ilpo's review:
"By doing this you had to add some gotos which makes code
control flow more complicated than it was in the original
changes. From reviewer's point of view, the code is more
complicated without guard() than with them."
To make that work on 6.12.y, two small additional prerequisites
are now part of the series (see new patches 3/10 and 4/10
below).
- Stop replacing guard()s with explicit lock/unlock pairs; v2
diff against upstream is now significantly smaller.
Series structure (10 patches):
Prerequisites (per Ilpo's original guidance, plus one implicit
guard-defs prereq):
1/10 serial: 8250: use serial_port_in/out() helpers [dbd26a886e94]
2/10 serial: 8250_dw: Comment possible corner cases ... [bd8cad85561b]
3/10 serial: introduce uart_port_lock() guard()s [0fd60b689b0d] (NEW in v2)
4/10 serial: 8250: convert serial8250_do_shutdown()
to scoped_guard() [partial b339809edda1] (NEW in v2)
BUSY deassert series:
5/10 serial: 8250: Protect LCR write in shutdown [59a33d83bbe6]
6/10 serial: 8250_dw: Avoid unnecessary LCR writes [8002d6d6d0d8]
7/10 serial: 8250: Add serial8250_handle_irq_locked() [8324a54f604d]
8/10 serial: 8250_dw: Rework dw8250_handle_irq() ... [883c5a2bc934]
9/10 serial: 8250_dw: Rework IIR_NO_INT handling ... [73a4ed8f9efa]
10/10 serial: 8250_dw: Ensure BUSY is deasserted [a7b9ce39fbe4]
Notes on the new prerequisites (3/10 and 4/10):
- Patch 3/10 cherry-picks commit 0fd60b689b0d ("serial: introduce
uart_port_lock() guard()s") unmodified (13 additive lines in
include/linux/serial_core.h). It introduces DEFINE_GUARD() /
DEFINE_LOCK_GUARD_1() invocations for uart_port_lock,
uart_port_lock_irq and uart_port_lock_irqsave, which are
required so that scoped_guard(uart_port_lock_irqsave, port) and
guard(uart_port_lock_irqsave)(port) compile in 6.12.y. The
underlying cleanup.h infrastructure (DEFINE_GUARD,
DEFINE_LOCK_GUARD_1) already exists in 6.12.y; only this small
serial-specific wiring is missing.
- Patch 4/10 is a *partial* backport of commit b339809edda1
("serial: 8250: use guard()s"), per Ilpo's original request:
"b339809edda1 ('serial: 8250: use guard()s')
(8250_port.c shutdown hunks)"
The commit is authored by Jiri Slaby (SUSE) and carries the
upstream Link/SoB chain, plus an explicit [Ionut: partial
backport - only the serial8250_do_shutdown() hunks from
b339809edda1] note. The remaining (much larger) refactor of
8250_port.c and 8250_core.c is not needed for the BUSY deassert
series and is intentionally not backported.
Notes on the BUSY series itself:
- Patch 6/7 of the original mainline BUSY series,
commit e0a368ae7953 ("serial: 8250: Add late synchronize_irq()
to shutdown to handle DW UART BUSY"), is *already* in 6.12.y as
commit 0bae1c670aa8 (Ilpo, 2026-02-03), so it is not re-sent
here. Functionally this means patches 5-10 above land on top of
the existing late-synchronize_irq() fix.
- Ilpo's other suggested-but-not-required prerequisites remain
*not* included:
* commit fc9ceb501e38 ("serial: 8250: sanitize
uart_port::serial_{in,out}() types") only causes a trivial
conflict in patch 10 in code that the BUSY change removes
anyway (per Ilpo's note);
* commit c213375e3283 ("serial: 8250_dw: Call dw8250_quirks()
conditionally") only causes a conflict in patch 9 around
the dw8250_setup_dma_filter() helper and the conditional
p->handle_irq assignment, neither of which exist in 6.12.y
and neither of which is needed for the BUSY fix.
Both of those conflicts are resolved trivially in patches 9 and
10 with explicit [Ionut: adapt to 6.12.y - ...] notes.
- Namespace export syntax: in 6.12.y both EXPORT_SYMBOL_NS_GPL()
and MODULE_IMPORT_NS() apply __stringify(ns) to the namespace
argument, so it must be a bare identifier. Mainline (where the
upstream patches were written) accepts a string literal. Patches
7 and 10 here use the bare-identifier form (SERIAL_8250) instead
of the upstream string form ("SERIAL_8250"); without this fix
the .vmlinux.export.c link step fails with "expected ':' or ')'
before 'SERIAL_8250'". This is noted in the [Ionut: ...] block
of the affected patches.
Only patches 4, 7, 9 and 10 carry "[Ionut: adapt to 6.12.y - ...]"
notes; the rest are clean cherry-picks of the upstream commits
using the standard "commit <hash> upstream." stable convention.
Build:
- HEAD of the series builds to a complete vmlinux on 6.12.87 with
CONFIG_SERIAL_8250=y, CONFIG_SERIAL_8250_DW=m on x86_64. The
.vmlinux.export.c link step (which v1 originally tripped on
during development) passes cleanly.
Testing plan:
- Same as v1: test on Intel platforms with DW APB UART
(snps,dw-apb-uart) running 6.12.x-rt (PREEMPT_RT) to confirm
the original symptom (LCR writes silently ignored under Rx
load -> baud / framing mismatch after set_termios) is gone.
Will report Tested-by once cycles complete.
Based on: linux-6.12.y at v6.12.87 (8bf2f55ef536).
Andy Shevchenko (1):
serial: 8250_dw: Comment possible corner cases in serial_out()
implementation
Ilpo Järvinen (6):
serial: 8250: Protect LCR write in shutdown
serial: 8250_dw: Avoid unnecessary LCR writes
serial: 8250: Add serial8250_handle_irq_locked()
serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
serial: 8250_dw: Ensure BUSY is deasserted
Jiri Slaby (SUSE) (3):
serial: 8250: use serial_port_in/out() helpers
serial: introduce uart_port_lock() guard()s
serial: 8250: convert serial8250_do_shutdown() to scoped_guard()
drivers/tty/serial/8250/8250.h | 25 +++
drivers/tty/serial/8250/8250_dw.c | 296 +++++++++++++++++++++++-----
drivers/tty/serial/8250/8250_fsl.c | 8 +-
drivers/tty/serial/8250/8250_omap.c | 2 +-
drivers/tty/serial/8250/8250_port.c | 90 +++++----
include/linux/serial_8250.h | 1 +
include/linux/serial_core.h | 13 ++
7 files changed, 338 insertions(+), 97 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v2 6.12.y 02/10] serial: 8250_dw: Comment possible corner cases in serial_out() implementation
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
commit bd8cad85561b8de36f099404c332bf3e3dbe90f5 upstream.
8250 DesignWare driver uses a few custom implementations of the serial_out().
These implementations are carefully made to avoid infinite loops. But this is
not obvious from looking at the code. Comment the possible corner cases in
the respective functions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20250317094021.1201512-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 3225011fd772..88c55336d50f 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -107,11 +107,23 @@ static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value)
return value;
}
+/*
+ * This function is being called as part of the uart_port::serial_out()
+ * routine. Hence, it must not call serial_port_out() or serial_out()
+ * against the modified registers here, i.e. LCR.
+ */
static void dw8250_force_idle(struct uart_port *p)
{
struct uart_8250_port *up = up_to_u8250p(p);
unsigned int lsr;
+ /*
+ * The following call currently performs serial_out()
+ * against the FCR register. Because it differs to LCR
+ * there will be no infinite loop, but if it ever gets
+ * modified, we might need a new custom version of it
+ * that avoids infinite recursion.
+ */
serial8250_clear_and_reinit_fifos(up);
/*
@@ -128,6 +140,11 @@ static void dw8250_force_idle(struct uart_port *p)
serial_port_in(p, UART_RX);
}
+/*
+ * This function is being called as part of the uart_port::serial_out()
+ * routine. Hence, it must not call serial_port_out() or serial_out()
+ * against the modified registers here, i.e. LCR.
+ */
static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = to_dw8250_data(p->private_data);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 01/10] serial: 8250: use serial_port_in/out() helpers
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Jiri Slaby (SUSE), Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
commit dbd26a886e94deb7fda9050f9195ccb41f9a5d93 upstream.
There are serial_port_in/out() helpers to be used instead of direct
p->serial_in/out(). Use them in various 8250 drivers.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
[v2]
* Use serial_port_in/out() and not serial_in/out() [Andy]
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # 8250_dw
Link: https://lore.kernel.org/r/20250317070046.24386-28-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
drivers/tty/serial/8250/8250_dw.c | 16 ++++++++--------
drivers/tty/serial/8250/8250_fsl.c | 8 ++++----
drivers/tty/serial/8250/8250_omap.c | 2 +-
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f17dc3de020c..3225011fd772 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -120,12 +120,12 @@ static void dw8250_force_idle(struct uart_port *p)
* enabled.
*/
if (up->fcr & UART_FCR_ENABLE_FIFO) {
- lsr = p->serial_in(p, UART_LSR);
+ lsr = serial_port_in(p, UART_LSR);
if (!(lsr & UART_LSR_DR))
return;
}
- (void)p->serial_in(p, UART_RX);
+ serial_port_in(p, UART_RX);
}
static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
@@ -139,7 +139,7 @@ static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
/* Make sure LCR write wasn't ignored */
while (tries--) {
- unsigned int lcr = p->serial_in(p, offset);
+ unsigned int lcr = serial_port_in(p, offset);
if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
@@ -260,7 +260,7 @@ static int dw8250_handle_irq(struct uart_port *p)
{
struct uart_8250_port *up = up_to_u8250p(p);
struct dw8250_data *d = to_dw8250_data(p->private_data);
- unsigned int iir = p->serial_in(p, UART_IIR);
+ unsigned int iir = serial_port_in(p, UART_IIR);
bool rx_timeout = (iir & 0x3f) == UART_IIR_RX_TIMEOUT;
unsigned int quirks = d->pdata->quirks;
unsigned int status;
@@ -281,7 +281,7 @@ static int dw8250_handle_irq(struct uart_port *p)
status = serial_lsr_in(up);
if (!(status & (UART_LSR_DR | UART_LSR_BI)))
- (void) p->serial_in(p, UART_RX);
+ serial_port_in(p, UART_RX);
uart_port_unlock_irqrestore(p, flags);
}
@@ -303,7 +303,7 @@ static int dw8250_handle_irq(struct uart_port *p)
if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
/* Clear the USR */
- (void)p->serial_in(p, d->pdata->usr_reg);
+ serial_port_in(p, d->pdata->usr_reg);
return 1;
}
@@ -390,7 +390,7 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
{
struct uart_8250_port *up = up_to_u8250p(p);
- unsigned int mcr = p->serial_in(p, UART_MCR);
+ unsigned int mcr = serial_port_in(p, UART_MCR);
if (up->capabilities & UART_CAP_IRDA) {
if (termios->c_line == N_IRDA)
@@ -398,7 +398,7 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
else
mcr &= ~DW_UART_MCR_SIRE;
- p->serial_out(p, UART_MCR, mcr);
+ serial_port_out(p, UART_MCR, mcr);
}
serial8250_do_set_ldisc(p, termios);
}
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index b4ed442082a8..59d3d2801c2e 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -32,7 +32,7 @@ int fsl8250_handle_irq(struct uart_port *port)
uart_port_lock_irqsave(&up->port, &flags);
- iir = port->serial_in(port, UART_IIR);
+ iir = serial_port_in(port, UART_IIR);
if (iir & UART_IIR_NO_INT) {
uart_port_unlock_irqrestore(&up->port, flags);
return 0;
@@ -54,12 +54,12 @@ int fsl8250_handle_irq(struct uart_port *port)
if (unlikely((iir & UART_IIR_ID) == UART_IIR_RLSI &&
(up->lsr_saved_flags & UART_LSR_BI))) {
up->lsr_saved_flags &= ~UART_LSR_BI;
- port->serial_in(port, UART_RX);
+ serial_port_in(port, UART_RX);
uart_port_unlock_irqrestore(&up->port, flags);
return 1;
}
- lsr = orig_lsr = up->port.serial_in(&up->port, UART_LSR);
+ lsr = orig_lsr = serial_port_in(port, UART_LSR);
/* Process incoming characters first */
if ((lsr & (UART_LSR_DR | UART_LSR_BI)) &&
@@ -71,7 +71,7 @@ int fsl8250_handle_irq(struct uart_port *port)
if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) {
unsigned long delay;
- up->ier = port->serial_in(port, UART_IER);
+ up->ier = serial_port_in(port, UART_IER);
if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
port->ops->stop_rx(port);
} else {
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 0f4ce0c69114..dc9e3e25d55f 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -688,7 +688,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
/* Synchronize UART_IER access against the console. */
uart_port_lock(port);
- up->ier = port->serial_in(port, UART_IER);
+ up->ier = serial_port_in(port, UART_IER);
if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
port->ops->stop_rx(port);
} else {
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6.12.y 03/10] serial: introduce uart_port_lock() guard()s
From: Ionut Nechita (Wind River) @ 2026-05-13 6:50 UTC (permalink / raw)
To: ilpo.jarvinen, gregkh
Cc: stable, andriy.shevchenko, wander, chris.friesen, linux-serial,
Jiri Slaby (SUSE), Ionut Nechita
In-Reply-To: <20260513065028.112468-1-ionut.nechita@windriver.com>
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
commit 0fd60b689b0dacce659253ec15cb3d3bf660e30b upstream.
Having this, guards like these work:
guard(uart_port_lock_irq)(&up->port);
or
scoped_guard(uart_port_lock_irqsave, port) {
...
}
See e.g. "serial: 8250: use guard()s" later in this series.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250814072456.182853-4-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
---
include/linux/serial_core.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 4ab65874a850..191154b8ada2 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -782,6 +782,19 @@ static inline void uart_port_unlock_irqrestore(struct uart_port *up, unsigned lo
spin_unlock_irqrestore(&up->lock, flags);
}
+DEFINE_GUARD(uart_port_lock, struct uart_port *, uart_port_lock(_T), uart_port_unlock(_T));
+DEFINE_GUARD_COND(uart_port_lock, _try, uart_port_trylock(_T));
+
+DEFINE_GUARD(uart_port_lock_irq, struct uart_port *, uart_port_lock_irq(_T),
+ uart_port_unlock_irq(_T));
+
+DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
+ uart_port_lock_irqsave(_T->lock, &_T->flags),
+ uart_port_unlock_irqrestore(_T->lock, _T->flags),
+ unsigned long flags);
+DEFINE_LOCK_GUARD_1_COND(uart_port_lock_irqsave, _try,
+ uart_port_trylock_irqsave(_T->lock, &_T->flags));
+
static inline int serial_port_in(struct uart_port *up, int offset)
{
return up->serial_in(up, offset);
--
2.54.0
^ permalink raw reply related
* [PATCH] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: w15303746062 @ 2026-05-13 6:45 UTC (permalink / raw)
To: marcel, luiz.dentz
Cc: linux-bluetooth, linux-serial, linux-kernel, Mingyu Wang
From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
A Use-After-Free (UAF) vulnerability and a subsequent General Protection
Fault (GPF) were observed in h5_recv() due to a race condition between
the initialization of the HCI UART line discipline and concurrent TTY
hangup via TIOCVHANGUP.
The issue arises because the workqueues (init_ready and write_work) are
only cancelled if the HCI_UART_PROTO_READY flag is set. However, during
the protocol initialization phase (HCI_UART_PROTO_INIT), the underlying
protocol (e.g., H5) may schedule work (such as sending sync/config
packets). If a hangup occurs before the setup completes and the READY
flag is set, hci_uart_tty_close() skips the cancel_work_sync() calls
and proceeds to free the `hu` struct.
When the delayed workqueue finally executes, it blindly dereferences
the freed `hu` struct, causing ODEBUG warnings and kernel panics.
Fix this by moving the cancel_work_sync() calls outside the
HCI_UART_PROTO_READY check, ensuring that any pending works are
unconditionally cancelled before the hci_uart structure is freed.
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
drivers/bluetooth/hci_ldisc.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..566e1c525ee2 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -544,14 +544,18 @@ static void hci_uart_tty_close(struct tty_struct *tty)
if (hdev)
hci_uart_close(hdev);
+ /*
+ * Always cancel workqueues unconditionally before freeing the hu
+ * struct, as they might be active during the PROTO_INIT phase.
+ */
+ cancel_work_sync(&hu->init_ready);
+ cancel_work_sync(&hu->write_work);
+
if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
percpu_down_write(&hu->proto_lock);
clear_bit(HCI_UART_PROTO_READY, &hu->flags);
percpu_up_write(&hu->proto_lock);
- cancel_work_sync(&hu->init_ready);
- cancel_work_sync(&hu->write_work);
-
if (hdev) {
if (test_bit(HCI_UART_REGISTERED, &hu->flags))
hci_unregister_dev(hdev);
--
2.34.1
^ permalink raw reply related
* BUG: General Protection Fault in h5_recv due to TTY line discipline race condition (TIOCSTI)
From: 王明煜 @ 2026-05-13 2:54 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz
Cc: linux-bluetooth, linux-kernel, linux-serial
Hi all,
We have discovered a severe NULL pointer dereference and subsequent General Protection Fault in `h5_recv()` within `drivers/bluetooth/hci_h5.c`. This crash is triggered by a race condition between the initialization of the HCI UART line discipline and concurrent data injection via the `TIOCSTI` ioctl.
This issue was found during fuzzing with a custom LLM-assisted device modeling framework.
### Crash Trace (KASAN)
Oops: general protection fault, probably for non-canonical address 0xdffffc000000005f: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000002f8-0x00000000000002ff]
CPU: 3 UID: 0 PID: 75833 Comm: syz.0.5605 Not tainted 6.18.0 #1 PREEMPT(full)
RIP: 0010:h5_recv+0xfc/0x8f0 drivers/bluetooth/hci_h5.c:572
...
Call Trace:
<TASK>
hci_uart_tty_receive+0x25b/0x800 drivers/bluetooth/hci_ldisc.c:627
tiocsti drivers/tty/tty_io.c:2290 [inline]
tty_ioctl+0x502/0x1690 drivers/tty/tty_io.c:2706
__x64_sys_ioctl+0x18f/0x210 fs/ioctl.c:583
do_syscall_64+0xcb/0xfa0 arch/x86/entry/syscall_64.c:94
...
### Vulnerability Analysis
The crash occurs due to an architectural race window during the line discipline setup:
1. Thread A executes `ioctl(HCIUARTSETPROTO)` to set the line discipline to HCI_UART_H5. The kernel sets the `HCI_UART_PROTO_INIT` bit in `hu->flags` and proceeds to call `hci_uart_register_dev()`, which eventually invokes `h5_open()`.
2. Before `h5_open()` finishes executing and allocates memory for `hu->priv` (i.e., before `hu->priv = h5;` is executed), Thread B concurrently executes `ioctl(TIOCSTI)`.
3. The `TIOCSTI` command forces data into the TTY receive path, calling `hci_uart_tty_receive()`.
4. Inside `hci_uart_tty_receive()`, the condition `!test_bit(HCI_UART_PROTO_INIT, &hu->flags)` evaluates to FALSE (because Thread A set it), allowing the execution to proceed without dropping the data.
5. The execution flows into `hu->proto->recv()` -> `h5_recv()`.
6. `h5_recv()` blindly dereferences `hu->priv` (which is still NULL), leading to the GPF when accessing `h5->rx_pending` on line 572: `BT_DBG("... %zu ...", ..., h5->rx_pending, ...);`.
### Proposed Fix Direction
While adding a simple `if (!h5) return 0;` at the beginning of `h5_recv()` prevents the crash, it seems the root cause lies in the permissiveness of the `HCI_UART_PROTO_INIT` state in `hci_uart_tty_receive()`, or the lack of proper synchronization between the `open` callback and the `recv` path during initialization.
Since we are not entirely familiar with the specific handshake requirements of the H5 protocol during the `PROTO_INIT` phase, we are reporting this vulnerability here rather than submitting a potentially flawed patch.
Please let us know if you need more information or the full reproducer.
Reported-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
Best regards,
Mingyu Wang
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: qcom: geni-se-qup: Add compatible for SA8797P SoC
From: Bjorn Andersson @ 2026-05-12 20:23 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Praveen Talari,
Konrad Dybcio, Dmitry Baryshkov, Bartosz Golaszewski,
Deepti Jaggi, linux-serial, devicetree, linux-arm-msm,
linux-kernel
In-Reply-To: <20260427005901.230237-1-shengchao.guo@oss.qualcomm.com>
On Mon, 27 Apr 2026 08:59:01 +0800, Shawn Guo wrote:
> Document GENI Serial Engine QUP Wrapper Controller on Nord SA8797P SoC
> which is compatible with SA8255P one.
>
>
Applied, thanks!
[1/1] dt-bindings: qcom: geni-se-qup: Add compatible for SA8797P SoC
commit: 35466ef5db1fbdff49c4142026c4c56514d5ff47
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* [tty:tty-next 28/31] drivers/tty/serial/max310x.c:1507:20: warning: 'max310x_regmap_name' defined but not used
From: kernel test robot @ 2026-05-12 19:53 UTC (permalink / raw)
To: Hugo Villeneuve; +Cc: oe-kbuild-all, linux-serial, Greg Kroah-Hartman
Hi Hugo,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next
head: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
commit: 20ffe4b3330a8bde9e933e9ba2323d5e9386caa5 [28/31] serial: max310x: allow driver to be built with SPI or I2C
config: powerpc64-randconfig-001-20260513 (https://download.01.org/0day-ci/archive/20260513/202605130334.e6V1YDQN-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260513/202605130334.e6V1YDQN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605130334.e6V1YDQN-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
drivers/tty/serial/max310x.c:1730:1: warning: label 'err_spi_register' defined but not used [-Wunused-label]
err_spi_register:
^~~~~~~~~~~~~~~~
At top level:
>> drivers/tty/serial/max310x.c:1507:20: warning: 'max310x_regmap_name' defined but not used [-Wunused-function]
static const char *max310x_regmap_name(u8 port_id)
^~~~~~~~~~~~~~~~~~~
drivers/tty/serial/max310x.c:1492:29: warning: 'regcfg' defined but not used [-Wunused-variable]
static struct regmap_config regcfg = {
^~~~~~
>> drivers/tty/serial/max310x.c:1464:13: warning: 'max310x_remove' defined but not used [-Wunused-function]
static void max310x_remove(struct device *dev)
^~~~~~~~~~~~~~
>> drivers/tty/serial/max310x.c:1280:12: warning: 'max310x_probe' defined but not used [-Wunused-function]
static int max310x_probe(struct device *dev, const struct max310x_devtype *devtype,
^~~~~~~~~~~~~
vim +/max310x_regmap_name +1507 drivers/tty/serial/max310x.c
f65444187a66bf Alexander Shiyan 2012-08-06 1463
70b4d23226c85a Uwe Kleine-König 2021-10-12 @1464 static void max310x_remove(struct device *dev)
f65444187a66bf Alexander Shiyan 2012-08-06 1465 {
f65444187a66bf Alexander Shiyan 2012-08-06 1466 struct max310x_port *s = dev_get_drvdata(dev);
88d5e520aa9701 abdoulaye berthe 2014-07-12 1467 int i;
f65444187a66bf Alexander Shiyan 2012-08-06 1468
6286767ad3afc8 Alexander Shiyan 2016-06-07 1469 for (i = 0; i < s->devtype->nr; i++) {
10d8b34a421716 Alexander Shiyan 2013-06-29 1470 cancel_work_sync(&s->p[i].tx_work);
e7b8a3ceff5e5c Alexander Shiyan 2014-02-07 1471 cancel_work_sync(&s->p[i].md_work);
5bdb48b501e836 Alexander Shiyan 2016-06-07 1472 cancel_work_sync(&s->p[i].rs_work);
5d888f1c32e228 Hugo Villeneuve 2024-01-18 1473
5d888f1c32e228 Hugo Villeneuve 2024-01-18 1474 if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
6286767ad3afc8 Alexander Shiyan 2016-06-07 1475 uart_remove_one_port(&max310x_uart, &s->p[i].port);
5d888f1c32e228 Hugo Villeneuve 2024-01-18 1476
9464833a765f66 Hugo Villeneuve 2024-01-18 1477 max310x_power(&s->p[i].port, 0);
10d8b34a421716 Alexander Shiyan 2013-06-29 1478 }
f65444187a66bf Alexander Shiyan 2012-08-06 1479
d3a8a252e177cf Alexander Shiyan 2014-02-10 1480 clk_disable_unprepare(s->clk);
f65444187a66bf Alexander Shiyan 2012-08-06 1481 }
f65444187a66bf Alexander Shiyan 2012-08-06 1482
58afc909772c9b Alexander Shiyan 2014-02-10 1483 static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
58afc909772c9b Alexander Shiyan 2014-02-10 1484 { .compatible = "maxim,max3107", .data = &max3107_devtype, },
58afc909772c9b Alexander Shiyan 2014-02-10 1485 { .compatible = "maxim,max3108", .data = &max3108_devtype, },
58afc909772c9b Alexander Shiyan 2014-02-10 1486 { .compatible = "maxim,max3109", .data = &max3109_devtype, },
58afc909772c9b Alexander Shiyan 2014-02-10 1487 { .compatible = "maxim,max14830", .data = &max14830_devtype },
58afc909772c9b Alexander Shiyan 2014-02-10 1488 { }
58afc909772c9b Alexander Shiyan 2014-02-10 1489 };
58afc909772c9b Alexander Shiyan 2014-02-10 1490 MODULE_DEVICE_TABLE(of, max310x_dt_ids);
58afc909772c9b Alexander Shiyan 2014-02-10 1491
27027a70e21f62 Alexander Shiyan 2014-02-10 1492 static struct regmap_config regcfg = {
27027a70e21f62 Alexander Shiyan 2014-02-10 1493 .reg_bits = 8,
27027a70e21f62 Alexander Shiyan 2014-02-10 1494 .val_bits = 8,
d584b65c0ddf20 Jan Kundrát 2017-12-13 1495 .write_flag_mask = MAX310X_WRITE_BIT,
c9615d34ce2619 wangkaiyuan 2024-03-18 1496 .cache_type = REGCACHE_MAPLE,
6ef281daf02059 Cosmin Tanislav 2022-06-05 1497 .max_register = MAX310X_REG_1F,
27027a70e21f62 Alexander Shiyan 2014-02-10 1498 .writeable_reg = max310x_reg_writeable,
27027a70e21f62 Alexander Shiyan 2014-02-10 1499 .volatile_reg = max310x_reg_volatile,
27027a70e21f62 Alexander Shiyan 2014-02-10 1500 .precious_reg = max310x_reg_precious,
3f42b142ea1171 Jan Kundrát 2023-04-05 1501 .writeable_noinc_reg = max310x_reg_noinc,
3f42b142ea1171 Jan Kundrát 2023-04-05 1502 .readable_noinc_reg = max310x_reg_noinc,
3f42b142ea1171 Jan Kundrát 2023-04-05 1503 .max_raw_read = MAX310X_FIFO_SIZE,
3f42b142ea1171 Jan Kundrát 2023-04-05 1504 .max_raw_write = MAX310X_FIFO_SIZE,
27027a70e21f62 Alexander Shiyan 2014-02-10 1505 };
27027a70e21f62 Alexander Shiyan 2014-02-10 1506
609aabb259d497 Hugo Villeneuve 2024-01-18 @1507 static const char *max310x_regmap_name(u8 port_id)
609aabb259d497 Hugo Villeneuve 2024-01-18 1508 {
609aabb259d497 Hugo Villeneuve 2024-01-18 1509 switch (port_id) {
609aabb259d497 Hugo Villeneuve 2024-01-18 1510 case 0: return "port0";
609aabb259d497 Hugo Villeneuve 2024-01-18 1511 case 1: return "port1";
609aabb259d497 Hugo Villeneuve 2024-01-18 1512 case 2: return "port2";
609aabb259d497 Hugo Villeneuve 2024-01-18 1513 case 3: return "port3";
609aabb259d497 Hugo Villeneuve 2024-01-18 1514 default:
609aabb259d497 Hugo Villeneuve 2024-01-18 1515 WARN_ON(true);
609aabb259d497 Hugo Villeneuve 2024-01-18 1516 return NULL;
609aabb259d497 Hugo Villeneuve 2024-01-18 1517 }
609aabb259d497 Hugo Villeneuve 2024-01-18 1518 }
609aabb259d497 Hugo Villeneuve 2024-01-18 1519
:::::: The code at line 1507 was first introduced by commit
:::::: 609aabb259d497578ffa2d66ced57c22044f821c serial: max310x: use separate regmap name for each port
:::::: TO: Hugo Villeneuve <hvilleneuve@dimonoff.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Praveen Talari @ 2026-05-12 17:14 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, Konrad Dybcio
Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
jyothi.seerapu, Praveen Talari
In-Reply-To: <20260512-add-tracepoints-for-qcom-geni-serial-v2-0-a5726421b3af@oss.qualcomm.com>
Add tracing to the Qualcomm GENI serial driver to improve runtime
observability.
Trace hooks are added at key points including termios and clock
configuration, manual control get/set, interrupt handling, and data
TX/RX paths.
Usage examples:
Enable all serial traces:
echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
clk_rate=7372800 clk_div=4 clk_idx=0
2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
rx_par=0x00000000 stop=0
2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
uart_manual_rfr=0x00000000
2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
geni_ios=0x00000001
2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
64 65 66 67 68
2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
64 65 66 67 68
2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
uart_manual_rfr=0x80000002
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e6b0a55f0cfb..9e2de074d799 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -7,6 +7,9 @@
/* Disable MMIO tracing to prevent excessive logging of unwanted MMIO traces */
#define __DISABLE_TRACE_MMIO__
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_geni_serial.h>
+
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/io.h>
@@ -225,7 +228,7 @@ static void qcom_geni_serial_config_port(struct uart_port *uport, int cfg_flags)
static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
{
unsigned int mctrl = TIOCM_DSR | TIOCM_CAR;
- u32 geni_ios;
+ u32 geni_ios = 0;
if (uart_console(uport)) {
mctrl |= TIOCM_CTS;
@@ -235,6 +238,8 @@ static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
mctrl |= TIOCM_CTS;
}
+ trace_geni_serial_get_mctrl(uport->dev, mctrl, geni_ios);
+
return mctrl;
}
@@ -253,6 +258,8 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
if (!(mctrl & TIOCM_RTS) && !uport->suspended)
uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY;
writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR);
+
+ trace_geni_serial_set_mctrl(uport->dev, mctrl, uart_manual_rfr);
}
static const char *qcom_geni_serial_get_type(struct uart_port *uport)
@@ -683,6 +690,8 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
xmit_size = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail,
UART_XMIT_SIZE);
+ trace_geni_serial_tx_data(uport->dev, tail, xmit_size);
+
qcom_geni_set_rs485_mode(uport, SER_RS485_RTS_ON_SEND);
qcom_geni_serial_setup_tx(uport, xmit_size);
@@ -909,8 +918,10 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
return;
}
- if (!drop)
+ if (!drop) {
+ trace_geni_serial_rx_data(uport->dev, port->rx_buf, rx_in);
handle_rx_uart(uport, rx_in);
+ }
ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
DMA_RX_BUF_SIZE,
@@ -1069,6 +1080,10 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
geni_status = readl(uport->membase + SE_GENI_STATUS);
dma = readl(uport->membase + SE_GENI_DMA_MODE_EN);
m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+
+ trace_geni_serial_irq(uport->dev, m_irq_status, s_irq_status,
+ dma_tx_status, dma_rx_status);
+
writel(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR);
writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
writel(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
@@ -1281,8 +1296,8 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned int baud)
return -EINVAL;
}
- dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n, clk_idx = %u\n",
- baud * sampling_rate, clk_rate, clk_div, clk_idx);
+ trace_geni_serial_clk_cfg(uport->dev, baud * sampling_rate, clk_rate,
+ clk_div, clk_idx);
uport->uartclk = clk_rate;
port->clk_rate = clk_rate;
@@ -1432,6 +1447,10 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
+
+ trace_geni_serial_set_termios(uport->dev, baud, bits_per_char,
+ tx_trans_cfg, tx_parity_cfg, rx_trans_cfg,
+ rx_parity_cfg, stop_bit_len);
}
#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Praveen Talari @ 2026-05-12 17:14 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, Konrad Dybcio
Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
jyothi.seerapu, Praveen Talari
In-Reply-To: <20260512-add-tracepoints-for-qcom-geni-serial-v2-0-a5726421b3af@oss.qualcomm.com>
Add tracepoint support to the Qualcomm GENI serial driver to provide
runtime visibility into driver behavior without requiring invasive debug
patches.
The trace events cover UART termios configuration, clock setup, modem
control state, interrupt status, and TX/RX data, making it easier to
diagnose communication issues in the field.
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v1->v2:
- Removed multiple TX/RX trace events, instead used
DECLARE_EVENT_CLASS and DEFINE_EVENT.
---
include/trace/events/qcom_geni_serial.h | 172 ++++++++++++++++++++++++++++++++
1 file changed, 172 insertions(+)
diff --git a/include/trace/events/qcom_geni_serial.h b/include/trace/events/qcom_geni_serial.h
new file mode 100644
index 000000000000..5e23827881d0
--- /dev/null
+++ b/include/trace/events/qcom_geni_serial.h
@@ -0,0 +1,172 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_serial
+
+#if !defined(_TRACE_QCOM_GENI_SERIAL_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_SERIAL_H
+
+#include <linux/device.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(geni_serial_set_termios,
+ TP_PROTO(struct device *dev, unsigned int baud,
+ unsigned int bits_per_char, u32 tx_trans_cfg,
+ u32 tx_parity_cfg, u32 rx_trans_cfg,
+ u32 rx_parity_cfg, u32 stop_bit_len),
+ TP_ARGS(dev, baud, bits_per_char, tx_trans_cfg, tx_parity_cfg,
+ rx_trans_cfg, rx_parity_cfg, stop_bit_len),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(unsigned int, baud)
+ __field(unsigned int, bits_per_char)
+ __field(u32, tx_trans_cfg)
+ __field(u32, tx_parity_cfg)
+ __field(u32, rx_trans_cfg)
+ __field(u32, rx_parity_cfg)
+ __field(u32, stop_bit_len)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->baud = baud;
+ __entry->bits_per_char = bits_per_char;
+ __entry->tx_trans_cfg = tx_trans_cfg;
+ __entry->tx_parity_cfg = tx_parity_cfg;
+ __entry->rx_trans_cfg = rx_trans_cfg;
+ __entry->rx_parity_cfg = rx_parity_cfg;
+ __entry->stop_bit_len = stop_bit_len;
+ ),
+
+ TP_printk("%s: baud=%u bpc=%u tx_trans=0x%08x tx_par=0x%08x rx_trans=0x%08x rx_par=0x%08x stop=%u",
+ __get_str(name), __entry->baud, __entry->bits_per_char,
+ __entry->tx_trans_cfg, __entry->tx_parity_cfg,
+ __entry->rx_trans_cfg, __entry->rx_parity_cfg,
+ __entry->stop_bit_len)
+);
+
+TRACE_EVENT(geni_serial_clk_cfg,
+ TP_PROTO(struct device *dev, unsigned int desired_rate,
+ unsigned long clk_rate, unsigned int clk_div,
+ unsigned int clk_idx),
+ TP_ARGS(dev, desired_rate, clk_rate, clk_div, clk_idx),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(unsigned int, desired_rate)
+ __field(unsigned long, clk_rate)
+ __field(unsigned int, clk_div)
+ __field(unsigned int, clk_idx)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->desired_rate = desired_rate;
+ __entry->clk_rate = clk_rate;
+ __entry->clk_div = clk_div;
+ __entry->clk_idx = clk_idx;
+ ),
+
+ TP_printk("%s: desired_rate=%u clk_rate=%lu clk_div=%u clk_idx=%u",
+ __get_str(name), __entry->desired_rate, __entry->clk_rate,
+ __entry->clk_div, __entry->clk_idx)
+);
+
+TRACE_EVENT(geni_serial_irq,
+ TP_PROTO(struct device *dev, u32 m_irq, u32 s_irq,
+ u32 dma_tx, u32 dma_rx),
+ TP_ARGS(dev, m_irq, s_irq, dma_tx, dma_rx),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(u32, m_irq)
+ __field(u32, s_irq)
+ __field(u32, dma_tx)
+ __field(u32, dma_rx)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->m_irq = m_irq;
+ __entry->s_irq = s_irq;
+ __entry->dma_tx = dma_tx;
+ __entry->dma_rx = dma_rx;
+ ),
+
+ TP_printk("%s: m_irq=0x%08x s_irq=0x%08x dma_tx=0x%08x dma_rx=0x%08x",
+ __get_str(name), __entry->m_irq, __entry->s_irq,
+ __entry->dma_tx, __entry->dma_rx)
+);
+
+DECLARE_EVENT_CLASS(geni_serial_data,
+
+ TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+
+ TP_ARGS(dev, buf, len),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(unsigned int, len)
+ __dynamic_array(u8, data, len)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->len = len;
+ memcpy(__get_dynamic_array(data), buf, len);
+ ),
+
+ TP_printk("%s: len=%u data=%s",
+ __get_str(name), __entry->len,
+ __print_hex(__get_dynamic_array(data), __entry->len))
+);
+
+DEFINE_EVENT(geni_serial_data, geni_serial_tx_data,
+
+ TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+
+ TP_ARGS(dev, buf, len)
+
+);
+
+DEFINE_EVENT(geni_serial_data, geni_serial_rx_data,
+
+ TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+
+ TP_ARGS(dev, buf, len)
+
+);
+
+TRACE_EVENT(geni_serial_set_mctrl,
+ TP_PROTO(struct device *dev, unsigned int mctrl,
+ u32 uart_manual_rfr),
+ TP_ARGS(dev, mctrl, uart_manual_rfr),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(unsigned int, mctrl)
+ __field(u32, uart_manual_rfr)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->mctrl = mctrl;
+ __entry->uart_manual_rfr = uart_manual_rfr;
+ ),
+
+ TP_printk("%s: mctrl=0x%04x uart_manual_rfr=0x%08x",
+ __get_str(name), __entry->mctrl, __entry->uart_manual_rfr)
+);
+
+TRACE_EVENT(geni_serial_get_mctrl,
+ TP_PROTO(struct device *dev, unsigned int mctrl, u32 geni_ios),
+ TP_ARGS(dev, mctrl, geni_ios),
+
+ TP_STRUCT__entry(__string(name, dev_name(dev))
+ __field(unsigned int, mctrl)
+ __field(u32, geni_ios)
+ ),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->mctrl = mctrl;
+ __entry->geni_ios = geni_ios;
+ ),
+
+ TP_printk("%s: mctrl=0x%04x geni_ios=0x%08x",
+ __get_str(name), __entry->mctrl, __entry->geni_ios)
+);
+
+#endif /* _TRACE_QCOM_GENI_SERIAL_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] Add tracepoints support for Qualcomm GENI Serial drivers
From: Praveen Talari @ 2026-05-12 17:14 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, Konrad Dybcio
Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
jyothi.seerapu, Praveen Talari
Add tracepoints to the Qualcomm GENI (Generic Interface) serial driver.
These trace events enable runtime debugging and performance analysis of
UART operations.
The trace events cover UART termios configuration, clock setup, manual
control state, interrupt status, and actual transmitted/received data in
hexadecimal format.
Usage examples:
Enable all serial traces:
echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
clk_rate=7372800 clk_div=4 clk_idx=0
2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
rx_par=0x00000000 stop=0
2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
uart_manual_rfr=0x00000000
2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
geni_ios=0x00000001
2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
64 65 66 67 68
2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
64 65 66 67 68
2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
uart_manual_rfr=0x80000002
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v2:
- removed multiple trace events for TX/RX events, instead used
DECLARE_EVENT_CLASS and DEFINE_EVENT.
- Link to v1: https://lore.kernel.org/r/20260506-add-tracepoints-for-qcom-geni-serial-v1-0-544b22612e08@oss.qualcomm.com
---
Praveen Talari (2):
serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
drivers/tty/serial/qcom_geni_serial.c | 27 ++++-
include/trace/events/qcom_geni_serial.h | 172 ++++++++++++++++++++++++++++++++
2 files changed, 195 insertions(+), 4 deletions(-)
---
base-commit: 1f5ffc672165ff851063a5fd044b727ab2517ae3
change-id: 20260427-add-tracepoints-for-qcom-geni-serial-948777218b7b
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH] serial: max310x: fix compile errors if CONFIG_SPI_MASTER is disabled
From: Hugo Villeneuve @ 2026-05-12 15:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve
Cc: hugo, linux-serial, kernel test robot, linux-kernel
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Since commit 20ffe4b3330a8 ("serial: max310x: allow driver to be built with
SPI or I2C"), if I2C is enabled and SPI_MASTER is disabled, we have these
compile errors:
drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
drivers/tty/serial/max310x.c: error: 'max310x_spi_driver' undeclared...
drivers/tty/serial/max310x.c: In function ‘max310x_uart_init’:
drivers/tty/serial/max310x.c: error: label ‘err_spi_register’
defined but not used...
drivers/tty/serial/max310x.c: error: ‘regcfg’ defined but not used
Fix by properly encapsulating i2c/spi code/variables in their respective
context with CONFIG_I2C and CONFIG_SPI_MASTER.
Fixes: 20ffe4b3330a8 ("serial: max310x: allow driver to be built with SPI or I2C")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605121847.N9DVLNg2-lkp@intel.com/
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
note: not Cc-ing stable as the commit is still in tty-next, and even if the
errors originate from original commit that added I2C support, they were not
trigerred because the driver could not be selected/compiled if
CONFIG_SPI_MASTER was disabled.
---
drivers/tty/serial/max310x.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 9f423b3b4201d..8380fa1b0c0eb 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1507,6 +1507,21 @@ static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, max310x_dt_ids);
+static const char *max310x_regmap_name(u8 port_id)
+{
+ switch (port_id) {
+ case 0: return "port0";
+ case 1: return "port1";
+ case 2: return "port2";
+ case 3: return "port3";
+ default:
+ WARN_ON(true);
+ return NULL;
+ }
+}
+
+#ifdef CONFIG_SPI_MASTER
+
static struct regmap_config regcfg = {
.reg_bits = 8,
.val_bits = 8,
@@ -1522,20 +1537,6 @@ static struct regmap_config regcfg = {
.max_raw_write = MAX310X_FIFO_SIZE,
};
-static const char *max310x_regmap_name(u8 port_id)
-{
- switch (port_id) {
- case 0: return "port0";
- case 1: return "port1";
- case 2: return "port2";
- case 3: return "port3";
- default:
- WARN_ON(true);
- return NULL;
- }
-}
-
-#ifdef CONFIG_SPI_MASTER
static int max310x_spi_extended_reg_enable(struct device *dev, bool enable)
{
struct max310x_port *s = dev_get_drvdata(dev);
@@ -1742,10 +1743,11 @@ static int __init max310x_uart_init(void)
#ifdef CONFIG_I2C
err_i2c_register:
- spi_unregister_driver(&max310x_spi_driver);
#endif
-
+#ifdef CONFIG_SPI_MASTER
+ spi_unregister_driver(&max310x_spi_driver);
err_spi_register:
+#endif
uart_unregister_driver(&max310x_uart);
return ret;
base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
--
2.47.3
^ permalink raw reply related
* [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo, stable
In-Reply-To: <cover.1778592805.git.jnilo@free.fr>
dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
lock held via guard(uart_port_lock_irqsave). The guard destructor is
plain uart_port_unlock_irqrestore(), so a SysRq character captured into
port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
being dispatched to handle_sysrq().
This is the same regression pattern as in serial8250_handle_irq(),
introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
dw8250_handle_irq() locking and IIR handling") moved the function to
the guard()-based locking scheme without using the sysrq-aware unlock
helper.
Switch to guard(uart_port_lock_sysrq_irqsave) so that captured
sysrq_ch is dispatched on scope exit, matching the fix in
serial8250_handle_irq().
Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 55e40c10f..237543fa7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
unsigned int quirks = d->pdata->quirks;
unsigned int status;
- guard(uart_port_lock_irqsave)(p);
+ guard(uart_port_lock_sysrq_irqsave)(p);
switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
case UART_IIR_NO_INT:
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo, stable
In-Reply-To: <cover.1778592805.git.jnilo@free.fr>
serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
(reached from serial8250_read_char()). Dispatch of that captured
character to handle_sysrq() is expected to happen at port-unlock time,
through uart_unlock_and_check_sysrq[_irqrestore]().
After commit 8324a54f604d ("serial: 8250: Add
serial8250_handle_irq_locked()") the function was reduced to a wrapper
that takes the port lock via guard(uart_port_lock_irqsave) whose
destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
unlock helper is no longer called, so port->sysrq_ch is captured but
never dispatched: BREAK + SysRq key is consumed silently.
This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
8250: fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address.
Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
is the sysrq-aware unlock helper, restoring the pre-split behaviour.
Update the Context: comment on serial8250_handle_irq_locked() so future
HW-specific 8250 wrappers know to use the same guard or the explicit
sysrq-aware unlock.
Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART produces the SysRq help dump in dmesg and the brk
counter in /proc/tty/driver/serial increments correctly.
Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_port.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e4e6a53eb..64f3487e8 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
}
/*
- * Context: port's lock must be held by the caller.
+ * Context: port's lock must be held by the caller. The caller must
+ * release it via guard(uart_port_lock_sysrq_irqsave) or
+ * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
+ * character on unlock.
*/
void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
{
@@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if (iir & UART_IIR_NO_INT)
return 0;
- guard(uart_port_lock_irqsave)(port);
+ guard(uart_port_lock_sysrq_irqsave)(port);
serial8250_handle_irq_locked(port, iir);
return 1;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/3] serial: core: introduce guard(uart_port_lock_sysrq_irqsave)
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo
In-Reply-To: <cover.1778592805.git.jnilo@free.fr>
uart_handle_break() and uart_prepare_sysrq_char() (in
include/linux/serial_core.h) capture a SysRq character into
port->sysrq_ch while the port lock is held and rely on the unlock
helper -- uart_unlock_and_check_sysrq_irqrestore() -- to dispatch the
captured character to handle_sysrq() on scope exit.
The existing guard(uart_port_lock_irqsave) cannot be used by IRQ
handlers that process RX, because its destructor calls plain
uart_port_unlock_irqrestore() and silently drops port->sysrq_ch.
Add a dedicated guard variant whose destructor is the sysrq-aware
unlock helper. Callers that may capture SysRq characters opt in by
using guard(uart_port_lock_sysrq_irqsave); the existing
uart_port_lock_irqsave guard keeps its current plain-unlock semantics
for the many callers that do not process RX.
The lock side is identical to uart_port_lock_irqsave -- only the
unlock-time behaviour differs. Naming mirrors
uart_unlock_and_check_sysrq_irqrestore() (sysrq before irqsave/irqrestore).
The new macro is placed after the CONFIG_MAGIC_SYSRQ_SERIAL block so
both definitions of uart_unlock_and_check_sysrq_irqrestore() (sysrq
enabled and disabled) are visible at expansion time. When
CONFIG_MAGIC_SYSRQ_SERIAL=n the destructor degenerates to plain
uart_port_unlock_irqrestore(), so there is no overhead.
No functional change on its own; users are converted in the following
patches.
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
include/linux/serial_core.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 4f7bbdd90..4fa079dc9 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1286,6 +1286,19 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
}
#endif /* CONFIG_MAGIC_SYSRQ_SERIAL */
+/*
+ * Variant of guard(uart_port_lock_irqsave) for IRQ handlers that may capture
+ * a SysRq character via uart_prepare_sysrq_char(). The destructor uses the
+ * sysrq-aware unlock helper so that a captured port->sysrq_ch is dispatched
+ * to handle_sysrq() on scope exit. The plain guard variant silently drops
+ * sysrq_ch and must not be used by callers that process RX.
+ */
+DEFINE_LOCK_GUARD_1(uart_port_lock_sysrq_irqsave, struct uart_port,
+ uart_port_lock_irqsave(_T->lock, &_T->flags),
+ uart_unlock_and_check_sysrq_irqrestore(_T->lock,
+ _T->flags),
+ unsigned long flags);
+
/*
* We do the SysRQ and SAK checking like this...
*/
--
2.43.0
^ permalink raw reply related
* [PATCH 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo
In-Reply-To: <5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr>
This series fixes a silent regression where a SysRq character entered as
BREAK + key on the serial console is consumed by the kernel but never
dispatched to handle_sysrq().
The root cause and two fix candidates were discussed in [1]. Following
Ilpo's suggestion, this series adds a dedicated lock-guard variant whose
destructor is the sysrq-aware unlock helper, and switches the two
affected IRQ handlers (serial8250_handle_irq and dw8250_handle_irq) to
use it. The plain guard(uart_port_lock_irqsave) keeps its current
semantics for the many callers that do not process RX.
Patch 1 introduces guard(uart_port_lock_sysrq_irqsave) in serial_core.h.
Patch 2 switches serial8250_handle_irq() and updates the Context comment
on serial8250_handle_irq_locked() so future HW-specific 8250
wrappers know which unlock variant is required.
Patch 3 switches dw8250_handle_irq() to the same guard.
Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART now produces the SysRq help dump in dmesg; the brk
counter in /proc/tty/driver/serial increments per BREAK as expected.
Build tested on tty-next (base 16e95bfb79b5).
[1] https://lore.kernel.org/linux-serial/5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr/
Jacques Nilo (3):
serial: core: introduce guard(uart_port_lock_sysrq_irqsave)
serial: 8250: dispatch SysRq character in serial8250_handle_irq()
serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
drivers/tty/serial/8250/8250_dw.c | 2 +-
drivers/tty/serial/8250/8250_port.c | 7 +++++--
include/linux/serial_core.h | 13 +++++++++++++
3 files changed, 19 insertions(+), 3 deletions(-)
base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
--
2.43.0
^ permalink raw reply
* Re: [REPORT] serial: 8250: BREAK + SysRq dispatch silently broken since 8324a54f604d
From: Ilpo Järvinen @ 2026-05-12 13:21 UTC (permalink / raw)
To: Jacques Nilo
Cc: Ilpo Järvinen, Greg Kroah-Hartman, Jiri Slaby, linux-serial,
LKML, Johan Hovold
In-Reply-To: <122f6431-2241-4367-be28-bfd3b31f5333@free.fr>
[-- Attachment #1: Type: text/plain, Size: 3670 bytes --]
On Tue, 12 May 2026, Jacques Nilo wrote:
> On Tue, 12 May 2026, Ilpo Järvinen wrote:
>
> > I seem to have come blind to the (unlock function) names. I'm sorry
> > about breaking this.
>
> No problem at all -- the asymmetry between the lock and unlock helper
> names is exactly the kind of thing that's easy to miss in a refactor.
>
> > 8250_dw's handle_irq also uses guard() which was the reason for this
> > change in the first place so it should be fixed as well.
>
> Confirmed -- dw8250_handle_irq() at drivers/tty/serial/8250/8250_dw.c:421
> does the same guard(uart_port_lock_irqsave)(p) before
> serial8250_handle_irq_locked(). Same bug.
>
> > > Option B -- fix the guard destructor in serial_core.h:
> >
> > There will be many such sites so this doesn't sound a great idea.
> >
> > I wonder why we couldn't instead do another DEFINE_GUARD() for the
> > sysrq case?
>
> Agreed, that's cleaner. The lock side is identical -- only the unlock
> needs the sysrq-aware variant -- so a second lock-guard macro keyed off
> the unlock destructor fits well:
>
> DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave_sysrq, struct uart_port,
> uart_port_lock_irqsave(_T->lock, &_T->flags),
> uart_unlock_and_check_sysrq_irqrestore(_T->lock,
> _T->flags),
> unsigned long flags);
>
> Callers that may capture sysrq_ch (currently serial8250_handle_irq and
> dw8250_handle_irq) opt in by using guard(uart_port_lock_irqsave_sysrq);
> the existing guard(uart_port_lock_irqsave) keeps its current plain-unlock
> semantics for everyone else.
>
> Naming-wise I'm not attached to "_sysrq" -- if you'd prefer something
> shorter (e.g. uart_port_rx_lock_irqsave) or aligned with another
> convention in the tree, happy to take direction.
I don't have strong preferences (I'm usually not good with names
anyway :-)). Somebody might object not placing _irqsave as last in the
name (and reversing the word order would be a bit inconsistent compared
with the unlock name).
> > I suppose thought the lockdep assert in serial8250_handle_irq_locked()
> > cannot discern that the correct one of them is being used by the
> > caller. But at least it's context comment should mention that the
> > correct guard/unlock variant should be used.
>
> Right -- both guards take port->lock so lockdep can't distinguish them.
> I'll update the Context: line on serial8250_handle_irq_locked() to spell
> out the requirement, e.g.:
>
> /*
> * Context: port's lock must be held by the caller, and must be released
> * via guard(uart_port_lock_irqsave_sysrq) or
> * uart_unlock_and_check_sysrq_irqrestore()
> so a SysRq character
> * captured by serial8250_read_char() is dispatched on unlock.
This would be less words to the same effect:
which captures SysRq character on unlock.
?
> */
>
> Plan, then, for a v1 patch series against tty-next:
>
> 1. include/linux/serial_core.h: add the new lock-guard macro.
> 2. drivers/tty/serial/8250/8250_port.c: switch serial8250_handle_irq()
> to guard(uart_port_lock_irqsave_sysrq); update the Context comment
> on serial8250_handle_irq_locked().
> 3. drivers/tty/serial/8250/8250_dw.c: switch dw8250_handle_irq() to
> the same guard.
>
> I'll mark it with Fixes: 8324a54f604d and Cc: stable for the kernels
> that carry the regression. Let me know if the naming or the Context
> wording wants adjusting before I send.
+
Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
--
i.
^ permalink raw reply
* Re: [REPORT] serial: 8250: BREAK + SysRq dispatch silently broken since 8324a54f604d
From: Jacques Nilo @ 2026-05-12 13:06 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold
In-Reply-To: <4bab4248-1c49-3f68-d327-fc00a4d7114b@linux.intel.com>
On Tue, 12 May 2026, Ilpo Järvinen wrote:
> I seem to have come blind to the (unlock function) names. I'm sorry
> about breaking this.
No problem at all -- the asymmetry between the lock and unlock helper
names is exactly the kind of thing that's easy to miss in a refactor.
> 8250_dw's handle_irq also uses guard() which was the reason for this
> change in the first place so it should be fixed as well.
Confirmed -- dw8250_handle_irq() at drivers/tty/serial/8250/8250_dw.c:421
does the same guard(uart_port_lock_irqsave)(p) before
serial8250_handle_irq_locked(). Same bug.
> > Option B -- fix the guard destructor in serial_core.h:
>
> There will be many such sites so this doesn't sound a great idea.
>
> I wonder why we couldn't instead do another DEFINE_GUARD() for the
> sysrq case?
Agreed, that's cleaner. The lock side is identical -- only the unlock
needs the sysrq-aware variant -- so a second lock-guard macro keyed off
the unlock destructor fits well:
DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave_sysrq, struct uart_port,
uart_port_lock_irqsave(_T->lock, &_T->flags),
uart_unlock_and_check_sysrq_irqrestore(_T->lock,
_T->flags),
unsigned long flags);
Callers that may capture sysrq_ch (currently serial8250_handle_irq and
dw8250_handle_irq) opt in by using guard(uart_port_lock_irqsave_sysrq);
the existing guard(uart_port_lock_irqsave) keeps its current plain-unlock
semantics for everyone else.
Naming-wise I'm not attached to "_sysrq" -- if you'd prefer something
shorter (e.g. uart_port_rx_lock_irqsave) or aligned with another
convention in the tree, happy to take direction.
> I suppose thought the lockdep assert in serial8250_handle_irq_locked()
> cannot discern that the correct one of them is being used by the
> caller. But at least it's context comment should mention that the
> correct guard/unlock variant should be used.
Right -- both guards take port->lock so lockdep can't distinguish them.
I'll update the Context: line on serial8250_handle_irq_locked() to spell
out the requirement, e.g.:
/*
* Context: port's lock must be held by the caller, and must be released
* via guard(uart_port_lock_irqsave_sysrq) or
* uart_unlock_and_check_sysrq_irqrestore() so a SysRq character
* captured by serial8250_read_char() is dispatched on unlock.
*/
Plan, then, for a v1 patch series against tty-next:
1. include/linux/serial_core.h: add the new lock-guard macro.
2. drivers/tty/serial/8250/8250_port.c: switch serial8250_handle_irq()
to guard(uart_port_lock_irqsave_sysrq); update the Context comment
on serial8250_handle_irq_locked().
3. drivers/tty/serial/8250/8250_dw.c: switch dw8250_handle_irq() to
the same guard.
I'll mark it with Fixes: 8324a54f604d and Cc: stable for the kernels
that carry the regression. Let me know if the naming or the Context
wording wants adjusting before I send.
Thanks for the quick read,
Jacques
Le 12/05/2026 à 14:58, Ilpo Järvinen a écrit :
> On Tue, 12 May 2026, Jacques Nilo wrote:
>
>> Hi,
>>
>> We hit what looks like a silent SysRq-over-serial regression on a 6.18
>> build of the 8250 driver. Posting as a report rather than a patch because
>> there are at least two reasonable fixes and I'd like a maintainer call
>> before sending one.
>>
>> Symptom
>> =======
>>
>> CONFIG_MAGIC_SYSRQ=y, CONFIG_MAGIC_SYSRQ_SERIAL=y,
>> CONFIG_SERIAL_8250_CONSOLE=y.
>>
>> A BREAK followed by a SysRq key on the console UART is consumed by the
>> kernel (BREAK counter in /proc/tty/driver/serial increments correctly)
>> but is never dispatched to handle_sysrq(). dmesg shows no "sysrq: ..."
>> line.
>>
>> `echo h > /proc/sysrq-trigger` still works, isolating the regression to
>> the serial input path. Verified end-to-end on an RTL8196E MIPS board
>> running 6.18.24; the affected code is in the generic 8250 core, so the
>> issue is not platform-specific.
>>
>> Path
>> ====
>>
>> serial8250_default_handle_irq()
>> -> serial8250_handle_irq() [8250_port.c:1835]
>> guard(uart_port_lock_irqsave)(port); [8250_port.c:1840]
>> serial8250_handle_irq_locked()
>> -> serial8250_rx_chars()
>> -> serial8250_read_char()
>> -> uart_handle_break() -- arms port->sysrq
>> -> uart_prepare_sysrq_char(port, ch) -- captures sysrq_ch
>> /* guard scope ends -> port unlock */
>>
>> The captured port->sysrq_ch is dispatched to handle_sysrq() at unlock
>> time -- but only by uart_unlock_and_check_sysrq[_irqrestore]() (see
>> include/linux/serial_core.h:1239). The scope guard's destructor at
>> serial_core.h:797 is plain uart_port_unlock_irqrestore(), which skips
>> the dispatch:
>>
>> DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
>> uart_port_lock_irqsave(_T->lock, &_T->flags),
>> uart_port_unlock_irqrestore(_T->lock, _T->flags),
>> unsigned long flags);
>>
>> So sysrq_ch stays in the struct until the next BREAK clears it.
>>
>> Bisection
>> =========
>>
>> commit 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
>>
>> Pre-split serial8250_handle_irq() used explicit uart_port_lock_irqsave()
>> + uart_unlock_and_check_sysrq_irqrestore(). The split moved the body into
>> _locked() and replaced the explicit lock pair with
>> guard(uart_port_lock_irqsave), losing the sysrq-aware unlock.
>>
>> This was the very condition Johan Hovold's 853a9ae29e978 ("serial: 8250:
>> fix handle_irq locking", 2021) introduced
>> uart_unlock_and_check_sysrq_irqrestore() to address -- the new helper was
>> deliberately the sysrq-aware variant. The guard() conversion undoes that
>> intent.
> I seem to have come blind to the (unlock function) names. I'm sorry about
> breaking this.
>
>> Reproducer
>> ==========
>>
>> On any 8250-driven console with CONFIG_MAGIC_SYSRQ_SERIAL=y:
>>
>> # On the host side:
>> python3 -c 'import os,fcntl,termios,time
>> fd=os.open("/dev/ttyUSB0",os.O_RDWR|os.O_NOCTTY)
>> fcntl.ioctl(fd,0x5427); time.sleep(0.3); fcntl.ioctl(fd,0x5428)
>> time.sleep(0.05); os.write(fd,b"h"); time.sleep(0.3)'
>>
>> # On the gateway:
>> grep brk /proc/tty/driver/serial # counter increments
>> dmesg | grep sysrq: # empty -- no dispatch
>>
>> Two ways to fix
>> ===============
>>
>> Option A -- surgical, only fix serial8250_handle_irq():
>>
>> int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>> {
>> unsigned long flags;
>>
>> if (iir & UART_IIR_NO_INT)
>> return 0;
>>
>> uart_port_lock_irqsave(port, &flags);
>> serial8250_handle_irq_locked(port, iir);
>> uart_unlock_and_check_sysrq_irqrestore(port, flags);
>>
>> return 1;
>> }
>>
>> Restores the pre-split behaviour. Doesn't touch the guard infrastructure.
>> Drawback: leaves uart_port_lock_irqsave() as a generic primitive that
>> silently swallows pending sysrq_ch in any other call site that processes
>> RX under the guard. There are no such sites today in 8250_port.c
>> (uart_prepare_sysrq_char is only reachable through serial8250_handle_irq),
>> but the trap remains.
> 8250_dw's handle_irq also uses guard() which was the reason for this
> change in the first place so it should be fixed as well.
>
>> Option B -- fix the guard destructor in serial_core.h:
>>
>> DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
>> uart_port_lock_irqsave(_T->lock, &_T->flags),
>> uart_unlock_and_check_sysrq_irqrestore(_T->lock,
>> _T->flags),
>> unsigned long flags);
>>
>> uart_unlock_and_check_sysrq_irqrestore() short-circuits to plain unlock
>> when !port->has_sysrq, so no overhead on non-sysrq ports. Fixes all
>> current and future guard(uart_port_lock_irqsave) users in one place.
>> Drawback: changes the semantics of a shared serial primitive. Some
>> callers in 8250_port.c run under that guard from non-RX contexts
>> (serial8250_set_mctrl, wait_for_xmitr, etc.); the only observable effect
>> there would be a one-time handle_sysrq() call if a previous BREAK left
>> sysrq_ch set -- functionally desirable, but a behaviour change worth
>> documenting.
> There will be many such sites so this doesn't sound a great idea.
>
> I wonder why we couldn't instead do another DEFINE_GUARD() for the sysrq
> case?
>
> I suppose thought the lockdep assert in serial8250_handle_irq_locked()
> cannot discern that the correct one of them is being used by the caller.
> But at least it's context comment should mention that the correct
> guard/unlock variant should be used.
>
>> I have a tested Option A patch against 6.18.24 (verified the dispatch
>> fires and produces the SysRq help dump). Happy to send it formally, or
>> to retarget to Option B if that's the preferred direction.
>>
>> Thanks,
>>
>> Jacques
>>
^ permalink raw reply
* Re: [REPORT] serial: 8250: BREAK + SysRq dispatch silently broken since 8324a54f604d
From: Ilpo Järvinen @ 2026-05-12 12:58 UTC (permalink / raw)
To: Jacques Nilo
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold
In-Reply-To: <5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr>
[-- Attachment #1: Type: text/plain, Size: 6161 bytes --]
On Tue, 12 May 2026, Jacques Nilo wrote:
> Hi,
>
> We hit what looks like a silent SysRq-over-serial regression on a 6.18
> build of the 8250 driver. Posting as a report rather than a patch because
> there are at least two reasonable fixes and I'd like a maintainer call
> before sending one.
>
> Symptom
> =======
>
> CONFIG_MAGIC_SYSRQ=y, CONFIG_MAGIC_SYSRQ_SERIAL=y,
> CONFIG_SERIAL_8250_CONSOLE=y.
>
> A BREAK followed by a SysRq key on the console UART is consumed by the
> kernel (BREAK counter in /proc/tty/driver/serial increments correctly)
> but is never dispatched to handle_sysrq(). dmesg shows no "sysrq: ..."
> line.
>
> `echo h > /proc/sysrq-trigger` still works, isolating the regression to
> the serial input path. Verified end-to-end on an RTL8196E MIPS board
> running 6.18.24; the affected code is in the generic 8250 core, so the
> issue is not platform-specific.
>
> Path
> ====
>
> serial8250_default_handle_irq()
> -> serial8250_handle_irq() [8250_port.c:1835]
> guard(uart_port_lock_irqsave)(port); [8250_port.c:1840]
> serial8250_handle_irq_locked()
> -> serial8250_rx_chars()
> -> serial8250_read_char()
> -> uart_handle_break() -- arms port->sysrq
> -> uart_prepare_sysrq_char(port, ch) -- captures sysrq_ch
> /* guard scope ends -> port unlock */
>
> The captured port->sysrq_ch is dispatched to handle_sysrq() at unlock
> time -- but only by uart_unlock_and_check_sysrq[_irqrestore]() (see
> include/linux/serial_core.h:1239). The scope guard's destructor at
> serial_core.h:797 is plain uart_port_unlock_irqrestore(), which skips
> the dispatch:
>
> DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
> uart_port_lock_irqsave(_T->lock, &_T->flags),
> uart_port_unlock_irqrestore(_T->lock, _T->flags),
> unsigned long flags);
>
> So sysrq_ch stays in the struct until the next BREAK clears it.
>
> Bisection
> =========
>
> commit 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
>
> Pre-split serial8250_handle_irq() used explicit uart_port_lock_irqsave()
> + uart_unlock_and_check_sysrq_irqrestore(). The split moved the body into
> _locked() and replaced the explicit lock pair with
> guard(uart_port_lock_irqsave), losing the sysrq-aware unlock.
>
> This was the very condition Johan Hovold's 853a9ae29e978 ("serial: 8250:
> fix handle_irq locking", 2021) introduced
> uart_unlock_and_check_sysrq_irqrestore() to address -- the new helper was
> deliberately the sysrq-aware variant. The guard() conversion undoes that
> intent.
I seem to have come blind to the (unlock function) names. I'm sorry about
breaking this.
> Reproducer
> ==========
>
> On any 8250-driven console with CONFIG_MAGIC_SYSRQ_SERIAL=y:
>
> # On the host side:
> python3 -c 'import os,fcntl,termios,time
> fd=os.open("/dev/ttyUSB0",os.O_RDWR|os.O_NOCTTY)
> fcntl.ioctl(fd,0x5427); time.sleep(0.3); fcntl.ioctl(fd,0x5428)
> time.sleep(0.05); os.write(fd,b"h"); time.sleep(0.3)'
>
> # On the gateway:
> grep brk /proc/tty/driver/serial # counter increments
> dmesg | grep sysrq: # empty -- no dispatch
>
> Two ways to fix
> ===============
>
> Option A -- surgical, only fix serial8250_handle_irq():
>
> int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> {
> unsigned long flags;
>
> if (iir & UART_IIR_NO_INT)
> return 0;
>
> uart_port_lock_irqsave(port, &flags);
> serial8250_handle_irq_locked(port, iir);
> uart_unlock_and_check_sysrq_irqrestore(port, flags);
>
> return 1;
> }
>
> Restores the pre-split behaviour. Doesn't touch the guard infrastructure.
> Drawback: leaves uart_port_lock_irqsave() as a generic primitive that
> silently swallows pending sysrq_ch in any other call site that processes
> RX under the guard. There are no such sites today in 8250_port.c
> (uart_prepare_sysrq_char is only reachable through serial8250_handle_irq),
> but the trap remains.
8250_dw's handle_irq also uses guard() which was the reason for this
change in the first place so it should be fixed as well.
> Option B -- fix the guard destructor in serial_core.h:
>
> DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
> uart_port_lock_irqsave(_T->lock, &_T->flags),
> uart_unlock_and_check_sysrq_irqrestore(_T->lock,
> _T->flags),
> unsigned long flags);
>
> uart_unlock_and_check_sysrq_irqrestore() short-circuits to plain unlock
> when !port->has_sysrq, so no overhead on non-sysrq ports. Fixes all
> current and future guard(uart_port_lock_irqsave) users in one place.
> Drawback: changes the semantics of a shared serial primitive. Some
> callers in 8250_port.c run under that guard from non-RX contexts
> (serial8250_set_mctrl, wait_for_xmitr, etc.); the only observable effect
> there would be a one-time handle_sysrq() call if a previous BREAK left
> sysrq_ch set -- functionally desirable, but a behaviour change worth
> documenting.
There will be many such sites so this doesn't sound a great idea.
I wonder why we couldn't instead do another DEFINE_GUARD() for the sysrq
case?
I suppose thought the lockdep assert in serial8250_handle_irq_locked()
cannot discern that the correct one of them is being used by the caller.
But at least it's context comment should mention that the correct
guard/unlock variant should be used.
> I have a tested Option A patch against 6.18.24 (verified the dispatch
> fires and produces the SysRq help dump). Happy to send it formally, or
> to retarget to Option B if that's the preferred direction.
>
> Thanks,
>
> Jacques
>
--
i.
^ permalink raw reply
* [REPORT] serial: 8250: BREAK + SysRq dispatch silently broken since 8324a54f604d
From: Jacques Nilo @ 2026-05-12 12:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold
Hi,
We hit what looks like a silent SysRq-over-serial regression on a 6.18
build of the 8250 driver. Posting as a report rather than a patch because
there are at least two reasonable fixes and I'd like a maintainer call
before sending one.
Symptom
=======
CONFIG_MAGIC_SYSRQ=y, CONFIG_MAGIC_SYSRQ_SERIAL=y,
CONFIG_SERIAL_8250_CONSOLE=y.
A BREAK followed by a SysRq key on the console UART is consumed by the
kernel (BREAK counter in /proc/tty/driver/serial increments correctly)
but is never dispatched to handle_sysrq(). dmesg shows no "sysrq: ..."
line.
`echo h > /proc/sysrq-trigger` still works, isolating the regression to
the serial input path. Verified end-to-end on an RTL8196E MIPS board
running 6.18.24; the affected code is in the generic 8250 core, so the
issue is not platform-specific.
Path
====
serial8250_default_handle_irq()
-> serial8250_handle_irq() [8250_port.c:1835]
guard(uart_port_lock_irqsave)(port); [8250_port.c:1840]
serial8250_handle_irq_locked()
-> serial8250_rx_chars()
-> serial8250_read_char()
-> uart_handle_break() -- arms port->sysrq
-> uart_prepare_sysrq_char(port, ch) -- captures
sysrq_ch
/* guard scope ends -> port unlock */
The captured port->sysrq_ch is dispatched to handle_sysrq() at unlock
time -- but only by uart_unlock_and_check_sysrq[_irqrestore]() (see
include/linux/serial_core.h:1239). The scope guard's destructor at
serial_core.h:797 is plain uart_port_unlock_irqrestore(), which skips
the dispatch:
DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
uart_port_lock_irqsave(_T->lock, &_T->flags),
uart_port_unlock_irqrestore(_T->lock, _T->flags),
unsigned long flags);
So sysrq_ch stays in the struct until the next BREAK clears it.
Bisection
=========
commit 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Pre-split serial8250_handle_irq() used explicit uart_port_lock_irqsave()
+ uart_unlock_and_check_sysrq_irqrestore(). The split moved the body into
_locked() and replaced the explicit lock pair with
guard(uart_port_lock_irqsave), losing the sysrq-aware unlock.
This was the very condition Johan Hovold's 853a9ae29e978 ("serial: 8250:
fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address -- the new helper was
deliberately the sysrq-aware variant. The guard() conversion undoes that
intent.
Reproducer
==========
On any 8250-driven console with CONFIG_MAGIC_SYSRQ_SERIAL=y:
# On the host side:
python3 -c 'import os,fcntl,termios,time
fd=os.open("/dev/ttyUSB0",os.O_RDWR|os.O_NOCTTY)
fcntl.ioctl(fd,0x5427); time.sleep(0.3); fcntl.ioctl(fd,0x5428)
time.sleep(0.05); os.write(fd,b"h"); time.sleep(0.3)'
# On the gateway:
grep brk /proc/tty/driver/serial # counter increments
dmesg | grep sysrq: # empty -- no dispatch
Two ways to fix
===============
Option A -- surgical, only fix serial8250_handle_irq():
int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
{
unsigned long flags;
if (iir & UART_IIR_NO_INT)
return 0;
uart_port_lock_irqsave(port, &flags);
serial8250_handle_irq_locked(port, iir);
uart_unlock_and_check_sysrq_irqrestore(port, flags);
return 1;
}
Restores the pre-split behaviour. Doesn't touch the guard infrastructure.
Drawback: leaves uart_port_lock_irqsave() as a generic primitive that
silently swallows pending sysrq_ch in any other call site that processes
RX under the guard. There are no such sites today in 8250_port.c
(uart_prepare_sysrq_char is only reachable through serial8250_handle_irq),
but the trap remains.
Option B -- fix the guard destructor in serial_core.h:
DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
uart_port_lock_irqsave(_T->lock, &_T->flags),
uart_unlock_and_check_sysrq_irqrestore(_T->lock,
_T->flags),
unsigned long flags);
uart_unlock_and_check_sysrq_irqrestore() short-circuits to plain unlock
when !port->has_sysrq, so no overhead on non-sysrq ports. Fixes all
current and future guard(uart_port_lock_irqsave) users in one place.
Drawback: changes the semantics of a shared serial primitive. Some
callers in 8250_port.c run under that guard from non-RX contexts
(serial8250_set_mctrl, wait_for_xmitr, etc.); the only observable effect
there would be a one-time handle_sysrq() call if a previous BREAK left
sysrq_ch set -- functionally desirable, but a behaviour change worth
documenting.
I have a tested Option A patch against 6.18.24 (verified the dispatch
fires and produces the SysRq help dump). Happy to send it formally, or
to retarget to Option B if that's the preferred direction.
Thanks,
Jacques
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox