Re: [PATCH v2 1/1] braille: nbcon: Allow to use a serial console with NBCON API as Braille console

[John Ogness <john.ogness@linutronix.de>]

On 2026-09-25, Petr Mladek <pmladek@suse.com> 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.
>
> When the associated serial console driver gets converted to the NBCON API,
> the braille write callback should use con->write_atomic() callback
> with an appropriate locking.
>
> It must be the atomic variant because it can be called under a spin_lock,
> for example via:
>
>   + kbd_event()
>     + kbd_keycode()
>       + atomic_notifier_call_chain(&keyboard_notifier_list)
> 	+ vt_notifier_call()
> 	  + vc_refresh()
> 	    + braille_write()
>
> In addition, it can be called from printk() in any context via
> the graphical tty driver (vt code) even when the Braille driver is not
> in console_list directly.
>
> The locking is inspired with nbcon_legacy_emit_next_record(),
> nbcon_kdb_try_acquire()/release(), and the original serial driver locking:
>
>   1. IRQs are explicitly disabled to prevent CPU migration and nested
>      calls into the serial driver code.
>
>   2. New nbcon_braille_try_acquire()/release() API allows to initialize
>      the write context and acquire the ownership. It is using
>      NBCON_PRIO_NORMAL because it competes only with the other operations
>      on the serial console driver which are serialized using
>      nbcon_device_try_acquire().
>
>   3. It uses a busy loop until it acquires the ownership. Otherwise,
>      the messages would get lost. [*]

There could only be ownership issues if userspace is playing with the
/dev/ttySx device node, which userspace should not be doing.

>   4. It does just the best effort when oops_in_progress is set.
>
> Also, adjust __serial8250_console_write() in the 8250 serial driver to
> exclude Braille consoles from the newline prepending logic.

Note that all NBCON drivers cause this issue, not just the 8250. Later I
mention why it does not matter and no changes to the 8250 are required.

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

Well, it still performs the "\n" -> "\r\n" conversions. But I guess that
is appropriate.

> [*] The busy loop is not safe on PREEMPT_RT where the current owner
>     might sleep. We will need another solution there.

If we are knowingly breaking PREEMPT_RT then this series should also
include:

diff --git a/drivers/accessibility/Kconfig b/drivers/accessibility/Kconfig
index 6b2f79d1f1b81..d4faa6e0e01b1 100644
--- a/drivers/accessibility/Kconfig
+++ b/drivers/accessibility/Kconfig
@@ -21,6 +21,7 @@ config A11Y_BRAILLE_CONSOLE
 	bool "Console on braille device"
 	depends on VT
 	depends on SERIAL_CORE_CONSOLE
+	depends on !PREEMPT_RT
 	help
 	  Enables console output on a braille device connected to a 8250
 	  serial port. For now only the VisioBraille device is supported.

The console_braille "driver" needs to be made into a proper driver and
make use of the serdev subsystem. I am currently working on this, but
the changes are not trivial. And, optimally, the vt_console should also
be switched over to NBCON. So this is not something we are going to get
fixed in an rc6. For that reason, I support moving forward with this
series until a proper solution is developed.

> Fixes: d3539347022a ("serial: 8250: Switch to nbcon console, take 2")
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  .../accessibility/braille/braille_console.c   | 43 ++++++++++++-
>  drivers/tty/serial/8250/8250_port.c           |  5 +-
>  include/linux/console.h                       |  8 +++
>  kernel/printk/nbcon.c                         | 64 +++++++++++++++++++
>  4 files changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
> index 06b43b678d6e..bae177cb8cf9 100644
> --- a/drivers/accessibility/braille/braille_console.c
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -62,14 +62,36 @@ 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;
> +	bool locked;
> +
>  	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().
> +		 * 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);
> +		do {
> +			locked = nbcon_braille_try_acquire(braille_co, &wctxt);
> +			if (!locked)
> +				cpu_relax();
> +		} while (!locked && !oops_in_progress);
> +	}
> +
>  	if (!memcmp(lastwrite, buf, WIDTH * sizeof(*buf)))
> -		return;
> +		goto release_nbcon;
>  	memcpy(lastwrite, buf, WIDTH * sizeof(*buf));
>  
>  #define SOH 1
> @@ -102,7 +124,24 @@ 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");

This needs to be caught in braille_register_console(), not here.

> +		}
> +	} else {
> +		braille_co->write(braille_co, data, c - data);
> +	}
> +
> +release_nbcon:
> +	if (braille_co->flags & CON_NBCON) {
> +		if (locked)
> +			nbcon_braille_release(&wctxt);
> +		local_irq_restore(flags);
> +	}
>  }
>  
>  /* 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))) {

This change is not necessary because there will never be
handovers/takeovers for Braille. It is not registered as a console.

>  		if (use_fifo)
>  			__serial8250_console_fifo_write(up, wctxt, "\n", 1);
>  		else
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 502d1abe3f50..d780f6de303a 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -615,6 +615,10 @@ extern bool nbcon_allow_unsafe_takeover(void);
>  extern bool nbcon_kdb_try_acquire(struct console *con,
>  				  struct nbcon_write_context *wctxt);
>  extern void nbcon_kdb_release(struct nbcon_write_context *wctxt);
> +extern bool nbcon_is_braille(struct nbcon_write_context *wctxt);

nbcon_is_braille() is not needed.

> +extern bool nbcon_braille_try_acquire(struct console *con,
> +				      struct nbcon_write_context *wctxt);
> +extern void nbcon_braille_release(struct nbcon_write_context *wctxt);
>  
>  /*
>   * Check if the given console is currently capable and allowed to print
> @@ -678,8 +682,12 @@ static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
>  static inline bool nbcon_kdb_try_acquire(struct console *con,
>  					 struct nbcon_write_context *wctxt) { return false; }
>  static inline void nbcon_kdb_release(struct nbcon_write_context *wctxt) { }
> +static inline bool nbcon_is_braille(struct nbcon_write_context *wctxt) { return false; }

nbcon_is_braille() is not needed.

>  static inline bool console_is_usable(struct console *con, short flags,
>  				     bool use_atomic) { return false; }
> +static inline bool nbcon_braille_try_acquire(struct console *con,
> +					     struct nbcon_write_context *wctxt) { return false; }
> +static inline void nbcon_braille_release(struct nbcon_write_context *wctxt) { }
>  #endif
>  
>  extern int console_set_on_cmdline;
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index d17704fe93ae..e5a0506330eb 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1887,6 +1887,7 @@ bool nbcon_device_try_acquire(struct console *con)
>  
>  	memset(ctxt, 0, sizeof(*ctxt));
>  	ctxt->console	= con;
> +	/* Keep in sync with nbcon_braille_try_acquire(). */
>  	ctxt->prio	= NBCON_PRIO_NORMAL;
>  
>  	if (!nbcon_context_try_acquire(ctxt, false))
> @@ -2002,3 +2003,66 @@ void nbcon_kdb_release(struct nbcon_write_context *wctxt)
>  	 */
>  	__nbcon_atomic_flush_pending_con(ctxt->console, prb_next_reserve_seq(prb));
>  }
> +
> +/**
> + * nbcon_is_braille - Checks whether the nbcon write context is using Braille console
> + *
> + * @wctxt:	checked nbcon write context
> + *
> + * Return: True when the write context is associated with a Braille console.
> + *	   Othrewise, return false.
> + *
> + * Context: Can be called in any context but only when Braille console is
> + *	registered and the struct console could not disappear.
> + */
> +bool nbcon_is_braille(struct nbcon_write_context *wctxt)
> +{
> +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +	struct console *con = ctxt->console;
> +
> +	return con && con->flags & CON_BRL;
> +}

nbcon_is_braille() is not needed.

> +
> +/**
> + * nbcon_braille_try_acquire - Try to acquire nbcon console for braille_write()
> + *
> + * @con:	The nbcon console to acquire
> + * @wctxt:	The nbcon write context to be used on success
> + *
> + * Context:	braille_write() for emitting a single buffer on Braille console.
> + *
> + * Return:	True if the console was acquired. False otherwise.
> + *
> + * Braille console is not registered as a proper printk consoles. Instead,
> + * it is integrated with the graphical virtual terminal.
> + *
> + * This function is going to synchronize the Braille write against other
> + * operations on the used serial port. The port can be used also for a user
> + * input but printk() won't emit the messages there directly. It means
> + * the other operations will get synchronized using nbcon_device_try_acquire().
> + */
> +bool nbcon_braille_try_acquire(struct console *con,
> +			   struct nbcon_write_context *wctxt)
> +{
> +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +
> +	memset(ctxt, 0, sizeof(*ctxt));
> +	ctxt->console = con;
> +	/* Keep in sync with nbcon_device_try_acquire(). */
> +	ctxt->prio = NBCON_PRIO_NORMAL;
> +
> +	return nbcon_context_try_acquire(ctxt, false);
> +}
> +
> +/**
> + * nbcon_braille_release - Release the nbcon console
> + *
> + * @wctxt:	The nbcon write context initialized by a successful
> + *		nbcon_braille_try_acquire()
> + */
> +void nbcon_braille_release(struct nbcon_write_context *wctxt)
> +{
> +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +
> +	nbcon_context_release(ctxt);
> +}

John Ogness