Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Revert "serial: 8250: Fix clearing FIFOs in RS485 mode again"

Commit f6aa5beb45be ("serial: 8250: Fix clearing FIFOs in RS485 mode
again") makes a change to FIFO clearing code which its commit message
suggests was intended to be specific to use with RS485 mode, however:

1) The change made does not just affect __do_stop_tx_rs485(), it also
affects other uses of serial8250_clear_fifos() including paths for
starting up, shutting down or auto-configuring a port regardless of
whether it's an RS485 port or not.

2) It makes the assumption that resetting the FIFOs is a no-op when
FIFOs are disabled, and as such it checks for this case & explicitly
avoids setting the FIFO reset bits when the FIFO enable bit is
clear. A reading of the PC16550D manual would suggest that this is
OK since the FIFO should automatically be reset if it is later
enabled, but we support many 16550-compatible devices and have never
required this auto-reset behaviour for at least the whole git era.
Starting to rely on it now seems risky, offers no benefit, and
indeed breaks at least the Ingenic JZ4780's UARTs which reads
garbage when the RX FIFO is enabled if we don't explicitly reset it.

3) By only resetting the FIFOs if they're enabled, the behaviour of
serial8250_do_startup() during boot now depends on what the value of
FCR is before the 8250 driver is probed. This in itself seems
questionable and leaves us with FCR=0 & no FIFO reset if the UART
was used by 8250_early, otherwise it depends upon what the
bootloader left behind.

4) Although the naming of serial8250_clear_fifos() may be unclear, it
is clear that callers of it expect that it will disable FIFOs. Both
serial8250_do_startup() & serial8250_do_shutdown() contain comments
to that effect, and other callers explicitly re-enable the FIFOs
after calling serial8250_clear_fifos(). The premise of that patch
that disabling the FIFOs is incorrect therefore seems wrong.

For these reasons, this reverts commit f6aa5beb45be ("serial: 8250: Fix
clearing FIFOs in RS485 mode again").

Signed-off-by: Paul Burton <paul.burton@mips.com>
Fixes: f6aa5beb45be ("serial: 8250: Fix clearing FIFOs in RS485 mode again").
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Daniel Jedrychowski <avistel@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: linux-mips@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: stable <stable@vger.kernel.org> # 4.10+
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Paul Burton and committed by
Greg Kroah-Hartman
3c9dc275 40e020c1

+5 -24
+5 -24
drivers/tty/serial/8250/8250_port.c
··· 552 552 */ 553 553 static void serial8250_clear_fifos(struct uart_8250_port *p) 554 554 { 555 - unsigned char fcr; 556 - unsigned char clr_mask = UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT; 557 - 558 555 if (p->capabilities & UART_CAP_FIFO) { 559 - /* 560 - * Make sure to avoid changing FCR[7:3] and ENABLE_FIFO bits. 561 - * In case ENABLE_FIFO is not set, there is nothing to flush 562 - * so just return. Furthermore, on certain implementations of 563 - * the 8250 core, the FCR[7:3] bits may only be changed under 564 - * specific conditions and changing them if those conditions 565 - * are not met can have nasty side effects. One such core is 566 - * the 8250-omap present in TI AM335x. 567 - */ 568 - fcr = serial_in(p, UART_FCR); 569 - 570 - /* FIFO is not enabled, there's nothing to clear. */ 571 - if (!(fcr & UART_FCR_ENABLE_FIFO)) 572 - return; 573 - 574 - fcr |= clr_mask; 575 - serial_out(p, UART_FCR, fcr); 576 - 577 - fcr &= ~clr_mask; 578 - serial_out(p, UART_FCR, fcr); 556 + serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO); 557 + serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO | 558 + UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT); 559 + serial_out(p, UART_FCR, 0); 579 560 } 580 561 } 581 562 ··· 1448 1467 * Enable previously disabled RX interrupts. 1449 1468 */ 1450 1469 if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) { 1451 - serial8250_clear_fifos(p); 1470 + serial8250_clear_and_reinit_fifos(p); 1452 1471 1453 1472 p->ier |= UART_IER_RLSI | UART_IER_RDI; 1454 1473 serial_port_out(&p->port, UART_IER, p->ier);