public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set.
@ 2008-05-12 10:20 Bryan Wu
  2008-05-12 10:44 ` Alan Cox
  2008-05-12 11:18 ` Russell King
  0 siblings, 2 replies; 3+ messages in thread
From: Bryan Wu @ 2008-05-12 10:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Oleksiy Kebkal, Oleksiy Kebkal, Russell King, Alan Cox,
	Andrew Morton, Bryan Wu

From: Oleksiy Kebkal <kebkal@gmail.com>

Current behaviour doesn't give any possibility to

1.  preconfigure port setting, because open operation implies RTS
   assertion and initial serial port setting may be wrong.

2.  realize peculiar serial flow control required by some hardware
   especially in embedded devices.

Signed-off-by: Oleksiy Kebkal <lesha@evologics.de>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/serial/serial_core.c |   42 ++++++++++++++++++++++++++++--------------
 1 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index eab0327..903360b 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -179,11 +179,15 @@ static int uart_startup(struct uart_state *state, int init_hw)
 			uart_change_speed(state, NULL);
 
 			/*
-			 * Setup the RTS and DTR signals once the
-			 * port is open and ready to respond.
+			 * Setup the RTS (in the case of hardware flow control)
+			 * and DTR signals once the port is open and ready to
+			 * respond.
 			 */
-			if (info->tty->termios->c_cflag & CBAUD)
-				uart_set_mctrl(port, TIOCM_RTS | TIOCM_DTR);
+			if (info->tty->termios->c_cflag & CBAUD) {
+				uart_set_mctrl(port, TIOCM_DTR);
+				if (info->flags & UIF_CTS_FLOW)
+					uart_set_mctrl(port, TIOCM_RTS);
+			}
 		}
 
 		if (info->flags & UIF_CTS_FLOW) {
@@ -224,10 +228,14 @@ static void uart_shutdown(struct uart_state *state)
 		info->flags &= ~UIF_INITIALIZED;
 
 		/*
-		 * Turn off DTR and RTS early.
+		 * Turn off DTR and RTS (in the case of hardware flow control)
+		 * early.
 		 */
-		if (!info->tty || (info->tty->termios->c_cflag & HUPCL))
-			uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
+		if (!info->tty || (info->tty->termios->c_cflag & HUPCL)) {
+			uart_clear_mctrl(port, TIOCM_DTR);
+			if (info->flags & UIF_CTS_FLOW)
+				uart_clear_mctrl(port, TIOCM_RTS);
+               }
 
 		/*
 		 * clear delta_msr_wait queue to avoid mem leaks: we may free
@@ -1190,14 +1198,16 @@ static void uart_set_termios(struct tty_struct *tty,
 	uart_change_speed(state, old_termios);
 
 	/* Handle transition to B0 status */
-	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD))
-		uart_clear_mctrl(state->port, TIOCM_RTS | TIOCM_DTR);
+	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD)) {
+		uart_clear_mctrl(state->port, TIOCM_DTR);
+		if (state->info->flags & UIF_CTS_FLOW)
+			uart_clear_mctrl(state->port, TIOCM_RTS);
+       }
 
 	/* Handle transition away from B0 status */
 	if (!(old_termios->c_cflag & CBAUD) && (cflag & CBAUD)) {
 		unsigned int mask = TIOCM_DTR;
-		if (!(cflag & CRTSCTS) ||
-		    !test_bit(TTY_THROTTLED, &tty->flags))
+		if (!test_bit(TTY_THROTTLED, &tty->flags))
 			mask |= TIOCM_RTS;
 		uart_set_mctrl(state->port, mask);
 	}
@@ -1441,10 +1451,14 @@ static void uart_update_termios(struct uart_state *state)
 		uart_change_speed(state, NULL);
 
 		/*
-		 * And finally enable the RTS and DTR signals.
+		 * And finally enable the RTS (in the of case hardware flow
+		 * control) and DTR signals.
 		 */
-		if (tty->termios->c_cflag & CBAUD)
-			uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
+		if (tty->termios->c_cflag & CBAUD) {
+			uart_set_mctrl(port, TIOCM_DTR);
+			if (state->info->flags & UIF_CTS_FLOW)
+				uart_set_mctrl(port, TIOCM_RTS);
+		}
 	}
 }
 
-- 
1.5.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set.
  2008-05-12 10:20 [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set Bryan Wu
@ 2008-05-12 10:44 ` Alan Cox
  2008-05-12 11:18 ` Russell King
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Cox @ 2008-05-12 10:44 UTC (permalink / raw)
  To: Bryan Wu
  Cc: linux-kernel, Oleksiy Kebkal, Oleksiy Kebkal, Russell King,
	Andrew Morton, Bryan Wu

> +			 * Setup the RTS (in the case of hardware flow control)
> +			 * and DTR signals once the port is open and ready to
> +			 * respond.
>  			 */
> -			if (info->tty->termios->c_cflag & CBAUD)
> -				uart_set_mctrl(port, TIOCM_RTS | TIOCM_DTR);
> +			if (info->tty->termios->c_cflag & CBAUD) {
> +				uart_set_mctrl(port, TIOCM_DTR);
> +				if (info->flags & UIF_CTS_FLOW)
> +					uart_set_mctrl(port, TIOCM_RTS);
> +			}

Seems fine but we've now got 5 line blocks of duplicated code - and code
we will need to complicate further in future if we add other flow control
systems.

statc void uart_set_flow_control(port, int onoff)
{
	..
}

Would be worth writing and using for these cases

(Yes I am being fussy: I am currently trying to stamp out lots of code
duplication in the tty and serial layers so I don't want more added as it
is getting out of hand)

Alan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set.
  2008-05-12 10:20 [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set Bryan Wu
  2008-05-12 10:44 ` Alan Cox
@ 2008-05-12 11:18 ` Russell King
  1 sibling, 0 replies; 3+ messages in thread
From: Russell King @ 2008-05-12 11:18 UTC (permalink / raw)
  To: Bryan Wu
  Cc: linux-kernel, Oleksiy Kebkal, Oleksiy Kebkal, Alan Cox,
	Andrew Morton

On Mon, May 12, 2008 at 06:20:14PM +0800, Bryan Wu wrote:
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index eab0327..903360b 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -179,11 +179,15 @@ static int uart_startup(struct uart_state *state, int init_hw)
>  			uart_change_speed(state, NULL);
>  
>  			/*
> -			 * Setup the RTS and DTR signals once the
> -			 * port is open and ready to respond.
> +			 * Setup the RTS (in the case of hardware flow control)
> +			 * and DTR signals once the port is open and ready to
> +			 * respond.
>  			 */
> -			if (info->tty->termios->c_cflag & CBAUD)
> -				uart_set_mctrl(port, TIOCM_RTS | TIOCM_DTR);
> +			if (info->tty->termios->c_cflag & CBAUD) {
> +				uart_set_mctrl(port, TIOCM_DTR);
> +				if (info->flags & UIF_CTS_FLOW)

We have access to termios->c_cflag here, please use that in preference
to testing info->flags.

> +					uart_set_mctrl(port, TIOCM_RTS);
> +			}
>  		}
>  
>  		if (info->flags & UIF_CTS_FLOW) {
> @@ -224,10 +228,14 @@ static void uart_shutdown(struct uart_state *state)
>  		info->flags &= ~UIF_INITIALIZED;
>  
>  		/*
> -		 * Turn off DTR and RTS early.
> +		 * Turn off DTR and RTS (in the case of hardware flow control)
> +		 * early.
>  		 */
> -		if (!info->tty || (info->tty->termios->c_cflag & HUPCL))
> -			uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> +		if (!info->tty || (info->tty->termios->c_cflag & HUPCL)) {
> +			uart_clear_mctrl(port, TIOCM_DTR);
> +			if (info->flags & UIF_CTS_FLOW)

Ditto.

> +				uart_clear_mctrl(port, TIOCM_RTS);
> +               }
>  
>  		/*
>  		 * clear delta_msr_wait queue to avoid mem leaks: we may free
> @@ -1190,14 +1198,16 @@ static void uart_set_termios(struct tty_struct *tty,
>  	uart_change_speed(state, old_termios);
>  
>  	/* Handle transition to B0 status */
> -	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD))
> -		uart_clear_mctrl(state->port, TIOCM_RTS | TIOCM_DTR);
> +	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD)) {
> +		uart_clear_mctrl(state->port, TIOCM_DTR);
> +		if (state->info->flags & UIF_CTS_FLOW)

Ditto.

> +			uart_clear_mctrl(state->port, TIOCM_RTS);
> +       }
>  
>  	/* Handle transition away from B0 status */
>  	if (!(old_termios->c_cflag & CBAUD) && (cflag & CBAUD)) {
>  		unsigned int mask = TIOCM_DTR;
> -		if (!(cflag & CRTSCTS) ||
> -		    !test_bit(TTY_THROTTLED, &tty->flags))
> +		if (!test_bit(TTY_THROTTLED, &tty->flags))
>  			mask |= TIOCM_RTS;

This is inconsistent with your other changes - with your change, it will
always set RTS when switching away from B0 status.

>  		uart_set_mctrl(state->port, mask);
>  	}
> @@ -1441,10 +1451,14 @@ static void uart_update_termios(struct uart_state *state)
>  		uart_change_speed(state, NULL);
>  
>  		/*
> -		 * And finally enable the RTS and DTR signals.
> +		 * And finally enable the RTS (in the of case hardware flow
> +		 * control) and DTR signals.
>  		 */
> -		if (tty->termios->c_cflag & CBAUD)
> -			uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> +		if (tty->termios->c_cflag & CBAUD) {
> +			uart_set_mctrl(port, TIOCM_DTR);
> +			if (state->info->flags & UIF_CTS_FLOW)

We have access to termios->c_cflag here, please use that in preference
to testing info->flags.

> +				uart_set_mctrl(port, TIOCM_RTS);
> +		}
>  	}
>  }
>  

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-05-12 11:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-12 10:20 [PATCH 1/1] [serial/core]: This patch leaves RTS alone when CRTSCTS is not set Bryan Wu
2008-05-12 10:44 ` Alan Cox
2008-05-12 11:18 ` Russell King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox