[U-Boot] [PATCH v2 0/3] Add generic early debug UART feature

This series adds debug UART infrastructure which can in principle be used on any architecture. It works best with those that don't need a stack to call functions (e.g. ARM, PowerPC).
This came up in a discussion on the mailing list here:
https://patchwork.ozlabs.org/patch/384613/
My concerns at the time were: - it doesn't need to be written in assembler - it doesn't need to be ARM-specific - it only supports one serial driver
This potentially creates a problem where to support n architectures and m serial drivers we need n * m lots of assembler-coded functions. For example, in Linux for ARM we have:
../plat-spear/include/plat/debug-macro.S ../mach-s5pc100/include/mach/debug-macro.S ../mach-shark/include/mach/debug-macro.S ../mach-spear3xx/include/mach/debug-macro.S ../mach-sa1100/include/mach/debug-macro.S ../mach-l7200/include/mach/debug-macro.S ../mach-mv78xx0/include/mach/debug-macro.S ../mach-realview/include/mach/debug-macro.S ../mach-mmp/include/mach/debug-macro.S ../mach-s3c64xx/include/mach/debug-macro.S ../mach-s5pv210/include/mach/debug-macro.S ../mach-clps711x/include/mach/debug-macro.S ../mach-versatile/include/mach/debug-macro.S ../mach-iop32x/include/mach/debug-macro.S ../mach-ebsa110/include/mach/debug-macro.S ../mach-ux500/include/mach/debug-macro.S ../mach-orion5x/include/mach/debug-macro.S ../mach-lpc32xx/include/mach/debug-macro.S ../mach-vt8500/include/mach/debug-macro.S ../mach-rpc/include/mach/debug-macro.S ../mach-kirkwood/include/mach/debug-macro.S ../mach-spear6xx/include/mach/debug-macro.S ../mach-footbridge/include/mach/debug-macro.S ../mach-davinci/include/mach/debug-macro.S ../plat-samsung/include/plat/debug-macro.S ../mach-spear13xx/include/mach/debug-macro.S ../mach-exynos/include/mach/debug-macro.S ../mach-exynos/include/mach/regs-debug.h ../mach-dove/include/mach/debug-macro.S ../mach-omap1/include/mach/debug-macro.S ../mach-u300/include/mach/debug-macro.S ../mach-bcm2835/include/mach/debug-macro.S ../mach-h720x/include/mach/debug-macro.S ../mach-ep93xx/include/mach/debug-macro.S ../mach-gemini/include/mach/debug-macro.S ../mach-s3c24xx/include/mach/debug-macro.S ../mach-at91/include/mach/debug-macro.S ../mach-ixp4xx/include/mach/debug-macro.S ../mach-nomadik/include/mach/debug-macro.S ../mach-cns3xxx/include/mach/debug-macro.S ../mach-mxs/include/mach/debug-macro.S ../mach-iop33x/include/mach/debug-macro.S ../mach-ks8695/include/mach/debug-macro.S ../mach-netx/include/mach/debug-macro.S ../mach-iop13xx/include/mach/debug-macro.S ../mach-integrator/include/mach/debug-macro.S ../mach-msm/include/mach/debug-macro.S ../mach-pxa/include/mach/debug-macro.S ../mach-s5p64x0/include/mach/debug-macro.S ../mach-prima2/include/mach/debug-macro.S ../mach-omap2/include/mach/debug-macro.S
This series provides a possible alternative. It works by allowing any serial driver to export one init function and provide a putc() function. These can be used to output debug data before the real serial driver is available. Only one debug UART can be used at a time, and its address and serial clock must be provided statically in Kconfig.
This implementation does not depend on driver model being set up, and it is possible for it to operate without a stack on some architectures (e.g. PowerPC, ARM). It provides the same features as the ARM-specific debug.S but with more UART and architecture support.
As an example, here is the code generated for printch() by gcc 4.8 on ARM using the ns16550 driver (Tegra Seaboard):
0013b3b0 <printch>: 13b3b0: e59f201c ldr r2, [pc, #28] ; 13b3d4 <printch+0x24> 13b3b4: e5d23305 ldrb r3, [r2, #773] ; 0x305 13b3b8: e6ef3073 uxtb r3, r3 13b3bc: e3130020 tst r3, #32 13b3c0: 0afffffb beq 13b3b4 <printch+0x4> 13b3c4: e6ef0070 uxtb r0, r0 13b3c8: e59f3004 ldr r3, [pc, #4] ; 13b3d4 <printch+0x24> 13b3cc: e5c30300 strb r0, [r3, #768] ; 0x300 13b3d0: e12fff1e bx lr 13b3d4: 70006000 .word 0x70006000
The PowerPC 4xx code (ELDK-5.3) for printhex8() is this:
0102db44 <printhex8>: 102db44: 3c c0 ef 60 lis r6,-4256 102db48: 3c e0 ef 60 lis r7,-4256 102db4c: 39 20 00 1c li r9,28 102db50: 60 c6 03 05 ori r6,r6,773 102db54: 60 e7 03 00 ori r7,r7,768 102db58: 7c 6a 4c 30 srw r10,r3,r9 102db5c: 55 4a 07 3e clrlwi r10,r10,28 102db60: 2b 8a 00 09 cmplwi cr7,r10,9 102db64: 39 0a 00 30 addi r8,r10,48 102db68: 40 9d 00 08 ble- cr7,102db70 <printhex8+0x2c> 102db6c: 39 0a 00 57 addi r8,r10,87 102db70: 7c 00 04 ac sync 102db74: 89 46 00 00 lbz r10,0(r6) 102db78: 0c 0a 00 00 twi 0,r10,0 102db7c: 4c 00 01 2c isync 102db80: 55 4a 06 b4 rlwinm r10,r10,0,26,26 102db84: 71 45 00 ff andi. r5,r10,255 102db88: 41 82 ff e8 beq+ 102db70 <printhex8+0x2c> 102db8c: 55 08 06 3e clrlwi r8,r8,24 102db90: 7c 00 04 ac sync 102db94: 99 07 00 00 stb r8,0(r7) 102db98: 2f 89 00 00 cmpwi cr7,r9,0 102db9c: 39 29 ff fc addi r9,r9,-4 102dba0: 40 9e ff b8 bne+ cr7,102db58 <printhex8+0x14> 102dba4: 4e 80 00 20 blr
However this is somewhat toolchain-dependent. For example, some toolchains may spill over the available registers and use the stack. It would be nice to have a way to tell gcc not to do that.
This is not offered as a catch-all solution, but I believe it has value for many use cases.
Changes in v2: - Add asmlinkage to exported functions - Split series out on its own - Add x86 support (asmlinkage, so it still needs a stack at present) - Add better cover letter
Simon Glass (3): serial: Support an early UART for debugging serial: ns16550: Add access functions that don't need platdata serial: ns16550: Support debug UART
drivers/serial/Kconfig | 59 ++++++++++++++++++++ drivers/serial/ns16550.c | 98 ++++++++++++++++++++++++++------- include/debug_uart.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+), 20 deletions(-) create mode 100644 include/debug_uart.h

This came up in a discussion on the mailing list here:
https://patchwork.ozlabs.org/patch/384613/
My concerns at the time were: - it doesn't need to be written in assembler - it doesn't need to be ARM-specific
This patch provides a possible alternative. It works by allowing any serial driver to export one init function and provide a putc() function. These can be used to output debug data before the real serial driver is available.
This implementation does not depend on driver model, and it is possible for it to operate without a stack on some architectures (e.g. PowerPC, ARM). It provides the same features as the ARM-specific debug.S but with more UART and architecture support.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add asmlinkage to exported functions
drivers/serial/Kconfig | 46 ++++++++++++++++ include/debug_uart.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 include/debug_uart.h
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index a0b6e02..268bb42 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -5,6 +5,52 @@ config DM_SERIAL If you want to use driver model for serial drivers, say Y. To use legacy serial drivers, say N.
+config DEBUG_UART + bool "Enable an early debug UART for debugging" + help + The debug UART is intended for use very early in U-Boot to debug + problems when an ICE or other debug mechanism is not available. + + To use it you should: + - Make sure your UART supports this interface + - Enable CONFIG_DEBUG_UART + - Enable the CONFIG for your UART to tell it to provide this interface + (e.g. CONFIG_DEBUG_UART_NS16550) + - Define the required settings as needed (see below) + - Call debug_uart_init() before use + - Call debug_uart_putc() to output a character + + Depending on your platform it may be possible to use this UART before + a stack is available. + + If your UART does not support this interface you can probably add + support quite easily. Remember that you cannot use driver model and + it is preferred to use no stack. + + You must not use this UART once driver model is working and the + serial drivers are up and running (done in serial_init()). Otherwise + the drivers may conflict and you will get strange output. + +config DEBUG_UART_BASE + hex "Base address of UART" + depends on DEBUG_UART + help + This is the base address of your UART for memory-mapped UARTs. + + A default should be provided by your board, but if not you will need + to use the correct value here. + +config DEBUG_UART_CLOCK + int "UART input clock" + depends on DEBUG_UART + help + The UART input clock determines the speed of the internal UART + circuitry. The baud rate is derived from this by dividing the input + clock down. + + A default should be provided by your board, but if not you will need + to use the correct value here. + config UNIPHIER_SERIAL bool "UniPhier on-chip UART support" depends on ARCH_UNIPHIER && DM_SERIAL diff --git a/include/debug_uart.h b/include/debug_uart.h new file mode 100644 index 0000000..f56797b --- /dev/null +++ b/include/debug_uart.h @@ -0,0 +1,139 @@ +/* + * Early debug UART support + * + * (C) Copyright 2014 Google, Inc + * Writte by Simon Glass sjg@chromium.org + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _DEBUG_UART_H +#define _DEBUG_UART_H + +#include <linux/linkage.h> + +/* + * The debug UART is intended for use very early in U-Boot to debug problems + * when an ICE or other debug mechanism is not available. + * + * To use it you should: + * - Make sure your UART supports this interface + * - Enable CONFIG_DEBUG_UART + * - Enable the CONFIG for your UART to tell it to provide this interface + * (e.g. CONFIG_DEBUG_UART_NS16550) + * - Define the required settings as needed (see below) + * - Call debug_uart_init() before use + * - Call printch() to output a character + * + * Depending on your platform it may be possible to use this UART before a + * stack is available. + * + * If your UART does not support this interface you can probably add support + * quite easily. Remember that you cannot use driver model and it is preferred + * to use no stack. + * + * You must not use this UART once driver model is working and the serial + * drivers are up and running (done in serial_init()). Otherwise the drivers + * may conflict and you will get strange output. + * + * + * To enable the debug UART in your serial driver: + * + * - #include <debug_uart.h> + * - Define debug_uart_init(), trying to avoid using the stack + * - Define _debug_uart_putc() as static inline (avoiding stack usage) + * - Immediately afterwards, add DEBUG_UART_FUNCS to define the rest of the + * functionality (printch(), etc.) + */ + +/** + * debug_uart_init() - Set up the debug UART ready for use + * + * This sets up the UART with the correct baud rate, etc. + * + * Available CONFIG is: + * + * - CONFIG_DEBUG_UART_BASE: Base address of UART + * - CONFIG_BAUDRATE: Requested baud rate + * - CONFIG_DEBUG_UART_CLOCK: Input clock for UART + */ +void debug_uart_init(void); + +/** + * printch() - Output a character to the debug UART + * + * @ch: Character to output + */ +asmlinkage void printch(int ch); + +/** + * printascii() - Output an ASCII string to the debug UART + * + * @str: String to output + */ +asmlinkage void printascii(const char *str); + +/** + * printhex2() - Output a 2-digit hex value + * + * @value: Value to output + */ +asmlinkage void printhex2(uint value); + +/** + * printhex4() - Output a 4-digit hex value + * + * @value: Value to output + */ +asmlinkage void printhex4(uint value); + +/** + * printhex8() - Output a 8-digit hex value + * + * @value: Value to output + */ +asmlinkage void printhex8(uint value); + +/* + * Now define some functions - this should be inserted into the serial driver + */ +#define DEBUG_UART_FUNCS \ + asmlinkage void printch(int ch) \ + { \ + _debug_uart_putc(ch); \ + } \ +\ + asmlinkage void printascii(const char *str) \ + { \ + while (*str) \ + _debug_uart_putc(*str++); \ + } \ +\ + static inline void printhex1(uint digit) \ + { \ + digit &= 0xf; \ + _debug_uart_putc(digit > 9 ? digit - 10 + 'a' : digit + '0'); \ + } \ +\ + static inline void printhex(uint value, int digits) \ + { \ + while (digits-- > 0) \ + printhex1(value >> (4 * digits)); \ + } \ +\ + asmlinkage void printhex2(uint value) \ + { \ + printhex(value, 2); \ + } \ +\ + asmlinkage void printhex4(uint value) \ + { \ + printhex(value, 4); \ + } \ +\ + asmlinkage void printhex8(uint value) \ + { \ + printhex(value, 8); \ + } + +#endif

On 26 January 2015 at 18:27, Simon Glass sjg@chromium.org wrote:
This came up in a discussion on the mailing list here:
https://patchwork.ozlabs.org/patch/384613/
My concerns at the time were:
- it doesn't need to be written in assembler
- it doesn't need to be ARM-specific
This patch provides a possible alternative. It works by allowing any serial driver to export one init function and provide a putc() function. These can be used to output debug data before the real serial driver is available.
This implementation does not depend on driver model, and it is possible for it to operate without a stack on some architectures (e.g. PowerPC, ARM). It provides the same features as the ARM-specific debug.S but with more UART and architecture support.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add asmlinkage to exported functions
drivers/serial/Kconfig | 46 ++++++++++++++++ include/debug_uart.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 include/debug_uart.h
As discussed let's bring this in.
Applied to u-boot-dm.
(Removed these Kconfig options from minnowmax_defconfig as the same time to avoid a build error. We can always enable it later)

For the debug UART we need to be able to provide any parameters before driver model is set up. Add parameters to the low-level access functions to make this possible.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
drivers/serial/ns16550.c | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 70c9462..57e6125 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -55,17 +55,9 @@ DECLARE_GLOBAL_DATA_PTR; #endif /* CONFIG_SYS_NS16550_IER */
#ifdef CONFIG_DM_SERIAL -static void ns16550_writeb(NS16550_t port, int offset, int value) -{ - struct ns16550_platdata *plat = port->plat; - unsigned char *addr;
- offset *= 1 << plat->reg_shift; - addr = map_sysmem(plat->base, 0) + offset; - /* - * As far as we know it doesn't make sense to support selection of - * these options at run-time, so use the existing CONFIG options. - */ +static inline void serial_out_shift(unsigned char *addr, int shift, int value) +{ #ifdef CONFIG_SYS_NS16550_PORT_MAPPED outb(value, (ulong)addr); #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN) @@ -73,19 +65,14 @@ static void ns16550_writeb(NS16550_t port, int offset, int value) #elif defined(CONFIG_SYS_NS16550_MEM32) && defined(CONFIG_SYS_BIG_ENDIAN) out_be32(addr, value); #elif defined(CONFIG_SYS_BIG_ENDIAN) - writeb(value, addr + (1 << plat->reg_shift) - 1); + writeb(value, addr + (1 << shift) - 1); #else writeb(value, addr); #endif }
-static int ns16550_readb(NS16550_t port, int offset) +static inline int serial_in_shift(unsigned char *addr, int shift) { - struct ns16550_platdata *plat = port->plat; - unsigned char *addr; - - offset *= 1 << plat->reg_shift; - addr = map_sysmem(plat->base, 0) + offset; #ifdef CONFIG_SYS_NS16550_PORT_MAPPED return inb((ulong)addr); #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN) @@ -93,12 +80,37 @@ static int ns16550_readb(NS16550_t port, int offset) #elif defined(CONFIG_SYS_NS16550_MEM32) && defined(CONFIG_SYS_BIG_ENDIAN) return in_be32(addr); #elif defined(CONFIG_SYS_BIG_ENDIAN) - return readb(addr + (1 << plat->reg_shift) - 1); + return readb(addr + (1 << reg_shift) - 1); #else return readb(addr); #endif }
+static void ns16550_writeb(NS16550_t port, int offset, int value) +{ + struct ns16550_platdata *plat = port->plat; + unsigned char *addr; + + offset *= 1 << plat->reg_shift; + addr = map_sysmem(plat->base, 0) + offset; + /* + * As far as we know it doesn't make sense to support selection of + * these options at run-time, so use the existing CONFIG options. + */ + serial_out_shift(addr, plat->reg_shift, value); +} + +static int ns16550_readb(NS16550_t port, int offset) +{ + struct ns16550_platdata *plat = port->plat; + unsigned char *addr; + + offset *= 1 << plat->reg_shift; + addr = map_sysmem(plat->base, 0) + offset; + + return serial_in_shift(addr, plat->reg_shift); +} + /* We can clean these up once everything is moved to driver model */ #define serial_out(value, addr) \ ns16550_writeb(com_port, addr - (unsigned char *)com_port, value)

On 26 January 2015 at 18:27, Simon Glass sjg@chromium.org wrote:
For the debug UART we need to be able to provide any parameters before driver model is set up. Add parameters to the low-level access functions to make this possible.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2: None
drivers/serial/ns16550.c | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-)
Applied to u-boot-dm.

Add debug UART functions to permit ns16550 to provide an early debug UART. Try to avoid using the stack so that this can be called from assembler before a stack is set up (at least on ARM and PowerPC).
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Split series out on its own - Add x86 support (asmlinkage, so it still needs a stack at present) - Add better cover letter
drivers/serial/Kconfig | 13 +++++++++++++ drivers/serial/ns16550.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 268bb42..9d0d4c3 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -31,6 +31,19 @@ config DEBUG_UART serial drivers are up and running (done in serial_init()). Otherwise the drivers may conflict and you will get strange output.
+choice + prompt "Select which UART will provide the debug UART" + depends on DEBUG_UART + +config DEBUG_UART_NS16550 + bool "ns16550" + help + Select this to enable a debug UART using the ns16550 driver. You + will need to provide parameters to make this work. The driver will + be available until the real driver model serial is running. + +endchoice + config DEBUG_UART_BASE hex "Base address of UART" depends on DEBUG_UART diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 57e6125..eb00f1c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -118,10 +118,15 @@ static int ns16550_readb(NS16550_t port, int offset) ns16550_readb(com_port, addr - (unsigned char *)com_port) #endif
-int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) +static inline int calc_divisor(NS16550_t port, int clock, int baudrate) { const unsigned int mode_x_div = 16;
+ return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate); +} + +int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) +{ #ifdef CONFIG_OMAP1510 /* If can't cleanly clock 115200 set div to 1 */ if ((clock == 12000000) && (baudrate == 115200)) { @@ -131,7 +136,7 @@ int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) port->osc_12m_sel = 0; /* clear if previsouly set */ #endif
- return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate); + return calc_divisor(port, clock, baudrate); }
static void NS16550_setbrg(NS16550_t com_port, int baud_divisor) @@ -231,6 +236,47 @@ int NS16550_tstc(NS16550_t com_port)
#endif /* CONFIG_NS16550_MIN_FUNCTIONS */
+#ifdef CONFIG_DEBUG_UART_NS16550 + +#include <debug_uart.h> + +void debug_uart_init(void) +{ + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; + int baud_divisor; + + /* + * We copy the code from above because it is already horribly messy. + * Trying to refactor to nicely remove the duplication doesn't seem + * feasible. The better fix is to move all users of this driver to + * driver model. + */ + baud_divisor = calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK, + CONFIG_BAUDRATE); + + serial_out_shift(&com_port->ier, 0, CONFIG_SYS_NS16550_IER); + serial_out_shift(&com_port->mcr, 0, UART_MCRVAL); + serial_out_shift(&com_port->fcr, 0, UART_FCRVAL); + + serial_out_shift(&com_port->lcr, 0, UART_LCR_BKSE | UART_LCRVAL); + serial_out_shift(&com_port->dll, 0, baud_divisor & 0xff); + serial_out_shift(&com_port->dlm, 0, (baud_divisor >> 8) & 0xff); + serial_out_shift(&com_port->lcr, 0, UART_LCRVAL); +} + +static inline void _debug_uart_putc(int ch) +{ + struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; + + while (!(serial_in_shift(&com_port->lsr, 0) & UART_LSR_THRE)) + ; + serial_out_shift(&com_port->thr, 0, ch); +} + +DEBUG_UART_FUNCS + +#endif + #ifdef CONFIG_DM_SERIAL static int ns16550_serial_putc(struct udevice *dev, const char ch) {

On 26 January 2015 at 18:27, Simon Glass sjg@chromium.org wrote:
Add debug UART functions to permit ns16550 to provide an early debug UART. Try to avoid using the stack so that this can be called from assembler before a stack is set up (at least on ARM and PowerPC).
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Split series out on its own
- Add x86 support (asmlinkage, so it still needs a stack at present)
- Add better cover letter
Applied to u-boot-dm.

+tom, and pruning the cc list a little
Hi,
On 26 January 2015 at 18:27, Simon Glass sjg@chromium.org wrote:
This series adds debug UART infrastructure which can in principle be used on any architecture. It works best with those that don't need a stack to call functions (e.g. ARM, PowerPC).
I'd really quite like to apply this series. I have used it on PowerPC and x86 and found it quite useful. Any objections?
- Simon

On Tue, Feb 17, 2015 at 08:22:17PM -0700, Simon Glass wrote:
+tom, and pruning the cc list a little
Hi,
On 26 January 2015 at 18:27, Simon Glass sjg@chromium.org wrote:
This series adds debug UART infrastructure which can in principle be used on any architecture. It works best with those that don't need a stack to call functions (e.g. ARM, PowerPC).
I'd really quite like to apply this series. I have used it on PowerPC and x86 and found it quite useful. Any objections?
No, lets take it and work from there.
participants (2)
-
Simon Glass
-
Tom Rini