serial: imx: fix RTS/CTS setting
diff mbox series

Message ID 20190614072801.3187-1-s.hauer@pengutronix.de
State New
Headers show
Series
  • serial: imx: fix RTS/CTS setting
Related show

Commit Message

Sascha Hauer June 14, 2019, 7:28 a.m. UTC
The correct setting of the RTS pin depends on the CRTSCTS termios setting:

- When CRTSCTS is disabled then RTS shall be controlled by the TIOCM_RTS
  flag.
- When CRTSCTS is enabled the expected behaviour of the RTS pin is:
  - When TIOCM_RTS is set then let the receiver control RTS.
  - When the TIOCM_RTS flag is cleared then RTS shall be deasserted (to let
    the upper layers throttle the transfer even when the FIFO in the UART has
    enough space).

This patch fixes this behaviour. Previously the RTS pin has always been
controlled by the receiver once the TIOCM_RTS flag was set and the CRTSCTS
setting hasn't been taken into account.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/tty/serial/imx.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Uwe Kleine-K├Ânig June 14, 2019, 7:48 a.m. UTC | #1
[expanded Cc: a bit]

Hello Sascha,

On Fri, Jun 14, 2019 at 09:28:01AM +0200, Sascha Hauer wrote:
> The correct setting of the RTS pin depends on the CRTSCTS termios setting:
> 
> - When CRTSCTS is disabled then RTS shall be controlled by the TIOCM_RTS
>   flag.
> - When CRTSCTS is enabled the expected behaviour of the RTS pin is:
>   - When TIOCM_RTS is set then let the receiver control RTS.
>   - When the TIOCM_RTS flag is cleared then RTS shall be deasserted (to let
>     the upper layers throttle the transfer even when the FIFO in the UART has
>     enough space).
> 
> This patch fixes this behaviour. Previously the RTS pin has always been
> controlled by the receiver once the TIOCM_RTS flag was set and the CRTSCTS
> setting hasn't been taken into account.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/tty/serial/imx.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 8b752e895053..0eddca6455ad 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -216,6 +216,7 @@ struct imx_port {
>  	unsigned int		dma_is_enabled:1;
>  	unsigned int		dma_is_rxing:1;
>  	unsigned int		dma_is_txing:1;
> +	unsigned int		crtscts:1;
>  	struct dma_chan		*dma_chan_rx, *dma_chan_tx;
>  	struct scatterlist	rx_sgl, tx_sgl[2];
>  	void			*rx_buf;
> @@ -967,9 +968,18 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  		u32 ucr2;
>  
>  		ucr2 = imx_uart_readl(sport, UCR2);
> +
>  		ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
> -		if (mctrl & TIOCM_RTS)
> -			ucr2 |= UCR2_CTS | UCR2_CTSC;
> +
> +		if (mctrl & TIOCM_RTS) {
> +			if (sport->crtscts)
> +				/* let the receiver control RTS */
> +				ucr2 |= UCR2_CTSC;
> +			else
> +				/* Force RTS active */
> +				ucr2 |= UCR2_CTS;
> +		}
> +

Other drivers check for

	port->status & UPSTAT_AUTORTS

instead of CRTSCTS. I didn't manage to grasp all the details from the
quick look I took, but maybe you should better do the same?

>  		imx_uart_writel(sport, ucr2, UCR2);
>  	}
>  
> @@ -1554,6 +1564,11 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
>  	else
>  		ucr2 = UCR2_SRST | UCR2_IRTS;
>  
> +	if (termios->c_cflag & CRTSCTS)
> +		sport->crtscts = true;
> +	else
> +		sport->crtscts = false;
> +
>  	if (termios->c_cflag & CRTSCTS) {

I'd put setting sport->crtscts in the following if block, maybe even in
the if (sport->have_rtscts) part that starts below here?

>  		if (sport->have_rtscts) {
>  			ucr2 &= ~UCR2_IRTS;

Best regards
Uwe

Patch
diff mbox series

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 8b752e895053..0eddca6455ad 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -216,6 +216,7 @@  struct imx_port {
 	unsigned int		dma_is_enabled:1;
 	unsigned int		dma_is_rxing:1;
 	unsigned int		dma_is_txing:1;
+	unsigned int		crtscts:1;
 	struct dma_chan		*dma_chan_rx, *dma_chan_tx;
 	struct scatterlist	rx_sgl, tx_sgl[2];
 	void			*rx_buf;
@@ -967,9 +968,18 @@  static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		u32 ucr2;
 
 		ucr2 = imx_uart_readl(sport, UCR2);
+
 		ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
-		if (mctrl & TIOCM_RTS)
-			ucr2 |= UCR2_CTS | UCR2_CTSC;
+
+		if (mctrl & TIOCM_RTS) {
+			if (sport->crtscts)
+				/* let the receiver control RTS */
+				ucr2 |= UCR2_CTSC;
+			else
+				/* Force RTS active */
+				ucr2 |= UCR2_CTS;
+		}
+
 		imx_uart_writel(sport, ucr2, UCR2);
 	}
 
@@ -1554,6 +1564,11 @@  imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	else
 		ucr2 = UCR2_SRST | UCR2_IRTS;
 
+	if (termios->c_cflag & CRTSCTS)
+		sport->crtscts = true;
+	else
+		sport->crtscts = false;
+
 	if (termios->c_cflag & CRTSCTS) {
 		if (sport->have_rtscts) {
 			ucr2 &= ~UCR2_IRTS;