From 535a07698b8b3e6f305673102d297262cae2360a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 4 Dec 2024 05:09:22 +0200 Subject: serial: 8250_pci: Share WCH IDs with parport_serial driver parport_serial driver uses subset of WCH IDs that are present in 8250_pci. Share them via pci_ids.h and switch parport_serial to use defined constants. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20241204031114.1029882-3-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- include/linux/pci_ids.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'include/linux') diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index d2402bf4aea2..de5deb1a0118 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2593,6 +2593,11 @@ #define PCI_VENDOR_ID_REDHAT 0x1b36 +#define PCI_VENDOR_ID_WCHIC 0x1c00 +#define PCI_DEVICE_ID_WCHIC_CH382_0S1P 0x3050 +#define PCI_DEVICE_ID_WCHIC_CH382_2S1P 0x3250 +#define PCI_DEVICE_ID_WCHIC_CH382_2S 0x3253 + #define PCI_VENDOR_ID_SILICOM_DENMARK 0x1c2c #define PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS 0x1c36 @@ -2647,6 +2652,12 @@ #define PCI_VENDOR_ID_AKS 0x416c #define PCI_DEVICE_ID_AKS_ALADDINCARD 0x0100 +#define PCI_VENDOR_ID_WCHCN 0x4348 +#define PCI_DEVICE_ID_WCHCN_CH353_4S 0x3453 +#define PCI_DEVICE_ID_WCHCN_CH353_2S1PF 0x5046 +#define PCI_DEVICE_ID_WCHCN_CH353_1S1P 0x5053 +#define PCI_DEVICE_ID_WCHCN_CH353_2S1P 0x7053 + #define PCI_VENDOR_ID_ACCESSIO 0x494f #define PCI_DEVICE_ID_ACCESSIO_WDG_CSM 0x22c0 -- cgit v1.3.1 From 910ef438e93cd2ceb70c72caea418710d648feef Mon Sep 17 00:00:00 2001 From: John Ogness Date: Tue, 7 Jan 2025 22:33:00 +0106 Subject: serial: 8250: Provide flag for IER toggling for RS485 For RS485 mode, if SER_RS485_RX_DURING_TX is not available, the console ->write() callback needs to enable/disable Tx. It does this by calling the ->rs485_start_tx() and ->rs485_stop_tx() callbacks. However, some of these callbacks also disable/enable interrupts and makes power management calls. This causes 2 problems for console writing: 1. A console write can occur in contexts that are illegal for pm_runtime_*(). It is not even necessary for console writing to use pm_runtime_*() because a console already does this in serial8250_console_setup() and serial8250_console_exit(). 2. The console ->write() callback already handles disabling/enabling the interrupts by properly restoring the previous IER value. Add an argument @toggle_ier to the ->rs485_start_tx() and ->rs485_stop_tx() callbacks to specify if they may disable/enable receive interrupts while using pm_runtime_*(). Console writing will not allow the toggling. For all call sites other than console writing there is no functional change. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Link: https://lore.kernel.org/r/20250107212702.169493-5-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250.h | 4 ++-- drivers/tty/serial/8250/8250_bcm2835aux.c | 4 ++-- drivers/tty/serial/8250/8250_omap.c | 2 +- drivers/tty/serial/8250/8250_port.c | 26 +++++++++++++++----------- include/linux/serial_8250.h | 4 ++-- 5 files changed, 22 insertions(+), 18 deletions(-) (limited to 'include/linux') diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index e5310c65cf52..11e05aa014e5 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -231,8 +231,8 @@ void serial8250_rpm_put_tx(struct uart_8250_port *p); int serial8250_em485_config(struct uart_port *port, struct ktermios *termios, struct serial_rs485 *rs485); -void serial8250_em485_start_tx(struct uart_8250_port *p); -void serial8250_em485_stop_tx(struct uart_8250_port *p); +void serial8250_em485_start_tx(struct uart_8250_port *p, bool toggle_ier); +void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier); void serial8250_em485_destroy(struct uart_8250_port *p); extern struct serial_rs485 serial8250_em485_supported; diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c index fdb53b54e99e..0609582a62f7 100644 --- a/drivers/tty/serial/8250/8250_bcm2835aux.c +++ b/drivers/tty/serial/8250/8250_bcm2835aux.c @@ -46,7 +46,7 @@ struct bcm2835aux_data { u32 cntl; }; -static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up) +static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up, bool toggle_ier) { if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) { struct bcm2835aux_data *data = dev_get_drvdata(up->port.dev); @@ -65,7 +65,7 @@ static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up) serial8250_out_MCR(up, UART_MCR_RTS); } -static void bcm2835aux_rs485_stop_tx(struct uart_8250_port *up) +static void bcm2835aux_rs485_stop_tx(struct uart_8250_port *up, bool toggle_ier) { if (up->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) serial8250_out_MCR(up, 0); diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 42b4aa56b902..c2b75e3f106d 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -365,7 +365,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) if (up->port.rs485.flags & SER_RS485_ENABLED && up->port.rs485_config == serial8250_em485_config) - serial8250_em485_stop_tx(up); + serial8250_em485_stop_tx(up, true); } /* diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 15abd95fcf06..d7976a21cca9 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -578,7 +578,7 @@ static int serial8250_em485_init(struct uart_8250_port *p) deassert_rts: if (p->em485->tx_stopped) - p->rs485_stop_tx(p); + p->rs485_stop_tx(p, true); return 0; } @@ -1398,10 +1398,11 @@ static void serial8250_stop_rx(struct uart_port *port) /** * serial8250_em485_stop_tx() - generic ->rs485_stop_tx() callback * @p: uart 8250 port + * @toggle_ier: true to allow enabling receive interrupts * * Generic callback usable by 8250 uart drivers to stop rs485 transmission. */ -void serial8250_em485_stop_tx(struct uart_8250_port *p) +void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier) { unsigned char mcr = serial8250_in_MCR(p); @@ -1422,8 +1423,10 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) { serial8250_clear_and_reinit_fifos(p); - p->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_port_out(&p->port, UART_IER, p->ier); + if (toggle_ier) { + p->ier |= UART_IER_RLSI | UART_IER_RDI; + serial_port_out(&p->port, UART_IER, p->ier); + } } } EXPORT_SYMBOL_GPL(serial8250_em485_stop_tx); @@ -1438,7 +1441,7 @@ static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t) serial8250_rpm_get(p); uart_port_lock_irqsave(&p->port, &flags); if (em485->active_timer == &em485->stop_tx_timer) { - p->rs485_stop_tx(p); + p->rs485_stop_tx(p, true); em485->active_timer = NULL; em485->tx_stopped = true; } @@ -1470,7 +1473,7 @@ static void __stop_tx_rs485(struct uart_8250_port *p, u64 stop_delay) em485->active_timer = &em485->stop_tx_timer; hrtimer_start(&em485->stop_tx_timer, ns_to_ktime(stop_delay), HRTIMER_MODE_REL); } else { - p->rs485_stop_tx(p); + p->rs485_stop_tx(p, true); em485->active_timer = NULL; em485->tx_stopped = true; } @@ -1559,6 +1562,7 @@ static inline void __start_tx(struct uart_port *port) /** * serial8250_em485_start_tx() - generic ->rs485_start_tx() callback * @up: uart 8250 port + * @toggle_ier: true to allow disabling receive interrupts * * Generic callback usable by 8250 uart drivers to start rs485 transmission. * Assumes that setting the RTS bit in the MCR register means RTS is high. @@ -1566,11 +1570,11 @@ static inline void __start_tx(struct uart_port *port) * stoppable by disabling the UART_IER_RDI interrupt. (Some chips set the * UART_LSR_DR bit even when UART_IER_RDI is disabled, foiling this approach.) */ -void serial8250_em485_start_tx(struct uart_8250_port *up) +void serial8250_em485_start_tx(struct uart_8250_port *up, bool toggle_ier) { unsigned char mcr = serial8250_in_MCR(up); - if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) + if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX) && toggle_ier) serial8250_stop_rx(&up->port); if (up->port.rs485.flags & SER_RS485_RTS_ON_SEND) @@ -1604,7 +1608,7 @@ static bool start_tx_rs485(struct uart_port *port) if (em485->tx_stopped) { em485->tx_stopped = false; - up->rs485_start_tx(up); + up->rs485_start_tx(up, true); if (up->port.rs485.delay_rts_before_send > 0) { em485->active_timer = &em485->start_tx_timer; @@ -3424,7 +3428,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, if (em485) { if (em485->tx_stopped) - up->rs485_start_tx(up); + up->rs485_start_tx(up, false); mdelay(port->rs485.delay_rts_before_send); } @@ -3462,7 +3466,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, if (em485) { mdelay(port->rs485.delay_rts_after_send); if (em485->tx_stopped) - up->rs485_stop_tx(up); + up->rs485_stop_tx(up, false); } serial_port_out(port, UART_IER, ier); diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index e0717c8393d7..144de7a7948d 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -161,8 +161,8 @@ struct uart_8250_port { void (*dl_write)(struct uart_8250_port *up, u32 value); struct uart_8250_em485 *em485; - void (*rs485_start_tx)(struct uart_8250_port *); - void (*rs485_stop_tx)(struct uart_8250_port *); + void (*rs485_start_tx)(struct uart_8250_port *up, bool toggle_ier); + void (*rs485_stop_tx)(struct uart_8250_port *up, bool toggle_ier); /* Serial port overrun backoff */ struct delayed_work overrun_backoff; -- cgit v1.3.1 From b63e6f60eab45b16a1bf734fef9035a4c4187cd5 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Tue, 7 Jan 2025 22:33:01 +0106 Subject: serial: 8250: Switch to nbcon console Implement the necessary callbacks to switch the 8250 console driver to perform as an nbcon console. Add implementations for the nbcon console callbacks: ->write_atomic() ->write_thread() ->device_lock() ->device_unlock() and add CON_NBCON to the initial @flags. All register access in the callbacks are within unsafe sections. The ->write_atomic() and ->write_thread() callbacks allow safe handover/takeover per byte and add a preceding newline if they take over from another context mid-line. For the ->write_atomic() callback, a new irq_work is used to defer modem control since it may be called from a context that does not allow waking up tasks. Note: A new __serial8250_clear_IER() is introduced for direct clearing of UART_IER. This will allow to restore the lockdep check to serial8250_clear_IER() in a follow-up commit. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Tested-by: Petr Mladek Link: https://lore.kernel.org/r/20250107212702.169493-6-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_core.c | 35 ++++++- drivers/tty/serial/8250/8250_port.c | 180 +++++++++++++++++++++++++++++------- include/linux/serial_8250.h | 13 ++- 3 files changed, 187 insertions(+), 41 deletions(-) (limited to 'include/linux') diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 2b70e82dffeb..3d5a2183ff13 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -388,12 +388,34 @@ void __init serial8250_register_ports(struct uart_driver *drv, struct device *de #ifdef CONFIG_SERIAL_8250_CONSOLE -static void univ8250_console_write(struct console *co, const char *s, - unsigned int count) +static void univ8250_console_write_atomic(struct console *co, + struct nbcon_write_context *wctxt) { struct uart_8250_port *up = &serial8250_ports[co->index]; - serial8250_console_write(up, s, count); + 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); } static int univ8250_console_setup(struct console *co, char *options) @@ -494,12 +516,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx, static struct console univ8250_console = { .name = "ttyS", - .write = univ8250_console_write, + .write_atomic = univ8250_console_write_atomic, + .write_thread = univ8250_console_write_thread, + .device_lock = univ8250_console_device_lock, + .device_unlock = univ8250_console_device_unlock, .device = uart_console_device, .setup = univ8250_console_setup, .exit = univ8250_console_exit, .match = univ8250_console_match, - .flags = CON_PRINTBUFFER | CON_ANYTIME, + .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_NBCON, .index = -1, .data = &serial8250_reg, }; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index d7976a21cca9..08466cf10d73 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -711,7 +711,12 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_put(p); } -static void serial8250_clear_IER(struct uart_8250_port *up) +/* + * 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) { if (up->capabilities & UART_CAP_UUE) serial_out(up, UART_IER, UART_IER_UUE); @@ -719,6 +724,11 @@ 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. @@ -1406,9 +1416,6 @@ 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 @@ -1424,6 +1431,16 @@ 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); } @@ -3303,7 +3320,11 @@ 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) @@ -3340,11 +3361,22 @@ 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, unsigned int count) +static void fifo_wait_for_lsr(struct uart_8250_port *up, + struct nbcon_write_context *wctxt, + 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; } @@ -3357,20 +3389,29 @@ static void fifo_wait_for_lsr(struct uart_8250_port *up, unsigned int count) * to get empty. */ static void serial8250_console_fifo_write(struct uart_8250_port *up, - const char *s, unsigned int count) + struct nbcon_write_context *wctxt) { - 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, fifosize); + fifo_wait_for_lsr(up, wctxt, 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; @@ -3378,6 +3419,8 @@ 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; } @@ -3386,39 +3429,57 @@ 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, tx_count); + 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); + } } /* - * 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. + * Print a string to the serial port trying not to disturb + * any possible real use of the port... * - * Doing runtime PM is really a bad idea for the kernel console. - * Thus, we assume the function is called when device is powered up. + * Doing runtime PM is really a bad idea for the kernel console. + * Thus, assume it is called when device is powered up. */ -void serial8250_console_write(struct uart_8250_port *up, const char *s, - unsigned int count) +void serial8250_console_write(struct uart_8250_port *up, + struct nbcon_write_context *wctxt, + bool is_atomic) { struct uart_8250_em485 *em485 = up->em485; struct uart_port *port = &up->port; - unsigned long flags; - unsigned int ier, use_fifo; - int locked = 1; - - touch_nmi_watchdog(); + unsigned int ier; + bool use_fifo; - if (oops_in_progress) - locked = uart_port_trylock_irqsave(port, &flags); - else - uart_port_lock_irqsave(port, &flags); + if (!nbcon_enter_unsafe(wctxt)) + return; /* - * First save the IER then disable the interrupts + * 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. */ 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))) { @@ -3432,6 +3493,18 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, 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 @@ -3452,10 +3525,23 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, */ !(up->port.flags & UPF_CONS_FLOW); + nbcon_exit_unsafe(wctxt); + if (likely(use_fifo)) - serial8250_console_fifo_write(up, s, count); + serial8250_console_fifo_write(up, wctxt); else - uart_console_write(port, s, count, serial8250_console_wait_putchar); + 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)); + } /* * Finally, wait for transmitter to become empty @@ -3478,11 +3564,18 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, * call it if we have saved something in the saved flags * while processing with interrupts off. */ - if (up->msr_saved_flags) - serial8250_modem_status(up); + 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 (locked) - uart_port_unlock_irqrestore(port, flags); + nbcon_exit_unsafe(wctxt); } static unsigned int probe_baud(struct uart_port *port) @@ -3500,8 +3593,24 @@ 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'; @@ -3511,6 +3620,9 @@ 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 144de7a7948d..57875c37023a 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -150,8 +150,17 @@ 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; @@ -202,8 +211,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, const char *s, - unsigned int count); +void serial8250_console_write(struct uart_8250_port *up, + struct nbcon_write_context *wctxt, bool in_atomic); int serial8250_console_setup(struct uart_port *port, char *options, bool probe); int serial8250_console_exit(struct uart_port *port); -- cgit v1.3.1 From 39d0be87438a0cc29151898c7fba24b43f2f3df8 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Sun, 12 Jan 2025 13:57:59 +0000 Subject: serial: kgdb_nmi: Remove unused knock code kgdb_nmi_poll_knock() has been unused since it was added in 2013 in commit 0c57dfcc6c1d ("tty/serial: Add kgdb_nmi driver") Remove it, the static helpers, and module parameters it used. (The comment explaining why it might be used sounds sensible, but it's never been wired up, perhaps it's worth doing somewhere?) Signed-off-by: Dr. David Alan Gilbert Link: https://lore.kernel.org/r/20250112135759.105541-1-linux@treblig.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/kgdb_nmi.c | 101 ------------------------------------------ include/linux/kgdb.h | 2 - 2 files changed, 103 deletions(-) (limited to 'include/linux') diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index e93850f6447a..2833708e369f 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -27,18 +27,6 @@ #include #include -static int kgdb_nmi_knock = 1; -module_param_named(knock, kgdb_nmi_knock, int, 0600); -MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command " \ - "must be used to enter the debugger; when set to 0, " \ - "hitting return key is enough to enter the debugger; " \ - "when set to -1, the debugger is entered immediately " \ - "upon NMI"); - -static char *kgdb_nmi_magic = "$3#33"; -module_param_named(magic, kgdb_nmi_magic, charp, 0600); -MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)"); - static atomic_t kgdb_nmi_num_readers = ATOMIC_INIT(0); static int kgdb_nmi_console_setup(struct console *co, char *options) @@ -95,95 +83,6 @@ struct kgdb_nmi_tty_priv { static struct tty_port *kgdb_nmi_port; -static void kgdb_tty_recv(int ch) -{ - struct kgdb_nmi_tty_priv *priv; - char c = ch; - - if (!kgdb_nmi_port || ch < 0) - return; - /* - * Can't use port->tty->driver_data as tty might be not there. Timer - * will check for tty and will get the ref, but here we don't have to - * do that, and actually, we can't: we're in NMI context, no locks are - * possible. - */ - priv = container_of(kgdb_nmi_port, struct kgdb_nmi_tty_priv, port); - kfifo_in(&priv->fifo, &c, 1); -} - -static int kgdb_nmi_poll_one_knock(void) -{ - static int n; - int c; - const char *magic = kgdb_nmi_magic; - size_t m = strlen(magic); - bool printch = false; - - c = dbg_io_ops->read_char(); - if (c == NO_POLL_CHAR) - return c; - - if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) { - return 1; - } else if (c == magic[n]) { - n = (n + 1) % m; - if (!n) - return 1; - printch = true; - } else { - n = 0; - } - - if (atomic_read(&kgdb_nmi_num_readers)) { - kgdb_tty_recv(c); - return 0; - } - - if (printch) { - kdb_printf("%c", c); - return 0; - } - - kdb_printf("\r%s %s to enter the debugger> %*s", - kgdb_nmi_knock ? "Type" : "Hit", - kgdb_nmi_knock ? magic : "", (int)m, ""); - while (m--) - kdb_printf("\b"); - return 0; -} - -/** - * kgdb_nmi_poll_knock - Check if it is time to enter the debugger - * - * "Serial ports are often noisy, especially when muxed over another port (we - * often use serial over the headset connector). Noise on the async command - * line just causes characters that are ignored, on a command line that blocked - * execution noise would be catastrophic." -- Colin Cross - * - * So, this function implements KGDB/KDB knocking on the serial line: we won't - * enter the debugger until we receive a known magic phrase (which is actually - * "$3#33", known as "escape to KDB" command. There is also a relaxed variant - * of knocking, i.e. just pressing the return key is enough to enter the - * debugger. And if knocking is disabled, the function always returns 1. - */ -bool kgdb_nmi_poll_knock(void) -{ - if (kgdb_nmi_knock < 0) - return true; - - while (1) { - int ret; - - ret = kgdb_nmi_poll_one_knock(); - if (ret == NO_POLL_CHAR) - return false; - else if (ret == 1) - break; - } - return true; -} - /* * The tasklet is cheap, it does not cause wakeups when reschedules itself, * instead it waits for the next tick. diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 76e891ee9e37..51ef131e66b7 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -309,11 +309,9 @@ extern unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs); #ifdef CONFIG_SERIAL_KGDB_NMI extern int kgdb_register_nmi_console(void); extern int kgdb_unregister_nmi_console(void); -extern bool kgdb_nmi_poll_knock(void); #else static inline int kgdb_register_nmi_console(void) { return 0; } static inline int kgdb_unregister_nmi_console(void) { return 0; } -static inline bool kgdb_nmi_poll_knock(void) { return true; } #endif extern int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops); -- cgit v1.3.1 From f79b163c42314a1f46f4bcc40a19c8a75cf1e7a3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 22 Jan 2025 10:35:56 +0100 Subject: 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 --- drivers/tty/serial/8250/8250_core.c | 35 +------ drivers/tty/serial/8250/8250_port.c | 180 +++++++----------------------------- include/linux/serial_8250.h | 13 +-- 3 files changed, 41 insertions(+), 187 deletions(-) (limited to 'include/linux') 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); -- cgit v1.3.1