The 8250 FIFOs indeed need to be cleared after stopping transmission in
RS485 mode without SER_RS485_RX_DURING_TX flag set. But there are two
problems with the approach taken by the previous patch from Fixes tag.
First, serial8250_clear_fifos() should clear fifos, but what it really
does is it enables the FIFOs unconditionally if present, clears them
and then sets the FCR register to zero, which effectively disables the
FIFOs. In case the FIFO is disabled, enabling it and clearing it makes
no sense and in fact can trigger misbehavior of the 8250 core. Moreover,
the FCR register may contain other FIFO configuration bits which may not
be writable unconditionally and writing them incorrectly can trigger
misbehavior of the 8250 core too. (ie. AM335x UART swallows the first
byte and retransmits the last byte twice because of this FCR write).
Second, serial8250_clear_and_reinit_fifos() completely reloads the FCR,
but what really has to happen at the end of the RS485 transmission is
clearing of the FIFOs and nothing else.
This patch repairs serial8250_clear_fifos() so that it really only
clears the FIFOs by operating on FCR[2:1] bits and leaves all the
other bits alone. It also undoes serial8250_clear_and_reinit_fifos()
from __do_stop_tx_rs485() as serial8250_clear_fifos() is sufficient.
Signed-off-by: Marek Vasut <marex@xxxxxxx>
Fixes: 2bed8a8e7072 ("Clearing FIFOs in RS485 emulation mode causes subsequent transmits to break")
Cc: Daniel Jedrychowski <avistel@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
NOTE: I am not entirely certain this won't break some other hardware,
esp. since there might be dependencies on the weird previous
behavior of the serial8250_clear_fifos() somewhere.
---
drivers/tty/serial/8250/8250_port.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 3f779d25ec0c..f776b3eafb96 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -552,11 +552,30 @@ static unsigned int serial_icr_read(struct uart_8250_port *up, int offset)
*/
static void serial8250_clear_fifos(struct uart_8250_port *p)
{
+ unsigned char fcr;
+ unsigned char clr_mask = UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT;
+
if (p->capabilities & UART_CAP_FIFO) {
- serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO);
- serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
- UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
- serial_out(p, UART_FCR, 0);
+ /*
+ * Make sure to avoid changing FCR[7:3] and ENABLE_FIFO bits.
+ * In case ENABLE_FIFO is not set, there is nothing to flush
+ * so just return. Furthermore, on certain implementations of
+ * the 8250 core, the FCR[7:3] bits may only be changed under
+ * specific conditions and changing them if those conditions
+ * are not met can have nasty side effects. One such core is
+ * the 8250-omap present in TI AM335x.
+ */
+ fcr = serial_in(p, UART_FCR);
+
+ /* FIFO is not enabled, there's nothing to clear. */
+ if (!(fcr & UART_FCR_ENABLE_FIFO))
+ return;
+
+ fcr |= clr_mask;
+ serial_out(p, UART_FCR, fcr);
+
+ fcr &= ~clr_mask;
+ serial_out(p, UART_FCR, fcr);
}
}
@@ -1448,7 +1467,7 @@ static void __do_stop_tx_rs485(struct uart_8250_port *p)
* Enable previously disabled RX interrupts.
*/
if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
- serial8250_clear_and_reinit_fifos(p);
+ serial8250_clear_fifos(p);
p->ier |= UART_IER_RLSI | UART_IER_RDI;
serial_port_out(&p->port, UART_IER, p->ier);
--
2.18.0
