[U-Boot] [PATCH] serial: stm32: Automatically generate CR when LF is observed

Currently the u-boot output looks rather odd when running the minicom terminal emulator in its default mode (just a string of "random" looking text down the right hand side of the screen).
This is caused by a combination of minicom not automatically wrapping lines and the stm32 serial driver never sending a carriage return.
Issue is trivially solved by automatically generating a CR whenever a LF is transmitted, Several other serial drivers implement this behaviour.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Kamil Lulko rev13@wp.pl --- drivers/serial/serial_stm32.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/serial/serial_stm32.c b/drivers/serial/serial_stm32.c index 3c80096..8c613db 100644 --- a/drivers/serial/serial_stm32.c +++ b/drivers/serial/serial_stm32.c @@ -81,6 +81,10 @@ static int stm32_serial_getc(void) static void stm32_serial_putc(const char c) { struct stm32_serial *usart = (struct stm32_serial *)USART_BASE; + + if (c == '\n') + stm32_serial_putc('\r'); + while ((readl(&usart->sr) & USART_SR_FLAG_TXE) == 0) ; writel(c, &usart->dr);

Strange, this was already posted by Kunhua Huang - then reverted in commit 698a12bef9e782dcd99c555a739c16eec8669f14. Anyway, yes this carriage return should be added there. I simply forgot it since I had implicit CR for each LF turned on in my terminal. Never thought this would cause so much havoc for users ;)
/Kamil
2015-05-12 21:25 GMT+02:00 Daniel Thompson daniel.thompson@linaro.org:
Currently the u-boot output looks rather odd when running the minicom terminal emulator in its default mode (just a string of "random" looking text down the right hand side of the screen).
This is caused by a combination of minicom not automatically wrapping lines and the stm32 serial driver never sending a carriage return.
Issue is trivially solved by automatically generating a CR whenever a LF is transmitted, Several other serial drivers implement this behaviour.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Kamil Lulko rev13@wp.pl
drivers/serial/serial_stm32.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/serial/serial_stm32.c b/drivers/serial/serial_stm32.c index 3c80096..8c613db 100644 --- a/drivers/serial/serial_stm32.c +++ b/drivers/serial/serial_stm32.c @@ -81,6 +81,10 @@ static int stm32_serial_getc(void) static void stm32_serial_putc(const char c) { struct stm32_serial *usart = (struct stm32_serial *)USART_BASE;
if (c == '\n')
stm32_serial_putc('\r');
while ((readl(&usart->sr) & USART_SR_FLAG_TXE) == 0) ; writel(c, &usart->dr);
-- 2.1.0

On Tue, May 12, 2015 at 10:35:55PM +0200, Kamil Lulko wrote:
Strange, this was already posted by Kunhua Huang - then reverted in commit 698a12bef9e782dcd99c555a739c16eec8669f14. Anyway, yes this carriage return should be added there. I simply forgot it since I had implicit CR for each LF turned on in my terminal. Never thought this would cause so much havoc for users ;)
I reverted it since the author said it wasn't needed with the other patch they did being applied. Daniel, can you confirm the odd behavior exists with top of tree? Thanks!

On Tue, May 12, 2015 at 05:56:36PM -0400, Tom Rini wrote:
On Tue, May 12, 2015 at 10:35:55PM +0200, Kamil Lulko wrote:
Strange, this was already posted by Kunhua Huang - then reverted in commit 698a12bef9e782dcd99c555a739c16eec8669f14. Anyway, yes this carriage return should be added there. I simply forgot it since I had implicit CR for each LF turned on in my terminal. Never thought this would cause so much havoc for users ;)
I reverted it since the author said it wasn't needed with the other patch they did being applied. Daniel, can you confirm the odd behavior exists with top of tree? Thanks!
-- Tom
Sorry about my wrong expression.
In these two patch "[U-Boot] [PATCH v2] stm32f4: fix serial output" and "[U-Boot] [PATCH v2] stm32f4: add serial print port" , there are same and similar code as below.
In "[U-Boot] [PATCH v2] stm32f4: fix serial output" :
struct stm32_serial *usart = (struct stm32_serial *)USART_BASE; + + if (c == '\n') + stm32_serial_putc('\r'); + while ((readl(&usart->sr) & USART_SR_FLAG_TXE) == 0) ; writel(c, &usart->dr);
In "[U-Boot] [PATCH v2] stm32f4: add serial print port" :
- struct stm32_serial *usart = (struct stm32_serial *)USART_BASE; + struct stm32_serial *usart = + (struct stm32_serial *)usart_base[USART_PORT]; + + if (c == '\n') + stm32_serial_putc('\r'); + while ((readl(&usart->sr) & USART_SR_FLAG_TXE) == 0) ; writel(c, &usart->dr);
If these "two patch" patch at the same time, it may happened conflict.
I have reply the mail in "[U-Boot] [U-Boot,v2] stm32f4: add serial print port" as below,
Sorry about the mistake of this patch. This patch has already include "[PATCH v2] stm32f4: fix serial output". If adopt this patch, patch "[PATCH v2] stm32f4: fix serial output" need to be discarded.
This means that "add serial print port" has already include "stm32f4: fix serial output". So, if patch "add serial print port", then patch "stm32f4: fix serial output" should not be patch.
But patch these "two patch" and then revert it. It will cause the code disappear.
Thanks to Daniel Thompson.

On 12/05/15 22:56, Tom Rini wrote:
On Tue, May 12, 2015 at 10:35:55PM +0200, Kamil Lulko wrote:
Strange, this was already posted by Kunhua Huang - then reverted in commit 698a12bef9e782dcd99c555a739c16eec8669f14. Anyway, yes this carriage return should be added there. I simply forgot it since I had implicit CR for each LF turned on in my terminal. Never thought this would cause so much havoc for users ;)
I reverted it since the author said it wasn't needed with the other patch they did being applied. Daniel, can you confirm the odd behavior exists with top of tree? Thanks!
Yes, the odd behavior still exists with top of tree.
That said, I lucked out here.
In truth I had the problem because my git tree was slightly more out of date than I thought it was (and because a google search for "serial u-boot stm32" before posting my patch didn't notice the code from Kunhua).
So... my patch won't apply to HEAD anyway but reverting the revert would be very welcome!
Daniel.
participants (4)
-
Daniel Thompson
-
Kamil Lulko
-
Kunhua Huang
-
Tom Rini