[PATCH 0/2] serial: qcom-geni: Convert console to nbcon and handle RX overflow

[PATCH 0/2] serial: qcom-geni: Convert console to nbcon and handle RX overflow

[Bjorn Andersson]

The Qualcomm GENI UART uses independent sequencers. The main
sequencer handles console TX and the secondary sequencer handles
FIFO RX.

The legacy console writer disables both interrupt domains while it
performs a potentially long polled M-side transfer. Besides blocking
printk callers for UART wire time, this leaves the small S-side FIFO
unserviced and can lose interactive console input through FIFO overrun.

This two-patch series converts the console to nbcon, then keep FIFO RX
active while console output is in progress.

Patch 1 adds nbcon threaded and atomic writers. It uses bounded
32-byte M-side commands so higher-priority atomic output can take over
without waiting for a complete printk record. Atomic output may cancel
normal M-side TX. The driver restarts queued TTY output with irq_work,
honors flow control, and prevents deferred work from accessing a shut
down or removed port.

Patch 2 keeps FIFO RX on the secondary sequencer and drains it while
the threaded writer refills console TX. It also defers SysRq delivery
from this new receive context until the UART port lock is released.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
Bjorn Andersson (2):
      serial: qcom-geni: Convert console to nbcon
      serial: qcom-geni: Keep FIFO RX active during console TX

 drivers/tty/serial/qcom_geni_serial.c | 272 ++++++++++++++++++++++++++--------
 1 file changed, 212 insertions(+), 60 deletions(-)
---
base-commit: 78bc8af4affb9a732504eb22eeac7d1e50883853
change-id: 20260723-qcom-geni-nbcon-55816a833d8c

Best regards,
--  
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

[PATCH 1/2] serial: qcom-geni: Convert console to nbcon

[Bjorn Andersson]

The legacy GENI console writer serializes every message around a
synchronous polled M-side transfer. It blocks printk callers for UART
wire time and cannot provide atomic output while normal console output is
active.

Convert the console to nbcon threaded and atomic writers. Use the UART
port lock as the device lock and bound threaded M-side commands so
urgent diagnostics can take over atomic output. The threaded writer is
batching the output in 32-source-byte commands, a value chosen to
balance the command setup overhead with atomic-handoff latency.

Atomic output can cancel an active normal TX command. Use irq_work to
resume queued TTY output afterward, honor flow control, and prevent the
deferred restart from accessing the port during shutdown or removal.

Assisted-by: OpenCode:GPT-5.5
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 166 ++++++++++++++++++++++++++++------
 1 file changed, 140 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 67b14fda4ff9..08427390c173 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/irq.h>
+#include <linux/irq_work.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/panic_notifier.h>
@@ -89,6 +90,7 @@
 #define DEF_TX_WM			2
 #define DEF_FIFO_WIDTH_BITS		32
 #define UART_RX_WM			2
+#define CONSOLE_TX_CHUNK_SIZE		32
 
 /* SE_UART_LOOPBACK_CFG */
 #define RX_TX_SORTED			BIT(0)
@@ -153,7 +155,8 @@ struct qcom_geni_serial_port {
 	bool rx_tx_swap;
 	bool cts_rts_swap;
 	bool manual_flow;
-
+	bool tx_kick_enabled;
+	struct irq_work tx_kick;
 	struct qcom_geni_private_data private_data;
 	const struct qcom_geni_device_data *dev_data;
 	struct dev_pm_domain_list *pd_list;
@@ -168,6 +171,8 @@ static struct uart_driver qcom_geni_uart_driver;
 static void __qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
 static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
 static int qcom_geni_serial_port_setup(struct uart_port *uport);
+static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport);
+static void qcom_geni_serial_resume_tx(struct uart_port *uport);
 
 static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
 {
@@ -539,49 +544,123 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
 	qcom_geni_serial_poll_tx_done(uport);
 }
 
-static void qcom_geni_serial_console_write(struct console *co, const char *s,
-			      unsigned int count)
+static void qcom_geni_serial_console_takeover(struct uart_port *uport,
+					       bool preserve_tx)
+{
+	if (qcom_geni_serial_main_active(uport)) {
+		struct qcom_geni_serial_port *port = to_dev_port(uport);
+
+		if (preserve_tx) {
+			if (port->tx_remaining == 0)
+				qcom_geni_serial_poll_tx_done(uport);
+			else
+				qcom_geni_serial_drain_fifo(uport);
+		}
+
+		qcom_geni_serial_cancel_tx_cmd(uport);
+	}
+}
+
+static void qcom_geni_serial_console_write_thread(struct console *co,
+						  struct nbcon_write_context *wctxt)
 {
+	struct qcom_geni_serial_port *port;
 	struct uart_port *uport;
+	unsigned int offset = 0;
+
+	port = get_port_from_line(co->index, true, NULL);
+	if (IS_ERR(port))
+		return;
+
+	uport = &port->uport;
+	if (!nbcon_enter_unsafe(wctxt))
+		return;
+
+	qcom_geni_serial_console_takeover(uport, true);
+	if (!nbcon_exit_unsafe(wctxt))
+		return;
+
+	while (offset < wctxt->len) {
+		/*
+		 * Printk records can be much larger than the FIFO. Limit one
+		 * M-side command to 32 source bytes so atomic console output can
+		 * take over between commands instead of waiting for the record.
+		 */
+		unsigned int count = min_t(unsigned int, wctxt->len - offset,
+						   CONSOLE_TX_CHUNK_SIZE);
+
+		if (!nbcon_enter_unsafe(wctxt))
+			return;
+
+		__qcom_geni_serial_console_write(uport, wctxt->outbuf + offset,
+						 count);
+		offset += count;
+
+		if (!nbcon_exit_unsafe(wctxt))
+			return;
+	}
+
+	if (!nbcon_enter_unsafe(wctxt))
+		return;
+
+	qcom_geni_serial_resume_tx(uport);
+	nbcon_exit_unsafe(wctxt);
+}
+
+static void qcom_geni_serial_console_write_atomic(struct console *co,
+						  struct nbcon_write_context *wctxt)
+{
 	struct qcom_geni_serial_port *port;
+	struct uart_port *uport;
 	u32 m_irq_en, s_irq_en;
-	bool locked = true;
-	unsigned long flags;
-
-	WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
 
 	port = get_port_from_line(co->index, true, NULL);
 	if (IS_ERR(port))
 		return;
 
 	uport = &port->uport;
-	if (oops_in_progress)
-		locked = uart_port_trylock_irqsave(uport, &flags);
-	else
-		uart_port_lock_irqsave(uport, &flags);
+	if (!nbcon_enter_unsafe(wctxt))
+		return;
 
 	m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
 	s_irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
 	writel(0, uport->membase + SE_GENI_M_IRQ_EN);
 	writel(0, uport->membase + SE_GENI_S_IRQ_EN);
 
-	if (qcom_geni_serial_main_active(uport)) {
-		/* Wait for completion or drain FIFO */
-		if (!locked || port->tx_remaining == 0)
-			qcom_geni_serial_poll_tx_done(uport);
-		else
-			qcom_geni_serial_drain_fifo(uport);
-
-		qcom_geni_serial_cancel_tx_cmd(uport);
-	}
-
-	__qcom_geni_serial_console_write(uport, s, count);
+	qcom_geni_serial_console_takeover(uport, false);
+	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len);
 
 	writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
 	writel(s_irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+	nbcon_exit_unsafe(wctxt);
 
-	if (locked)
-		uart_port_unlock_irqrestore(uport, flags);
+	/* Restart TTY data left queued when atomic output canceled M TX. */
+	if (READ_ONCE(port->tx_kick_enabled))
+		irq_work_queue(&port->tx_kick);
+}
+
+static void qcom_geni_serial_console_device_lock(struct console *co,
+						  unsigned long *flags)
+{
+	struct qcom_geni_serial_port *port;
+
+	port = get_port_from_line(co->index, true, NULL);
+	if (IS_ERR(port))
+		return;
+
+	__uart_port_lock_irqsave(&port->uport, flags);
+}
+
+static void qcom_geni_serial_console_device_unlock(struct console *co,
+						    unsigned long flags)
+{
+	struct qcom_geni_serial_port *port;
+
+	port = get_port_from_line(co->index, true, NULL);
+	if (IS_ERR(port))
+		return;
+
+	__uart_port_unlock_irqrestore(&port->uport, flags);
 }
 
 static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
@@ -738,6 +817,29 @@ static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport)
 	writel(irq_en, uport->membase +	SE_GENI_M_IRQ_EN);
 }
 
+/* Caller holds the UART port lock. */
+static void qcom_geni_serial_resume_tx(struct uart_port *uport)
+{
+	if (!uart_tx_stopped(uport) &&
+	    !kfifo_is_empty(&uport->state->port.xmit_fifo))
+		qcom_geni_serial_start_tx_fifo(uport);
+}
+
+static void qcom_geni_serial_restart_tx(struct irq_work *work)
+{
+	struct qcom_geni_serial_port *port = container_of(work,
+						      struct qcom_geni_serial_port, tx_kick);
+	struct uart_port *uport = &port->uport;
+
+	if (!READ_ONCE(port->tx_kick_enabled) || !uport->state || uport->suspended)
+		return;
+
+	uart_port_lock(uport);
+	if (READ_ONCE(port->tx_kick_enabled) && uport->state && !uport->suspended)
+		qcom_geni_serial_resume_tx(uport);
+	uart_port_unlock(uport);
+}
+
 static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport)
 {
 	u32 irq_en;
@@ -1182,6 +1284,11 @@ static int setup_fifos(struct qcom_geni_serial_port *port)
 
 static void qcom_geni_serial_shutdown(struct uart_port *uport)
 {
+	struct qcom_geni_serial_port *port = to_dev_port(uport);
+
+	/* Atomic console output queues tx_kick without taking the port lock. */
+	WRITE_ONCE(port->tx_kick_enabled, false);
+	irq_work_sync(&port->tx_kick);
 	disable_irq(uport->irq);
 
 	uart_port_lock_irq(uport);
@@ -1281,6 +1388,7 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
 	uart_port_unlock_irq(uport);
 
 	enable_irq(uport->irq);
+	WRITE_ONCE(port->tx_kick_enabled, true);
 
 	return 0;
 }
@@ -1627,10 +1735,13 @@ static void console_unregister(struct uart_driver *drv)
 
 static struct console cons_ops = {
 	.name = "ttyMSM",
-	.write = qcom_geni_serial_console_write,
+	.write_atomic = qcom_geni_serial_console_write_atomic,
+	.write_thread = qcom_geni_serial_console_write_thread,
+	.device_lock = qcom_geni_serial_console_device_lock,
+	.device_unlock = qcom_geni_serial_console_device_unlock,
 	.device = uart_console_device,
 	.setup = qcom_geni_console_setup,
-	.flags = CON_PRINTBUFFER,
+	.flags = CON_PRINTBUFFER | CON_NBCON,
 	.index = -1,
 	.data = &qcom_geni_console_driver,
 };
@@ -1946,6 +2057,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 
 	port->private_data.drv = drv;
 	uport->private_data = &port->private_data;
+	init_irq_work(&port->tx_kick, qcom_geni_serial_restart_tx);
 	platform_set_drvdata(pdev, port);
 
 	irq_set_status_flags(uport->irq, IRQ_NOAUTOEN);
@@ -2000,6 +2112,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
 
 	atomic_notifier_chain_unregister(&panic_notifier_list, &port->panic_nb);
 
+	WRITE_ONCE(port->tx_kick_enabled, false);
+	irq_work_sync(&port->tx_kick);
 	dev_pm_clear_wake_irq(&pdev->dev);
 	device_init_wakeup(&pdev->dev, false);
 	ida_free(&port_ida, uport->line);

-- 
2.53.0

[PATCH 2/2] serial: qcom-geni: Keep FIFO RX active during console TX

[Bjorn Andersson]

The GENI main sequencer handles console TX while the secondary sequencer
handles FIFO RX. Before nbcon, the legacy console writer disabled both
interrupt domains while it performed a long polled M-side transfer. This
left the small S-side FIFO unserviced, allowing console input to overrun
and be lost.

The nbcon conversion replaces IRQ masking with the UART port lock, but a
threaded console write still prevents the RX handler from draining the
FIFO. Keep S-side RX enabled independently of M-side TX and drain it
while refilling each bounded console command. This preserves interactive
input during console output.

Atomic output masks only M-side TX state, leaving FIFO RX handling
independent. The threaded writer can also detect a SysRq character while
it drains RX, so defer delivery until device_unlock() drops the UART port
lock, as the existing IRQ path does with uart_unlock_and_check_sysrq().

Assisted-by: OpenCode:GPT-5.5
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 112 +++++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 37 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 08427390c173..a473d5521067 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -173,6 +173,7 @@ static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
 static int qcom_geni_serial_port_setup(struct uart_port *uport);
 static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport);
 static void qcom_geni_serial_resume_tx(struct uart_port *uport);
+static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport);
 
 static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
 {
@@ -493,7 +494,7 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, unsigned char ch)
 
 static void
 __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
-				 unsigned int count)
+				 unsigned int count, bool poll_rx)
 {
 	struct qcom_geni_private_data *private_data = uport->private_data;
 
@@ -525,6 +526,8 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
 		if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
 						M_TX_FIFO_WATERMARK_EN, true))
 			break;
+		if (poll_rx)
+			qcom_geni_serial_poll_rx_fifo_locked(uport);
 		chars_to_write = min_t(size_t, count - i, avail / 2);
 		uart_console_write(uport, s + i, chars_to_write,
 						qcom_geni_serial_wr_char);
@@ -593,7 +596,7 @@ static void qcom_geni_serial_console_write_thread(struct console *co,
 			return;
 
 		__qcom_geni_serial_console_write(uport, wctxt->outbuf + offset,
-						 count);
+						 count, true);
 		offset += count;
 
 		if (!nbcon_exit_unsafe(wctxt))
@@ -612,7 +615,7 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
 {
 	struct qcom_geni_serial_port *port;
 	struct uart_port *uport;
-	u32 m_irq_en, s_irq_en;
+	u32 m_irq_en;
 
 	port = get_port_from_line(co->index, true, NULL);
 	if (IS_ERR(port))
@@ -623,15 +626,14 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
 		return;
 
 	m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
-	s_irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
-	writel(0, uport->membase + SE_GENI_M_IRQ_EN);
-	writel(0, uport->membase + SE_GENI_S_IRQ_EN);
+	writel(m_irq_en & ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN),
+		uport->membase + SE_GENI_M_IRQ_EN);
 
+	/* Atomic console output takes priority over an active normal TX command. */
 	qcom_geni_serial_console_takeover(uport, false);
-	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len);
+	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len, false);
 
 	writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
-	writel(s_irq_en, uport->membase + SE_GENI_S_IRQ_EN);
 	nbcon_exit_unsafe(wctxt);
 
 	/* Restart TTY data left queued when atomic output canceled M TX. */
@@ -655,12 +657,25 @@ static void qcom_geni_serial_console_device_unlock(struct console *co,
 						    unsigned long flags)
 {
 	struct qcom_geni_serial_port *port;
+#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
+	u8 sysrq_ch;
+#endif
 
 	port = get_port_from_line(co->index, true, NULL);
 	if (IS_ERR(port))
 		return;
 
+#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
+	/* The threaded console writer can receive a SysRq character. */
+	sysrq_ch = port->uport.sysrq_ch;
+	port->uport.sysrq_ch = 0;
+#endif
 	__uart_port_unlock_irqrestore(&port->uport, flags);
+
+#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
+	if (sysrq_ch)
+		handle_sysrq(sysrq_ch);
+#endif
 }
 
 static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
@@ -902,6 +917,34 @@ static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool drop)
 	handle_rx_console(uport, total_bytes, drop);
 }
 
+/* Caller holds the UART port lock. */
+static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport)
+{
+	struct qcom_geni_serial_port *port = to_dev_port(uport);
+	struct tty_port *tport = &uport->state->port;
+	u32 s_irq_status;
+	bool drop_rx = false;
+
+	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
+	writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
+
+	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
+		uport->icount.overrun++;
+		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
+	}
+
+	if (s_irq_status & (S_GP_IRQ_0_EN | S_GP_IRQ_1_EN)) {
+		if (s_irq_status & S_GP_IRQ_0_EN)
+			uport->icount.parity++;
+		drop_rx = true;
+	} else if (s_irq_status & (S_GP_IRQ_2_EN | S_GP_IRQ_3_EN)) {
+		uport->icount.brk++;
+		port->brk = true;
+	}
+
+	qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
+}
+
 static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
 {
 	u32 irq_en;
@@ -912,10 +955,6 @@ static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
 	irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
 	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
 
-	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
-	irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
-	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
-
 	if (!qcom_geni_serial_secondary_active(uport))
 		return;
 
@@ -949,10 +988,6 @@ static void qcom_geni_serial_start_rx_fifo(struct uart_port *uport)
 	irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
 	irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
 	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
-
-	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
-	irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
-	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
 }
 
 static void qcom_geni_serial_stop_rx_dma(struct uart_port *uport)
@@ -1182,25 +1217,11 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
 
 	uart_port_lock(uport);
 
-	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
 	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
-	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
 	dma_rx_status = readl(uport->membase + SE_DMA_RX_IRQ_STAT);
-	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);
 	writel(dma_rx_status, uport->membase + SE_DMA_RX_IRQ_CLR);
 
-	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
-		goto out_unlock;
-
 	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
 		uport->icount.overrun++;
 		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
@@ -1215,12 +1236,35 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
 		port->brk = true;
 	}
 
+	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
+	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
+	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(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
+
+	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
+		goto handle_rx;
+
 	if (dma) {
 		if (dma_tx_status & TX_DMA_DONE) {
 			qcom_geni_serial_handle_tx_dma(uport);
 			qcom_geni_set_rs485_mode(uport, SER_RS485_RTS_AFTER_SEND);
+		}
+	} else if (m_irq_status & m_irq_en &
+		   (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN)) {
+		qcom_geni_serial_handle_tx_fifo(uport,
+				m_irq_status & M_CMD_DONE_EN,
+				geni_status & M_GENI_CMD_ACTIVE);
 	}
 
+handle_rx:
+	if (dma) {
 		if (dma_rx_status) {
 			if (dma_rx_status & RX_RESET_DONE)
 				goto out_unlock;
@@ -1237,12 +1281,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
 				qcom_geni_serial_handle_rx_dma(uport, drop_rx);
 		}
 	} else {
-		if (m_irq_status & m_irq_en &
-		    (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
-			qcom_geni_serial_handle_tx_fifo(uport,
-					m_irq_status & M_CMD_DONE_EN,
-					geni_status & M_GENI_CMD_ACTIVE);
-
 		if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
 			qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
 	}
@@ -1632,7 +1670,7 @@ static void qcom_geni_serial_earlycon_write(struct console *con,
 {
 	struct earlycon_device *dev = con->data;
 
-	__qcom_geni_serial_console_write(&dev->port, s, n);
+	__qcom_geni_serial_console_write(&dev->port, s, n, false);
 }
 
 #ifdef CONFIG_CONSOLE_POLL

-- 
2.53.0

Re: [PATCH 1/2] serial: qcom-geni: Convert console to nbcon

[Konrad Dybcio]

On 7/29/26 11:44 PM, Bjorn Andersson wrote:
> The legacy GENI console writer serializes every message around a
> synchronous polled M-side transfer. It blocks printk callers for UART
> wire time and cannot provide atomic output while normal console output is
> active.
> 
> Convert the console to nbcon threaded and atomic writers. Use the UART
> port lock as the device lock and bound threaded M-side commands so
> urgent diagnostics can take over atomic output. The threaded writer is
> batching the output in 32-source-byte commands, a value chosen to
> balance the command setup overhead with atomic-handoff latency.
> 
> Atomic output can cancel an active normal TX command. Use irq_work to
> resume queued TTY output afterward, honor flow control, and prevent the
> deferred restart from accessing the port during shutdown or removal.
> 
> Assisted-by: OpenCode:GPT-5.5
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---

[...]

> +	while (offset < wctxt->len) {

Other nbcon drivers access wctxt->len in write_thread with READ_ONCE,
I believe we need to do the same

[...]

> +static void qcom_geni_serial_console_write_atomic(struct console *co,
> +						  struct nbcon_write_context *wctxt)
> +{
>  	struct qcom_geni_serial_port *port;
> +	struct uart_port *uport;
>  	u32 m_irq_en, s_irq_en;
> -	bool locked = true;
> -	unsigned long flags;
> -
> -	WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);

Unrelated?

[...]

> +/* Caller holds the UART port lock. */
> +static void qcom_geni_serial_resume_tx(struct uart_port *uport)
> +{
> +	if (!uart_tx_stopped(uport) &&
> +	    !kfifo_is_empty(&uport->state->port.xmit_fifo))
> +		qcom_geni_serial_start_tx_fifo(uport);
> +}

This is a single-use oneliner, but perhaps having 3+2 conditions
is better than having 5 conditions to an if statement..


> +
> +static void qcom_geni_serial_restart_tx(struct irq_work *work)
> +{
> +	struct qcom_geni_serial_port *port = container_of(work,
> +						      struct qcom_geni_serial_port, tx_kick);
> +	struct uart_port *uport = &port->uport;
> +
> +	if (!READ_ONCE(port->tx_kick_enabled) || !uport->state || uport->suspended)
> +		return;
> +
> +	uart_port_lock(uport);
> +	if (READ_ONCE(port->tx_kick_enabled) && uport->state && !uport->suspended)
> +		qcom_geni_serial_resume_tx(uport);
> +	uart_port_unlock(uport);
> +}
> +
>  static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport)
>  {
>  	u32 irq_en;
> @@ -1182,6 +1284,11 @@ static int setup_fifos(struct qcom_geni_serial_port *port)
>  
>  static void qcom_geni_serial_shutdown(struct uart_port *uport)
>  {
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +
> +	/* Atomic console output queues tx_kick without taking the port lock. */

non-atomic?

Konrad

Re: [PATCH 2/2] serial: qcom-geni: Keep FIFO RX active during console TX

[Konrad Dybcio]

On 7/29/26 11:44 PM, Bjorn Andersson wrote:
> The GENI main sequencer handles console TX while the secondary sequencer
> handles FIFO RX. Before nbcon, the legacy console writer disabled both
> interrupt domains while it performed a long polled M-side transfer. This
> left the small S-side FIFO unserviced, allowing console input to overrun
> and be lost.
> 
> The nbcon conversion replaces IRQ masking with the UART port lock, but a
> threaded console write still prevents the RX handler from draining the
> FIFO. Keep S-side RX enabled independently of M-side TX and drain it
> while refilling each bounded console command. This preserves interactive
> input during console output.
> 
> Atomic output masks only M-side TX state, leaving FIFO RX handling
> independent. The threaded writer can also detect a SysRq character while
> it drains RX, so defer delivery until device_unlock() drops the UART port
> lock, as the existing IRQ path does with uart_unlock_and_check_sysrq().
> 
> Assisted-by: OpenCode:GPT-5.5
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---

I think this looks alright, although my ability here to judge is not
superb..

+Cc Mukesh and the reviewers list

Konrad

>  drivers/tty/serial/qcom_geni_serial.c | 112 +++++++++++++++++++++++-----------
>  1 file changed, 75 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 08427390c173..a473d5521067 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -173,6 +173,7 @@ static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>  static int qcom_geni_serial_port_setup(struct uart_port *uport);
>  static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport);
>  static void qcom_geni_serial_resume_tx(struct uart_port *uport);
> +static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport);
>  
>  static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
>  {
> @@ -493,7 +494,7 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, unsigned char ch)
>  
>  static void
>  __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
> -				 unsigned int count)
> +				 unsigned int count, bool poll_rx)
>  {
>  	struct qcom_geni_private_data *private_data = uport->private_data;
>  
> @@ -525,6 +526,8 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
>  		if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
>  						M_TX_FIFO_WATERMARK_EN, true))
>  			break;
> +		if (poll_rx)
> +			qcom_geni_serial_poll_rx_fifo_locked(uport);
>  		chars_to_write = min_t(size_t, count - i, avail / 2);
>  		uart_console_write(uport, s + i, chars_to_write,
>  						qcom_geni_serial_wr_char);
> @@ -593,7 +596,7 @@ static void qcom_geni_serial_console_write_thread(struct console *co,
>  			return;
>  
>  		__qcom_geni_serial_console_write(uport, wctxt->outbuf + offset,
> -						 count);
> +						 count, true);
>  		offset += count;
>  
>  		if (!nbcon_exit_unsafe(wctxt))
> @@ -612,7 +615,7 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
>  {
>  	struct qcom_geni_serial_port *port;
>  	struct uart_port *uport;
> -	u32 m_irq_en, s_irq_en;
> +	u32 m_irq_en;
>  
>  	port = get_port_from_line(co->index, true, NULL);
>  	if (IS_ERR(port))
> @@ -623,15 +626,14 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
>  		return;
>  
>  	m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	s_irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
> -	writel(0, uport->membase + SE_GENI_M_IRQ_EN);
> -	writel(0, uport->membase + SE_GENI_S_IRQ_EN);
> +	writel(m_irq_en & ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN),
> +		uport->membase + SE_GENI_M_IRQ_EN);
>  
> +	/* Atomic console output takes priority over an active normal TX command. */
>  	qcom_geni_serial_console_takeover(uport, false);
> -	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len);
> +	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len, false);
>  
>  	writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> -	writel(s_irq_en, uport->membase + SE_GENI_S_IRQ_EN);
>  	nbcon_exit_unsafe(wctxt);
>  
>  	/* Restart TTY data left queued when atomic output canceled M TX. */
> @@ -655,12 +657,25 @@ static void qcom_geni_serial_console_device_unlock(struct console *co,
>  						    unsigned long flags)
>  {
>  	struct qcom_geni_serial_port *port;
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	u8 sysrq_ch;
> +#endif
>  
>  	port = get_port_from_line(co->index, true, NULL);
>  	if (IS_ERR(port))
>  		return;
>  
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	/* The threaded console writer can receive a SysRq character. */
> +	sysrq_ch = port->uport.sysrq_ch;
> +	port->uport.sysrq_ch = 0;
> +#endif
>  	__uart_port_unlock_irqrestore(&port->uport, flags);
> +
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	if (sysrq_ch)
> +		handle_sysrq(sysrq_ch);
> +#endif
>  }
>  
>  static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
> @@ -902,6 +917,34 @@ static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool drop)
>  	handle_rx_console(uport, total_bytes, drop);
>  }
>  
> +/* Caller holds the UART port lock. */
> +static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport)
> +{
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +	struct tty_port *tport = &uport->state->port;
> +	u32 s_irq_status;
> +	bool drop_rx = false;
> +
> +	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
> +	writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
> +
> +	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
> +		uport->icount.overrun++;
> +		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
> +	}
> +
> +	if (s_irq_status & (S_GP_IRQ_0_EN | S_GP_IRQ_1_EN)) {
> +		if (s_irq_status & S_GP_IRQ_0_EN)
> +			uport->icount.parity++;
> +		drop_rx = true;
> +	} else if (s_irq_status & (S_GP_IRQ_2_EN | S_GP_IRQ_3_EN)) {
> +		uport->icount.brk++;
> +		port->brk = true;
> +	}
> +
> +	qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
> +}
> +
>  static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
>  {
>  	u32 irq_en;
> @@ -912,10 +955,6 @@ static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
>  	irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
>  	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
>  
> -	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
> -	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> -
>  	if (!qcom_geni_serial_secondary_active(uport))
>  		return;
>  
> @@ -949,10 +988,6 @@ static void qcom_geni_serial_start_rx_fifo(struct uart_port *uport)
>  	irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
>  	irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
>  	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
> -
> -	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
> -	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
>  }
>  
>  static void qcom_geni_serial_stop_rx_dma(struct uart_port *uport)
> @@ -1182,25 +1217,11 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>  
>  	uart_port_lock(uport);
>  
> -	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
>  	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
> -	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
>  	dma_rx_status = readl(uport->membase + SE_DMA_RX_IRQ_STAT);
> -	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);
>  	writel(dma_rx_status, uport->membase + SE_DMA_RX_IRQ_CLR);
>  
> -	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
> -		goto out_unlock;
> -
>  	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
>  		uport->icount.overrun++;
>  		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
> @@ -1215,12 +1236,35 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>  		port->brk = true;
>  	}
>  
> +	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
> +	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
> +	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(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
> +
> +	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
> +		goto handle_rx;
> +
>  	if (dma) {
>  		if (dma_tx_status & TX_DMA_DONE) {
>  			qcom_geni_serial_handle_tx_dma(uport);
>  			qcom_geni_set_rs485_mode(uport, SER_RS485_RTS_AFTER_SEND);
> +		}
> +	} else if (m_irq_status & m_irq_en &
> +		   (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN)) {
> +		qcom_geni_serial_handle_tx_fifo(uport,
> +				m_irq_status & M_CMD_DONE_EN,
> +				geni_status & M_GENI_CMD_ACTIVE);
>  	}
>  
> +handle_rx:
> +	if (dma) {
>  		if (dma_rx_status) {
>  			if (dma_rx_status & RX_RESET_DONE)
>  				goto out_unlock;
> @@ -1237,12 +1281,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>  				qcom_geni_serial_handle_rx_dma(uport, drop_rx);
>  		}
>  	} else {
> -		if (m_irq_status & m_irq_en &
> -		    (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
> -			qcom_geni_serial_handle_tx_fifo(uport,
> -					m_irq_status & M_CMD_DONE_EN,
> -					geni_status & M_GENI_CMD_ACTIVE);
> -
>  		if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
>  			qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
>  	}
> @@ -1632,7 +1670,7 @@ static void qcom_geni_serial_earlycon_write(struct console *con,
>  {
>  	struct earlycon_device *dev = con->data;
>  
> -	__qcom_geni_serial_console_write(&dev->port, s, n);
> +	__qcom_geni_serial_console_write(&dev->port, s, n, false);
>  }
>  
>  #ifdef CONFIG_CONSOLE_POLL
> 

Re: [PATCH 1/2] serial: qcom-geni: Convert console to nbcon

[Aniket RANDIVE]

On 7/30/2026 3:14 AM, Bjorn Andersson wrote:
> The legacy GENI console writer serializes every message around a
> synchronous polled M-side transfer. It blocks printk callers for UART
> wire time and cannot provide atomic output while normal console output is
> active.
> 
> Convert the console to nbcon threaded and atomic writers. Use the UART
> port lock as the device lock and bound threaded M-side commands so
> urgent diagnostics can take over atomic output. The threaded writer is
> batching the output in 32-source-byte commands, a value chosen to
> balance the command setup overhead with atomic-handoff latency.
> 
> Atomic output can cancel an active normal TX command. Use irq_work to
> resume queued TTY output afterward, honor flow control, and prevent the
> deferred restart from accessing the port during shutdown or removal.
> 
> Assisted-by: OpenCode:GPT-5.5
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 166 ++++++++++++++++++++++++++++------
>   1 file changed, 140 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 67b14fda4ff9..08427390c173 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -16,6 +16,7 @@
>   #include <linux/io.h>
>   #include <linux/iopoll.h>
>   #include <linux/irq.h>
> +#include <linux/irq_work.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/panic_notifier.h>
> @@ -89,6 +90,7 @@
>   #define DEF_TX_WM			2
>   #define DEF_FIFO_WIDTH_BITS		32
>   #define UART_RX_WM			2
> +#define CONSOLE_TX_CHUNK_SIZE		32

I noticed that CONSOLE_TX_CHUNK_SIZE is limited to 32 bytes, effectively 
using only half of the 16-word TX FIFO. Was 64 bytes tested and found 
problematic, or is 32 bytes simply a conservative default?

>   
>   /* SE_UART_LOOPBACK_CFG */
>   #define RX_TX_SORTED			BIT(0)
> @@ -153,7 +155,8 @@ struct qcom_geni_serial_port {
>   	bool rx_tx_swap;
>   	bool cts_rts_swap;
>   	bool manual_flow;
> -
> +	bool tx_kick_enabled;
> +	struct irq_work tx_kick;
>   	struct qcom_geni_private_data private_data;
>   	const struct qcom_geni_device_data *dev_data;
>   	struct dev_pm_domain_list *pd_list;
> @@ -168,6 +171,8 @@ static struct uart_driver qcom_geni_uart_driver;
>   static void __qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>   static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>   static int qcom_geni_serial_port_setup(struct uart_port *uport);
> +static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport);
> +static void qcom_geni_serial_resume_tx(struct uart_port *uport);
>   
>   static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
>   {
> @@ -539,49 +544,123 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
>   	qcom_geni_serial_poll_tx_done(uport);
>   }
>   
> -static void qcom_geni_serial_console_write(struct console *co, const char *s,
> -			      unsigned int count)
> +static void qcom_geni_serial_console_takeover(struct uart_port *uport,
> +					       bool preserve_tx)
> +{
> +	if (qcom_geni_serial_main_active(uport)) {
> +		struct qcom_geni_serial_port *port = to_dev_port(uport);
> +
> +		if (preserve_tx) {
> +			if (port->tx_remaining == 0)
> +				qcom_geni_serial_poll_tx_done(uport);
> +			else
> +				qcom_geni_serial_drain_fifo(uport);
> +		}
> +
> +		qcom_geni_serial_cancel_tx_cmd(uport);
> +	}
> +}
> +
> +static void qcom_geni_serial_console_write_thread(struct console *co,
> +						  struct nbcon_write_context *wctxt)
>   {
> +	struct qcom_geni_serial_port *port;
>   	struct uart_port *uport;
> +	unsigned int offset = 0;
> +
> +	port = get_port_from_line(co->index, true, NULL);
> +	if (IS_ERR(port))
> +		return;
> +
> +	uport = &port->uport;
> +	if (!nbcon_enter_unsafe(wctxt))
> +		return;
> +
> +	qcom_geni_serial_console_takeover(uport, true);
> +	if (!nbcon_exit_unsafe(wctxt))
> +		return;

When nbcon_enter_unsafe()/exit_unsafe() report lost ownership, 
write_thread() returns after console_takeover() has cancelled the
active M command, skipping resume_tx(). As far as I can tell TX still
recovers: M_IRQ_EN and TX_WATERMARK_REG are untouched, so the latched
watermark re-asserts on the drained FIFO and the ISR reaches
handle_tx_fifo() with active=false, which re-issues setup_tx() for the 
remaining xmit_fifo data.

Is that the intended fallback? A comment would help, since it is an
implicit dependency on the !active recovery path rather than anything
local to write_thread().

Thanks,
Aniket

> +
> +	while (offset < wctxt->len) {
> +		/*
> +		 * Printk records can be much larger than the FIFO. Limit one
> +		 * M-side command to 32 source bytes so atomic console output can
> +		 * take over between commands instead of waiting for the record.
> +		 */
> +		unsigned int count = min_t(unsigned int, wctxt->len - offset,
> +						   CONSOLE_TX_CHUNK_SIZE);
> +
> +		if (!nbcon_enter_unsafe(wctxt))
> +			return;
> +
> +		__qcom_geni_serial_console_write(uport, wctxt->outbuf + offset,
> +						 count);
> +		offset += count;
> +
> +		if (!nbcon_exit_unsafe(wctxt))
> +			return;
> +	}
> +
> +	if (!nbcon_enter_unsafe(wctxt))
> +		return;
> +
> +	qcom_geni_serial_resume_tx(uport);
> +	nbcon_exit_unsafe(wctxt);
> +}
> +
> +static void qcom_geni_serial_console_write_atomic(struct console *co,
> +						  struct nbcon_write_context *wctxt)
> +{
>   	struct qcom_geni_serial_port *port;
> +	struct uart_port *uport;
>   	u32 m_irq_en, s_irq_en;
> -	bool locked = true;
> -	unsigned long flags;
> -
> -	WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
>   
>   	port = get_port_from_line(co->index, true, NULL);
>   	if (IS_ERR(port))
>   		return;
>   
>   	uport = &port->uport;
> -	if (oops_in_progress)
> -		locked = uart_port_trylock_irqsave(uport, &flags);
> -	else
> -		uart_port_lock_irqsave(uport, &flags);
> +	if (!nbcon_enter_unsafe(wctxt))
> +		return;
>   
>   	m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
>   	s_irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
>   	writel(0, uport->membase + SE_GENI_M_IRQ_EN);
>   	writel(0, uport->membase + SE_GENI_S_IRQ_EN);
>   
> -	if (qcom_geni_serial_main_active(uport)) {
> -		/* Wait for completion or drain FIFO */
> -		if (!locked || port->tx_remaining == 0)
> -			qcom_geni_serial_poll_tx_done(uport);
> -		else
> -			qcom_geni_serial_drain_fifo(uport);
> -
> -		qcom_geni_serial_cancel_tx_cmd(uport);
> -	}
> -
> -	__qcom_geni_serial_console_write(uport, s, count);
> +	qcom_geni_serial_console_takeover(uport, false);
> +	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len);
>   
>   	writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
>   	writel(s_irq_en, uport->membase + SE_GENI_S_IRQ_EN);
> +	nbcon_exit_unsafe(wctxt);
>   
> -	if (locked)
> -		uart_port_unlock_irqrestore(uport, flags);
> +	/* Restart TTY data left queued when atomic output canceled M TX. */
> +	if (READ_ONCE(port->tx_kick_enabled))
> +		irq_work_queue(&port->tx_kick);
> +}
> +
> +static void qcom_geni_serial_console_device_lock(struct console *co,
> +						  unsigned long *flags)
> +{
> +	struct qcom_geni_serial_port *port;
> +
> +	port = get_port_from_line(co->index, true, NULL);
> +	if (IS_ERR(port))
> +		return;
> +
> +	__uart_port_lock_irqsave(&port->uport, flags);
> +}
> +
> +static void qcom_geni_serial_console_device_unlock(struct console *co,
> +						    unsigned long flags)
> +{
> +	struct qcom_geni_serial_port *port;
> +
> +	port = get_port_from_line(co->index, true, NULL);
> +	if (IS_ERR(port))
> +		return;
> +
> +	__uart_port_unlock_irqrestore(&port->uport, flags);
>   }
>   
>   static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
> @@ -738,6 +817,29 @@ static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport)
>   	writel(irq_en, uport->membase +	SE_GENI_M_IRQ_EN);
>   }
>   
> +/* Caller holds the UART port lock. */
> +static void qcom_geni_serial_resume_tx(struct uart_port *uport)
> +{
> +	if (!uart_tx_stopped(uport) &&
> +	    !kfifo_is_empty(&uport->state->port.xmit_fifo))
> +		qcom_geni_serial_start_tx_fifo(uport);
> +}
> +
> +static void qcom_geni_serial_restart_tx(struct irq_work *work)
> +{
> +	struct qcom_geni_serial_port *port = container_of(work,
> +						      struct qcom_geni_serial_port, tx_kick);
> +	struct uart_port *uport = &port->uport;
> +
> +	if (!READ_ONCE(port->tx_kick_enabled) || !uport->state || uport->suspended)
> +		return;
> +
> +	uart_port_lock(uport);
> +	if (READ_ONCE(port->tx_kick_enabled) && uport->state && !uport->suspended)
> +		qcom_geni_serial_resume_tx(uport);
> +	uart_port_unlock(uport);
> +}
> +
>   static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport)
>   {
>   	u32 irq_en;
> @@ -1182,6 +1284,11 @@ static int setup_fifos(struct qcom_geni_serial_port *port)
>   
>   static void qcom_geni_serial_shutdown(struct uart_port *uport)
>   {
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +
> +	/* Atomic console output queues tx_kick without taking the port lock. */
> +	WRITE_ONCE(port->tx_kick_enabled, false);
> +	irq_work_sync(&port->tx_kick);
>   	disable_irq(uport->irq);
>   
>   	uart_port_lock_irq(uport);
> @@ -1281,6 +1388,7 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
>   	uart_port_unlock_irq(uport);
>   
>   	enable_irq(uport->irq);
> +	WRITE_ONCE(port->tx_kick_enabled, true);
>   
>   	return 0;
>   }
> @@ -1627,10 +1735,13 @@ static void console_unregister(struct uart_driver *drv)
>   
>   static struct console cons_ops = {
>   	.name = "ttyMSM",
> -	.write = qcom_geni_serial_console_write,
> +	.write_atomic = qcom_geni_serial_console_write_atomic,
> +	.write_thread = qcom_geni_serial_console_write_thread,
> +	.device_lock = qcom_geni_serial_console_device_lock,
> +	.device_unlock = qcom_geni_serial_console_device_unlock,
>   	.device = uart_console_device,
>   	.setup = qcom_geni_console_setup,
> -	.flags = CON_PRINTBUFFER,
> +	.flags = CON_PRINTBUFFER | CON_NBCON,
>   	.index = -1,
>   	.data = &qcom_geni_console_driver,
>   };
> @@ -1946,6 +2057,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>   
>   	port->private_data.drv = drv;
>   	uport->private_data = &port->private_data;
> +	init_irq_work(&port->tx_kick, qcom_geni_serial_restart_tx);
>   	platform_set_drvdata(pdev, port);
>   
>   	irq_set_status_flags(uport->irq, IRQ_NOAUTOEN);
> @@ -2000,6 +2112,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>   
>   	atomic_notifier_chain_unregister(&panic_notifier_list, &port->panic_nb);
>   
> +	WRITE_ONCE(port->tx_kick_enabled, false);
> +	irq_work_sync(&port->tx_kick);
>   	dev_pm_clear_wake_irq(&pdev->dev);
>   	device_init_wakeup(&pdev->dev, false);
>   	ida_free(&port_ida, uport->line);
> 

Re: [PATCH 2/2] serial: qcom-geni: Keep FIFO RX active during console TX

[Aniket RANDIVE]

On 7/30/2026 3:14 AM, Bjorn Andersson wrote:
> The GENI main sequencer handles console TX while the secondary sequencer
> handles FIFO RX. Before nbcon, the legacy console writer disabled both
> interrupt domains while it performed a long polled M-side transfer. This
> left the small S-side FIFO unserviced, allowing console input to overrun
> and be lost.
> 
> The nbcon conversion replaces IRQ masking with the UART port lock, but a
> threaded console write still prevents the RX handler from draining the
> FIFO. Keep S-side RX enabled independently of M-side TX and drain it
> while refilling each bounded console command. This preserves interactive
> input during console output.
> 
> Atomic output masks only M-side TX state, leaving FIFO RX handling
> independent. The threaded writer can also detect a SysRq character while
> it drains RX, so defer delivery until device_unlock() drops the UART port
> lock, as the existing IRQ path does with uart_unlock_and_check_sysrq().
> 
> Assisted-by: OpenCode:GPT-5.5
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 112 +++++++++++++++++++++++-----------
>   1 file changed, 75 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 08427390c173..a473d5521067 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -173,6 +173,7 @@ static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>   static int qcom_geni_serial_port_setup(struct uart_port *uport);
>   static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport);
>   static void qcom_geni_serial_resume_tx(struct uart_port *uport);
> +static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport);
>   
>   static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
>   {
> @@ -493,7 +494,7 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, unsigned char ch)
>   
>   static void
>   __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
> -				 unsigned int count)
> +				 unsigned int count, bool poll_rx)
>   {
>   	struct qcom_geni_private_data *private_data = uport->private_data;
>   
> @@ -525,6 +526,8 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
>   		if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
>   						M_TX_FIFO_WATERMARK_EN, true))
>   			break;
> +		if (poll_rx)
> +			qcom_geni_serial_poll_rx_fifo_locked(uport);
>   		chars_to_write = min_t(size_t, count - i, avail / 2);
>   		uart_console_write(uport, s + i, chars_to_write,
>   						qcom_geni_serial_wr_char);
> @@ -593,7 +596,7 @@ static void qcom_geni_serial_console_write_thread(struct console *co,
>   			return;
>   
>   		__qcom_geni_serial_console_write(uport, wctxt->outbuf + offset,
> -						 count);
> +						 count, true);
>   		offset += count;
>   
>   		if (!nbcon_exit_unsafe(wctxt))
> @@ -612,7 +615,7 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
>   {
>   	struct qcom_geni_serial_port *port;
>   	struct uart_port *uport;
> -	u32 m_irq_en, s_irq_en;
> +	u32 m_irq_en;
>   
>   	port = get_port_from_line(co->index, true, NULL);
>   	if (IS_ERR(port))
> @@ -623,15 +626,14 @@ static void qcom_geni_serial_console_write_atomic(struct console *co,
>   		return;
>   
>   	m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	s_irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
> -	writel(0, uport->membase + SE_GENI_M_IRQ_EN);
> -	writel(0, uport->membase + SE_GENI_S_IRQ_EN);
> +	writel(m_irq_en & ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN),
> +		uport->membase + SE_GENI_M_IRQ_EN);
>   
> +	/* Atomic console output takes priority over an active normal TX command. */
>   	qcom_geni_serial_console_takeover(uport, false);
> -	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len);
> +	__qcom_geni_serial_console_write(uport, wctxt->outbuf, wctxt->len, false);
>   
>   	writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> -	writel(s_irq_en, uport->membase + SE_GENI_S_IRQ_EN);
>   	nbcon_exit_unsafe(wctxt);
>   
>   	/* Restart TTY data left queued when atomic output canceled M TX. */
> @@ -655,12 +657,25 @@ static void qcom_geni_serial_console_device_unlock(struct console *co,
>   						    unsigned long flags)
>   {
>   	struct qcom_geni_serial_port *port;
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	u8 sysrq_ch;
> +#endif
>   
>   	port = get_port_from_line(co->index, true, NULL);
>   	if (IS_ERR(port))
>   		return;
>   
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	/* The threaded console writer can receive a SysRq character. */
> +	sysrq_ch = port->uport.sysrq_ch;
> +	port->uport.sysrq_ch = 0;
> +#endif

Nit: the SysRq handling here duplicates 
uart_unlock_and_check_sysrq_irqrestore(). I assume device_unlock() needs 
to use __uart_port_unlock_irqrestore(), which prevents reuse of the 
helper. A short comment documenting that constraint would help future 
readers understand the duplication.

Thanks,
Aniket

>   	__uart_port_unlock_irqrestore(&port->uport, flags);
> +
> +#ifdef CONFIG_MAGIC_SYSRQ_SERIAL
> +	if (sysrq_ch)
> +		handle_sysrq(sysrq_ch);
> +#endif
>   }
>   
>   static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
> @@ -902,6 +917,34 @@ static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool drop)
>   	handle_rx_console(uport, total_bytes, drop);
>   }
>   
> +/* Caller holds the UART port lock. */
> +static void qcom_geni_serial_poll_rx_fifo_locked(struct uart_port *uport)
> +{
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +	struct tty_port *tport = &uport->state->port;
> +	u32 s_irq_status;
> +	bool drop_rx = false;
> +
> +	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
> +	writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
> +
> +	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
> +		uport->icount.overrun++;
> +		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
> +	}
> +
> +	if (s_irq_status & (S_GP_IRQ_0_EN | S_GP_IRQ_1_EN)) {
> +		if (s_irq_status & S_GP_IRQ_0_EN)
> +			uport->icount.parity++;
> +		drop_rx = true;
> +	} else if (s_irq_status & (S_GP_IRQ_2_EN | S_GP_IRQ_3_EN)) {
> +		uport->icount.brk++;
> +		port->brk = true;
> +	}
> +
> +	qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
> +}
> +
>   static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
>   {
>   	u32 irq_en;
> @@ -912,10 +955,6 @@ static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
>   	irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
>   	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
>   
> -	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
> -	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> -
>   	if (!qcom_geni_serial_secondary_active(uport))
>   		return;
>   
> @@ -949,10 +988,6 @@ static void qcom_geni_serial_start_rx_fifo(struct uart_port *uport)
>   	irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
>   	irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
>   	writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
> -
> -	irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> -	irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
> -	writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
>   }
>   
>   static void qcom_geni_serial_stop_rx_dma(struct uart_port *uport)
> @@ -1182,25 +1217,11 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>   
>   	uart_port_lock(uport);
>   
> -	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
>   	s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
> -	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
>   	dma_rx_status = readl(uport->membase + SE_DMA_RX_IRQ_STAT);
> -	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);
>   	writel(dma_rx_status, uport->membase + SE_DMA_RX_IRQ_CLR);
>   
> -	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
> -		goto out_unlock;
> -
>   	if (s_irq_status & S_RX_FIFO_WR_ERR_EN) {
>   		uport->icount.overrun++;
>   		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
> @@ -1215,12 +1236,35 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>   		port->brk = true;
>   	}
>   
> +	m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
> +	dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
> +	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(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
> +
> +	if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
> +		goto handle_rx;
> +
>   	if (dma) {
>   		if (dma_tx_status & TX_DMA_DONE) {
>   			qcom_geni_serial_handle_tx_dma(uport);
>   			qcom_geni_set_rs485_mode(uport, SER_RS485_RTS_AFTER_SEND);
> +		}
> +	} else if (m_irq_status & m_irq_en &
> +		   (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN)) {
> +		qcom_geni_serial_handle_tx_fifo(uport,
> +				m_irq_status & M_CMD_DONE_EN,
> +				geni_status & M_GENI_CMD_ACTIVE);
>   	}
>   
> +handle_rx:
> +	if (dma) {
>   		if (dma_rx_status) {
>   			if (dma_rx_status & RX_RESET_DONE)
>   				goto out_unlock;
> @@ -1237,12 +1281,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
>   				qcom_geni_serial_handle_rx_dma(uport, drop_rx);
>   		}
>   	} else {
> -		if (m_irq_status & m_irq_en &
> -		    (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
> -			qcom_geni_serial_handle_tx_fifo(uport,
> -					m_irq_status & M_CMD_DONE_EN,
> -					geni_status & M_GENI_CMD_ACTIVE);
> -
>   		if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
>   			qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
>   	}
> @@ -1632,7 +1670,7 @@ static void qcom_geni_serial_earlycon_write(struct console *con,
>   {
>   	struct earlycon_device *dev = con->data;
>   
> -	__qcom_geni_serial_console_write(&dev->port, s, n);
> +	__qcom_geni_serial_console_write(&dev->port, s, n, false);
>   }
>   
>   #ifdef CONFIG_CONSOLE_POLL
>