On Fri, 2016-07-29 at 09:46:04 +0200, Helmut Grohne wrote:
> 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 or
> cdns_uart_startup, 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.
>
> Signed-off-by: Helmut Grohne <h.grohne@xxxxxxxxxx>
> ---
> drivers/tty/serial/xilinx_uartps.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index acaf83c..c07ee6f 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -152,6 +152,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
> @@ -1039,23 +1040,14 @@ static struct uart_port *cdns_uart_get_port(int id)
>
> #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);
> }
>
> @@ -1116,7 +1108,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)) !=
Nit: There should be spaces around the '|' operator
> + CDNS_UART_SR_TXEMPTY)
> + cpu_relax();
Acked-by: Sören Brinkmann <soren.brinkmann@xxxxxxxxxx>
Sören
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
