[<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