serial: 8250_dw: Ensure BUSY is deasserted

DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
Existance of BUSY depends on uart_16550_compatible, if UART HW is
configured with it those registers can always be written.

There currently is dw8250_force_idle() which attempts to achieve
non-BUSY state by disabling FIFO, however, the solution is unreliable
when Rx keeps getting more and more characters.

Create a sequence of operations that ensures UART cannot keep BUSY
asserted indefinitely. The new sequence relies on enabling loopback mode
temporarily to prevent incoming Rx characters keeping UART BUSY.

Ensure no Tx in ongoing while the UART is switches into the loopback
mode (requires exporting serial8250_fifo_wait_for_lsr_thre() and adding
DMA Tx pause/resume functions).

According to tests performed by Adriana Nicolae <adriana@arista.com>,
simply disabling FIFO or clearing FIFOs only once does not always
ensure BUSY is deasserted but up to two tries may be needed. This could
be related to ongoing Rx of a character (a guess, not known for sure).
Therefore, retry FIFO clearing a few times (retry limit 4 is arbitrary
number but using, e.g., p->fifosize seems overly large). Tests
performed by others did not exhibit similar challenge but it does not
seem harmful to leave the FIFO clearing loop in place for all DW UARTs
with BUSY functionality.

Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
writes. In case of plain LCR writes, opportunistically try to update
LCR first and only invoke dw8250_idle_enter() if the write did not
succeed (it has been observed that in practice most LCR writes do
succeed without complications).

This issue was first reported by qianfan Zhao who put lots of debugging
effort into understanding the solution space.

Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround")
Fixes: 7d4008ebb1c9 ("tty: add a DesignWare 8250 driver")
Cc: stable <stable@kernel.org>
Reported-by: qianfan Zhao <qianfanguijin@163.com>
Link: https://lore.kernel.org/linux-serial/289bb78a-7509-1c5c-2923-a04ed3b6487d@163.com/
Reported-by: Adriana Nicolae <adriana@arista.com>
Link: https://lore.kernel.org/linux-serial/20250819182322.3451959-1-adriana@arista.com/
Reported-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Bandal, Shankar <shankar.bandal@intel.com>
Tested-by: Murthy, Shanth <shanth.murthy@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://patch.msgid.link/20260203171049.4353-8-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Ilpo Järvinen and committed by

Greg Kroah-Hartman 3 months ago a7b9ce39 e0a368ae

+165 -59

3 changed files

expand all

drivers

tty

serial

8250

8250.h

8250_dw.c

8250_port.c

+25

drivers/tty/serial/8250/8250.h

··· 175 175 return value; 176 176 } 177 177 178 + void serial8250_clear_fifos(struct uart_8250_port *p); 178 179 void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p); 180 + void serial8250_fifo_wait_for_lsr_thre(struct uart_8250_port *up, unsigned int count); 179 181 180 182 void serial8250_rpm_get(struct uart_8250_port *p); 181 183 void serial8250_rpm_put(struct uart_8250_port *p); ··· 402 400 403 401 return dma && dma->tx_running; 404 402 } 403 + 404 + static inline void serial8250_tx_dma_pause(struct uart_8250_port *p) 405 + { 406 + struct uart_8250_dma *dma = p->dma; 407 + 408 + if (!dma->tx_running) 409 + return; 410 + 411 + dmaengine_pause(dma->txchan); 412 + } 413 + 414 + static inline void serial8250_tx_dma_resume(struct uart_8250_port *p) 415 + { 416 + struct uart_8250_dma *dma = p->dma; 417 + 418 + if (!dma->tx_running) 419 + return; 420 + 421 + dmaengine_resume(dma->txchan); 422 + } 405 423 #else 406 424 static inline int serial8250_tx_dma(struct uart_8250_port *p) 407 425 { ··· 443 421 { 444 422 return false; 445 423 } 424 + 425 + static inline void serial8250_tx_dma_pause(struct uart_8250_port *p) { } 426 + static inline void serial8250_tx_dma_resume(struct uart_8250_port *p) { } 446 427 #endif 447 428 448 429 static inline int ns16550a_goto_highspeed(struct uart_8250_port *up)

+125 -46

drivers/tty/serial/8250/8250_dw.c

··· 16 16 #include <linux/delay.h> 17 17 #include <linux/device.h> 18 18 #include <linux/io.h> 19 + #include <linux/lockdep.h> 19 20 #include <linux/mod_devicetable.h> 20 21 #include <linux/module.h> 21 22 #include <linux/notifier.h> ··· 47 46 #define DW_UART_IIR_IID GENMASK(3, 0) 48 47 49 48 #define DW_UART_MCR_SIRE BIT(6) 49 + 50 + #define DW_UART_USR_BUSY BIT(0) 50 51 51 52 /* Renesas specific register fields */ 52 53 #define RZN1_UART_xDMACR_DMA_EN BIT(0) ··· 92 89 93 90 unsigned int skip_autocfg:1; 94 91 unsigned int uart_16550_compatible:1; 92 + unsigned int in_idle:1; 95 93 96 94 u8 no_int_count; 97 95 }; ··· 125 121 return value; 126 122 } 127 123 128 - /* 129 - * This function is being called as part of the uart_port::serial_out() 130 - * routine. Hence, it must not call serial_port_out() or serial_out() 131 - * against the modified registers here, i.e. LCR. 132 - */ 133 - static void dw8250_force_idle(struct uart_port *p) 124 + static void dw8250_idle_exit(struct uart_port *p) 134 125 { 126 + struct dw8250_data *d = to_dw8250_data(p->private_data); 135 127 struct uart_8250_port *up = up_to_u8250p(p); 136 - unsigned int lsr; 137 128 138 - /* 139 - * The following call currently performs serial_out() 140 - * against the FCR register. Because it differs to LCR 141 - * there will be no infinite loop, but if it ever gets 142 - * modified, we might need a new custom version of it 143 - * that avoids infinite recursion. 144 - */ 145 - serial8250_clear_and_reinit_fifos(up); 129 + if (d->uart_16550_compatible) 130 + return; 146 131 147 - /* 148 - * With PSLVERR_RESP_EN parameter set to 1, the device generates an 149 - * error response when an attempt to read an empty RBR with FIFO 150 - * enabled. 151 - */ 152 - if (up->fcr & UART_FCR_ENABLE_FIFO) { 153 - lsr = serial_port_in(p, UART_LSR); 154 - if (!(lsr & UART_LSR_DR)) 155 - return; 132 + if (up->capabilities & UART_CAP_FIFO) 133 + serial_port_out(p, UART_FCR, up->fcr); 134 + serial_port_out(p, UART_MCR, up->mcr); 135 + serial_port_out(p, UART_IER, up->ier); 136 + 137 + /* DMA Rx is restarted by IRQ handler as needed. */ 138 + if (up->dma) 139 + serial8250_tx_dma_resume(up); 140 + 141 + d->in_idle = 0; 142 + } 143 + 144 + /* 145 + * Ensure BUSY is not asserted. If DW UART is configured with 146 + * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while 147 + * BUSY is asserted. 148 + * 149 + * Context: port's lock must be held 150 + */ 151 + static int dw8250_idle_enter(struct uart_port *p) 152 + { 153 + struct dw8250_data *d = to_dw8250_data(p->private_data); 154 + unsigned int usr_reg = d->pdata ? d->pdata->usr_reg : DW_UART_USR; 155 + struct uart_8250_port *up = up_to_u8250p(p); 156 + int retries; 157 + u32 lsr; 158 + 159 + lockdep_assert_held_once(&p->lock); 160 + 161 + if (d->uart_16550_compatible) 162 + return 0; 163 + 164 + d->in_idle = 1; 165 + 166 + /* Prevent triggering interrupt from RBR filling */ 167 + serial_port_out(p, UART_IER, 0); 168 + 169 + if (up->dma) { 170 + serial8250_rx_dma_flush(up); 171 + if (serial8250_tx_dma_running(up)) 172 + serial8250_tx_dma_pause(up); 156 173 } 157 174 158 - serial_port_in(p, UART_RX); 175 + /* 176 + * Wait until Tx becomes empty + one extra frame time to ensure all bits 177 + * have been sent on the wire. 178 + * 179 + * FIXME: frame_time delay is too long with very low baudrates. 180 + */ 181 + serial8250_fifo_wait_for_lsr_thre(up, p->fifosize); 182 + ndelay(p->frame_time); 183 + 184 + serial_port_out(p, UART_MCR, up->mcr | UART_MCR_LOOP); 185 + 186 + retries = 4; /* Arbitrary limit, 2 was always enough in tests */ 187 + do { 188 + serial8250_clear_fifos(up); 189 + if (!(serial_port_in(p, usr_reg) & DW_UART_USR_BUSY)) 190 + break; 191 + /* FIXME: frame_time delay is too long with very low baudrates. */ 192 + ndelay(p->frame_time); 193 + } while (--retries); 194 + 195 + lsr = serial_lsr_in(up); 196 + if (lsr & UART_LSR_DR) { 197 + serial_port_in(p, UART_RX); 198 + up->lsr_saved_flags = 0; 199 + } 200 + 201 + /* Now guaranteed to have BUSY deasserted? Just sanity check */ 202 + if (serial_port_in(p, usr_reg) & DW_UART_USR_BUSY) { 203 + dw8250_idle_exit(p); 204 + return -EBUSY; 205 + } 206 + 207 + return 0; 208 + } 209 + 210 + static void dw8250_set_divisor(struct uart_port *p, unsigned int baud, 211 + unsigned int quot, unsigned int quot_frac) 212 + { 213 + struct uart_8250_port *up = up_to_u8250p(p); 214 + int ret; 215 + 216 + ret = dw8250_idle_enter(p); 217 + if (ret < 0) 218 + return; 219 + 220 + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); 221 + if (!(serial_port_in(p, UART_LCR) & UART_LCR_DLAB)) 222 + goto idle_failed; 223 + 224 + serial_dl_write(up, quot); 225 + serial_port_out(p, UART_LCR, up->lcr); 226 + 227 + idle_failed: 228 + dw8250_idle_exit(p); 159 229 } 160 230 161 231 /* 162 232 * This function is being called as part of the uart_port::serial_out() 163 - * routine. Hence, it must not call serial_port_out() or serial_out() 164 - * against the modified registers here, i.e. LCR. 233 + * routine. Hence, special care must be taken when serial_port_out() or 234 + * serial_out() against the modified registers here, i.e. LCR (d->in_idle is 235 + * used to break recursion loop). 165 236 */ 166 237 static void dw8250_check_lcr(struct uart_port *p, unsigned int offset, u32 value) 167 238 { 168 239 struct dw8250_data *d = to_dw8250_data(p->private_data); 169 - void __iomem *addr = p->membase + (offset << p->regshift); 170 - int tries = 1000; 240 + u32 lcr; 241 + int ret; 171 242 172 243 if (offset != UART_LCR || d->uart_16550_compatible) 173 244 return; 174 245 246 + lcr = serial_port_in(p, UART_LCR); 247 + 175 248 /* Make sure LCR write wasn't ignored */ 176 - while (tries--) { 177 - u32 lcr = serial_port_in(p, offset); 249 + if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) 250 + return; 178 251 179 - if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) 180 - return; 252 + if (d->in_idle) 253 + goto write_err; 181 254 182 - dw8250_force_idle(p); 255 + ret = dw8250_idle_enter(p); 256 + if (ret < 0) 257 + goto write_err; 183 258 184 - #ifdef CONFIG_64BIT 185 - if (p->type == PORT_OCTEON) 186 - __raw_writeq(value & 0xff, addr); 187 - else 188 - #endif 189 - if (p->iotype == UPIO_MEM32) 190 - writel(value, addr); 191 - else if (p->iotype == UPIO_MEM32BE) 192 - iowrite32be(value, addr); 193 - else 194 - writeb(value, addr); 195 - } 259 + serial_port_out(p, UART_LCR, value); 260 + dw8250_idle_exit(p); 261 + return; 262 + 263 + write_err: 196 264 /* 197 265 * FIXME: this deadlocks if port->lock is already held 198 266 * dev_err(p->dev, "Couldn't set LCR to %d\n", value); 199 267 */ 268 + return; /* Silences "label at the end of compound statement" */ 200 269 } 201 270 202 271 /* ··· 709 632 p->type = PORT_8250; 710 633 p->flags = UPF_FIXED_PORT; 711 634 p->dev = dev; 635 + 712 636 p->set_ldisc = dw8250_set_ldisc; 713 637 p->set_termios = dw8250_set_termios; 638 + p->set_divisor = dw8250_set_divisor; 714 639 715 640 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); 716 641 if (!data)

+15 -13

drivers/tty/serial/8250/8250_port.c

··· 489 489 /* 490 490 * FIFO support. 491 491 */ 492 - static void serial8250_clear_fifos(struct uart_8250_port *p) 492 + void serial8250_clear_fifos(struct uart_8250_port *p) 493 493 { 494 494 if (p->capabilities & UART_CAP_FIFO) { 495 495 serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO); ··· 498 498 serial_out(p, UART_FCR, 0); 499 499 } 500 500 } 501 + EXPORT_SYMBOL_NS_GPL(serial8250_clear_fifos, "SERIAL_8250"); 501 502 502 503 static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t); 503 504 static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t); ··· 3199 3198 } 3200 3199 EXPORT_SYMBOL_GPL(serial8250_set_defaults); 3201 3200 3201 + void serial8250_fifo_wait_for_lsr_thre(struct uart_8250_port *up, unsigned int count) 3202 + { 3203 + unsigned int i; 3204 + 3205 + for (i = 0; i < count; i++) { 3206 + if (wait_for_lsr(up, UART_LSR_THRE)) 3207 + return; 3208 + } 3209 + } 3210 + EXPORT_SYMBOL_NS_GPL(serial8250_fifo_wait_for_lsr_thre, "SERIAL_8250"); 3211 + 3202 3212 #ifdef CONFIG_SERIAL_8250_CONSOLE 3203 3213 3204 3214 static void serial8250_console_putchar(struct uart_port *port, unsigned char ch) ··· 3251 3239 serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS); 3252 3240 } 3253 3241 3254 - static void fifo_wait_for_lsr(struct uart_8250_port *up, unsigned int count) 3255 - { 3256 - unsigned int i; 3257 - 3258 - for (i = 0; i < count; i++) { 3259 - if (wait_for_lsr(up, UART_LSR_THRE)) 3260 - return; 3261 - } 3262 - } 3263 - 3264 3242 /* 3265 3243 * Print a string to the serial port using the device FIFO 3266 3244 * ··· 3269 3267 3270 3268 while (s != end) { 3271 3269 /* Allow timeout for each byte of a possibly full FIFO */ 3272 - fifo_wait_for_lsr(up, fifosize); 3270 + serial8250_fifo_wait_for_lsr_thre(up, fifosize); 3273 3271 3274 3272 for (i = 0; i < fifosize && s != end; ++i) { 3275 3273 if (*s == '\n' && !cr_sent) { ··· 3287 3285 * Allow timeout for each byte written since the caller will only wait 3288 3286 * for UART_LSR_BOTH_EMPTY using the timeout of a single character 3289 3287 */ 3290 - fifo_wait_for_lsr(up, tx_count); 3288 + serial8250_fifo_wait_for_lsr_thre(up, tx_count); 3291 3289 } 3292 3290 3293 3291 /*

Configure Feed

Configure Feed