From 12ed0528b32592aec4d97f34a5e67b096aa5a07d Mon Sep 17 00:00:00 2001 From: Daniel Leung Date: Wed, 25 Jun 2025 09:56:19 -0700 Subject: [PATCH] Revert "drivers: serial: ns16550: Fix TX IRQ not triggered... ...when FIFO is empty" This reverts commit 47e43d552e813c1f9acc4e999c5dd59008d72905. This is breaking sample.sensor.shell.pytest where characters are either missing or repeated when printing to the console. Originally this is just for RISC-V with PLIC interrupt controller. That was made more general to avoid having arch specific code in a generic driver. And now it is breaking on non-RISC-V platforms. Note that the QEMU RISC-V boards all have PLIC as interrupt controller and are passing sensor shell pytest without this workaround. So this does not seem to be an issue with PLIC and NS16550. So revert the commit for now. Fixes #92187 Signed-off-by: Daniel Leung --- drivers/serial/Kconfig.ns16550 | 11 -------- drivers/serial/uart_ns16550.c | 49 ---------------------------------- 2 files changed, 60 deletions(-) diff --git a/drivers/serial/Kconfig.ns16550 b/drivers/serial/Kconfig.ns16550 index 64114235431..4c6676cdab6 100644 --- a/drivers/serial/Kconfig.ns16550 +++ b/drivers/serial/Kconfig.ns16550 @@ -103,17 +103,6 @@ config UART_NS16550_WA_ISR_REENABLE_INTERRUPT the IER is being toggled to re-assert interrupts at the end of ISR to nudge the host interrupt controller to fire the ISR again. -config UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - bool "Callback directly when the TX FIFO is already empty" - default y if SHELL_BACKEND_SERIAL - depends on UART_INTERRUPT_DRIVEN - help - When calling uart_ns16550_irq_tx_enable() to wait for the TX FIFO - ready interrupt, but this interrupt will only be triggered if the - current state is not empty. Therefore, if the current state is - empty, you need to solve this problem by calling the callback - function directly. - endmenu endif # UART_NS16550 diff --git a/drivers/serial/uart_ns16550.c b/drivers/serial/uart_ns16550.c index 0885bd983dd..8b1381e59d8 100644 --- a/drivers/serial/uart_ns16550.c +++ b/drivers/serial/uart_ns16550.c @@ -369,10 +369,6 @@ struct uart_ns16550_dev_data { void *cb_data; /**< Callback function arg */ #endif -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - uint8_t sw_tx_irq; /**< software tx ready flag */ -#endif - #if UART_NS16550_DLF_ENABLED uint8_t dlf; /**< DLF value */ #endif @@ -937,10 +933,6 @@ static void uart_ns16550_poll_out(const struct device *dev, ns16550_outbyte(dev_cfg, THR(dev), c); -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - data->sw_tx_irq = 0; /**< clean up */ -#endif - k_spin_unlock(&data->lock, key); } @@ -988,12 +980,6 @@ static int uart_ns16550_fifo_fill(const struct device *dev, ns16550_outbyte(dev_cfg, THR(dev), tx_data[i]); } -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - if (i != 0) { - data->sw_tx_irq = 0; /**< clean up */ - } -#endif - k_spin_unlock(&data->lock, key); return i; @@ -1056,26 +1042,6 @@ static void uart_ns16550_irq_tx_enable(const struct device *dev) #endif ns16550_outbyte(dev_cfg, IER(dev), ns16550_inbyte(dev_cfg, IER(dev)) | IER_TBE); -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - if (ns16550_inbyte(dev_cfg, LSR(dev)) & LSR_THRE) { - k_spin_unlock(&data->lock, key); - /* - * The TX FIFO ready interrupt will be triggered if only if - * when the pre-state is not empty. Thus, if the pre-state is - * already empty, try to call the callback routine directly - * to resolve it. - */ - int irq_lock_key = arch_irq_lock(); - - if (data->cb && (ns16550_inbyte(dev_cfg, LSR(dev)) & LSR_THRE)) { - data->sw_tx_irq = 1; /**< set tx ready */ - data->cb(dev, data->cb_data); - } - arch_irq_unlock(irq_lock_key); - return; - } -#endif - k_spin_unlock(&data->lock, key); } @@ -1090,10 +1056,6 @@ static void uart_ns16550_irq_tx_disable(const struct device *dev) const struct uart_ns16550_dev_config * const dev_cfg = dev->config; k_spinlock_key_t key = k_spin_lock(&data->lock); -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - data->sw_tx_irq = 0; /**< clean up */ -#endif - ns16550_outbyte(dev_cfg, IER(dev), ns16550_inbyte(dev_cfg, IER(dev)) & (~IER_TBE)); @@ -1134,17 +1096,6 @@ static int uart_ns16550_irq_tx_ready(const struct device *dev) int ret = ((IIRC(dev) & IIR_ID) == IIR_THRE) ? 1 : 0; -#ifdef CONFIG_UART_NS16550_WA_TX_FIFO_EMPTY_INTERRUPT - if (ret == 0 && data->sw_tx_irq) { - /**< replace resoult when there is a software solution */ - const struct uart_ns16550_dev_config * const dev_cfg = dev->config; - - if (ns16550_inbyte(dev_cfg, IER(dev)) & IER_TBE) { - ret = 1; - } - } -#endif - k_spin_unlock(&data->lock, key); return ret;