patch "serial: 8250: Fix clearing FIFOs in RS485 mode again" added to tty-testing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    serial: 8250: Fix clearing FIFOs in RS485 mode again

to my tty git tree which can be found at
    git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
in the tty-testing branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will be merged to the tty-next branch sometime soon,
after it passes testing, and the merge window is open.

If you have any questions about this process, please let me know.


>From b268203fc327a8f480ef70b70acace7f4a80f525 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marex@xxxxxxx>
Date: Mon, 3 Sep 2018 02:44:52 +0200
Subject: serial: 8250: Fix clearing FIFOs in RS485 mode again

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>
Cc: stable <stable@xxxxxxxxxxxxxxx> # let it bake a bit before merging
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
 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.19.0





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux