[PATCH] kdb: Put a cr before lf when working w/ a non open console port

[PATCH] kdb: Put a cr before lf when working w/ a non open console port

[Doug Anderson]

In (efe2f29 kgdboc,kdb: Allow kdb to work on a non open console port)
support was added to directly use the "write_char" functions when
doing kdb over a non-open console port.  This is great, but it ends up
bypassing the normal code in uart_console_write() that adds a carriage
return before any newlines.

Add similar code in kdb so we don't get things like:
[0]kdb>
       [0]kdb>
              [0]kdb>

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 kernel/debug/kdb/kdb_io.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 14ff484..513ed44 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -698,6 +698,9 @@ kdb_printit:
 			len = retlen;
 			cp = kdb_buffer;
 			while (len--) {
+				if (*cp == '\n')
+					dbg_io_ops->write_char('\r');
+
 				dbg_io_ops->write_char(*cp);
 				cp++;
 			}
@@ -759,6 +762,9 @@ kdb_printit:
 			len = strlen(moreprompt);
 			cp = moreprompt;
 			while (len--) {
+				if (*cp == '\n')
+					dbg_io_ops->write_char('\r');
+
 				dbg_io_ops->write_char(*cp);
 				cp++;
 			}
-- 
1.9.1.423.g4596e3a

Re: [PATCH] kdb: Put a cr before lf when working w/ a non open console port

[Doug Anderson]

Hi,

On Fri, Apr 18, 2014 at 10:17 AM, Doug Anderson <dianders@chromium.org> wrote:
> In (efe2f29 kgdboc,kdb: Allow kdb to work on a non open console port)
> support was added to directly use the "write_char" functions when
> doing kdb over a non-open console port.  This is great, but it ends up
> bypassing the normal code in uart_console_write() that adds a carriage
> return before any newlines.
>
> Add similar code in kdb so we don't get things like:
> [0]kdb>
>        [0]kdb>
>               [0]kdb>
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  kernel/debug/kdb/kdb_io.c | 6 ++++++
>  1 file changed, 6 insertions(+)

I've been digging through the code and I've finally figured out where
this happens on some other platforms (I'm using samsung/serial
myself).

The 8250 driver has it hardcoded in serial8250_put_poll_char().  Next
week I'll include a removal of that code with v2 of this patch unless
I hear any objections.  I think doing things in kdb_io is nice
because:

* we don't need to duplicate the code in everyone's "put_poll_char" functions.

* we don't end up running the code in kgdb mode, only in kdb mode.

I could also move my change to be in uart_poll_put_char() which
_would_ (I believe) apply to both kdb / kgdb if that was desirable for
some reason.

-Doug

[PATCH v2] serial_core: Commonalize crlf when working w/ a non open console port

[Doug Anderson]

In (efe2f29 kgdboc,kdb: Allow kdb to work on a non open console port)
support was added to directly use the "write_char" functions when
doing kdb over a non-open console port.  This is great, but it ends up
bypassing the normal code in uart_console_write() that adds a carriage
return before any newlines.

There appears to have been a trend to add this support directly in
some console driver's poll_put_char() functions.  This had a few side
effects, including:
- In this case we were doing LFCR, not CRLF.  This was fixed in
  uart_console_write() back in (d358788 [SERIAL] kernel console should
  send CRLF not LFCR)
- Not all serial drivers had the LFCR code in their poll_put_char()
  functions.  In my case I was running serial/samsung.c which lacked
  it.

I've moved the handling to uart_poll_put_char() to fix the above
problems.  Now when I use kdb (and don't point console= to the same
UART) I no longer get:

[0]kdb>
       [0]kdb>
              [0]kdb>

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Moved from kdb to serial_core

 drivers/tty/serial/8250/8250_core.c | 5 -----
 drivers/tty/serial/pch_uart.c       | 5 -----
 drivers/tty/serial/pxa.c            | 5 -----
 drivers/tty/serial/serial_core.c    | 3 +++
 drivers/tty/serial/serial_txx9.c    | 5 -----
 5 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 81f909c..ffee982 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1926,13 +1926,8 @@ static void serial8250_put_poll_char(struct uart_port *port,
 	wait_for_xmitr(up, BOTH_EMPTY);
 	/*
 	 *	Send the character out.
-	 *	If a LF, also do CR...
 	 */
 	serial_port_out(port, UART_TX, c);
-	if (c == 10) {
-		wait_for_xmitr(up, BOTH_EMPTY);
-		serial_port_out(port, UART_TX, 13);
-	}
 
 	/*
 	 *	Finally, wait for transmitter to become empty
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 0931b3f..11e631d 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1588,13 +1588,8 @@ static void pch_uart_put_poll_char(struct uart_port *port,
 	wait_for_xmitr(priv, UART_LSR_THRE);
 	/*
 	 * Send the character out.
-	 * If a LF, also do CR...
 	 */
 	iowrite8(c, priv->membase + PCH_UART_THR);
-	if (c == 10) {
-		wait_for_xmitr(priv, UART_LSR_THRE);
-		iowrite8(13, priv->membase + PCH_UART_THR);
-	}
 
 	/*
 	 * Finally, wait for transmitter to become empty
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index f9f20f3..9e7ee39 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -711,13 +711,8 @@ static void serial_pxa_put_poll_char(struct uart_port *port,
 	wait_for_xmitr(up);
 	/*
 	 *	Send the character out.
-	 *	If a LF, also do CR...
 	 */
 	serial_out(up, UART_TX, c);
-	if (c == 10) {
-		wait_for_xmitr(up);
-		serial_out(up, UART_TX, 13);
-	}
 
 	/*
 	 *	Finally, wait for transmitter to become empty
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index f26834d..5dba976 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2236,6 +2236,9 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
 		return;
 
 	port = state->uart_port;
+
+	if (ch == '\n')
+		port->ops->poll_put_char(port, '\r');
 	port->ops->poll_put_char(port, ch);
 }
 #endif
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 90a080b..60f49b9 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -535,13 +535,8 @@ static void serial_txx9_put_poll_char(struct uart_port *port, unsigned char c)
 	wait_for_xmitr(up);
 	/*
 	 *	Send the character out.
-	 *	If a LF, also do CR...
 	 */
 	sio_out(up, TXX9_SITFIFO, c);
-	if (c == 10) {
-		wait_for_xmitr(up);
-		sio_out(up, TXX9_SITFIFO, 13);
-	}
 
 	/*
 	 *	Finally, wait for transmitter to become empty
-- 
1.9.1.423.g4596e3a

Re: [PATCH v2] serial_core: Commonalize crlf when working w/ a non open console port

[One Thousand Gnomes]

> I've moved the handling to uart_poll_put_char() to fix the above
> problems.  Now when I use kdb (and don't point console= to the same
> UART) I no longer get:
> 
> [0]kdb>
>        [0]kdb>
>               [0]kdb>
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>

This seems sensible

Reviewed-by: Alan Cox <alan@linux.intel.com>