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

[Petr Mladek <pmladek@suse.com>]

On Tue 2026-09-29 12:47:26, John Ogness wrote:
> 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.

Yup.

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

Good point!

> Later I mention why it does not matter and no changes to the 8250 are required.

I am afraid that it matters see below.


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

OK, it keeps "\n" when it is there. The problem is that
braille_write()  writes incomplete lines most of the time, see
https://lore.kernel.org/all/arW4C9TYane57IGN@end/

For example, I see the following on the serial console in the Braille mode:

<paste>
E>[                                       E>[                                       E>[  O                                    *>[  OK                                   A>[  OK                                   A>[  OK                                   A>[  OK  ]                                <>[  OK  ]                                <>[  OK  ] R                              N>[  OK  ] Re                             
       >[  OK  ] Rea                            J>[  OK  ] Reac                           	>[  OK  ] Reach                          A>[  OK  ] Reache                         D>[  OK  ] Reached                        @>[  OK  ] Reached                        @>[  OK  ] Reached t           
</paste>

So we need to avoid the extra newlines added by the driver code.

It would be better to handle this on the printk() code level.
But the write*() callbacks do not return any error value :-/


> > [*] 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.

Good point. Will add this in v3.

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

Sounds like a good long term plan. But I agree that we need another solution
in the meantime.

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

Will do in v3.

Thanks a lot review.

Best Regards,
Petr