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

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

On 2026-10-01, Petr Mladek <pmladek@suse.com> wrote:
> The Braille console is not registered in console_list. Instead, it is
> integrated with the virtual terminal (VT) and shows what is displayed
> on the terminal. It writes the data using con->write*() callback
> of the associated serial console driver.
>
> braille_write() is called from the VT code under console_lock().
> The associated serial console driver can be converted to the NBCON API
> though. The situation is similar to flushing nbcon consoles in the legacy
> loop when some boot consoles are still registered, see
> nbcon_legacy_emit_next_record().
>
> But there is a big difference though. braille_write() is not directly
> called from the code paths flushing registered consoles. The VT code
> expects that braille_write() succeeds. It does not replay the message
> when it can't acquire the ownership. As a result, braille_write():
>
>   + must try harder to get the ownership.
>
>   + has to be synchronized only against non-printk serial console
>     which depend nbcon_device_try_acquire() using NBCON_PRIO_NORMAL.
>
> Let's look at it from another side and try to simulate the original
> locking using the NBCON API:
>
>   1. Take con->device_lock(), aka the port->lock in the legacy serial
>      console driver.
>
>   2. Acquire nbcon context to provide some synchronization for a panic()
>      context. Use NBCON_PRIO_NORMAL because it contends only with
>      nbcon_device_try_acquire() users. Do it in a busy loop. It should
>      always succeed when con->device_lock() succeeded because all other
>      users do the same. The only exception is when the context get acquired
>      by a CPU handling panic.
>
>   3. In panic, disable interrupts and try to acquire the nbcon context.
>      Use NBCON_PRIO_PANIC. And try even an unsafe takeover because
>      otherwise the Braille console won't see the text shown during panic().
>      It is similar to the oops_in_progress/trylock handling in the legacy
>      serial console driver.
>
> Finally, avoid the newline prepending logic in the existing serial console
> drivers when they are used as a Braille console. As explained above,
> the Braille console shows the last modified line on the terminal (VT).
> braille_write() is called when single characters are added. Most
> messages are not ended by newline. Anyway, the VT code does not have
> logic to reply partially printed messages.
>
> Fixes: 13189fa73afa ("printk: nbcon: Rely on kthreads for normal operation")
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Minor comments follow, but no need to send a v5 for them. You could fix
them up on your own.

Reviewed-by: John Ogness <john.ogness@linutronix.de>

> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
> index 06b43b678d6e..fc6cd3530b28 100644
> --- a/drivers/accessibility/braille/braille_console.c
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -353,13 +406,16 @@ int braille_register_console(struct console *console, int index,
>  	if (!console_options)
>  		/* Only support VisioBraille for now */
>  		console_options = "57600o8";
> +
>  	if (braille_co)
>  		return -ENODEV;
> +
>  	if (console->setup) {
>  		ret = console->setup(console, console_options);
>  		if (ret != 0)
>  			return ret;
>  	}
> +
>  	console->flags |= CON_ENABLED;
>  	console->index = index;
>  	braille_co = console;

This hunk is pure whitespace cleanup. It is a leftover from when you
actually made changes here. I am OK if you keep it, but I would drop
this hunk.

> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index d17704fe93ae..8dac9bda4fe0 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -2002,3 +2003,81 @@ 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.

One more:  Otherwise

I ran a spellcheck on the whole patch this time, so we should be good
after this.

Thanks for doing the heavy lifting on this so quickly.

John