Re: [PATCH 2/2] braille: nbcon: Use nbcon atomic console callbacks

[John Ogness]

On 2026-09-22, Petr Mladek <pmladek@xxxxxxxx> wrote:
> The Braille console is integrated with the virtual terminal (VT) and
> writes its data using the legacy con->write() callback of the associated
> serial console driver.
>
> However, once the underlying console driver is converted to the NBCON
> API, it should use the con->write_atomic() callback, which must be
> synchronized by acquiring the nbcon console context.
>
> Adapt braille_write() to handle NBCON consoles:
>
> 1. Acquire the nbcon console ownership using NBCON_PRIO_EMERGENCY
>    priority before printing, disabling local interrupts to prevent
>    any nested calls into the driver.
> 2. Call the con->write_atomic() callback to write the buffer.
> 3. Release the nbcon console ownership and restore local interrupts.
>
> Also, adjust __serial8250_console_write() in the 8250 serial driver to
> exclude Braille consoles from the newline prepending logic. The serial
> port is not used for standard printk logging which might be interrupted
> in the middle of the operation. In the Braille mode, the serial driver
> is supposed to write exactly what it gets. In fact, it does not print
> any newlines at all in this case.
>
> Fixes: d3539347022a ("serial: 8250: Switch to nbcon console, take 2")
> Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
> ---
>  .../accessibility/braille/braille_console.c   | 38 ++++++++++++++++++-
>  drivers/tty/serial/8250/8250_port.c           |  5 ++-
>  2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
> index 06b43b678d6e..cd1019d478db 100644
> --- a/drivers/accessibility/braille/braille_console.c
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -62,14 +62,32 @@ static void braille_write(u16 *buf)
>  {
>  	static u16 lastwrite[WIDTH];
>  	unsigned char data[1 + 1 + 2*WIDTH + 2 + 1], csum = 0, *c;
> +	struct nbcon_write_context wctxt = { };
> +	unsigned long flags;
> +
>  	u16 out;
>  	int i;
>  
>  	if (!braille_co)
>  		return;
>  
> +	if (braille_co->flags & CON_NBCON) {
> +		/*
> +		 * Braille console might be called from unknown context via
> +		 * vt_console_print() from console_unlock() from printk().

printk()?

> +		 * Use the atomic callback and synchronize it just using
> +		 * the console context. Disable interrupts to prevent a nested
> +		 * call into the driver code which might cause a deadlock when
> +		 * trying to acquire the console ownership, see
> +		 * __nbcon_atomic_flush_pending_con().
> +		 */
> +		local_irq_save(flags);

This is not acceptable for PREEMPT_RT.

local_lock_irqsave() should be appropriate since this driver is not
called from printk(). ...if we want to take this route. More below...

> +		while (!nbcon_braille_try_acquire(braille_co, &wctxt))
> +			cpu_relax();
> +	}
> +
>  	if (!memcmp(lastwrite, buf, WIDTH * sizeof(*buf)))
> -		return;
> +		goto release_nbcon;
>  	memcpy(lastwrite, buf, WIDTH * sizeof(*buf));
>  
>  #define SOH 1
> @@ -102,7 +120,23 @@ static void braille_write(u16 *buf)
>  	*c++ = csum;
>  	*c++ = ETX;
>  
> -	braille_co->write(braille_co, data, c - data);
> +	if (braille_co->flags & CON_NBCON) {
> +		if (braille_co->write_atomic &&
> +		    !braille_co->flags & CON_NBCON_ATOMIC_UNSAFE) {
> +			nbcon_write_context_set_buf(&wctxt, (char *)data, c - data);
> +			braille_co->write_atomic(braille_co, &wctxt);
> +		} else {
> +			pr_warn_once("Braille requires a safe braille_co->write_atomic callback\n");
> +		}
> +	} else {
> +		braille_co->write(braille_co, data, c - data);
> +	}
> +
> +release_nbcon:
> +	if (braille_co->flags & CON_NBCON) {
> +		nbcon_braille_release(&wctxt);
> +		local_irq_restore(flags);
> +	}
>  }

If we want to "convert" the Braille console to NBCON, we need to have
established contexts so that we know what we can do. kdb was a horrible
hack that was only acceptable because when kdb is active, the whole
system goes out to lunch. But for the Braille console we need some
sanity because this is a live system.

AFAICT braille_write() is called from interrupt-handler and task
contexts. Can we buffer the writes and wake a dedicated kthread to
processes them using ->lock() and ->write_thread()? We can use spinlocks
to synchronize the FIFO buffer.

>  /* Follow the VC cursor*/
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 38fa45e74a37..6eb0b439e433 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3417,8 +3417,11 @@ static void __serial8250_console_write(struct uart_8250_port *up,
>  	 * If the console printer did not fully output the previous line, it
>  	 * must have been handed or taken over. Insert a newline in order to
>  	 * maintain clean output.
> +	 *
> +	 * Braille consoles are an exception. The serial port is not used
> +	 * for printk(). The driver is supposed to write exactly what it gets.
>  	 */
> -	if (!up->console_line_ended) {
> +	if (unlikely(!up->console_line_ended && !nbcon_is_braille(wctxt))) {

Indeed. I am just wondering if it should be more generic.

nbcon_is_registered() ?
nbcon_supports_printk() ?

Or do we really want Braille to be the one and only exception.

Also note that this code exists in other NBCON drivers as well. At some
point we will need to create a general solution to adding \n on
takeovers.

John