Skip to content

Commit

Permalink
serial: xuartps: reduce hardware TX race condition
Browse files Browse the repository at this point in the history
After sending data to the uart, the driver was waiting until the TX
FIFO was empty (for every single char written). After that, TX was
disabled by writing the original TX state to the status register. At
that time however, the state machine could still be shifting
characters. Not waiting can result in strange hardware states,
especially when coupled with calls to cdns_uart_set_termios, whose
symptom generally is garbage characters being received from uart or a
hang.

According to UG585, the TACTIVE bit of the channel status register
indicates the shifter operation and we should be waiting for that bit
to clear.

Sending characters does not require the TX FIFO to be empty, but merely
to not be full. So cdns_uart_console_putchar is updated accordingly.

During tests with an instrumented kernel and an oscilloscope, we could
determine that the chance of a race is reduced by this patch. It is not
removed entirely. On the oscilloscope, one can see that disabling the
transmitter early can result in the transmission hanging in the middle
of a character for a tiny duration. This hiccup is enough to
desynchronize with a remote device for a sequence of characters until a
data bit doesn't match the start or stop bits anymore.

Link: https://www.spinics.net/lists/linux-serial/msg23156.html
Link: https://www.spinics.net/lists/linux-serial/msg26139.html
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Helmut Grohne authored and Greg Kroah-Hartman committed Jun 28, 2018
1 parent 68d12bb commit de4ed39
Showing 1 changed file with 7 additions and 12 deletions.
19 changes: 7 additions & 12 deletions drivers/tty/serial/xilinx_uartps.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
#define CDNS_UART_SR_TXEMPTY 0x00000008 /* TX FIFO empty */
#define CDNS_UART_SR_TXFULL 0x00000010 /* TX FIFO full */
#define CDNS_UART_SR_RXTRIG 0x00000001 /* Rx Trigger */
#define CDNS_UART_SR_TACTIVE 0x00000800 /* TX state machine active */

/* baud dividers min/max values */
#define CDNS_UART_BDIV_MIN 4
Expand Down Expand Up @@ -1098,24 +1099,15 @@ static const struct uart_ops cdns_uart_ops = {
};

#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
/**
* cdns_uart_console_wait_tx - Wait for the TX to be full
* @port: Handle to the uart port structure
*/
static void cdns_uart_console_wait_tx(struct uart_port *port)
{
while (!(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXEMPTY))
barrier();
}

/**
* cdns_uart_console_putchar - write the character to the FIFO buffer
* @port: Handle to the uart port structure
* @ch: Character to be written
*/
static void cdns_uart_console_putchar(struct uart_port *port, int ch)
{
cdns_uart_console_wait_tx(port);
while (readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)
cpu_relax();
writel(ch, port->membase + CDNS_UART_FIFO);
}

Expand Down Expand Up @@ -1206,7 +1198,10 @@ static void cdns_uart_console_write(struct console *co, const char *s,
writel(ctrl, port->membase + CDNS_UART_CR);

uart_console_write(port, s, count, cdns_uart_console_putchar);
cdns_uart_console_wait_tx(port);
while ((readl(port->membase + CDNS_UART_SR) &
(CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE)) !=
CDNS_UART_SR_TXEMPTY)
cpu_relax();

writel(ctrl, port->membase + CDNS_UART_CR);

Expand Down

0 comments on commit de4ed39

Please sign in to comment.