From: Greg Kroah-Hartman Date: Wed, 22 Jan 2025 09:35:56 +0000 (+0100) Subject: Revert "serial: 8250: Switch to nbcon console" X-Git-Tag: v6.14-rc1~59^2 X-Git-Url: https://repo.jachan.dev/linux.git/commitdiff_plain/f79b163c42314a1f46f4bcc40a19c8a75cf1e7a3 Revert "serial: 8250: Switch to nbcon console" This reverts commit b63e6f60eab45b16a1bf734fef9035a4c4187cd5. kernel test robot has found problems with this commit so revert it for now. Link: https://lore.kernel.org/r/202501221029.fb0d574d-lkp@intel.com Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202501221029.fb0d574d-lkp@intel.com Cc: John Ogness Cc: Petr Mladek Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 3ab372b28cf3..6f676bb37ac3 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -388,34 +388,12 @@ void __init serial8250_register_ports(struct uart_driver *drv, struct device *de #ifdef CONFIG_SERIAL_8250_CONSOLE -static void univ8250_console_write_atomic(struct console *co, - struct nbcon_write_context *wctxt) +static void univ8250_console_write(struct console *co, const char *s, + unsigned int count) { struct uart_8250_port *up = &serial8250_ports[co->index]; - serial8250_console_write(up, wctxt, true); -} - -static void univ8250_console_write_thread(struct console *co, - struct nbcon_write_context *wctxt) -{ - struct uart_8250_port *up = &serial8250_ports[co->index]; - - serial8250_console_write(up, wctxt, false); -} - -static void univ8250_console_device_lock(struct console *co, unsigned long *flags) -{ - struct uart_port *up = &serial8250_ports[co->index].port; - - __uart_port_lock_irqsave(up, flags); -} - -static void univ8250_console_device_unlock(struct console *co, unsigned long flags) -{ - struct uart_port *up = &serial8250_ports[co->index].port; - - __uart_port_unlock_irqrestore(up, flags); + serial8250_console_write(up, s, count); } static int univ8250_console_setup(struct console *co, char *options) @@ -516,15 +494,12 @@ static int univ8250_console_match(struct console *co, char *name, int idx, static struct console univ8250_console = { .name = "ttyS", - .write_atomic = univ8250_console_write_atomic, - .write_thread = univ8250_console_write_thread, - .device_lock = univ8250_console_device_lock, - .device_unlock = univ8250_console_device_unlock, + .write = univ8250_console_write, .device = uart_console_device, .setup = univ8250_console_setup, .exit = univ8250_console_exit, .match = univ8250_console_match, - .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_NBCON, + .flags = CON_PRINTBUFFER | CON_ANYTIME, .index = -1, .data = &serial8250_reg, }; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 08466cf10d73..d7976a21cca9 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -711,12 +711,7 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_put(p); } -/* - * Only to be used directly by the callback helper serial8250_console_write(), - * which may not require the port lock. Use serial8250_clear_IER() instead for - * all other cases. - */ -static void __serial8250_clear_IER(struct uart_8250_port *up) +static void serial8250_clear_IER(struct uart_8250_port *up) { if (up->capabilities & UART_CAP_UUE) serial_out(up, UART_IER, UART_IER_UUE); @@ -724,11 +719,6 @@ static void __serial8250_clear_IER(struct uart_8250_port *up) serial_out(up, UART_IER, 0); } -static inline void serial8250_clear_IER(struct uart_8250_port *up) -{ - __serial8250_clear_IER(up); -} - #ifdef CONFIG_SERIAL_8250_RSA /* * Attempts to turn on the RSA FIFO. Returns zero on failure. @@ -1416,6 +1406,9 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier) { unsigned char mcr = serial8250_in_MCR(p); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) mcr |= UART_MCR_RTS; else @@ -1431,16 +1424,6 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier) serial8250_clear_and_reinit_fifos(p); if (toggle_ier) { - /* - * Port locked to synchronize UART_IER access against - * the console. The lockdep_assert must be restricted - * to this condition because only here is it - * guaranteed that the port lock is held. The other - * hardware access in this function is synchronized - * by console ownership. - */ - lockdep_assert_held_once(&p->port.lock); - p->ier |= UART_IER_RLSI | UART_IER_RDI; serial_port_out(&p->port, UART_IER, p->ier); } @@ -3320,11 +3303,7 @@ EXPORT_SYMBOL_GPL(serial8250_set_defaults); static void serial8250_console_putchar(struct uart_port *port, unsigned char ch) { - struct uart_8250_port *up = up_to_u8250p(port); - serial_port_out(port, UART_TX, ch); - - up->console_line_ended = (ch == '\n'); } static void serial8250_console_wait_putchar(struct uart_port *port, unsigned char ch) @@ -3361,22 +3340,11 @@ static void serial8250_console_restore(struct uart_8250_port *up) serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS); } -static void fifo_wait_for_lsr(struct uart_8250_port *up, - struct nbcon_write_context *wctxt, - unsigned int count) +static void fifo_wait_for_lsr(struct uart_8250_port *up, unsigned int count) { unsigned int i; for (i = 0; i < count; i++) { - /* - * Pass the ownership as quickly as possible to a higher - * priority context. Otherwise, its attempt to take over - * the ownership might timeout. The new owner will wait - * for UART_LSR_THRE before reusing the fifo. - */ - if (!nbcon_can_proceed(wctxt)) - return; - if (wait_for_lsr(up, UART_LSR_THRE)) return; } @@ -3389,29 +3357,20 @@ static void fifo_wait_for_lsr(struct uart_8250_port *up, * to get empty. */ static void serial8250_console_fifo_write(struct uart_8250_port *up, - struct nbcon_write_context *wctxt) + const char *s, unsigned int count) { + const char *end = s + count; unsigned int fifosize = up->tx_loadsz; struct uart_port *port = &up->port; - const char *s = wctxt->outbuf; - const char *end = s + wctxt->len; unsigned int tx_count = 0; bool cr_sent = false; unsigned int i; while (s != end) { /* Allow timeout for each byte of a possibly full FIFO */ - fifo_wait_for_lsr(up, wctxt, fifosize); + fifo_wait_for_lsr(up, fifosize); - /* - * Fill the FIFO. If a handover or takeover occurs, writing - * must be aborted since wctxt->outbuf and wctxt->len are no - * longer valid. - */ for (i = 0; i < fifosize && s != end; ++i) { - if (!nbcon_enter_unsafe(wctxt)) - return; - if (*s == '\n' && !cr_sent) { serial8250_console_putchar(port, '\r'); cr_sent = true; @@ -3419,8 +3378,6 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up, serial8250_console_putchar(port, *s++); cr_sent = false; } - - nbcon_exit_unsafe(wctxt); } tx_count = i; } @@ -3429,57 +3386,39 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up, * Allow timeout for each byte written since the caller will only wait * for UART_LSR_BOTH_EMPTY using the timeout of a single character */ - fifo_wait_for_lsr(up, wctxt, tx_count); -} - -static void serial8250_console_byte_write(struct uart_8250_port *up, - struct nbcon_write_context *wctxt) -{ - struct uart_port *port = &up->port; - const char *s = wctxt->outbuf; - const char *end = s + wctxt->len; - - /* - * Write out the message. If a handover or takeover occurs, writing - * must be aborted since wctxt->outbuf and wctxt->len are no longer - * valid. - */ - while (s != end) { - if (!nbcon_enter_unsafe(wctxt)) - return; - - uart_console_write(port, s++, 1, serial8250_console_wait_putchar); - - nbcon_exit_unsafe(wctxt); - } + fifo_wait_for_lsr(up, tx_count); } /* - * Print a string to the serial port trying not to disturb - * any possible real use of the port... + * Print a string to the serial port trying not to disturb + * any possible real use of the port... + * + * The console_lock must be held when we get here. * - * Doing runtime PM is really a bad idea for the kernel console. - * Thus, assume it is called when device is powered up. + * Doing runtime PM is really a bad idea for the kernel console. + * Thus, we assume the function is called when device is powered up. */ -void serial8250_console_write(struct uart_8250_port *up, - struct nbcon_write_context *wctxt, - bool is_atomic) +void serial8250_console_write(struct uart_8250_port *up, const char *s, + unsigned int count) { struct uart_8250_em485 *em485 = up->em485; struct uart_port *port = &up->port; - unsigned int ier; - bool use_fifo; + unsigned long flags; + unsigned int ier, use_fifo; + int locked = 1; - if (!nbcon_enter_unsafe(wctxt)) - return; + touch_nmi_watchdog(); + + if (oops_in_progress) + locked = uart_port_trylock_irqsave(port, &flags); + else + uart_port_lock_irqsave(port, &flags); /* - * First, save the IER, then disable the interrupts. The special - * variant to clear the IER is used because console printing may - * occur without holding the port lock. + * First save the IER then disable the interrupts */ ier = serial_port_in(port, UART_IER); - __serial8250_clear_IER(up); + serial8250_clear_IER(up); /* check scratch reg to see if port powered off during system sleep */ if (up->canary && (up->canary != serial_port_in(port, UART_SCR))) { @@ -3493,18 +3432,6 @@ void serial8250_console_write(struct uart_8250_port *up, mdelay(port->rs485.delay_rts_before_send); } - /* If ownership was lost, no writing is allowed */ - if (!nbcon_can_proceed(wctxt)) - goto skip_write; - - /* - * If 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. - */ - if (!up->console_line_ended) - uart_console_write(port, "\n", 1, serial8250_console_wait_putchar); - use_fifo = (up->capabilities & UART_CAP_FIFO) && /* * BCM283x requires to check the fifo @@ -3525,23 +3452,10 @@ void serial8250_console_write(struct uart_8250_port *up, */ !(up->port.flags & UPF_CONS_FLOW); - nbcon_exit_unsafe(wctxt); - if (likely(use_fifo)) - serial8250_console_fifo_write(up, wctxt); + serial8250_console_fifo_write(up, s, count); else - serial8250_console_byte_write(up, wctxt); -skip_write: - /* - * If ownership was lost, this context must reacquire ownership and - * re-enter the unsafe section in order to perform final actions - * (such as re-enabling interrupts). - */ - if (!nbcon_can_proceed(wctxt)) { - do { - nbcon_reacquire_nobuf(wctxt); - } while (!nbcon_enter_unsafe(wctxt)); - } + uart_console_write(port, s, count, serial8250_console_wait_putchar); /* * Finally, wait for transmitter to become empty @@ -3564,18 +3478,11 @@ skip_write: * call it if we have saved something in the saved flags * while processing with interrupts off. */ - if (up->msr_saved_flags) { - /* - * For atomic, it must be deferred to irq_work because this - * may be a context that does not permit waking up tasks. - */ - if (is_atomic) - irq_work_queue(&up->modem_status_work); - else - serial8250_modem_status(up); - } + if (up->msr_saved_flags) + serial8250_modem_status(up); - nbcon_exit_unsafe(wctxt); + if (locked) + uart_port_unlock_irqrestore(port, flags); } static unsigned int probe_baud(struct uart_port *port) @@ -3593,24 +3500,8 @@ static unsigned int probe_baud(struct uart_port *port) return (port->uartclk / 16) / quot; } -/* - * irq_work handler to perform modem control. Only triggered via - * ->write_atomic() callback because it may be in a scheduler or - * NMI context, unable to wake tasks. - */ -static void modem_status_handler(struct irq_work *iwp) -{ - struct uart_8250_port *up = container_of(iwp, struct uart_8250_port, modem_status_work); - struct uart_port *port = &up->port; - - uart_port_lock(port); - serial8250_modem_status(up); - uart_port_unlock(port); -} - int serial8250_console_setup(struct uart_port *port, char *options, bool probe) { - struct uart_8250_port *up = up_to_u8250p(port); int baud = 9600; int bits = 8; int parity = 'n'; @@ -3620,9 +3511,6 @@ int serial8250_console_setup(struct uart_port *port, char *options, bool probe) if (!port->iobase && !port->membase) return -ENODEV; - up->console_line_ended = true; - init_irq_work(&up->modem_status_work, modem_status_handler); - if (options) uart_parse_options(options, &baud, &parity, &bits, &flow); else if (probe) diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 57875c37023a..144de7a7948d 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -150,17 +150,8 @@ struct uart_8250_port { #define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS u16 lsr_saved_flags; u16 lsr_save_mask; - - /* - * Track when a console line has been fully written to the - * hardware, i.e. true when the most recent byte written to - * UART_TX by the console was '\n'. - */ - bool console_line_ended; - #define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA unsigned char msr_saved_flags; - struct irq_work modem_status_work; struct uart_8250_dma *dma; const struct uart_8250_ops *ops; @@ -211,8 +202,8 @@ void serial8250_tx_chars(struct uart_8250_port *up); unsigned int serial8250_modem_status(struct uart_8250_port *up); void serial8250_init_port(struct uart_8250_port *up); void serial8250_set_defaults(struct uart_8250_port *up); -void serial8250_console_write(struct uart_8250_port *up, - struct nbcon_write_context *wctxt, bool in_atomic); +void serial8250_console_write(struct uart_8250_port *up, const char *s, + unsigned int count); int serial8250_console_setup(struct uart_port *port, char *options, bool probe); int serial8250_console_exit(struct uart_port *port);