All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mxs: uart: allow setting RTS from software
@ 2012-12-12 16:31 ` Steffen Trumtrar
  0 siblings, 0 replies; 8+ messages in thread
From: Steffen Trumtrar @ 2012-12-12 16:31 UTC (permalink / raw)
  To: linux-serial
  Cc: stable, linux-arm-kernel, Alan Cox, Huang Shijie,
	Steffen Trumtrar

With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
mainline mxs-uart driver now sets RTSEN only when hardware flow control is
enabled via software. It is not possible any longer to set RTS manually via
software. However, the manual modification is a valid operation.
Regain the possibility to set RTS via software and only set RTSEN when hardware
flow control is explicitly enabled via settermios cflag CRTSCTS.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/tty/serial/mxs-auart.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 6db23b0..9b62c4b 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
-	ctrl &= ~AUART_CTRL2_RTSEN;
+	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
 	if (mctrl & TIOCM_RTS) {
 		if (tty_port_cts_enabled(&u->state->port))
 			ctrl |= AUART_CTRL2_RTSEN;
+		else
+			ctrl |= AUART_CTRL2_RTS;
 	}
 
 	s->ctrl = mctrl;
@@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
 		}
 		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
 	} else {
+		u->flags |= ASYNC_CTS_FLOW;
+		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
+	} else {
+		u->flags &= ~ASYNC_CTS_FLOW;
 		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
 	}
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] mxs: uart: allow setting RTS from software
@ 2012-12-12 16:31 ` Steffen Trumtrar
  0 siblings, 0 replies; 8+ messages in thread
From: Steffen Trumtrar @ 2012-12-12 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
mainline mxs-uart driver now sets RTSEN only when hardware flow control is
enabled via software. It is not possible any longer to set RTS manually via
software. However, the manual modification is a valid operation.
Regain the possibility to set RTS via software and only set RTSEN when hardware
flow control is explicitly enabled via settermios cflag CRTSCTS.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/tty/serial/mxs-auart.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 6db23b0..9b62c4b 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
-	ctrl &= ~AUART_CTRL2_RTSEN;
+	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
 	if (mctrl & TIOCM_RTS) {
 		if (tty_port_cts_enabled(&u->state->port))
 			ctrl |= AUART_CTRL2_RTSEN;
+		else
+			ctrl |= AUART_CTRL2_RTS;
 	}
 
 	s->ctrl = mctrl;
@@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
 		}
 		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
 	} else {
+		u->flags |= ASYNC_CTS_FLOW;
+		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
+	} else {
+		u->flags &= ~ASYNC_CTS_FLOW;
 		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
 	}
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] mxs: uart: allow setting RTS from software
  2012-12-12 16:31 ` Steffen Trumtrar
@ 2012-12-13  2:20   ` Huang Shijie
  -1 siblings, 0 replies; 8+ messages in thread
From: Huang Shijie @ 2012-12-13  2:20 UTC (permalink / raw)
  To: Steffen Trumtrar; +Cc: linux-serial, stable, linux-arm-kernel, Alan Cox

于 2012年12月13日 00:31, Steffen Trumtrar 写道:
> With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> enabled via software. It is not possible any longer to set RTS manually via
> software. However, the manual modification is a valid operation.
> Regain the possibility to set RTS via software and only set RTSEN when hardware
Is there any user case to set the RTS via software?
I am confused at this.
> flow control is explicitly enabled via settermios cflag CRTSCTS.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/tty/serial/mxs-auart.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 6db23b0..9b62c4b 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>  
>  	u32 ctrl = readl(u->membase + AUART_CTRL2);
>  
> -	ctrl &= ~AUART_CTRL2_RTSEN;
> +	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
>  	if (mctrl & TIOCM_RTS) {
>  		if (tty_port_cts_enabled(&u->state->port))
>  			ctrl |= AUART_CTRL2_RTSEN;
> +		else
> +			ctrl |= AUART_CTRL2_RTS;
>  	}
>  
>  	s->ctrl = mctrl;
> @@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
>  		}
>  		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
>  	} else {
> +		u->flags |= ASYNC_CTS_FLOW;
> +		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> +	} else {
> +		u->flags &= ~ASYNC_CTS_FLOW;
bug.

Your code adds TWO "else" here.

thanks
Huang Shijie
>  		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
>  	}
>  

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH] mxs: uart: allow setting RTS from software
@ 2012-12-13  2:20   ` Huang Shijie
  0 siblings, 0 replies; 8+ messages in thread
From: Huang Shijie @ 2012-12-13  2:20 UTC (permalink / raw)
  To: linux-arm-kernel

? 2012?12?13? 00:31, Steffen Trumtrar ??:
> With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> enabled via software. It is not possible any longer to set RTS manually via
> software. However, the manual modification is a valid operation.
> Regain the possibility to set RTS via software and only set RTSEN when hardware
Is there any user case to set the RTS via software?
I am confused at this.
> flow control is explicitly enabled via settermios cflag CRTSCTS.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/tty/serial/mxs-auart.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 6db23b0..9b62c4b 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>  
>  	u32 ctrl = readl(u->membase + AUART_CTRL2);
>  
> -	ctrl &= ~AUART_CTRL2_RTSEN;
> +	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
>  	if (mctrl & TIOCM_RTS) {
>  		if (tty_port_cts_enabled(&u->state->port))
>  			ctrl |= AUART_CTRL2_RTSEN;
> +		else
> +			ctrl |= AUART_CTRL2_RTS;
>  	}
>  
>  	s->ctrl = mctrl;
> @@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
>  		}
>  		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
>  	} else {
> +		u->flags |= ASYNC_CTS_FLOW;
> +		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> +	} else {
> +		u->flags &= ~ASYNC_CTS_FLOW;
bug.

Your code adds TWO "else" here.

thanks
Huang Shijie
>  		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
>  	}
>  

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] mxs: uart: allow setting RTS from software
  2012-12-13  2:20   ` Huang Shijie
@ 2012-12-13  8:40     ` Steffen Trumtrar
  -1 siblings, 0 replies; 8+ messages in thread
From: Steffen Trumtrar @ 2012-12-13  8:40 UTC (permalink / raw)
  To: Huang Shijie; +Cc: linux-serial, stable, linux-arm-kernel, Alan Cox

Hi,

On Thu, Dec 13, 2012 at 10:20:27AM +0800, Huang Shijie wrote:
> 于 2012年12月13日 00:31, Steffen Trumtrar 写道:
> > With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> > mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> > enabled via software. It is not possible any longer to set RTS manually via
> > software. However, the manual modification is a valid operation.
> > Regain the possibility to set RTS via software and only set RTSEN when hardware
> Is there any user case to set the RTS via software?
> I am confused at this.

there are cases where the RTS is used in a non-standard way. And the always-on
hardware flowcontrol breaks these (e.g. Echelon FT5000).
And as stated in the commit message, it is a valid operation to use software
flow control.

> > flow control is explicitly enabled via settermios cflag CRTSCTS.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >  drivers/tty/serial/mxs-auart.c |    8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> > index 6db23b0..9b62c4b 100644
> > --- a/drivers/tty/serial/mxs-auart.c
> > +++ b/drivers/tty/serial/mxs-auart.c
> > @@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
> >  
> >  	u32 ctrl = readl(u->membase + AUART_CTRL2);
> >  
> > -	ctrl &= ~AUART_CTRL2_RTSEN;
> > +	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
> >  	if (mctrl & TIOCM_RTS) {
> >  		if (tty_port_cts_enabled(&u->state->port))
> >  			ctrl |= AUART_CTRL2_RTSEN;
> > +		else
> > +			ctrl |= AUART_CTRL2_RTS;
> >  	}
> >  
> >  	s->ctrl = mctrl;
> > @@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
> >  		}
> >  		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> >  	} else {
> > +		u->flags |= ASYNC_CTS_FLOW;
> > +		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> > +	} else {
> > +		u->flags &= ~ASYNC_CTS_FLOW;
> bug.
> 
> Your code adds TWO "else" here.
> 

Oops, wrong merge resolution. Will fix that.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH] mxs: uart: allow setting RTS from software
@ 2012-12-13  8:40     ` Steffen Trumtrar
  0 siblings, 0 replies; 8+ messages in thread
From: Steffen Trumtrar @ 2012-12-13  8:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Dec 13, 2012 at 10:20:27AM +0800, Huang Shijie wrote:
> ? 2012?12?13? 00:31, Steffen Trumtrar ??:
> > With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> > mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> > enabled via software. It is not possible any longer to set RTS manually via
> > software. However, the manual modification is a valid operation.
> > Regain the possibility to set RTS via software and only set RTSEN when hardware
> Is there any user case to set the RTS via software?
> I am confused at this.

there are cases where the RTS is used in a non-standard way. And the always-on
hardware flowcontrol breaks these (e.g. Echelon FT5000).
And as stated in the commit message, it is a valid operation to use software
flow control.

> > flow control is explicitly enabled via settermios cflag CRTSCTS.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >  drivers/tty/serial/mxs-auart.c |    8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> > index 6db23b0..9b62c4b 100644
> > --- a/drivers/tty/serial/mxs-auart.c
> > +++ b/drivers/tty/serial/mxs-auart.c
> > @@ -412,10 +412,12 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
> >  
> >  	u32 ctrl = readl(u->membase + AUART_CTRL2);
> >  
> > -	ctrl &= ~AUART_CTRL2_RTSEN;
> > +	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
> >  	if (mctrl & TIOCM_RTS) {
> >  		if (tty_port_cts_enabled(&u->state->port))
> >  			ctrl |= AUART_CTRL2_RTSEN;
> > +		else
> > +			ctrl |= AUART_CTRL2_RTS;
> >  	}
> >  
> >  	s->ctrl = mctrl;
> > @@ -678,6 +680,10 @@ static void mxs_auart_settermios(struct uart_port *u,
> >  		}
> >  		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> >  	} else {
> > +		u->flags |= ASYNC_CTS_FLOW;
> > +		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
> > +	} else {
> > +		u->flags &= ~ASYNC_CTS_FLOW;
> bug.
> 
> Your code adds TWO "else" here.
> 

Oops, wrong merge resolution. Will fix that.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] mxs: uart: allow setting RTS from software
  2012-12-12 16:31 ` Steffen Trumtrar
@ 2012-12-14 14:11   ` Ben Hutchings
  -1 siblings, 0 replies; 8+ messages in thread
From: Ben Hutchings @ 2012-12-14 14:11 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-serial, stable, linux-arm-kernel, Alan Cox, Huang Shijie

On Wed, Dec 12, 2012 at 05:31:53PM +0100, Steffen Trumtrar wrote:
> With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> enabled via software. It is not possible any longer to set RTS manually via
> software. However, the manual modification is a valid operation.
> Regain the possibility to set RTS via software and only set RTSEN when hardware
> flow control is explicitly enabled via settermios cflag CRTSCTS.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
[...]

This is not the correct way to submit a patch to stable.  See
Documentation/stable_kernel_rules.txt

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH] mxs: uart: allow setting RTS from software
@ 2012-12-14 14:11   ` Ben Hutchings
  0 siblings, 0 replies; 8+ messages in thread
From: Ben Hutchings @ 2012-12-14 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 12, 2012 at 05:31:53PM +0100, Steffen Trumtrar wrote:
> With the patch "serial: mxs-auart: fix the wrong RTS hardware flow control" the
> mainline mxs-uart driver now sets RTSEN only when hardware flow control is
> enabled via software. It is not possible any longer to set RTS manually via
> software. However, the manual modification is a valid operation.
> Regain the possibility to set RTS via software and only set RTSEN when hardware
> flow control is explicitly enabled via settermios cflag CRTSCTS.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
[...]

This is not the correct way to submit a patch to stable.  See
Documentation/stable_kernel_rules.txt

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-12-14 14:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-12 16:31 [PATCH] mxs: uart: allow setting RTS from software Steffen Trumtrar
2012-12-12 16:31 ` Steffen Trumtrar
2012-12-13  2:20 ` Huang Shijie
2012-12-13  2:20   ` Huang Shijie
2012-12-13  8:40   ` Steffen Trumtrar
2012-12-13  8:40     ` Steffen Trumtrar
2012-12-14 14:11 ` Ben Hutchings
2012-12-14 14:11   ` Ben Hutchings

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.