linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios
@ 2016-06-13 19:02 Bjorn Andersson
  2016-06-15 12:58 ` Pramod Gurav
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Bjorn Andersson @ 2016-06-13 19:02 UTC (permalink / raw)
  To: Andy Gross, Stephen Boyd, David Brown
  Cc: Greg Kroah-Hartman, Jiri Slaby, open list:ARM/QUALCOMM SUPPORT,
	open list:ARM/QUALCOMM SUPPORT, open list:SERIAL DRIVERS,
	open list

Upon opening the tty, uart_open() ends up calling msm_set_baud_rate()
which resets the uart block. If this happens as we're coming out of
msm_console_write() a full fifo worth of console output will be
discarded.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

As reported here:
https://bugs.96boards.org/show_bug.cgi?id=378

 drivers/tty/serial/msm_serial.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b7d80bd57db9..93a10ac44933 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -813,23 +813,6 @@ static unsigned int msm_get_mctrl(struct uart_port *port)
 	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR | TIOCM_RTS;
 }
 
-static void msm_reset(struct uart_port *port)
-{
-	struct msm_port *msm_port = UART_TO_MSM(port);
-
-	/* reset everything */
-	msm_write(port, UART_CR_CMD_RESET_RX, UART_CR);
-	msm_write(port, UART_CR_CMD_RESET_TX, UART_CR);
-	msm_write(port, UART_CR_CMD_RESET_ERR, UART_CR);
-	msm_write(port, UART_CR_CMD_RESET_BREAK_INT, UART_CR);
-	msm_write(port, UART_CR_CMD_RESET_CTS, UART_CR);
-	msm_write(port, UART_CR_CMD_SET_RFR, UART_CR);
-
-	/* Disable DM modes */
-	if (msm_port->is_uartdm)
-		msm_write(port, 0, UARTDM_DMEN);
-}
-
 static void msm_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 	unsigned int mr;
@@ -972,7 +955,6 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud,
 	msm_write(port, 10, UART_TFWR);
 
 	msm_write(port, UART_CR_CMD_PROTECTION_EN, UART_CR);
-	msm_reset(port);
 
 	/* Enable RX and TX */
 	msm_write(port, UART_CR_TX_ENABLE | UART_CR_RX_ENABLE, UART_CR);
-- 
2.5.0

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

* Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios
  2016-06-13 19:02 [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios Bjorn Andersson
@ 2016-06-15 12:58 ` Pramod Gurav
  2016-06-16  1:11 ` Stephen Boyd
  2016-06-16 18:24 ` [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate Bjorn Andersson
  2 siblings, 0 replies; 8+ messages in thread
From: Pramod Gurav @ 2016-06-15 12:58 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Stephen Boyd, David Brown, Greg Kroah-Hartman,
	Jiri Slaby, open list:ARM/QUALCOMM SUPPORT,
	open list:ARM/QUALCOMM SUPPORT, open list:SERIAL DRIVERS,
	open list

On 14 June 2016 at 00:32, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> Upon opening the tty, uart_open() ends up calling msm_set_baud_rate()
> which resets the uart block. If this happens as we're coming out of
> msm_console_write() a full fifo worth of console output will be
> discarded.
>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> As reported here:
> https://bugs.96boards.org/show_bug.cgi?id=378
>
>  drivers/tty/serial/msm_serial.c | 18 ------------------
>  1 file changed, 18 deletions(-)

Thanks for the patch.

I no longer see these corruptions with this patch. This is what I used
to see on my DB410C with debain FS:
http://paste.ubuntu.com/17319106/

Tested-by: Pramod Gurav <pramod.gurav@linaro.org>

Regards,
Pramod

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

* Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios
  2016-06-13 19:02 [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios Bjorn Andersson
  2016-06-15 12:58 ` Pramod Gurav
@ 2016-06-16  1:11 ` Stephen Boyd
  2016-06-16 18:24 ` [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate Bjorn Andersson
  2 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2016-06-16  1:11 UTC (permalink / raw)
  To: Bjorn Andersson, Andy Gross, David Brown
  Cc: Greg Kroah-Hartman, Jiri Slaby, open list:ARM/QUALCOMM SUPPORT,
	open list:ARM/QUALCOMM SUPPORT, open list:SERIAL DRIVERS,
	open list

On 06/13/2016 12:02 PM, Bjorn Andersson wrote:
> Upon opening the tty, uart_open() ends up calling msm_set_baud_rate()
> which resets the uart block. If this happens as we're coming out of
> msm_console_write() a full fifo worth of console output will be
> discarded.
>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> As reported here:
> https://bugs.96boards.org/show_bug.cgi?id=378

Urgh. As mentioned in commit a12f1b406f2d (tty: serial: msm: Reset
uartdm after baud rate change, 2014-10-29) we actually need to reset the
hardware sometimes. Perhaps as discussed over IRC we need to take a
different approach here and only reset the hardware if the baud actually
changes? One way to test this would be to try running a getty on the
console and see if input still works.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate
  2016-06-13 19:02 [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios Bjorn Andersson
  2016-06-15 12:58 ` Pramod Gurav
  2016-06-16  1:11 ` Stephen Boyd
@ 2016-06-16 18:24 ` Bjorn Andersson
  2016-06-17 10:02   ` Nicolas Dechesne
  2016-06-20 23:54   ` Stephen Boyd
  2 siblings, 2 replies; 8+ messages in thread
From: Bjorn Andersson @ 2016-06-16 18:24 UTC (permalink / raw)
  To: Andy Gross, Stephen Boyd, David Brown
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-msm, linux-soc,
	linux-serial, linux-kernel

msm_set_termios() is called whenever the tty is opened. Setting the baud
rate requires a full reset of the msm serial block, even when the rate
is unchanged. In the case when the same uart is used as console this
reset will discard any console output data still being clocked out of
the TX fifo.

By skipping the rate-change in the case where the baud rate is unchanged
since last request we can avoid the reset and the discarding of the
data.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/tty/serial/msm_serial.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b7d80bd57db9..206149f104fa 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -76,6 +76,7 @@ struct msm_port {
 	bool			break_detected;
 	struct msm_dma		tx_dma;
 	struct msm_dma		rx_dma;
+	unsigned int		last_baud;
 };
 
 static void msm_handle_tx(struct uart_port *port);
@@ -1072,11 +1073,16 @@ static void msm_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (dma->chan) /* Terminate if any */
 		msm_stop_dma(port, dma);
 
-	/* calculate and set baud rate */
+	/* calculate and set baud rate, if changed from last request */
 	baud = uart_get_baud_rate(port, termios, old, 300, 4000000);
-	baud = msm_set_baud_rate(port, baud, &flags);
-	if (tty_termios_baud_rate(termios))
-		tty_termios_encode_baud_rate(termios, baud, baud);
+	if (baud != msm_port->last_baud) {
+		msm_port->last_baud = baud;
+
+		baud = msm_set_baud_rate(port, baud, &flags);
+		if (tty_termios_baud_rate(termios))
+			tty_termios_encode_baud_rate(termios, baud, baud);
+		uart_update_timeout(port, termios->c_cflag, baud);
+	}
 
 	/* calculate parity */
 	mr = msm_read(port, UART_MR2);
@@ -1134,8 +1140,6 @@ static void msm_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
 		port->read_status_mask |= UART_SR_RX_BREAK;
 
-	uart_update_timeout(port, termios->c_cflag, baud);
-
 	/* Try to use DMA */
 	msm_start_rx_dma(msm_port);
 
-- 
2.5.0

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

* Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate
  2016-06-16 18:24 ` [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate Bjorn Andersson
@ 2016-06-17 10:02   ` Nicolas Dechesne
  2016-06-21  9:43     ` Nicolas Dechesne
  2016-06-20 23:54   ` Stephen Boyd
  1 sibling, 1 reply; 8+ messages in thread
From: Nicolas Dechesne @ 2016-06-17 10:02 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Stephen Boyd, David Brown, Greg Kroah-Hartman,
	Jiri Slaby, linux-arm-msm, linux-soc, linux-serial, lkml

On Thu, Jun 16, 2016 at 9:24 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> msm_set_termios() is called whenever the tty is opened. Setting the baud
> rate requires a full reset of the msm serial block, even when the rate
> is unchanged. In the case when the same uart is used as console this
> reset will discard any console output data still being clocked out of
> the TX fifo.
>
> By skipping the rate-change in the case where the baud rate is unchanged
> since last request we can avoid the reset and the discarding of the
> data.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Tested-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>

I no longer see this type of corruption on serial console

[    6.325118] ALSA device �[    6.333338] Freeing unused kernel
memory: 572K (ffffffc000c10000 - ffffffc000c9f000)
[   13.800269]  remoteproc2: remote processor a20400�[  OK  ] Started
Start the WCN core.
[ �         Starting Update UTMP about System Runlevel Changes...

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

* Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate
  2016-06-16 18:24 ` [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate Bjorn Andersson
  2016-06-17 10:02   ` Nicolas Dechesne
@ 2016-06-20 23:54   ` Stephen Boyd
  1 sibling, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2016-06-20 23:54 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, David Brown, Greg Kroah-Hartman, Jiri Slaby,
	linux-arm-msm, linux-soc, linux-serial, linux-kernel

On 06/16, Bjorn Andersson wrote:
> msm_set_termios() is called whenever the tty is opened. Setting the baud
> rate requires a full reset of the msm serial block, even when the rate
> is unchanged. In the case when the same uart is used as console this
> reset will discard any console output data still being clocked out of
> the TX fifo.
> 
> By skipping the rate-change in the case where the baud rate is unchanged
> since last request we can avoid the reset and the discarding of the
> data.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate
  2016-06-17 10:02   ` Nicolas Dechesne
@ 2016-06-21  9:43     ` Nicolas Dechesne
  2016-06-22  9:59       ` Srinivas Kandagatla
  0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Dechesne @ 2016-06-21  9:43 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Stephen Boyd, David Brown, Greg Kroah-Hartman,
	Jiri Slaby, linux-arm-msm, linux-soc, linux-serial, lkml

On Fri, Jun 17, 2016 at 1:02 PM, Nicolas Dechesne
<nicolas.dechesne@linaro.org> wrote:
> <bjorn.andersson@linaro.org> wrote:
>> msm_set_termios() is called whenever the tty is opened. Setting the baud
>> rate requires a full reset of the msm serial block, even when the rate
>> is unchanged. In the case when the same uart is used as console this
>> reset will discard any console output data still being clocked out of
>> the TX fifo.
>>
>> By skipping the rate-change in the case where the baud rate is unchanged
>> since last request we can avoid the reset and the discarding of the
>> data.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
>
> Tested-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>
> I no longer see this type of corruption on serial console
>
> [    6.325118] ALSA device �[    6.333338] Freeing unused kernel
> memory: 572K (ffffffc000c10000 - ffffffc000c9f000)
> [   13.800269]  remoteproc2: remote processor a20400�[  OK  ] Started
> Start the WCN core.
> [ �         Starting Update UTMP about System Runlevel Changes...


oops. today i tried on APQ8064 boards , both IFC6410Plus and SD
600eval, and I can no longer 'type' into the serial console, I can get
debug messages though. Reverting this change 'fixes' the problem.

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

* Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate
  2016-06-21  9:43     ` Nicolas Dechesne
@ 2016-06-22  9:59       ` Srinivas Kandagatla
  0 siblings, 0 replies; 8+ messages in thread
From: Srinivas Kandagatla @ 2016-06-22  9:59 UTC (permalink / raw)
  To: Nicolas Dechesne, Bjorn Andersson
  Cc: Andy Gross, Stephen Boyd, David Brown, Greg Kroah-Hartman,
	Jiri Slaby, linux-arm-msm, linux-soc, linux-serial, lkml



On 21/06/16 10:43, Nicolas Dechesne wrote:
> On Fri, Jun 17, 2016 at 1:02 PM, Nicolas Dechesne
> <nicolas.dechesne@linaro.org> wrote:
>> <bjorn.andersson@linaro.org> wrote:
>>> msm_set_termios() is called whenever the tty is opened. Setting the baud
>>> rate requires a full reset of the msm serial block, even when the rate
>>> is unchanged. In the case when the same uart is used as console this
>>> reset will discard any console output data still being clocked out of
>>> the TX fifo.
>>>
>>> By skipping the rate-change in the case where the baud rate is unchanged
>>> since last request we can avoid the reset and the discarding of the
>>> data.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>>
>> Tested-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>>
>> I no longer see this type of corruption on serial console
>>
>> [    6.325118] ALSA device �[    6.333338] Freeing unused kernel
>> memory: 572K (ffffffc000c10000 - ffffffc000c9f000)
>> [   13.800269]  remoteproc2: remote processor a20400�[  OK  ] Started
>> Start the WCN core.
>> [ �         Starting Update UTMP about System Runlevel Changes...
>
>
> oops. today i tried on APQ8064 boards , both IFC6410Plus and SD
> 600eval, and I can no longer 'type' into the serial console, I can get
> debug messages though. Reverting this change 'fixes' the problem.

Am also hitting the same issue as Nico on IFC6410 with this patch.


> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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

end of thread, other threads:[~2016-06-22 10:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 19:02 [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios Bjorn Andersson
2016-06-15 12:58 ` Pramod Gurav
2016-06-16  1:11 ` Stephen Boyd
2016-06-16 18:24 ` [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate Bjorn Andersson
2016-06-17 10:02   ` Nicolas Dechesne
2016-06-21  9:43     ` Nicolas Dechesne
2016-06-22  9:59       ` Srinivas Kandagatla
2016-06-20 23:54   ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).