[v1,10/10] tty/serial: atmel: Prevent a warning on suspend

Message ID 20170908153604.28383-11-romain.izard.pro@gmail.com
State New
Headers show
Series
  • Various patches for SAMA5D2 backup mode
Related show

Commit Message

Romain Izard Sept. 8, 2017, 3:36 p.m.
The atmel serial port driver reported the following warning on suspend:
atmel_usart f8020000.serial: ttyS1: Unable to drain transmitter

As the ATMEL_US_TXEMPTY status bit in ATMEL_US_CSR is always cleared
when the transmitter is disabled, we need to know the transmitter's
state to return the real fifo state. And as ATMEL_US_CR is write-only,
it is necessary to save the state of the transmitter in a local
variable, and update the variable when TXEN and TXDIS is written in
ATMEL_US_CR.

After those changes, atmel_tx_empty can return "empty" on suspend, the
warning in uart_suspend_port disappears, and suspending is 20ms shorter
for each enabled Atmel serial port.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 drivers/tty/serial/atmel_serial.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Romain Izard Sept. 11, 2017, 9:52 a.m. | #1
2017-09-08 17:36 GMT+02:00 Romain Izard <romain.izard.pro@gmail.com>:
> The atmel serial port driver reported the following warning on suspend:
> atmel_usart f8020000.serial: ttyS1: Unable to drain transmitter
>
> As the ATMEL_US_TXEMPTY status bit in ATMEL_US_CSR is always cleared
> when the transmitter is disabled, we need to know the transmitter's
> state to return the real fifo state. And as ATMEL_US_CR is write-only,
> it is necessary to save the state of the transmitter in a local
> variable, and update the variable when TXEN and TXDIS is written in
> ATMEL_US_CR.
>
> After those changes, atmel_tx_empty can return "empty" on suspend, the
> warning in uart_suspend_port disappears, and suspending is 20ms shorter
> for each enabled Atmel serial port.
>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
>  drivers/tty/serial/atmel_serial.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..195c0d1b594e 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -171,6 +171,7 @@ struct atmel_uart_port {
>         bool                    has_hw_timer;
>         struct timer_list       uart_timer;
>
> +       bool                    tx_stopped;
>         bool                    suspended;
>         unsigned int            pending;
>         unsigned int            pending_status;
> @@ -380,6 +381,10 @@ static int atmel_config_rs485(struct uart_port *port,
>   */
>  static u_int atmel_tx_empty(struct uart_port *port)
>  {
> +       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +
> +       if (atmel_port->tx_stopped)
> +               return TIOCSER_TEMT;
>         return (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXEMPTY) ?
>                 TIOCSER_TEMT :
>                 0;
> @@ -485,6 +490,7 @@ static void atmel_stop_tx(struct uart_port *port)
>          * is fully transmitted.
>          */
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
> +       atmel_port->tx_stopped = true;
>
>         /* Disable interrupts */
>         atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> @@ -492,6 +498,7 @@ static void atmel_stop_tx(struct uart_port *port)
>         if ((port->rs485.flags & SER_RS485_ENABLED) &&
>             !(port->rs485.flags & SER_RS485_RX_DURING_TX))
>                 atmel_start_rx(port);
> +
>  }
>
>  /*
> @@ -521,6 +528,7 @@ static void atmel_start_tx(struct uart_port *port)
>
>         /* re-enable the transmitter */
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> +       atmel_port->tx_stopped = false;
>  }
>
>  /*
> @@ -1866,6 +1874,7 @@ static int atmel_startup(struct uart_port *port)
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
>         /* enable xmit & rcvr */
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> +       atmel_port->tx_stopped = false;
>
>         setup_timer(&atmel_port->uart_timer,
>                         atmel_uart_timer_callback,
> @@ -2122,6 +2131,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>
>         /* disable receiver and transmitter */
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
> +       atmel_port->tx_stopped = true;
>
>         /* mode */
>         if (port->rs485.flags & SER_RS485_ENABLED) {
> @@ -2207,6 +2217,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>         atmel_uart_writel(port, ATMEL_US_BRGR, quot);
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> +       atmel_port->tx_stopped = false;
>
>         /* restore interrupts */
>         atmel_uart_writel(port, ATMEL_US_IER, imr);
> @@ -2450,6 +2461,7 @@ static void atmel_console_write(struct console *co, const char *s, u_int count)
>
>         /* Make sure that tx path is actually able to send characters */
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> +       atmel_port->tx_stopped = false;
>
>         uart_console_write(port, s, count, atmel_console_putchar);
>
> @@ -2528,6 +2540,7 @@ static int __init atmel_console_setup(struct console *co, char *options)
>         atmel_uart_writel(port, ATMEL_US_IDR, -1);
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
>         atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> +       atmel_port->tx_stopped = false;
>
>         if (options)
>                 uart_parse_options(options, &baud, &parity, &bits, &flow);
> --
> 2.11.0
>

Unfortunately this patch was broken when I reported it from my branch to
the v4.13, as it does not build because of the missing declaration of
'atmel_port' in 'atmel_console_setup'.

I'll send a corrected version for v2.

Patch

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 7551cab438ff..195c0d1b594e 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -171,6 +171,7 @@  struct atmel_uart_port {
 	bool			has_hw_timer;
 	struct timer_list	uart_timer;
 
+	bool			tx_stopped;
 	bool			suspended;
 	unsigned int		pending;
 	unsigned int		pending_status;
@@ -380,6 +381,10 @@  static int atmel_config_rs485(struct uart_port *port,
  */
 static u_int atmel_tx_empty(struct uart_port *port)
 {
+	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+
+	if (atmel_port->tx_stopped)
+		return TIOCSER_TEMT;
 	return (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXEMPTY) ?
 		TIOCSER_TEMT :
 		0;
@@ -485,6 +490,7 @@  static void atmel_stop_tx(struct uart_port *port)
 	 * is fully transmitted.
 	 */
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
+	atmel_port->tx_stopped = true;
 
 	/* Disable interrupts */
 	atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
@@ -492,6 +498,7 @@  static void atmel_stop_tx(struct uart_port *port)
 	if ((port->rs485.flags & SER_RS485_ENABLED) &&
 	    !(port->rs485.flags & SER_RS485_RX_DURING_TX))
 		atmel_start_rx(port);
+
 }
 
 /*
@@ -521,6 +528,7 @@  static void atmel_start_tx(struct uart_port *port)
 
 	/* re-enable the transmitter */
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+	atmel_port->tx_stopped = false;
 }
 
 /*
@@ -1866,6 +1874,7 @@  static int atmel_startup(struct uart_port *port)
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	/* enable xmit & rcvr */
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+	atmel_port->tx_stopped = false;
 
 	setup_timer(&atmel_port->uart_timer,
 			atmel_uart_timer_callback,
@@ -2122,6 +2131,7 @@  static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* disable receiver and transmitter */
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
+	atmel_port->tx_stopped = true;
 
 	/* mode */
 	if (port->rs485.flags & SER_RS485_ENABLED) {
@@ -2207,6 +2217,7 @@  static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 	atmel_uart_writel(port, ATMEL_US_BRGR, quot);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+	atmel_port->tx_stopped = false;
 
 	/* restore interrupts */
 	atmel_uart_writel(port, ATMEL_US_IER, imr);
@@ -2450,6 +2461,7 @@  static void atmel_console_write(struct console *co, const char *s, u_int count)
 
 	/* Make sure that tx path is actually able to send characters */
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+	atmel_port->tx_stopped = false;
 
 	uart_console_write(port, s, count, atmel_console_putchar);
 
@@ -2528,6 +2540,7 @@  static int __init atmel_console_setup(struct console *co, char *options)
 	atmel_uart_writel(port, ATMEL_US_IDR, -1);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+	atmel_port->tx_stopped = false;
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);