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@xxxxxxxxxxxxxxxx>
> ---

[...]

> +	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