serial: Move carriage return before line feed for some serial drivers

PATCH New Alison Wang 6 participants 13 messages Feb 25, 2016 – Mar 2, 2016 Arm Samsung Exynos

Share & tools

Alison Wang

author

2016-02-25 02:41 UTC #0 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.

Signed-off-by Alison Wang <[email protected]>

---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)

drivers/serial/serial_mxc.c +4 -4

index 51485c0..1563bb3 100644
@@ -164,15 +164,15 @@ static int mxc_serial_getc(void)
 
 static void mxc_serial_putc(const char c)
 {
+	/* If \n, also do \r */
+	if (c == '\n')
+		serial_putc('\r');
+
 	__REG(UART_PHYS + UTXD) = c;
 
 	/* wait for transmitter to be ready */
 	while (!(__REG(UART_PHYS + UTS) & UTS_TXEMPTY))
 		WATCHDOG_RESET();
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc ('\r');
 }
 
 /*

drivers/serial/serial_pxa.c +4 -4

index 8fbcc10..1eb19ec 100644
@@ -156,6 +156,10 @@ void pxa_putc_dev(unsigned int uart_index, const char c)
 {
 	struct pxa_uart_regs *uart_regs;
 
+	/* If \n, also do \r */
+	if (c == '\n')
+		pxa_putc_dev(uart_index, '\r');
+
 	uart_regs = pxa_uart_index_to_regs(uart_index);
 	if (!uart_regs)
 		hang();

@@ -163,10 +167,6 @@ void pxa_putc_dev(unsigned int uart_index, const char c)
 	while (!(readl(&uart_regs->lsr) & LSR_TEMT))
 		WATCHDOG_RESET();
 	writel(c, &uart_regs->thr);
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		pxa_putc_dev (uart_index,'\r');
 }
 
 /*

drivers/serial/serial_s3c24x0.c +4 -4

index d4e7df2..0f0878a 100644
@@ -135,14 +135,14 @@ static void _serial_putc(const char c, const int dev_index)
 {
 	struct s3c24x0_uart *uart = s3c24x0_get_base_uart(dev_index);
 
+	/* If \n, also do \r */
+	if (c == '\n')
+		serial_putc('\r');
+
 	while (!(readl(&uart->utrstat) & 0x2))
 		/* wait for room in the tx FIFO */ ;
 
 	writeb(c, &uart->utxh);
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc('\r');
 }
 
 static inline void serial_putc_dev(unsigned int dev_index, const char c)

drivers/serial/usbtty.c +4 -3

index 75f0ec3..2e19813 100644
@@ -434,11 +434,12 @@ void usbtty_putc(struct stdio_dev *dev, const char c)
 	if (!usbtty_configured ())
 		return;
 
-	buf_push (&usbtty_output, &c, 1);
 	/* If \n, also do \r */
 	if (c == '\n')
 		buf_push (&usbtty_output, "\r", 1);
 
+	buf_push(&usbtty_output, &c, 1);
+
 	/* Poll at end to handle new data... */
 	if ((usbtty_output.size + 2) >= usbtty_output.totalsize) {
 		usbtty_poll ();

@@ -498,8 +499,8 @@ void usbtty_puts(struct stdio_dev *dev, const char *str)
 		n = next_nl_pos (str);
 
 		if (str[n] == '\n') {
-			__usbtty_puts (str, n + 1);
-			__usbtty_puts ("\r", 1);
+			__usbtty_puts("\r", 1);
+			__usbtty_puts(str, n + 1);
 			str += (n + 1);
 			len -= (n + 1);
 		} else {

Signature · 1 line

2.1.0.27.g96db324

Bin Meng

2016-02-25 03:00 UTC #1 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Quoted 4 lines · Alison Wang

In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.

Quoted 8 lines

Signed-off-by Alison Wang <[email protected]>

---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)

Reviewed-by Bin Meng <[email protected]>

I guess dm serial drivers will be in another patch?

Regards,
Bin

Huan Wang

2016-02-25 03:02 UTC #2 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi, Bin,

Quoted 15 lines

Quoted 4 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Quoted 8 lines
Signed-off-by Alison Wang <[email protected]>
---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)

Quoted 4 lines

Reviewed-by Bin Meng <[email protected]>
I guess dm serial drivers will be in another patch?

[Alison Wang] Yes, two patches.

Best Regards,
Alison Wang

Bin Meng

2016-02-25 03:17 UTC #3 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi Alison,

Quoted 17 lines · Huan Wang

Hi, Bin,
Quoted 15 lines
Quoted 12 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Signed-off-by Alison Wang <[email protected]>
---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)

Quoted 6 lines

Quoted 4 lines
Reviewed-by Bin Meng <[email protected]>
I guess dm serial drivers will be in another patch?
[Alison Wang] Yes, two patches.

I would suggest in the future you send all these 3 patches (serial-ulass, legacy serial, dm serial) in a series. This helps people to track.

Regards,
Bin

Huan Wang

2016-02-25 04:58 UTC #4 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi, Bin,

Quoted 20 lines · Huan Wang

Quoted 19 lines
Hi, Bin,
Quoted 17 lines
Quoted 4 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Quoted 8 lines
Signed-off-by Alison Wang <[email protected]>
---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)
Reviewed-by Bin Meng <[email protected]>

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
I guess dm serial drivers will be in another patch?
[Alison Wang] Yes, two patches.
I would suggest in the future you send all these 3 patches (serial-ulass, legacy serial, dm serial) in a series. This helps people to track.

[Alison Wang] dm serial? Sorry, I may misunderstand you. I only have 2 patches, one is for serial-ulass(that's my meaning about dm serial), the other is for legacy serial. I didn't find such error in other dm serial drivers.

Best Regards,
Alison Wang

Bin Meng

2016-02-25 05:46 UTC #5 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi Alison,

Quoted 27 lines · Huan Wang

Hi, Bin,
Quoted 25 lines · Huan Wang
Quoted 23 lines
Hi, Bin,
Quoted 15 lines
Quoted 4 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Quoted 8 lines
Signed-off-by Alison Wang <[email protected]>
---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)
Quoted 4 lines
Reviewed-by Bin Meng <[email protected]>
I guess dm serial drivers will be in another patch?
[Alison Wang] Yes, two patches.

Quoted 7 lines

Quoted 3 lines
I would suggest in the future you send all these 3 patches (serial-ulass, legacy serial, dm serial) in a series. This helps people to track.
[Alison Wang] dm serial? Sorry, I may misunderstand you. I only have 2 patches, one is for serial-ulass(that's my meaning about dm serial), the other is for legacy serial. I didn't find such error in other dm serial drivers.

serial_lpuart.c and serial_arc.c has this handling, and they should be removed.

Regards,
Bin

Huan Wang

2016-03-02 02:49 UTC #6 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

In reply to Bin Meng

Hi, Bin,

Quoted 20 lines · Huan Wang

Quoted 19 lines
Hi, Bin,
Quoted 17 lines
Quoted 4 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Quoted 8 lines
Signed-off-by Alison Wang <[email protected]>
---
 drivers/serial/serial_mxc.c     | 8 ++++----
 drivers/serial/serial_pxa.c     | 8 ++++----
 drivers/serial/serial_s3c24x0.c | 8 ++++----
 drivers/serial/usbtty.c         | 7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)
Reviewed-by Bin Meng <[email protected]>

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
I guess dm serial drivers will be in another patch?
[Alison Wang] Yes, two patches.
I would suggest in the future you send all these 3 patches (serial-ulass, legacy serial, dm serial) in a series. This helps people to track.

[Alison Wang] Sorry to reply late. I will send a series. Thanks for your advice.

Best Regards,
Alison Wang

James Chargin

2016-02-25 14:19 UTC #7 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

In reply to Alison Wang

This is a request for more information.

Quoted 3 lines · Alison Wang

In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.

You write "In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule"

Please provide a reference. I'd probably benefit from deeper knowledge of this subject. I had not heard this rule before.

Thank you,
Jim

Quoted 112 lines

Signed-off-by Alison Wang <[email protected]>

---
  drivers/serial/serial_mxc.c     | 8 ++++----
  drivers/serial/serial_pxa.c     | 8 ++++----
  drivers/serial/serial_s3c24x0.c | 8 ++++----
  drivers/serial/usbtty.c         | 7 ++++---
  4 files changed, 16 insertions(+), 15 deletions(-)

drivers/serial/serial_mxc.c +4 -4

index 51485c0..1563bb3 100644
@@ -164,15 +164,15 @@ static int mxc_serial_getc(void)
 
  static void mxc_serial_putc(const char c)
  {
+	/* If \n, also do \r */
+	if (c == '\n')
+		serial_putc('\r');
+
  	__REG(UART_PHYS + UTXD) = c;
 
  	/* wait for transmitter to be ready */
  	while (!(__REG(UART_PHYS + UTS) & UTS_TXEMPTY))
  		WATCHDOG_RESET();
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc ('\r');
  }
 
  /*

drivers/serial/serial_pxa.c +4 -4

index 8fbcc10..1eb19ec 100644
@@ -156,6 +156,10 @@ void pxa_putc_dev(unsigned int uart_index, const char c)
  {
  	struct pxa_uart_regs *uart_regs;
 
+	/* If \n, also do \r */
+	if (c == '\n')
+		pxa_putc_dev(uart_index, '\r');
+
  	uart_regs = pxa_uart_index_to_regs(uart_index);
  	if (!uart_regs)
  		hang();

@@ -163,10 +167,6 @@ void pxa_putc_dev(unsigned int uart_index, const char c)
  	while (!(readl(&uart_regs->lsr) & LSR_TEMT))
  		WATCHDOG_RESET();
  	writel(c, &uart_regs->thr);
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		pxa_putc_dev (uart_index,'\r');
  }
 
  /*

drivers/serial/serial_s3c24x0.c +4 -4

index d4e7df2..0f0878a 100644
@@ -135,14 +135,14 @@ static void _serial_putc(const char c, const int dev_index)
  {
  	struct s3c24x0_uart *uart = s3c24x0_get_base_uart(dev_index);
 
+	/* If \n, also do \r */
+	if (c == '\n')
+		serial_putc('\r');
+
  	while (!(readl(&uart->utrstat) & 0x2))
  		/* wait for room in the tx FIFO */ ;
 
  	writeb(c, &uart->utxh);
-
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc('\r');
  }
 
  static inline void serial_putc_dev(unsigned int dev_index, const char c)

drivers/serial/usbtty.c +4 -3

index 75f0ec3..2e19813 100644
@@ -434,11 +434,12 @@ void usbtty_putc(struct stdio_dev *dev, const char c)
  	if (!usbtty_configured ())
  		return;
 
-	buf_push (&usbtty_output, &c, 1);
  	/* If \n, also do \r */
  	if (c == '\n')
  		buf_push (&usbtty_output, "\r", 1);
 
+	buf_push(&usbtty_output, &c, 1);
+
  	/* Poll at end to handle new data... */
  	if ((usbtty_output.size + 2) >= usbtty_output.totalsize) {
  		usbtty_poll ();

@@ -498,8 +499,8 @@ void usbtty_puts(struct stdio_dev *dev, const char *str)
  		n = next_nl_pos (str);
 
  		if (str[n] == '\n') {
-			__usbtty_puts (str, n + 1);
-			__usbtty_puts ("\r", 1);
+			__usbtty_puts("\r", 1);
+			__usbtty_puts(str, n + 1);
  			str += (n + 1);
  			len -= (n + 1);
  		} else {

Signature · 3 lines

Jim Chargin
AJA Video Systems                       jimc at aja.com
(530) 271-3334                          http://www.aja.com

Huan Wang

2016-03-02 04:41 UTC #8 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi, Jim,

Quoted 14 lines

-----Original Message-----
From: James Chargin [mailto:jimccrown at gmail.com]
Sent: Thursday, February 25, 2016 10:19 PM
To: u-boot at lists.denx.de; b18965 at freescale.com
Subject: Re: [U-Boot] [PATCH] serial: Move carriage return before line feed for some serial drivers
This is a request for more information.
Quoted 3 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.

Quoted 6 lines

You write "In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule"
Please provide a reference. I'd probably benefit from deeper knowledge of this subject. I had not heard this rule before.

[Alison Wang] I think you could refer to https://en.wikipedia.org/wiki/Newline#History .

Best Regards,
Alison Wang

Marek Vasut

2016-02-25 17:55 UTC #9 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

In reply to Alison Wang

Quoted 5 lines · Alison Wang

In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Signed-off-by Alison Wang <[email protected]>

Why isn't this CR-LF stuff handled in common code instead ? It'd be much better than having billion copies of the exact same logic (some of them more buggy than the others) in drivers.

Bin Meng

2016-02-26 01:30 UTC #10 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Hi Marek,

Quoted 11 lines · Marek Vasut

Quoted 5 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Signed-off-by Alison Wang <[email protected]>
Why isn't this CR-LF stuff handled in common code instead ? It'd be much better than having billion copies of the exact same logic (some of them more buggy than the others) in drivers.

The CR-LF stuff is already handled in the common code, but only for the DM version, see serial-uclass.c. This patch is to fix non-DM version serial drivers. When those drivers get converted to DM, these handling should be removed.

Regards,
Bin

Simon Glass

2016-02-26 01:56 UTC #11 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

In reply to Marek Vasut

Hi Marek,

Quoted 11 lines · Marek Vasut

Quoted 5 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Signed-off-by Alison Wang <[email protected]>
Why isn't this CR-LF stuff handled in common code instead ? It'd be much better than having billion copies of the exact same logic (some of them more buggy than the others) in drivers.

With driver model it is.

Regards,
Simon

Marek Vasut

2016-02-26 09:45 UTC #12 lore raw

PATCH 1/1 serial: Move carriage return before line feed for some serial drivers

Quoted 10 lines · Simon Glass

Hi Marek,
Quoted 6 lines · On 25 February 2016 at 10:55, Marek Vasut <[email protected]…
Quoted 5 lines · Alison Wang
In general, a carriage return needs to execute before a line feed. The patch is to change some serial drivers based on this rule, such as serial_mxc.c, serial_pxa.c, serial_s3c24x0.c and usbtty.c.
Signed-off-by Alison Wang <[email protected]

Quoted 8 lines

<mailto:[email protected]>>
Quoted 5 lines
Why isn't this CR-LF stuff handled in common code instead ? It'd be much better than having billion copies of the exact same logic (some of them more buggy than the others) in drivers.
With driver model it is.

OK, thanks.