lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54AFF683.2000808@atmel.com>
Date:	Fri, 9 Jan 2015 16:40:51 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	Cyrille Pitchen <cyrille.pitchen@...el.com>,
	<gregkh@...uxfoundation.org>, <wenyou.yang@...el.com>,
	<ludovic.desroches@...el.com>, <leilei.zhao@...el.com>,
	<voice.shen@...el.com>, <josh.wu@...el.com>,
	<linux-serial@...r.kernel.org>,
	Richard Genoud <richard.genoud@...il.com>
CC:	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 4/5] tty/serial: at91: fix RTS line management when hardware
 handshake is enabled

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@...el.com>

Indeed. It fixes a pretty important misbehavior.

Acked-by: Nicolas Ferre <nicolas.ferre@...el.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ