Re: [PATCH 1/2] serial: qcom-geni: Convert console to nbcon
Konrad Dybcio <[email protected]> Thu, 30 Jul 2026 18:19:12 +0200
| Newsgroups | gmane.linux.serial,gmane.linux.ports.arm.msm,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> > --- [...] > + 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