Re: [U-Boot] [PATCH] debug_uart: output CR along with LF
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
The patch below fixes it, and keeps your change:
Thanks, Tim
---
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value); * Now define some functions - this should be inserted into the serial driver */ #define DEBUG_UART_FUNCS \ - void printch(int ch) \ +\ + static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \ \ + void printch(int ch) \ + { \ + _printch(ch); \ + } \ +\ void printascii(const char *str) \ { \ while (*str) \ - printch(*str++); \ + _printch(*str++); \ } \ \ static inline void printhex1(uint digit) \
Hi Tim,
2016-04-05 0:16 GMT+09:00 Tim Chick Tim.Chick@mediatek.com:
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
The patch below fixes it, and keeps your change:
I think the current implementation of debug_uart requires the stack, anyway.
I am not sure your patch is the right approach because your solution seems dependent on your compiler behavior.
I tried your patch with CONFIG_DEBUG_UART_ANNOUNCE=y, but my ARM compiler still produced debug_uart_init code with the stack.
00101004 <printascii>: 101004: e92d4038 push {r3, r4, r5, lr} 101008: e2405001 sub r5, r0, #1 10100c: ea000005 b 101028 <printascii+0x24> 101010: e354000a cmp r4, #10 101014: 1a000001 bne 101020 <printascii+0x1c> 101018: e3a0000d mov r0, #13 10101c: ebffffdf bl 100fa0 <_debug_uart_putc> 101020: e1a00004 mov r0, r4 101024: ebffffdd bl 100fa0 <_debug_uart_putc> 101028: e5f54001 ldrb r4, [r5, #1]! 10102c: e3540000 cmp r4, #0 101030: 1afffff6 bne 101010 <printascii+0xc> 101034: e8bd8038 pop {r3, r4, r5, pc}
00101038 <debug_uart_init>: 101038: e92d4008 push {r3, lr} 10103c: ebffffdf bl 100fc0 <_debug_uart_init> 101040: e59f0004 ldr r0, [pc, #4] ; 10104c <debug_uart_init+0x14> 101044: e8bd4008 pop {r3, lr} 101048: eaffffed b 101004 <printascii> 10104c: 0010a079 .word 0x0010a079
Please see below:
Hi Tim,
2016-04-05 0:16 GMT+09:00 Tim Chick Tim.Chick@mediatek.com:
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
The patch below fixes it, and keeps your change:
I think the current implementation of debug_uart requires the stack, anyway.
No, I'm quite sure it does not, on MIPS at least. I do not have a stack. My platform has no SRAM, so I don't have a stack until I can get the DRAM working.
I am not sure your patch is the right approach because your solution seems dependent on your compiler behavior.
I believe it just relies on inline, which is used elsewhere to enable debug_uart to work without a stack.
I tried your patch with CONFIG_DEBUG_UART_ANNOUNCE=y, but my ARM compiler still produced debug_uart_init code with the stack.
It shouldn't do, it should honor the inline keyword I think. I'm using gcc on mips, and get (with my code):
9c023598 <printascii>: 9c023598: 2406000a li a2,10 9c02359c: 3c02b000 lui v0,0xb000 9c0235a0: 1000000e b 9c0235dc <printascii+0x44> 9c0235a4: 2405000d li a1,13 9c0235a8: 14660006 bne v1,a2,9c0235c4 <printascii+0x2c> 9c0235ac: 24840001 addiu a0,a0,1 9c0235b0: 90480c14 lbu t0,3092(v0)
static inline void _debug_uart_putc(int ch) { struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) 9c0235b4: 31080020 andi t0,t0,0x20 9c0235b8: 1100fffd beqz t0,9c0235b0 <printascii+0x18> 9c0235bc: 00000000 nop 9c0235c0: a0450c00 sb a1,3072(v0) 9c0235c4: 90480c14 lbu t0,3092(v0)
...
You can see it inlines the code. With your change, it pushed data to the stack and called printascii as a subroutine.
Thanks, Tim
Hi Tim,
2016-04-04 17:16 GMT+02:00 Tim Chick Tim.Chick@mediatek.com:
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
do you call debug_uart_init() in lowlevel_init()?
The patch below fixes it, and keeps your change:
Thanks, Tim
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value);
- Now define some functions - this should be inserted into the serial driver
*/ #define DEBUG_UART_FUNCS \
void printch(int ch) \+\
static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \\
void printch(int ch) \{ \_printch(ch); \} \+\ void printascii(const char *str) \ { \ while (*str) \
printch(*str++); \
_printch(*str++); \ } \\ static inline void printhex1(uint digit) \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Sorry for top posting. Not in the office at the moment.
Yes, I call debug_uart_init() before I have SDRAM, in lowlevel_init(). I need the debug uart to help me debug lowlevel_init!
Thanks, Tim
-----Original Message----- From: Daniel Schwierzeck [mailto:daniel.schwierzeck@gmail.com] Sent: 07 April 2016 17:48 To: Tim Chick Tim.Chick@mediatek.com Cc: yamada.masahiro@socionext.com; u-boot@lists.denx.de; Simon Glass sjg@chromium.org; Stefan Roese sr@denx.de Subject: Re: [U-Boot] [PATCH] debug_uart: output CR along with LF
Hi Tim,
2016-04-04 17:16 GMT+02:00 Tim Chick Tim.Chick@mediatek.com:
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
do you call debug_uart_init() in lowlevel_init()?
The patch below fixes it, and keeps your change:
Thanks, Tim
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value);
- Now define some functions - this should be inserted into the serial driver
*/ #define DEBUG_UART_FUNCS \
void printch(int ch) \+\
static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \\
void printch(int ch) \{ \_printch(ch); \} \+\ void printascii(const char *str) \ { \ while (*str) \
printch(*str++); \
_printch(*str++); \ } \\ static inline void printhex1(uint digit) \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Tim,
On 7 April 2016 at 11:20, Tim Chick Tim.Chick@mediatek.com wrote:
Sorry for top posting. Not in the office at the moment.
Yes, I call debug_uart_init() before I have SDRAM, in lowlevel_init(). I need the debug uart to help me debug lowlevel_init!
Thanks, Tim
-----Original Message----- From: Daniel Schwierzeck [mailto:daniel.schwierzeck@gmail.com] Sent: 07 April 2016 17:48 To: Tim Chick Tim.Chick@mediatek.com Cc: yamada.masahiro@socionext.com; u-boot@lists.denx.de; Simon Glass sjg@chromium.org; Stefan Roese sr@denx.de Subject: Re: [U-Boot] [PATCH] debug_uart: output CR along with LF
Hi Tim,
2016-04-04 17:16 GMT+02:00 Tim Chick Tim.Chick@mediatek.com:
Hi Masahiro,
This patch breaks the debug_uart on my MIPS board. It means printascii now uses the stack, and my board does not have a stack when debug_uart_init is called. debug_uart_init calls printascii if DEBUG_UART_ANNOUNCE is defined.
do you call debug_uart_init() in lowlevel_init()?
The patch below fixes it, and keeps your change:
Yes your patch looks correct to me. I have also used the debug UART without a stack.
Reviewed-by: Simon Glass sjg@chromium.org
Thanks, Tim
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value);
- Now define some functions - this should be inserted into the serial
driver */ #define DEBUG_UART_FUNCS \
void printch(int ch) \+\
static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \\
void printch(int ch) \{ \_printch(ch); \} \+\ void printascii(const char *str) \ { \ while (*str) \
printch(*str++); \
_printch(*str++); \ } \\ static inline void printhex1(uint digit) \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
--
- Daniel
************* Email Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
On 20/04/2016 15:40, Simon Glass wrote:
Hi Tim,
On 7 April 2016 at 11:20, Tim Chick Tim.Chick@mediatek.com wrote:
Sorry for top posting. Not in the office at the moment.
Yes, I call debug_uart_init() before I have SDRAM, in lowlevel_init(). I need the debug uart to help me debug lowlevel_init!
The patch below fixes it, and keeps your change:
Yes your patch looks correct to me. I have also used the debug UART without a stack.
OK. What needs to be done to get it applied?
Shall I submit as a "normal" patch?
Thanks, Tim
Reviewed-by: Simon Glass sjg@chromium.org
Thanks, Tim
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value);
- Now define some functions - this should be inserted into the serial
driver */ #define DEBUG_UART_FUNCS \
void printch(int ch) \+\
static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \\
void printch(int ch) \{ \_printch(ch); \} \+\ void printascii(const char *str) \ { \ while (*str) \
printch(*str++); \
_printch(*str++); \ } \\ static inline void printhex1(uint digit) \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Tom,
On 22 April 2016 at 12:19, Tim Chick Tim.Chick@mediatek.com wrote:
On 20/04/2016 15:40, Simon Glass wrote:
Hi Tim,
On 7 April 2016 at 11:20, Tim Chick Tim.Chick@mediatek.com wrote:
Sorry for top posting. Not in the office at the moment.
Yes, I call debug_uart_init() before I have SDRAM, in lowlevel_init(). I need the debug uart to help me debug lowlevel_init!
The patch below fixes it, and keeps your change:
Yes your patch looks correct to me. I have also used the debug UART without a stack.
OK. What needs to be done to get it applied?
Shall I submit as a "normal" patch?
Yes, try using patman.
Regards, Simon
Thanks, Tim
Reviewed-by: Simon Glass sjg@chromium.org
Thanks, Tim
diff --git a/include/debug_uart.h b/include/debug_uart.h index 0d640b9..2980ae6 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -115,17 +115,23 @@ void printhex8(uint value);
- Now define some functions - this should be inserted into the serial
driver */ #define DEBUG_UART_FUNCS \
void printch(int ch) \+\
static inline void _printch(int ch) \ { \ if (ch == '\n') \ _debug_uart_putc('\r'); \ _debug_uart_putc(ch); \ } \\
void printch(int ch) \{ \_printch(ch); \} \+\ void printascii(const char *str) \ { \ while (*str) \
printch(*str++); \
_printch(*str++); \ } \\ static inline void printhex1(uint digit) \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
************* Email Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
participants (4)
-
Daniel Schwierzeck -
Masahiro Yamada -
Simon Glass -
Tim Chick