Re: [PATCH v2 9/9] tty/serial: atmel: Prevent a warning on suspend

[Richard Genoud]

On 15/09/2017 16:04, Romain Izard wrote:
> The atmel serial port driver reported the following warning on suspend:
> atmel_usart f802.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 
> ---
>  drivers/tty/serial/atmel_serial.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..783af6648be0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -171,6 +171,7 @@ struct atmel_uart_port {
>   boolhas_hw_timer;
>   struct timer_list   uart_timer;
>  
> + booltx_stopped;
>   boolsuspended;
>   unsigned intpending;
>   unsigned intpending_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);
> +
>  }
This line feed is not needed.
Otherwise,

Acked-by: Richard Genoud 

>  
>  /*
> @@ -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(_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);
>  
> @@ -2511,6 +2523,7 @@ static int __init atmel_console_setup(struct console 
> *co, char *options)
>  {
>   int ret;
>   struct uart_port *port = _ports[co->index].uart;
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>   int baud = 115200;
>   int bits = 8;
>   int parity = 'n';
> @@ -2528,6 +2541,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 

Re: [PATCH v2 9/9] tty/serial: atmel: Prevent a warning on suspend

[Nicolas Ferre]

On 15/09/2017 at 16:04, Romain Izard wrote:
> The atmel serial port driver reported the following warning on suspend:
> atmel_usart f802.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 

Tested-by: Nicolas Ferre 
on sama5d2 Xplained.

Acked-by: Nicolas Ferre 

> ---
>  drivers/tty/serial/atmel_serial.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..783af6648be0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -171,6 +171,7 @@ struct atmel_uart_port {
>   boolhas_hw_timer;
>   struct timer_list   uart_timer;
>  
> + booltx_stopped;
>   boolsuspended;
>   unsigned intpending;
>   unsigned intpending_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(_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);
>  
> @@ -2511,6 +2523,7 @@ static int __init atmel_console_setup(struct console 
> *co, char *options)
>  {
>   int ret;
>   struct uart_port *port = _ports[co->index].uart;
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>   int baud = 115200;
>   int bits = 8;
>   int parity = 'n';
> @@ -2528,6 +2541,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);
> + 

[PATCH v2 9/9] tty/serial: atmel: Prevent a warning on suspend

[Romain Izard]

The atmel serial port driver reported the following warning on suspend:
atmel_usart f802.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 
---
 drivers/tty/serial/atmel_serial.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index 7551cab438ff..783af6648be0 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -171,6 +171,7 @@ struct atmel_uart_port {
boolhas_hw_timer;
struct timer_list   uart_timer;
 
+   booltx_stopped;
boolsuspended;
unsigned intpending;
unsigned intpending_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(_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);
 
@@ -2511,6 +2523,7 @@ static int __init atmel_console_setup(struct console *co, 
char *options)
 {
int ret;
struct uart_port *port = _ports[co->index].uart;
+   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
int baud = 115200;
int bits = 8;
int parity = 'n';
@@ -2528,6 +2541,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, , , , );
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html