Thanks for your comment. > On Thu, Jan 09, 2025 at 01:04:36AM +0000, Toshiyuki Sato (Fujitsu) wrote: > > Hi, Greg. Thanks for the comment. > > > > > On Wed, Jan 08, 2025 at 12:47:30AM +0000, Toshiyuki Sato wrote: > > > > Implement the callbacks required for an NBCON console [0] on the > > > > amba-pl011 console driver. > > > > The codes for legacy console are retained, and the module > > > > parameter (use_nbcon) allows switching between legacy and NBCON. > > > > The default is off (use legacy console) for now. > > > > > > > > Referred to the NBCON implementation work for 8250 [1] and imx [2]. > > > > > > > > The normal-priority write_thread checks for console ownership > > > > each time a character is printed. > > > > write_atomic holds the console ownership until the entire string > > > > is printed. > > > > > > > > UART register operations are protected from other contexts by > > > > uart_port_lock, except for a final flush(nbcon_atomic_flush_unsafe) > > > > on panic. > > > > > > > > The patch has been verified to correctly handle the output and > > > > competition of messages with different priorities and flushing > > > > panic message to console after nmi panic using ARM64 QEMU and > > > > a physical machine(A64FX). > > > > > > > > [0] https://lore.kernel.org/all/ZuRRTbapH0DCj334@xxxxxxxxxxxxxxx/ > > > > [1] > > > > https://lore.kernel.org/all/20240913140538.221708-1-john.ogness@linutronix.d > > > e/T/ > > > > [2] > > > > https://lore.kernel.org/linux-arm-kernel/20240913-serial-imx-nbcon-v3-1-4c62 > > > 7302335b@xxxxxxxxxx/T/ > > > > > > > > Signed-off-by: Toshiyuki Sato <fj6611ie@xxxxxxxxxxxxxxxxx> > > > > --- > > > > drivers/tty/serial/amba-pl011.c | 113 > > > ++++++++++++++++++++++++++++++++ > > > > 1 file changed, 113 insertions(+) > > > > > > > > diff --git a/drivers/tty/serial/amba-pl011.c > b/drivers/tty/serial/amba-pl011.c > > > > index 69b7a3e1e..52fab3170 100644 > > > > --- a/drivers/tty/serial/amba-pl011.c > > > > +++ b/drivers/tty/serial/amba-pl011.c > > > > @@ -41,6 +41,7 @@ > > > > #include <linux/sizes.h> > > > > #include <linux/io.h> > > > > #include <linux/acpi.h> > > > > +#include <linux/moduleparam.h> > > > > > > > > #define UART_NR 14 > > > > > > > > @@ -263,6 +264,7 @@ struct uart_amba_port { > > > > char type[12]; > > > > bool rs485_tx_started; > > > > unsigned int rs485_tx_drain_interval; /* usecs */ > > > > + bool console_line_ended; > > > > #ifdef CONFIG_DMA_ENGINE > > > > /* DMA stuff */ > > > > unsigned int dmacr; /* dma control reg */ > > > > @@ -274,6 +276,10 @@ struct uart_amba_port { > > > > #endif > > > > }; > > > > > > > > +/* if non-zero, the console is nbcon. */ > > > > +static int use_nbcon; > > > > +module_param(use_nbcon, int, 0444); > > > > > > Why is a module parameter needed here? That feels wrong and not > > > scalable at all. What happens if you have multiple devices, which one > > > is nbcon and which isn't? > > > > This module parameter is for pl011 driver. > > Yes, and module parameters are from the 1990's, let's not add new ones > please. > > > With this patch implemented, only one type will be usable, depending > > on the value of use_nbcon. > > I thought it would be more user-friendly if legacy operation could be > > selected via boot parameter. > > Would it be better to make the patch purely nbcon-compatible, without > > retaining the legacy functionality? > > Why would you want the "legacy" functionality? Aren't we converting all > consoles over to use the new stuff? As you say, it should be replaced with nbcon. We will reconsider a v2 patch that supports only nbcon. Regards, Toshiyuki Sato
