Re: [PATCH 4/5] tty/serial: at91: fix RTS line management when hardware handshake is enabled

[Nicolas Ferre <nicolas.ferre@atmel.com>]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> This patch fixes many bugs in the code dealing with the hardware handshake.
> 
> As an example, in atmel_set_termios(), we used to test whether the CRTSCTS
> c_cflag was set. If so, we selected the "Hardware Handshake" mode through the
> Mode Register. However, few lines below the mode was reset to "Normal" (0).
> So there was no way to select the "Hardware Handshake" mode. To fix this issue,
> we moved the CRTSCRTS c_cflag test AFTER the mode has been reset to "Normal".
> 
> Also setting the RTSEN and RTSDIS bits in the Control Register has different
> results whether the USART is set in "Normal" or "Hardware Handshake" mode:
> 
> 1) "Normal" mode
> - the RTSEN bit forces the RTS line to low level, which tells the remote peer
>   that we are ready to received new data.
> - the RTSDIS bit forces the RTS line to high level, which tells the remote peer
>   to stop sending new data.
> 
> 2) "Hardware Handshake" mode
> - the RTSEN bit forces the RTS line to high level.
> - the RTSDIS bit lets the hardware control the RTS line.
> 
> WARNING:
> when FIFOs are not available or not enabled, the RTS line is controlled by the
> PDC. This is why using the Hardware Handshake mode requires using the PDC
> channel for reception. However the Hardware Handshake mode DOES NOT work with
> DMA controller since it cannot control the RTS line.
> Future designs with FIFOs will introduce a new feature: the RTS line will be
> controlled by the RX FIFO using thresholds. This patch was tested with this new
> design.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Indeed. It fixes a pretty important misbehavior.

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks.

> ---
>  drivers/tty/serial/atmel_serial.c | 91 ++++++++++++++++++++++++++-------------
>  1 file changed, 61 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 92a8b26..9e0c636 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -341,13 +341,37 @@ static u_int atmel_tx_empty(struct uart_port *port)
>  static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  {
>  	unsigned int control = 0;
> -	unsigned int mode;
> +	unsigned int mode = UART_GET_MR(port);
> +	unsigned int rts_paused, rts_ready;
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
> +	/* override mode to RS485 if needed, otherwise keep the current mode */
> +	if (port->rs485.flags & SER_RS485_ENABLED) {
> +		if ((port->rs485.delay_rts_after_send) > 0)
> +			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
> +		mode &= ~ATMEL_US_USMODE;
> +		mode |= ATMEL_US_USMODE_RS485;
> +	}
> +
> +	/* set the RTS line state according to the mode */
> +	if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
> +		/* force RTS line to high level */
> +		rts_paused = ATMEL_US_RTSEN;
> +
> +		/* give the control of the RTS line back to the hardware */
> +		rts_ready = ATMEL_US_RTSDIS;
> +	} else {
> +		/* force RTS line to high level */
> +		rts_paused = ATMEL_US_RTSDIS;
> +
> +		/* force RTS line to low level */
> +		rts_ready = ATMEL_US_RTSEN;
> +	}
> +
>  	if (mctrl & TIOCM_RTS)
> -		control |= ATMEL_US_RTSEN;
> +		control |= rts_ready;
>  	else
> -		control |= ATMEL_US_RTSDIS;
> +		control |= rts_paused;
>  
>  	if (mctrl & TIOCM_DTR)
>  		control |= ATMEL_US_DTREN;
> @@ -359,23 +383,12 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  	mctrl_gpio_set(atmel_port->gpios, mctrl);
>  
>  	/* Local loopback mode? */
> -	mode = UART_GET_MR(port) & ~ATMEL_US_CHMODE;
> +	mode &= ~ATMEL_US_CHMODE;
>  	if (mctrl & TIOCM_LOOP)
>  		mode |= ATMEL_US_CHMODE_LOC_LOOP;
>  	else
>  		mode |= ATMEL_US_CHMODE_NORMAL;
>  
> -	/* Resetting serial mode to RS232 (0x0) */
> -	mode &= ~ATMEL_US_USMODE;
> -
> -	if (port->rs485.flags & SER_RS485_ENABLED) {
> -		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if ((port->rs485.delay_rts_after_send) > 0)
> -			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
> -		mode |= ATMEL_US_USMODE_RS485;
> -	} else {
> -		dev_dbg(port->dev, "Setting UART to RS232\n");
> -	}
>  	UART_PUT_MR(port, mode);
>  }
>  
> @@ -1921,12 +1934,14 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  			      struct ktermios *old)
>  {
>  	unsigned long flags;
> -	unsigned int mode, imr, quot, baud;
> +	unsigned int old_mode, mode, imr, quot, baud;
> +
> +	/* save the current mode register */
> +	mode = old_mode = UART_GET_MR(port);
>  
> -	/* Get current mode register */
> -	mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
> -					| ATMEL_US_NBSTOP | ATMEL_US_PAR
> -					| ATMEL_US_USMODE);
> +	/* reset the mode, clock divisor, parity, stop bits and data size */
> +	mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL | ATMEL_US_NBSTOP |
> +		  ATMEL_US_PAR | ATMEL_US_USMODE);
>  
>  	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>  	quot = uart_get_divisor(port, baud);
> @@ -1971,12 +1986,6 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  	} else
>  		mode |= ATMEL_US_PAR_NONE;
>  
> -	/* hardware handshake (RTS/CTS) */
> -	if (termios->c_cflag & CRTSCTS)
> -		mode |= ATMEL_US_USMODE_HWHS;
> -	else
> -		mode |= ATMEL_US_USMODE_NORMAL;
> -
>  	spin_lock_irqsave(&port->lock, flags);
>  
>  	port->read_status_mask = ATMEL_US_OVRE;
> @@ -2020,18 +2029,40 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  	/* disable receiver and transmitter */
>  	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
>  
> -	/* Resetting serial mode to RS232 (0x0) */
> -	mode &= ~ATMEL_US_USMODE;
> -
> +	/* mode */
>  	if (port->rs485.flags & SER_RS485_ENABLED) {
>  		if ((port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> +	} else if (termios->c_cflag & CRTSCTS) {
> +		/* RS232 with hardware handshake (RTS/CTS) */
> +		mode |= ATMEL_US_USMODE_HWHS;
> +	} else {
> +		/* RS232 without hadware handshake */
> +		mode |= ATMEL_US_USMODE_NORMAL;
>  	}
>  
> -	/* set the parity, stop bits and data size */
> +	/* set the mode, clock divisor, parity, stop bits and data size */
>  	UART_PUT_MR(port, mode);
>  
> +	/*
> +	 * when switching the mode, set the RTS line state according to the
> +	 * new mode, otherwise keep the former state
> +	 */
> +	if ((old_mode & ATMEL_US_USMODE) != (mode & ATMEL_US_USMODE)) {
> +		unsigned int rts_state;
> +
> +		if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
> +			/* let the hardware control the RTS line */
> +			rts_state = ATMEL_US_RTSDIS;
> +		} else {
> +			/* force RTS line to low level */
> +			rts_state = ATMEL_US_RTSEN;
> +		}
> +
> +		UART_PUT_CR(port, rts_state);
> +	}
> +
>  	/* set the baud rate */
>  	UART_PUT_BRGR(port, quot);
>  	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> 


-- 
Nicolas Ferre