[RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[Bjorn Andersson]

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

Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[Pramod Gurav]

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

Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[Stephen Boyd]

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

[PATCH v2] tty: serial: msm: Don't reconfigure same baud rate

[Bjorn Andersson]

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

Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate

[Nicolas Dechesne]

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

Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate

[Nicolas Dechesne]

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.

Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate

[Srinivas Kandagatla]

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
>

Re: [PATCH v2] tty: serial: msm: Don't reconfigure same baud rate

[Stephen Boyd]

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