[U-Boot] [PATCH 0/6] Serial cleanup series
This series does minor cleanup on the serial subsystem. The first patch implements default_serial_puts() to put an end to endless duplication of while (*s) { putc(*s++); } variations. The further three patches make use of it and clean up the serial code slightly.
The remaining two patches annotate the serial core with proper kerneldoc documentation and add a simple template for it.
Marek Vasut (6): serial: Implement default_serial_puts() serial: Use default_serial_puts() in drivers serial: Reorder serial_assign() serial: Reorder get_current() kerneldoc: Annotate drivers/serial/serial.c kerneldoc: stdio: tmpl: Add stdio template
arch/mips/cpu/mips32/au1x00/au1x00_serial.c | 8 +- arch/mips/cpu/mips32/incaip/asc_serial.c | 10 +- arch/mips/cpu/xburst/jz_serial.c | 8 +- arch/powerpc/cpu/mpc5xx/serial.c | 10 +- arch/powerpc/cpu/mpc8220/uart.c | 9 +- arch/powerpc/cpu/mpc8260/serial_scc.c | 9 +- arch/powerpc/cpu/mpc8260/serial_smc.c | 9 +- arch/powerpc/cpu/mpc85xx/serial_scc.c | 9 +- arch/sparc/cpu/leon2/serial.c | 9 +- arch/sparc/cpu/leon3/serial.c | 9 +- board/Marvell/common/serial.c | 9 +- board/bmw/serial.c | 10 +- board/cogent/serial.c | 8 +- board/esd/cpci750/serial.c | 10 +- board/evb64260/serial.c | 9 +- board/pcippc2/fpga_serial.c | 7 - board/pcippc2/fpga_serial.h | 1 - board/pcippc2/pcippc2.c | 3 +- board/prodrive/p3mx/serial.c | 10 +- doc/DocBook/Makefile | 2 +- doc/DocBook/stdio.tmpl | 46 +++++++ drivers/serial/altera_jtag_uart.c | 8 +- drivers/serial/altera_uart.c | 9 +- drivers/serial/atmel_usart.c | 8 +- drivers/serial/lpc32xx_hsuart.c | 8 +- drivers/serial/mcfuart.c | 9 +- drivers/serial/ns9750_serial.c | 15 +- drivers/serial/opencores_yanu.c | 10 +- drivers/serial/s3c4510b_uart.c | 6 +- drivers/serial/s3c64xx.c | 8 +- drivers/serial/serial.c | 196 +++++++++++++++++++++++++-- drivers/serial/serial_clps7111.c | 9 +- drivers/serial/serial_imx.c | 9 +- drivers/serial/serial_ixp.c | 9 +- drivers/serial/serial_ks8695.c | 9 +- drivers/serial/serial_lh7a40x.c | 9 +- drivers/serial/serial_lpc2292.c | 9 +- drivers/serial/serial_mxc.c | 9 +- drivers/serial/serial_netarm.c | 9 +- drivers/serial/serial_pl01x.c | 9 +- drivers/serial/serial_s3c44b0.c | 9 +- drivers/serial/serial_sa1100.c | 9 +- drivers/serial/serial_sh.c | 9 +- include/serial.h | 2 + 44 files changed, 271 insertions(+), 321 deletions(-) create mode 100644 doc/DocBook/stdio.tmpl
Cc: Tom Rini trini@ti.com
U-Boot contains a lot of duplicit implementations of serial_puts() call which just pipes single characters into the port in loop. Implement function that does this behavior into common code, so others can make easy use of it.
This function is called default_serial_puts() and it's sole purpose is to call putc() in loop on the whole string passed to it.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 7 +++++++ include/serial.h | 2 ++ 2 files changed, 9 insertions(+)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 9550cbd..da41cd5 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -271,6 +271,13 @@ void serial_puts(const char *s) get_current()->puts(s); }
+void default_serial_puts(const char *s) +{ + struct serial_device *dev = get_current(); + while (*s) + dev->putc(*s++); +} + #if CONFIG_POST & CONFIG_SYS_POST_UART static const int bauds[] = CONFIG_SYS_BAUDRATE_TABLE;
diff --git a/include/serial.h b/include/serial.h index a8d23f5..14f863e 100644 --- a/include/serial.h +++ b/include/serial.h @@ -20,6 +20,8 @@ struct serial_device { struct serial_device *next; };
+void default_serial_puts(const char *s); + extern struct serial_device serial_smc_device; extern struct serial_device serial_scc_device; extern struct serial_device *default_serial_console(void);
On Sat, Oct 06, 2012 at 02:07:01PM -0000, Marek Vasut wrote:
U-Boot contains a lot of duplicit implementations of serial_puts() call which just pipes single characters into the port in loop. Implement function that does this behavior into common code, so others can make easy use of it.
This function is called default_serial_puts() and it's sole purpose is to call putc() in loop on the whole string passed to it.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com
This along with the rest of the series, applied to u-boot/master, thanks!
Replace the in-place ad-hoc implementation of serial_puts() within the drivers with default_serial_puts() call. This cuts down on the code duplication quite a bit.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- arch/mips/cpu/mips32/au1x00/au1x00_serial.c | 8 +------- arch/mips/cpu/mips32/incaip/asc_serial.c | 10 +--------- arch/mips/cpu/xburst/jz_serial.c | 8 +------- arch/powerpc/cpu/mpc5xx/serial.c | 10 +--------- arch/powerpc/cpu/mpc8220/uart.c | 9 +-------- arch/powerpc/cpu/mpc8260/serial_scc.c | 9 +-------- arch/powerpc/cpu/mpc8260/serial_smc.c | 9 +-------- arch/powerpc/cpu/mpc85xx/serial_scc.c | 9 +-------- arch/sparc/cpu/leon2/serial.c | 9 +-------- arch/sparc/cpu/leon3/serial.c | 9 +-------- board/Marvell/common/serial.c | 9 +-------- board/bmw/serial.c | 10 +--------- board/cogent/serial.c | 8 +------- board/esd/cpci750/serial.c | 10 +--------- board/evb64260/serial.c | 9 +-------- board/pcippc2/fpga_serial.c | 7 ------- board/pcippc2/fpga_serial.h | 1 - board/pcippc2/pcippc2.c | 3 ++- board/prodrive/p3mx/serial.c | 10 +--------- drivers/serial/altera_jtag_uart.c | 8 +------- drivers/serial/altera_uart.c | 9 +-------- drivers/serial/atmel_usart.c | 8 +------- drivers/serial/lpc32xx_hsuart.c | 8 +------- drivers/serial/mcfuart.c | 9 +-------- drivers/serial/ns9750_serial.c | 15 +-------------- drivers/serial/opencores_yanu.c | 10 +--------- drivers/serial/s3c4510b_uart.c | 6 ++---- drivers/serial/s3c64xx.c | 8 +------- drivers/serial/serial_clps7111.c | 9 +-------- drivers/serial/serial_imx.c | 9 +-------- drivers/serial/serial_ixp.c | 9 +-------- drivers/serial/serial_ks8695.c | 9 +-------- drivers/serial/serial_lh7a40x.c | 9 +-------- drivers/serial/serial_lpc2292.c | 9 +-------- drivers/serial/serial_mxc.c | 9 +-------- drivers/serial/serial_netarm.c | 9 +-------- drivers/serial/serial_pl01x.c | 9 +-------- drivers/serial/serial_s3c44b0.c | 9 +-------- drivers/serial/serial_sa1100.c | 9 +-------- drivers/serial/serial_sh.c | 9 +-------- 40 files changed, 40 insertions(+), 306 deletions(-)
diff --git a/arch/mips/cpu/mips32/au1x00/au1x00_serial.c b/arch/mips/cpu/mips32/au1x00/au1x00_serial.c index 0beac98..3e85b90 100644 --- a/arch/mips/cpu/mips32/au1x00/au1x00_serial.c +++ b/arch/mips/cpu/mips32/au1x00/au1x00_serial.c @@ -103,12 +103,6 @@ static void au1x00_serial_putc(const char c) *uart_tx = (u32)c; }
-static void au1x00_serial_puts(const char *s) -{ - while (*s) - serial_putc(*s++); -} - static int au1x00_serial_getc(void) { volatile u32 *uart_rx = (volatile u32*)(UART0_ADDR+UART_RX); @@ -137,7 +131,7 @@ static struct serial_device au1x00_serial_drv = { .stop = NULL, .setbrg = au1x00_serial_setbrg, .putc = au1x00_serial_putc, - .puts = au1x00_serial_puts, + .puts = default_serial_puts, .getc = au1x00_serial_getc, .tstc = au1x00_serial_tstc, }; diff --git a/arch/mips/cpu/mips32/incaip/asc_serial.c b/arch/mips/cpu/mips32/incaip/asc_serial.c index 08949f4..6f0e4f2 100644 --- a/arch/mips/cpu/mips32/incaip/asc_serial.c +++ b/arch/mips/cpu/mips32/incaip/asc_serial.c @@ -236,14 +236,6 @@ static void asc_serial_putc(const char c) } }
-static void asc_serial_puts(const char *s) -{ - while (*s) - { - serial_putc (*s++); - } -} - static int asc_serial_getc(void) { ulong symbol_mask; @@ -292,7 +284,7 @@ static struct serial_device asc_serial_drv = { .stop = NULL, .setbrg = asc_serial_setbrg, .putc = asc_serial_putc, - .puts = asc_serial_puts, + .puts = default_serial_puts, .getc = asc_serial_getc, .tstc = asc_serial_tstc, }; diff --git a/arch/mips/cpu/xburst/jz_serial.c b/arch/mips/cpu/xburst/jz_serial.c index 3199007..a147657 100644 --- a/arch/mips/cpu/xburst/jz_serial.c +++ b/arch/mips/cpu/xburst/jz_serial.c @@ -109,19 +109,13 @@ static int jz_serial_getc(void) return readb(&uart->rbr_thr_dllr); }
-static void jz_serial_puts(const char *s) -{ - while (*s) - serial_putc(*s++); -} - static struct serial_device jz_serial_drv = { .name = "jz_serial", .start = jz_serial_init, .stop = NULL, .setbrg = jz_serial_setbrg, .putc = jz_serial_putc, - .puts = jz_serial_puts, + .puts = default_serial_puts, .getc = jz_serial_getc, .tstc = jz_serial_tstc, }; diff --git a/arch/powerpc/cpu/mpc5xx/serial.c b/arch/powerpc/cpu/mpc5xx/serial.c index 6ef8be8..732856a 100644 --- a/arch/powerpc/cpu/mpc5xx/serial.c +++ b/arch/powerpc/cpu/mpc5xx/serial.c @@ -161,21 +161,13 @@ static void mpc5xx_serial_setbrg(void) #endif }
-static void mpc5xx_serial_puts(const char *s) -{ - while (*s) { - serial_putc(*s); - ++s; - } -} - static struct serial_device mpc5xx_serial_drv = { .name = "mpc5xx_serial", .start = mpc5xx_serial_init, .stop = NULL, .setbrg = mpc5xx_serial_setbrg, .putc = mpc5xx_serial_putc, - .puts = mpc5xx_serial_puts, + .puts = default_serial_puts, .getc = mpc5xx_serial_getc, .tstc = mpc5xx_serial_tstc, }; diff --git a/arch/powerpc/cpu/mpc8220/uart.c b/arch/powerpc/cpu/mpc8220/uart.c index 25d4472..772528f 100644 --- a/arch/powerpc/cpu/mpc8220/uart.c +++ b/arch/powerpc/cpu/mpc8220/uart.c @@ -84,13 +84,6 @@ static void mpc8220_serial_putc(const char c) psc->xmitbuf[0] = c; }
-static void mpc8220_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static int mpc8220_serial_getc(void) { volatile psc8220_t *psc = (psc8220_t *) PSC_BASE; @@ -132,7 +125,7 @@ static struct serial_device mpc8220_serial_drv = { .stop = NULL, .setbrg = mpc8220_serial_setbrg, .putc = mpc8220_serial_putc, - .puts = mpc8220_serial_puts, + .puts = default_serial_puts, .getc = mpc8220_serial_getc, .tstc = mpc8220_serial_tstc, }; diff --git a/arch/powerpc/cpu/mpc8260/serial_scc.c b/arch/powerpc/cpu/mpc8260/serial_scc.c index ab77558..ab2a2b2 100644 --- a/arch/powerpc/cpu/mpc8260/serial_scc.c +++ b/arch/powerpc/cpu/mpc8260/serial_scc.c @@ -217,13 +217,6 @@ static void mpc8260_scc_serial_putc(const char c) tbdf->cbd_sc |= BD_SC_READY; }
-static void mpc8260_scc_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static int mpc8260_scc_serial_getc(void) { volatile cbd_t *rbdf; @@ -267,7 +260,7 @@ static struct serial_device mpc8260_scc_serial_drv = { .stop = NULL, .setbrg = mpc8260_scc_serial_setbrg, .putc = mpc8260_scc_serial_putc, - .puts = mpc8260_scc_serial_puts, + .puts = default_serial_puts, .getc = mpc8260_scc_serial_getc, .tstc = mpc8260_scc_serial_tstc, }; diff --git a/arch/powerpc/cpu/mpc8260/serial_smc.c b/arch/powerpc/cpu/mpc8260/serial_smc.c index 7edde9a..feba1f6 100644 --- a/arch/powerpc/cpu/mpc8260/serial_smc.c +++ b/arch/powerpc/cpu/mpc8260/serial_smc.c @@ -216,13 +216,6 @@ static void mpc8260_smc_serial_putc(const char c) rtx->txbd.cbd_sc |= BD_SC_READY; }
-static void mpc8260_smc_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static int mpc8260_smc_serial_getc(void) { volatile smc_uart_t *up; @@ -270,7 +263,7 @@ static struct serial_device mpc8260_smc_serial_drv = { .stop = NULL, .setbrg = mpc8260_smc_serial_setbrg, .putc = mpc8260_smc_serial_putc, - .puts = mpc8260_smc_serial_puts, + .puts = default_serial_puts, .getc = mpc8260_smc_serial_getc, .tstc = mpc8260_smc_serial_tstc, }; diff --git a/arch/powerpc/cpu/mpc85xx/serial_scc.c b/arch/powerpc/cpu/mpc85xx/serial_scc.c index fe9af55..6345362 100644 --- a/arch/powerpc/cpu/mpc85xx/serial_scc.c +++ b/arch/powerpc/cpu/mpc85xx/serial_scc.c @@ -220,13 +220,6 @@ static void mpc85xx_serial_putc(const char c) tbdf->cbd_sc |= BD_SC_READY; }
-static void mpc85xx_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static int mpc85xx_serial_getc(void) { volatile cbd_t *rbdf; @@ -268,7 +261,7 @@ static struct serial_device mpc85xx_serial_drv = { .stop = NULL, .setbrg = mpc85xx_serial_setbrg, .putc = mpc85xx_serial_putc, - .puts = mpc85xx_serial_puts, + .puts = default_serial_puts, .getc = mpc85xx_serial_getc, .tstc = mpc85xx_serial_tstc, }; diff --git a/arch/sparc/cpu/leon2/serial.c b/arch/sparc/cpu/leon2/serial.c index 16fffb6..40d5b01 100644 --- a/arch/sparc/cpu/leon2/serial.c +++ b/arch/sparc/cpu/leon2/serial.c @@ -105,13 +105,6 @@ static void leon2_serial_putc(const char c) leon2_serial_putc_raw(c); }
-static void leon2_serial_puts(const char *s) -{ - while (*s) { - serial_putc(*s++); - } -} - static int leon2_serial_getc(void) { LEON2_regs *leon2 = (LEON2_regs *) LEON2_PREGS; @@ -172,7 +165,7 @@ static struct serial_device leon2_serial_drv = { .stop = NULL, .setbrg = leon2_serial_setbrg, .putc = leon2_serial_putc, - .puts = leon2_serial_puts, + .puts = default_serial_puts, .getc = leon2_serial_getc, .tstc = leon2_serial_tstc, }; diff --git a/arch/sparc/cpu/leon3/serial.c b/arch/sparc/cpu/leon3/serial.c index c4f3ee8..838d451 100644 --- a/arch/sparc/cpu/leon3/serial.c +++ b/arch/sparc/cpu/leon3/serial.c @@ -99,13 +99,6 @@ static void leon3_serial_putc(const char c) leon3_serial_putc_raw(c); }
-static void leon3_serial_puts(const char *s) -{ - while (*s) { - serial_putc(*s++); - } -} - static int leon3_serial_getc(void) { if (!leon3_apbuart) @@ -146,7 +139,7 @@ static struct serial_device leon3_serial_drv = { .stop = NULL, .setbrg = leon3_serial_setbrg, .putc = leon3_serial_putc, - .puts = leon3_serial_puts, + .puts = default_serial_puts, .getc = leon3_serial_getc, .tstc = leon3_serial_tstc, }; diff --git a/board/Marvell/common/serial.c b/board/Marvell/common/serial.c index 1327c62..4a780c3 100644 --- a/board/Marvell/common/serial.c +++ b/board/Marvell/common/serial.c @@ -139,20 +139,13 @@ static void marvell_serial_setbrg(void)
#endif /* CONFIG_MPSC */
-static void marvell_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device marvell_serial_drv = { .name = "marvell_serial", .start = marvell_serial_init, .stop = NULL, .setbrg = marvell_serial_setbrg, .putc = marvell_serial_putc, - .puts = marvell_serial_puts, + .puts = default_serial_puts, .getc = marvell_serial_getc, .tstc = marvell_serial_tstc, }; diff --git a/board/bmw/serial.c b/board/bmw/serial.c index 08f449c..29a579f 100644 --- a/board/bmw/serial.c +++ b/board/bmw/serial.c @@ -58,14 +58,6 @@ static void bmw_serial_putc(const char c) NS16550_putc (console, c); }
-static void bmw_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - - static int bmw_serial_getc(void) { return NS16550_getc (console); @@ -89,7 +81,7 @@ static struct serial_device bmw_serial_drv = { .stop = NULL, .setbrg = bmw_serial_setbrg, .putc = bmw_serial_putc, - .puts = bmw_serial_puts, + .puts = default_serial_puts, .getc = bmw_serial_getc, .tstc = bmw_serial_tstc, }; diff --git a/board/cogent/serial.c b/board/cogent/serial.c index cd4a976..20631d1 100644 --- a/board/cogent/serial.c +++ b/board/cogent/serial.c @@ -68,12 +68,6 @@ static void cogent_serial_putc(const char c) cma_mb_reg_write (&mbsp->ser_thr, c); }
-static void cogent_serial_puts(const char *s) -{ - while (*s != '\0') - serial_putc (*s++); -} - static int cogent_serial_getc(void) { cma_mb_serial *mbsp = (cma_mb_serial *) CMA_MB_SERIAL_BASE; @@ -96,7 +90,7 @@ static struct serial_device cogent_serial_drv = { .stop = NULL, .setbrg = cogent_serial_setbrg, .putc = cogent_serial_putc, - .puts = cogent_serial_puts, + .puts = default_serial_puts, .getc = cogent_serial_getc, .tstc = cogent_serial_tstc, }; diff --git a/board/esd/cpci750/serial.c b/board/esd/cpci750/serial.c index 25f8950..160e0e0 100644 --- a/board/esd/cpci750/serial.c +++ b/board/esd/cpci750/serial.c @@ -75,21 +75,13 @@ static void cpci750_serial_setbrg(void) galbrg_set_baudrate (CONFIG_MPSC_PORT, gd->baudrate); }
- -static void cpci750_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device cpci750_serial_drv = { .name = "cpci750_serial", .start = cpci750_serial_init, .stop = NULL, .setbrg = cpci750_serial_setbrg, .putc = cpci750_serial_putc, - .puts = cpci750_serial_puts, + .puts = default_serial_puts, .getc = cpci750_serial_getc, .tstc = cpci750_serial_tstc, }; diff --git a/board/evb64260/serial.c b/board/evb64260/serial.c index 9fd4298..b9ca1d7 100644 --- a/board/evb64260/serial.c +++ b/board/evb64260/serial.c @@ -139,20 +139,13 @@ static void evb64260_serial_setbrg(void)
#endif /* CONFIG_MPSC */
-static void evb64260_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device evb64260_serial_drv = { .name = "evb64260_serial", .start = evb64260_serial_init, .stop = NULL, .setbrg = evb64260_serial_setbrg, .putc = evb64260_serial_putc, - .puts = evb64260_serial_puts, + .puts = default_serial_puts, .getc = evb64260_serial_getc, .tstc = evb64260_serial_tstc, }; diff --git a/board/pcippc2/fpga_serial.c b/board/pcippc2/fpga_serial.c index 5f89d9b..de61ca0 100644 --- a/board/pcippc2/fpga_serial.c +++ b/board/pcippc2/fpga_serial.c @@ -73,13 +73,6 @@ void fpga_serial_putc (char c) } }
-void fpga_serial_puts (const char *s) -{ - while (*s) { - fpga_serial_print (*s++); - } -} - int fpga_serial_getc (void) { while ((in8 (UART (LSR)) & 0x01) == 0); diff --git a/board/pcippc2/fpga_serial.h b/board/pcippc2/fpga_serial.h index 5275014..106fbf7 100644 --- a/board/pcippc2/fpga_serial.h +++ b/board/pcippc2/fpga_serial.h @@ -26,7 +26,6 @@
extern void fpga_serial_init (int); extern void fpga_serial_putc (char); -extern void fpga_serial_puts (const char *); extern int fpga_serial_getc (void); extern int fpga_serial_tstc (void); extern void fpga_serial_setbrg (void); diff --git a/board/pcippc2/pcippc2.c b/board/pcippc2/pcippc2.c index 4a91458..5e6fc58 100644 --- a/board/pcippc2/pcippc2.c +++ b/board/pcippc2/pcippc2.c @@ -29,6 +29,7 @@ #include <watchdog.h> #include <pci.h> #include <netdev.h> +#include <serial.h>
#include "hardware.h" #include "pcippc2.h" @@ -129,7 +130,7 @@ int misc_init_r (void) fpga_serial_init (sconsole_get_baudrate ());
sconsole_putc = fpga_serial_putc; - sconsole_puts = fpga_serial_puts; + sconsole_puts = default_serial_puts; sconsole_getc = fpga_serial_getc; sconsole_tstc = fpga_serial_tstc; sconsole_setbrg = fpga_serial_setbrg; diff --git a/board/prodrive/p3mx/serial.c b/board/prodrive/p3mx/serial.c index 2f4d294..3536933 100644 --- a/board/prodrive/p3mx/serial.c +++ b/board/prodrive/p3mx/serial.c @@ -75,21 +75,13 @@ static void p3mx_serial_setbrg(void) galbrg_set_baudrate (CONFIG_MPSC_PORT, gd->baudrate); }
- -static void p3mx_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device p3mx_serial_drv = { .name = "p3mx_serial", .start = p3mx_serial_init, .stop = NULL, .setbrg = p3mx_serial_setbrg, .putc = p3mx_serial_putc, - .puts = p3mx_serial_puts, + .puts = default_serial_puts, .getc = p3mx_serial_getc, .tstc = p3mx_serial_tstc, }; diff --git a/drivers/serial/altera_jtag_uart.c b/drivers/serial/altera_jtag_uart.c index 654b501..28319ba 100644 --- a/drivers/serial/altera_jtag_uart.c +++ b/drivers/serial/altera_jtag_uart.c @@ -59,12 +59,6 @@ static void altera_jtag_serial_putc(char c) writel ((unsigned char)c, &jtag->data); }
-static void altera_jtag_serial_puts(const char *s) -{ - while (*s != 0) - serial_putc (*s++); -} - static int altera_jtag_serial_tstc(void) { return ( readl (&jtag->control) & NIOS_JTAG_RRDY); @@ -91,7 +85,7 @@ static struct serial_device altera_jtag_serial_drv = { .stop = NULL, .setbrg = altera_jtag_serial_setbrg, .putc = altera_jtag_serial_putc, - .puts = altera_jtag_serial_puts, + .puts = default_serial_puts, .getc = altera_jtag_serial_getc, .tstc = altera_jtag_serial_tstc, }; diff --git a/drivers/serial/altera_uart.c b/drivers/serial/altera_uart.c index 27550ed..118cd58 100644 --- a/drivers/serial/altera_uart.c +++ b/drivers/serial/altera_uart.c @@ -82,13 +82,6 @@ static void altera_serial_putc(char c) writel ((unsigned char)c, &uart->txdata); }
-static void altera_serial_puts(const char *s) -{ - while (*s != 0) { - serial_putc (*s++); - } -} - static int altera_serial_tstc(void) { return (readl (&uart->status) & NIOS_UART_RRDY); @@ -107,7 +100,7 @@ static struct serial_device altera_serial_drv = { .stop = NULL, .setbrg = altera_serial_setbrg, .putc = altera_serial_putc, - .puts = altera_serial_puts, + .puts = default_serial_puts, .getc = altera_serial_getc, .tstc = altera_serial_tstc, }; diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c index 1303031..c4d7432 100644 --- a/drivers/serial/atmel_usart.c +++ b/drivers/serial/atmel_usart.c @@ -86,12 +86,6 @@ static void atmel_serial_putc(char c) writel(c, &usart->thr); }
-static void atmel_serial_puts(const char *s) -{ - while (*s) - serial_putc(*s++); -} - static int atmel_serial_getc(void) { atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; @@ -113,7 +107,7 @@ static struct serial_device atmel_serial_drv = { .stop = NULL, .setbrg = atmel_serial_setbrg, .putc = atmel_serial_putc, - .puts = atmel_serial_puts, + .puts = default_serial_puts, .getc = atmel_serial_getc, .tstc = atmel_serial_tstc, }; diff --git a/drivers/serial/lpc32xx_hsuart.c b/drivers/serial/lpc32xx_hsuart.c index 02429b5..7559916 100644 --- a/drivers/serial/lpc32xx_hsuart.c +++ b/drivers/serial/lpc32xx_hsuart.c @@ -77,19 +77,13 @@ static int lpc32xx_serial_init(void) return 0; }
-static void lpc32xx_serial_puts(const char *s) -{ - while (*s) - serial_putc(*s++); -} - static struct serial_device lpc32xx_serial_drv = { .name = "lpc32xx_serial", .start = lpc32xx_serial_init, .stop = NULL, .setbrg = lpc32xx_serial_setbrg, .putc = lpc32xx_serial_putc, - .puts = lpc32xx_serial_puts, + .puts = default_serial_puts, .getc = lpc32xx_serial_getc, .tstc = lpc32xx_serial_tstc, }; diff --git a/drivers/serial/mcfuart.c b/drivers/serial/mcfuart.c index 00a7114..8ea9af0 100644 --- a/drivers/serial/mcfuart.c +++ b/drivers/serial/mcfuart.c @@ -87,13 +87,6 @@ static void mcf_serial_putc(const char c) uart->utb = c; }
-static void mcf_serial_puts(const char *s) -{ - while (*s) { - serial_putc(*s++); - } -} - static int mcf_serial_getc(void) { volatile uart_t *uart = (volatile uart_t *)(CONFIG_SYS_UART_BASE); @@ -136,7 +129,7 @@ static struct serial_device mcf_serial_drv = { .stop = NULL, .setbrg = mcf_serial_setbrg, .putc = mcf_serial_putc, - .puts = mcf_serial_puts, + .puts = default_serial_puts, .getc = mcf_serial_getc, .tstc = mcf_serial_tstc, }; diff --git a/drivers/serial/ns9750_serial.c b/drivers/serial/ns9750_serial.c index cb545c4..85fc68a 100644 --- a/drivers/serial/ns9750_serial.c +++ b/drivers/serial/ns9750_serial.c @@ -100,19 +100,6 @@ static void ns9750_serial_putc(const char c) }
/*********************************************************************** - * @Function: serial_puts - * @Return: n/a - * @Descr: writes non-zero string to the FIFO. - ***********************************************************************/ - -static void ns9750_serial_puts(const char *s) -{ - while (*s) { - serial_putc( *s++ ); - } -} - -/*********************************************************************** * @Function: serial_getc * @Return: the character read * @Descr: performs only 8bit accesses to the FIFO. No error handling @@ -215,7 +202,7 @@ static struct serial_device ns9750_serial_drv = { .stop = NULL, .setbrg = ns9750_serial_setbrg, .putc = ns9750_serial_putc, - .puts = ns9750_serial_puts, + .puts = default_serial_puts, .getc = ns9750_serial_getc, .tstc = ns9750_serial_tstc, }; diff --git a/drivers/serial/opencores_yanu.c b/drivers/serial/opencores_yanu.c index 49bccf3..4ca6ef0 100644 --- a/drivers/serial/opencores_yanu.c +++ b/drivers/serial/opencores_yanu.c @@ -161,14 +161,6 @@ static void oc_serial_putc(char c) writel((unsigned char)c, &uart->data); }
-static void oc_serial_puts(const char *s) -{ - while (*s != 0) { - serial_putc (*s++); - } -} - - static int oc_serial_tstc(void) { unsigned status ; @@ -195,7 +187,7 @@ static struct serial_device oc_serial_drv = { .stop = NULL, .setbrg = oc_serial_setbrg, .putc = oc_serial_putc, - .puts = oc_serial_puts, + .puts = default_serial_puts, .getc = oc_serial_getc, .tstc = oc_serial_tstc, }; diff --git a/drivers/serial/s3c4510b_uart.c b/drivers/serial/s3c4510b_uart.c index 423d26e..c460229 100644 --- a/drivers/serial/s3c4510b_uart.c +++ b/drivers/serial/s3c4510b_uart.c @@ -199,12 +199,10 @@ static int s3c4510b_serial_getc(void)
static void s3c4510b_serial_puts(const char *s) { - while (*s) { - serial_putc (*s++); - } + default_serial_puts(s);
/* busy wait for tx complete */ - while ( !uart->m_stat.bf.txComplete); + while (!uart->m_stat.bf.txComplete);
/* clear break */ uart->m_ctrl.bf.sendBreak = 0; diff --git a/drivers/serial/s3c64xx.c b/drivers/serial/s3c64xx.c index 9ab8a28..ea8d734 100644 --- a/drivers/serial/s3c64xx.c +++ b/drivers/serial/s3c64xx.c @@ -166,19 +166,13 @@ static int s3c64xx_serial_tstc(void) return uart->UTRSTAT & 0x1; }
-static void s3c64xx_serial_puts(const char *s) -{ - while (*s) - serial_putc(*s++); -} - static struct serial_device s3c64xx_serial_drv = { .name = "s3c64xx_serial", .start = s3c64xx_serial_init, .stop = NULL, .setbrg = s3c64xx_serial_setbrg, .putc = s3c64xx_serial_putc, - .puts = s3c64xx_serial_puts, + .puts = default_serial_puts, .getc = s3c64xx_serial_getc, .tstc = s3c64xx_serial_tstc, }; diff --git a/drivers/serial/serial_clps7111.c b/drivers/serial/serial_clps7111.c index 65473e8..c292ed8 100644 --- a/drivers/serial/serial_clps7111.c +++ b/drivers/serial/serial_clps7111.c @@ -112,20 +112,13 @@ static int clps7111_serial_getc(void) return IO_UARTDR1 & 0xff; }
-static void clps7111_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device clps7111_serial_drv = { .name = "clps7111_serial", .start = clps7111_serial_init, .stop = NULL, .setbrg = clps7111_serial_setbrg, .putc = clps7111_serial_putc, - .puts = clps7111_serial_puts, + .puts = default_serial_puts, .getc = clps7111_serial_getc, .tstc = clps7111_serial_tstc, }; diff --git a/drivers/serial/serial_imx.c b/drivers/serial/serial_imx.c index 6c075b5..9b9be44 100644 --- a/drivers/serial/serial_imx.c +++ b/drivers/serial/serial_imx.c @@ -214,20 +214,13 @@ static int imx_serial_tstc(void) return 1; }
-static void imx_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device imx_serial_drv = { .name = "imx_serial", .start = imx_serial_init, .stop = NULL, .setbrg = imx_serial_setbrg, .putc = imx_serial_putc, - .puts = imx_serial_puts, + .puts = default_serial_puts, .getc = imx_serial_getc, .tstc = imx_serial_tstc, }; diff --git a/drivers/serial/serial_ixp.c b/drivers/serial/serial_ixp.c index c8b3658..09a3df4 100644 --- a/drivers/serial/serial_ixp.c +++ b/drivers/serial/serial_ixp.c @@ -121,20 +121,13 @@ static int ixp_serial_getc(void) return (char) RBR(CONFIG_SYS_IXP425_CONSOLE) & 0xff; }
-static void ixp_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device ixp_serial_drv = { .name = "ixp_serial", .start = ixp_serial_init, .stop = NULL, .setbrg = ixp_serial_setbrg, .putc = ixp_serial_putc, - .puts = ixp_serial_puts, + .puts = default_serial_puts, .getc = ixp_serial_getc, .tstc = ixp_serial_tstc, }; diff --git a/drivers/serial/serial_ks8695.c b/drivers/serial/serial_ks8695.c index 60e8007..8b1c974 100644 --- a/drivers/serial/serial_ks8695.c +++ b/drivers/serial/serial_ks8695.c @@ -102,13 +102,6 @@ static int ks8695_serial_tstc(void) return 0; }
-static void ks8695_serial_puts(const char *s) -{ - char c; - while ((c = *s++) != 0) - serial_putc(c); -} - static int ks8695_serial_getc(void) { volatile struct ks8695uart *uartp = KS8695_UART_ADDR; @@ -124,7 +117,7 @@ static struct serial_device ks8695_serial_drv = { .stop = NULL, .setbrg = ks8695_serial_setbrg, .putc = ks8695_serial_putc, - .puts = ks8695_serial_puts, + .puts = default_serial_puts, .getc = ks8695_serial_getc, .tstc = ks8695_serial_tstc, }; diff --git a/drivers/serial/serial_lh7a40x.c b/drivers/serial/serial_lh7a40x.c index 6c96285..68e958b 100644 --- a/drivers/serial/serial_lh7a40x.c +++ b/drivers/serial/serial_lh7a40x.c @@ -175,20 +175,13 @@ static int lh7a40x_serial_tstc(void) return(!(uart->status & UART_RXFE)); }
-static void lh7a40x_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device lh7a40x_serial_drv = { .name = "lh7a40x_serial", .start = lh7a40x_serial_init, .stop = NULL, .setbrg = lh7a40x_serial_setbrg, .putc = lh7a40x_serial_putc, - .puts = lh7a40x_serial_puts, + .puts = default_serial_puts, .getc = lh7a40x_serial_getc, .tstc = lh7a40x_serial_tstc, }; diff --git a/drivers/serial/serial_lpc2292.c b/drivers/serial/serial_lpc2292.c index fcab202..8abc476 100644 --- a/drivers/serial/serial_lpc2292.c +++ b/drivers/serial/serial_lpc2292.c @@ -89,13 +89,6 @@ static int lpc2292_serial_getc(void) return GET8(U0RBR); }
-static void lpc2292_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - /* Test if there is a byte to read */ static int lpc2292_serial_tstc(void) { @@ -108,7 +101,7 @@ static struct serial_device lpc2292_serial_drv = { .stop = NULL, .setbrg = lpc2292_serial_setbrg, .putc = lpc2292_serial_putc, - .puts = lpc2292_serial_puts, + .puts = default_serial_puts, .getc = lpc2292_serial_getc, .tstc = lpc2292_serial_tstc, }; diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index b0612f5..9227d64 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -187,13 +187,6 @@ static int mxc_serial_tstc(void) return 1; }
-static void mxc_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - /* * Initialise the serial port with the given baudrate. The settings * are always 8 data bits, no parity, 1 stop bit, no start bits. @@ -228,7 +221,7 @@ static struct serial_device mxc_serial_drv = { .stop = NULL, .setbrg = mxc_serial_setbrg, .putc = mxc_serial_putc, - .puts = mxc_serial_puts, + .puts = default_serial_puts, .getc = mxc_serial_getc, .tstc = mxc_serial_tstc, }; diff --git a/drivers/serial/serial_netarm.c b/drivers/serial/serial_netarm.c index d30adc3..44d7c50 100644 --- a/drivers/serial/serial_netarm.c +++ b/drivers/serial/serial_netarm.c @@ -182,20 +182,13 @@ static int netarm_serial_getc(void) return ch_uint & 0xff; }
-static void netarm_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device netarm_serial_drv = { .name = "netarm_serial", .start = netarm_serial_init, .stop = NULL, .setbrg = netarm_serial_setbrg, .putc = netarm_serial_putc, - .puts = netarm_serial_puts, + .puts = default_serial_puts, .getc = netarm_serial_getc, .tstc = netarm_serial_tstc, }; diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c index 7db7b65..b331be7 100644 --- a/drivers/serial/serial_pl01x.c +++ b/drivers/serial/serial_pl01x.c @@ -179,13 +179,6 @@ static void pl01x_serial_putc(const char c) pl01x_putc (CONSOLE_PORT, c); }
-static void pl01x_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static int pl01x_serial_getc(void) { return pl01x_getc (CONSOLE_PORT); @@ -259,7 +252,7 @@ static struct serial_device pl01x_serial_drv = { .stop = NULL, .setbrg = pl01x_serial_setbrg, .putc = pl01x_serial_putc, - .puts = pl01x_serial_puts, + .puts = default_serial_puts, .getc = pl01x_serial_getc, .tstc = pl01x_serial_tstc, }; diff --git a/drivers/serial/serial_s3c44b0.c b/drivers/serial/serial_s3c44b0.c index a4428e0..9cae843 100644 --- a/drivers/serial/serial_s3c44b0.c +++ b/drivers/serial/serial_s3c44b0.c @@ -209,20 +209,13 @@ static int s3c44b0_serial_getc(void) } }
-static void s3c44b0_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device s3c44b0_serial_drv = { .name = "s3c44b0_serial", .start = s3c44b0_serial_init, .stop = NULL, .setbrg = s3c44b0_serial_setbrg, .putc = s3c44b0_serial_putc, - .puts = s3c44b0_serial_puts, + .puts = default_serial_puts, .getc = s3c44b0_serial_getc, .tstc = s3c44b0_serial_tstc, }; diff --git a/drivers/serial/serial_sa1100.c b/drivers/serial/serial_sa1100.c index c6b34db..3c0f4c5 100644 --- a/drivers/serial/serial_sa1100.c +++ b/drivers/serial/serial_sa1100.c @@ -153,20 +153,13 @@ static int sa1100_serial_getc(void) #endif }
-static void sa1100_serial_puts(const char *s) -{ - while (*s) { - serial_putc (*s++); - } -} - static struct serial_device sa1100_serial_drv = { .name = "sa1100_serial", .start = sa1100_serial_init, .stop = NULL, .setbrg = sa1100_serial_setbrg, .putc = sa1100_serial_putc, - .puts = sa1100_serial_puts, + .puts = default_serial_puts, .getc = sa1100_serial_getc, .tstc = sa1100_serial_tstc, }; diff --git a/drivers/serial/serial_sh.c b/drivers/serial/serial_sh.c index 640861a..8a09621 100644 --- a/drivers/serial/serial_sh.c +++ b/drivers/serial/serial_sh.c @@ -134,13 +134,6 @@ static void sh_serial_putc(const char c) serial_raw_putc(c); }
-static void sh_serial_puts(const char *s) -{ - char c; - while ((c = *s++) != 0) - serial_putc(c); -} - static int sh_serial_tstc(void) { return serial_rx_fifo_level() ? 1 : 0; @@ -194,7 +187,7 @@ static struct serial_device sh_serial_drv = { .stop = NULL, .setbrg = sh_serial_setbrg, .putc = sh_serial_putc, - .puts = sh_serial_puts, + .puts = default_serial_puts, .getc = sh_serial_getc, .tstc = sh_serial_tstc, };
Reorder serial_assign() function to get rid of the extra level of indentation. Also, adjust the return value to be -EINVAL instead of positive one to be more consistent.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index da41cd5..1054494 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -26,6 +26,7 @@ #include <stdio_dev.h> #include <post.h> #include <linux/compiler.h> +#include <errno.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -203,13 +204,13 @@ int serial_assign(const char *name) struct serial_device *s;
for (s = serial_devices; s; s = s->next) { - if (strcmp(s->name, name) == 0) { - serial_current = s; - return 0; - } + if (strcmp(s->name, name)) + continue; + serial_current = s; + return 0; }
- return 1; + return -EINVAL; }
void serial_reinit_all(void)
On Sat, Oct 06, 2012 at 05:07:03PM -0700, Marek Vasut wrote:
Reorder serial_assign() function to get rid of the extra level of indentation. Also, adjust the return value to be -EINVAL instead of positive one to be more consistent.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com
drivers/serial/serial.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index da41cd5..1054494 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -26,6 +26,7 @@ #include <stdio_dev.h> #include <post.h> #include <linux/compiler.h> +#include <errno.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -203,13 +204,13 @@ int serial_assign(const char *name) struct serial_device *s;
for (s = serial_devices; s; s = s->next) {
if (strcmp(s->name, name) == 0) {serial_current = s;return 0;}
if (strcmp(s->name, name))continue;serial_current = s;return 0;
- return 1;
- return -EINVAL;
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
-Allen
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
-Allen
Best regards, Marek Vasut
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
-Allen
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Regards, Simon
-Allen
nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
Best regards, Marek Vasut
On Thu, Oct 25, 2012 at 12:03:47PM -0700, Marek Vasut wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
Adding Joe Hershberger to cc
I think its safe to remove the call to serial_assign() altogether, as it's written now it will always fail. Reading through doc/driver-model/UDM-serial.txt the CONFIG_SERIAL_MULTI case should be handled transparently underneath iomux. The part that still not clear to me is what mechanism is supposed to be used to select the current serial port in a CONFIG_SERIAL_MULTI configuration? AFAICT the only caller of serial_assign() that actually passes the name of a serial device is in serial_initialize():
serial_assign(default_serial_console()->name);
Should there be another environemnt variable that maps the stdio_dev "serial" to the current serial_device so you could do something like:
; get input from first serial port and USB keyboard # setenv serial eserial0 # setenv stdin serial,usbkbd
; get input from second serial port and USB keyboard # setenv serial eserial1 # setenv stdin seriak,usbkbd
-Allen
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
Regards, Simon
Best regards, Marek Vasut
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
Hi Marek, the change to return value here broke serial output on tegra. What I see is that the serial device name (s->name) is "eserial0" as set by serial_ns16550.c, and the name passed in from the stdout environment is "serial" so they don't match and it fails. This always used to be ok because the return code didn't indicate failure and iomux_doenv() would continue on happily, but now it causes iomux_doenv() to fail and no printfs() work after that.
Not sure what the right fix is, should stdout really be set to "eserial0"? It seems "serial" should mean "the default serial device" which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
-Allen
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/25/12 14:19, Allen Martin wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
> Hi Marek, the change to return value here broke serial > output on tegra. What I see is that the serial device > name (s->name) is "eserial0" as set by > serial_ns16550.c, and the name passed in from the > stdout environment is "serial" so they don't match and > it fails. This always used to be ok because the > return code didn't indicate failure and iomux_doenv() > would continue on happily, but now it causes > iomux_doenv() to fail and no printfs() work after > that. > > Not sure what the right fix is, should stdout really be > set to "eserial0"? It seems "serial" should mean "the > default serial device" which for the normal case is the > one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
Andrew's recent set of patches for am335x means I do. If I follow correctly, you're describing the case where >1 port for a driver is known, we default to say 0 but want to use 1, via the env?
- -- Tom
On Thu, Oct 25, 2012 at 02:27:24PM -0700, Tom Rini wrote:
- PGP Signed by an unknown key
On 10/25/12 14:19, Allen Martin wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: > Dear Allen Martin, > > [...] > >> Hi Marek, the change to return value here broke serial >> output on tegra. What I see is that the serial device >> name (s->name) is "eserial0" as set by >> serial_ns16550.c, and the name passed in from the >> stdout environment is "serial" so they don't match and >> it fails. This always used to be ok because the >> return code didn't indicate failure and iomux_doenv() >> would continue on happily, but now it causes >> iomux_doenv() to fail and no printfs() work after >> that. >> >> Not sure what the right fix is, should stdout really be >> set to "eserial0"? It seems "serial" should mean "the >> default serial device" which for the normal case is the >> one and only device. > > Looking at the source, the obvious course of action is to > fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
Andrew's recent set of patches for am335x means I do. If I follow correctly, you're describing the case where >1 port for a driver is known, we default to say 0 but want to use 1, via the env?
Yes, exactly. I assume that's what the current calls to serial_assign() were supposed to be doing, but weren't.
-Allen
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote:
Dear Allen Martin,
[...]
> Hi Marek, the change to return value here broke serial output on > tegra. What I see is that the serial device name (s->name) is > "eserial0" as set by serial_ns16550.c, and the name passed in from the > stdout environment is "serial" so they don't match and it fails. This > always used to be ok because the return code didn't indicate failure > and iomux_doenv() would continue on happily, but now it causes > iomux_doenv() to fail and no printfs() work after that. > > Not sure what the right fix is, should stdout really be set to > "eserial0"? It seems "serial" should mean "the default serial device" > which for the normal case is the one and only device.
Looking at the source, the obvious course of action is to fix iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting) that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
-Joe
Dear Joe Hershberger,
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com
wrote:
On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: > Dear Allen Martin, > > [...] > > > Hi Marek, the change to return value here broke serial output on > > tegra. What I see is that the serial device name (s->name) is > > "eserial0" as set by serial_ns16550.c, and the name passed in > > from the stdout environment is "serial" so they don't match and > > it fails. This always used to be ok because the return code > > didn't indicate failure and iomux_doenv() would continue on > > happily, but now it causes iomux_doenv() to fail and no > > printfs() work after that. > > > > Not sure what the right fix is, should stdout really be set to > > "eserial0"? It seems "serial" should mean "the default serial > > device" which for the normal case is the one and only device. > > Looking at the source, the obvious course of action is to fix > iomux.c .
I've been looking at this call to serial_assign() from iomux.c and I'm not convinced this code does anything meaningful at all. It passes the name of a struct stdio_dev device which serial_assign() then tries to match against the registered struct serial_devices, which will never match.
What I don't understand is the case where you have a board that actually has more than one physical serial port and how the mapping from stdio_dev to serial_device happens.
Also, looking at the code to cmd_nvedit, I think your change also broke "setenv stdout" for boards that don't define CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't go down this code path, but it looks identical to the code in iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting)
I know, he's such a bastard, always breaking stuff and interfering with other people's work !
that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
Actually, I'd like to merge the serial stuff and stdio stuff into one. So "setenv stdX serial" would be replaced with "setenv stdX <serial_driver_name>". I think that's the approach to take. But it'd break many boards.
Best regards, Marek Vasut
On Fri, Oct 26, 2012 at 03:22:32AM -0700, Marek Vasut wrote:
Dear Joe Hershberger,
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com
wrote:
> On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: >> Dear Allen Martin, >> >> [...] >> >> > Hi Marek, the change to return value here broke serial output on >> > tegra. What I see is that the serial device name (s->name) is >> > "eserial0" as set by serial_ns16550.c, and the name passed in >> > from the stdout environment is "serial" so they don't match and >> > it fails. This always used to be ok because the return code >> > didn't indicate failure and iomux_doenv() would continue on >> > happily, but now it causes iomux_doenv() to fail and no >> > printfs() work after that. >> > >> > Not sure what the right fix is, should stdout really be set to >> > "eserial0"? It seems "serial" should mean "the default serial >> > device" which for the normal case is the one and only device. >> >> Looking at the source, the obvious course of action is to fix >> iomux.c . > > I've been looking at this call to serial_assign() from iomux.c and > I'm not convinced this code does anything meaningful at all. It > passes the name of a struct stdio_dev device which serial_assign() > then tries to match against the registered struct serial_devices, > which will never match. > > What I don't understand is the case where you have a board that > actually has more than one physical serial port and how the mapping > from stdio_dev to serial_device happens. > > Also, looking at the code to cmd_nvedit, I think your change also > broke "setenv stdout" for boards that don't define > CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't > go down this code path, but it looks identical to the code in > iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting)
I know, he's such a bastard, always breaking stuff and interfering with other people's work !
that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
Actually, I'd like to merge the serial stuff and stdio stuff into one. So "setenv stdX serial" would be replaced with "setenv stdX <serial_driver_name>". I think that's the approach to take. But it'd break many boards.
I think that would be fine if we can also preserve "setenv stdin serial" to use the default serial device. That will mean no changes for boards that have only one serial port, which is the vast majority.
-Allen
Hi Marek,
On Fri, Oct 26, 2012 at 5:22 AM, Marek Vasut marex@denx.de wrote:
Dear Joe Hershberger,
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
Hi,
On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com
wrote:
> On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: >> Dear Allen Martin, >> >> [...] >> >> > Hi Marek, the change to return value here broke serial output on >> > tegra. What I see is that the serial device name (s->name) is >> > "eserial0" as set by serial_ns16550.c, and the name passed in >> > from the stdout environment is "serial" so they don't match and >> > it fails. This always used to be ok because the return code >> > didn't indicate failure and iomux_doenv() would continue on >> > happily, but now it causes iomux_doenv() to fail and no >> > printfs() work after that. >> > >> > Not sure what the right fix is, should stdout really be set to >> > "eserial0"? It seems "serial" should mean "the default serial >> > device" which for the normal case is the one and only device. >> >> Looking at the source, the obvious course of action is to fix >> iomux.c . > > I've been looking at this call to serial_assign() from iomux.c and > I'm not convinced this code does anything meaningful at all. It > passes the name of a struct stdio_dev device which serial_assign() > then tries to match against the registered struct serial_devices, > which will never match. > > What I don't understand is the case where you have a board that > actually has more than one physical serial port and how the mapping > from stdio_dev to serial_device happens. > > Also, looking at the code to cmd_nvedit, I think your change also > broke "setenv stdout" for boards that don't define > CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't > go down this code path, but it looks identical to the code in > iomux.c
Sorry if I missed it - what was the resolution here? Should we revert that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting)
I know, he's such a bastard, always breaking stuff and interfering with other people's work !
:)
that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
Actually, I'd like to merge the serial stuff and stdio stuff into one. So "setenv stdX serial" would be replaced with "setenv stdX <serial_driver_name>". I think that's the approach to take. But it'd break many boards.
I think that's fine. We should be able to make it work both ways. In other words, as long as there is only one serial device registered, then "serial" will be accepted. The actual name of the driver is also always accepted.
-Joe
On Fri, Oct 26, 2012 at 11:39:48AM -0700, Joe Hershberger wrote:
Hi Marek,
On Fri, Oct 26, 2012 at 5:22 AM, Marek Vasut marex@denx.de wrote:
Dear Joe Hershberger,
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote:
Dear Simon Glass,
> Hi, > > On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin amartin@nvidia.com
wrote:
> > On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: > >> Dear Allen Martin, > >> > >> [...] > >> > >> > Hi Marek, the change to return value here broke serial output on > >> > tegra. What I see is that the serial device name (s->name) is > >> > "eserial0" as set by serial_ns16550.c, and the name passed in > >> > from the stdout environment is "serial" so they don't match and > >> > it fails. This always used to be ok because the return code > >> > didn't indicate failure and iomux_doenv() would continue on > >> > happily, but now it causes iomux_doenv() to fail and no > >> > printfs() work after that. > >> > > >> > Not sure what the right fix is, should stdout really be set to > >> > "eserial0"? It seems "serial" should mean "the default serial > >> > device" which for the normal case is the one and only device. > >> > >> Looking at the source, the obvious course of action is to fix > >> iomux.c . > > > > I've been looking at this call to serial_assign() from iomux.c and > > I'm not convinced this code does anything meaningful at all. It > > passes the name of a struct stdio_dev device which serial_assign() > > then tries to match against the registered struct serial_devices, > > which will never match. > > > > What I don't understand is the case where you have a board that > > actually has more than one physical serial port and how the mapping > > from stdio_dev to serial_device happens. > > > > Also, looking at the code to cmd_nvedit, I think your change also > > broke "setenv stdout" for boards that don't define > > CONFIG_CONSOLE_MUX. We always have this on for tegra, so we don't > > go down this code path, but it looks identical to the code in > > iomux.c > > Sorry if I missed it - what was the resolution here? Should we revert > that change?
Definitelly not. We should fix the iomux.c , possibly by flipping the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting)
I know, he's such a bastard, always breaking stuff and interfering with other people's work !
:)
that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
Actually, I'd like to merge the serial stuff and stdio stuff into one. So "setenv stdX serial" would be replaced with "setenv stdX <serial_driver_name>". I think that's the approach to take. But it'd break many boards.
I think that's fine. We should be able to make it work both ways. In other words, as long as there is only one serial device registered, then "serial" will be accepted. The actual name of the driver is also always accepted.
Or, ever better we could just say "serial" always maps to default_serial_console() and that should map exactly to existing functionality regardless if there is one or more serial devices. Then any board that wants more precise control can use the name of the serial driver instead.
-Allen
Dear Allen Martin,
On Fri, Oct 26, 2012 at 11:39:48AM -0700, Joe Hershberger wrote:
Hi Marek,
On Fri, Oct 26, 2012 at 5:22 AM, Marek Vasut marex@denx.de wrote:
Dear Joe Hershberger,
Hi Allen,
On Thu, Oct 25, 2012 at 4:19 PM, Allen Martin amartin@nvidia.com wrote:
On Thu, Oct 25, 2012 at 02:02:55PM -0700, Simon Glass wrote:
Hi,
On Thu, Oct 25, 2012 at 12:03 PM, Marek Vasut marex@denx.de wrote: > Dear Simon Glass, > >> Hi, >> >> On Mon, Oct 22, 2012 at 10:23 AM, Allen Martin >> amartin@nvidia.com
wrote:
>> > On Sat, Oct 20, 2012 at 01:19:00AM -0700, Marek Vasut wrote: >> >> Dear Allen Martin, >> >> >> >> [...] >> >> >> >> > Hi Marek, the change to return value here broke serial >> >> > output on tegra. What I see is that the serial device >> >> > name (s->name) is "eserial0" as set by serial_ns16550.c, >> >> > and the name passed in from the stdout environment is >> >> > "serial" so they don't match and it fails. This always >> >> > used to be ok because the return code didn't indicate >> >> > failure and iomux_doenv() would continue on happily, but >> >> > now it causes iomux_doenv() to fail and no printfs() work >> >> > after that. >> >> > >> >> > Not sure what the right fix is, should stdout really be set >> >> > to "eserial0"? It seems "serial" should mean "the default >> >> > serial device" which for the normal case is the one and >> >> > only device. >> >> >> >> Looking at the source, the obvious course of action is to fix >> >> iomux.c . >> > >> > I've been looking at this call to serial_assign() from iomux.c >> > and I'm not convinced this code does anything meaningful at >> > all. It passes the name of a struct stdio_dev device which >> > serial_assign() then tries to match against the registered >> > struct serial_devices, which will never match. >> > >> > What I don't understand is the case where you have a board >> > that actually has more than one physical serial port and how >> > the mapping from stdio_dev to serial_device happens. >> > >> > Also, looking at the code to cmd_nvedit, I think your change >> > also broke "setenv stdout" for boards that don't define >> > CONFIG_CONSOLE_MUX. We always have this on for tegra, so we >> > don't go down this code path, but it looks identical to the >> > code in iomux.c >> >> Sorry if I missed it - what was the resolution here? Should we >> revert that change? > > Definitelly not. We should fix the iomux.c , possibly by flipping > the inequation mark as a short term solution.
OK that's fine. Is someone working on a patch?
I'll send out my proposal for a patch. Unfortunately I don't have a board with multiple serial ports to correctly test CONFIG_SERIAL_MULTI
One of the boards I'm working on does this (has more than one). At least before the serial rework from Marek, the stdout was either the serial device directly (each serial device was added as a console device as well) so the serial setting was redundant. You could just set them directly to the serial port (which is more flexible).
I had two patches (not sent to ML before Marek made them highly conflicting)
I know, he's such a bastard, always breaking stuff and interfering with other people's work !
: :) :
that take the opposite approach you were, since it preserves the flexibility. It removed the "serial" setting to each of the std* variables and instead sets it to the default serial device. I'll remake that patch on top of the new serial landscape sometime soon.
Actually, I'd like to merge the serial stuff and stdio stuff into one. So "setenv stdX serial" would be replaced with "setenv stdX <serial_driver_name>". I think that's the approach to take. But it'd break many boards.
I think that's fine. We should be able to make it work both ways. In other words, as long as there is only one serial device registered, then "serial" will be accepted. The actual name of the driver is also always accepted.
Or, ever better we could just say "serial" always maps to default_serial_console() and that should map exactly to existing functionality regardless if there is one or more serial devices. Then any board that wants more precise control can use the name of the serial driver instead.
So default_serial_console() will return a char * instead of driver pointer ... good.
Best regards, Marek Vasut
Reorder the get_current() function to make it a bit more readable. The code does not grow and there is minor change in the code logic, where dev != NULL is now checked in any case.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 1054494..57e3b75 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -225,20 +225,23 @@ static struct serial_device *get_current(void) { struct serial_device *dev;
- if (!(gd->flags & GD_FLG_RELOC) || !serial_current) { + if (!(gd->flags & GD_FLG_RELOC)) dev = default_serial_console(); + else if (!serial_current) + dev = default_serial_console(); + else + dev = serial_current;
- /* We must have a console device */ - if (!dev) { + /* We must have a console device */ + if (!dev) { #ifdef CONFIG_SPL_BUILD - puts("Cannot find console\n"); - hang(); + puts("Cannot find console\n"); + hang(); #else - panic("Cannot find console\n"); + panic("Cannot find console\n"); #endif - } - } else - dev = serial_current; + } + return dev; }
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 57e3b75..7eb4838 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -33,10 +33,27 @@ DECLARE_GLOBAL_DATA_PTR; static struct serial_device *serial_devices; static struct serial_device *serial_current;
+/** + * serial_null() - Void registration routine of a serial driver + * + * This routine implements a void registration routine of a serial driver. The + * registration routine of a particular driver is aliased to this empty function + * in case the driver is not compiled into U-Boot. + */ static void serial_null(void) { }
+/** + * serial_initfunc() - Do a forward declaration of driver registration routine + * @name: Name of the real driver registration routine. + * + * This macro expands onto forward declaration of a driver registration routine, + * which is then used below in serial_initialize() function. The declaration is + * made weak and aliases to serial_null() so in case the driver is not compiled + * in, the function is still declared and can be used, but aliases to + * serial_null() and thus is optimized away. + */ #define serial_initfunc(name) \ void name(void) \ __attribute__((weak, alias("serial_null"))); @@ -94,6 +111,15 @@ serial_initfunc(s3c44b0_serial_initialize); serial_initfunc(sa1100_serial_initialize); serial_initfunc(sh_serial_initialize);
+/** + * serial_register() - Register serial driver with serial driver core + * @dev: Pointer to the serial driver structure + * + * This function registers the serial driver supplied via @dev with serial + * driver core, thus making U-Boot aware of it and making it available for + * U-Boot to use. On platforms that still require manual relocation of constant + * variables, relocation of the supplied structure is performed. + */ void serial_register(struct serial_device *dev) { #ifdef CONFIG_NEEDS_MANUAL_RELOC @@ -117,6 +143,14 @@ void serial_register(struct serial_device *dev) serial_devices = dev; }
+/** + * serial_initialize() - Register all compiled-in serial port drivers + * + * This function registers all serial port drivers that are compiled into + * the U-Boot binary with the serial core, thus making them available to + * U-Boot to use. Lastly, this function assigns a default serial port to + * the serial core. That serial port is then used as a default output. + */ void serial_initialize(void) { mpc8xx_serial_initialize(); @@ -175,6 +209,13 @@ void serial_initialize(void) serial_assign(default_serial_console()->name); }
+/** + * serial_stdio_init() - Register serial ports with STDIO core + * + * This function generates a proxy driver for each serial port driver. These + * proxy drivers then register with the STDIO core, making the serial drivers + * available as STDIO devices. + */ void serial_stdio_init(void) { struct stdio_dev dev; @@ -199,6 +240,17 @@ void serial_stdio_init(void) } }
+/** + * serial_assign() - Select the serial output device by name + * @name: Name of the serial driver to be used as default output + * + * This function configures the serial output multiplexing by selecting which + * serial device will be used as default. In case the STDIO "serial" device is + * selected as stdin/stdout/stderr, the serial device previously configured by + * this function will be used for the particular operation. + * + * Returns 0 on success, negative on error. + */ int serial_assign(const char *name) { struct serial_device *s; @@ -213,6 +265,12 @@ int serial_assign(const char *name) return -EINVAL; }
+/** + * serial_reinit_all() - Reinitialize all compiled-in serial ports + * + * This function reinitializes all serial ports that are compiled into U-Boot + * by calling their serial_start() functions. + */ void serial_reinit_all(void) { struct serial_device *s; @@ -221,6 +279,20 @@ void serial_reinit_all(void) s->start(); }
+/** + * get_current() - Return pointer to currently selected serial port + * + * This function returns a pointer to currently selected serial port. The + * currently selected serial port is altered by serial_assign() function. + * + * In case this function is called before relocation or before any serial + * port is configured, this function calls default_serial_console() to + * determine the serial port. Otherwise, the configured serial port is + * returned. + * + * Returns pointer to the currently selected serial port on success, + * NULL on error. + */ static struct serial_device *get_current(void) { struct serial_device *dev; @@ -245,36 +317,110 @@ static struct serial_device *get_current(void) return dev; }
+/** + * serial_init() - Initialize currently selected serial port + * + * This function initializes the currently selected serial port. This usually + * involves setting up the registers of that particular port, enabling clock + * and such. This function uses the get_current() call to determine which port + * is selected. + * + * Returns 0 on success, negative on error. + */ int serial_init(void) { return get_current()->start(); }
+/** + * serial_setbrg() - Configure baud-rate of currently selected serial port + * + * This function configures the baud-rate of the currently selected serial + * port. The baud-rate is retrieved from global data within the serial port + * driver. This function uses the get_current() call to determine which port + * is selected. + * + * Returns 0 on success, negative on error. + */ void serial_setbrg(void) { get_current()->setbrg(); }
+/** + * serial_getc() - Read single character from currently selected serial port + * + * This function retrieves a single character from currently selected serial + * port. In case there is no character in the serial port fifo, this function + * will block and wait for the character indefinitelly. This function uses the + * get_current() call to determine which port is selected. + * + * Returns the character on success, negative on error. + */ int serial_getc(void) { return get_current()->getc(); }
+/** + * serial_tstc() - Test if data is available on currently selected serial port + * + * This function tests if a single character is available on currently selected + * serial port. In case there is no character in the serial port buffer, this + * function will not block. This function uses the get_current() call to + * determine which port is selected. + * + * Returns positive if character is available, zero otherwise. + */ int serial_tstc(void) { return get_current()->tstc(); }
+/** + * serial_putc() - Output single character via currently selected serial port + * @c: Single character to be output from the serial port. + * + * This function outputs a single character via currently selected serial port. + * This character is written into the fifo of the port and the function waits + * for the character to be emitted, therefore this function may block for a + * short amount of time. This function uses the get_current() call to determine + * which port is selected. + */ void serial_putc(const char c) { get_current()->putc(c); }
+/** + * serial_puts() - Output string via currently selected serial port + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string via currently selected serial + * port. This function behaves as an accelerator in case the hardware has a + * larger fifo buffer. The whole string that is to be output is available to + * the function implementing the hardware manipulation. The function waits + * for the whole string to be emitted, therefore this function may block for a + * some amount of time. This function uses the get_current() call to determine + * which port is selected. + */ void serial_puts(const char *s) { get_current()->puts(s); }
+/** + * default_serial_puts() - Output string by calling serial_putc() in loop + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string by calling serial_putc() + * in a loop. Most drivers do not fifo larger than one byte, thus this function + * precisely implements their serial_puts(). + * + * To optimize the number of get_current() calls, this function only calls + * get_current() once and then directly accesses the putc() call of the + * &struct serial_device . + */ void default_serial_puts(const char *s) { struct serial_device *dev = get_current(); @@ -285,6 +431,17 @@ void default_serial_puts(const char *s) #if CONFIG_POST & CONFIG_SYS_POST_UART static const int bauds[] = CONFIG_SYS_BAUDRATE_TABLE;
+/** + * uart_post_test() - Test the currently selected serial port using POST + * @flags: POST framework flags + * + * Do a loopback test of the currently selected serial port. This function + * is only useful in the context of the POST testing framwork. The serial + * port is firstly configured into loopback mode and then characters are + * sent through it. + * + * Returns 0 on success, value otherwise. + */ /* Mark weak until post/cpu/.../uart.c migrate over */ __weak int uart_post_test(int flags)
On Sun, Oct 07, 2012 at 02:07:05AM +0200, Marek Vasut wrote:
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com
Please re-wrap the comments you're adding, they're too wide in many cases and right at the edge in others (textwidth=72 or 75 is my preference).
On Mon, Oct 08, 2012 at 12:37:43PM -0700, Tom Rini wrote:
On Sun, Oct 07, 2012 at 02:07:05AM +0200, Marek Vasut wrote:
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com
Please re-wrap the comments you're adding, they're too wide in many cases and right at the edge in others (textwidth=72 or 75 is my preference).
For the record, I appreciate that Marek re-wrapped his text but indeed 80-wide comments are allowed and should only result in my cussing to myself.
Dear Tom Rini,
On Mon, Oct 08, 2012 at 12:37:43PM -0700, Tom Rini wrote:
On Sun, Oct 07, 2012 at 02:07:05AM +0200, Marek Vasut wrote:
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com
Please re-wrap the comments you're adding, they're too wide in many cases and right at the edge in others (textwidth=72 or 75 is my preference).
For the record, I appreciate that Marek re-wrapped his text but indeed 80-wide comments are allowed and should only result in my cussing to myself.
I adjusted my email client to handle this actually ...
Best regards, Marek Vasut
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+)
V2: Rewrap the documentation.
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 57e3b75..51ddad7 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -33,10 +33,28 @@ DECLARE_GLOBAL_DATA_PTR; static struct serial_device *serial_devices; static struct serial_device *serial_current;
+/** + * serial_null() - Void registration routine of a serial driver + * + * This routine implements a void registration routine of a serial + * driver. The registration routine of a particular driver is aliased + * to this empty function in case the driver is not compiled into + * U-Boot. + */ static void serial_null(void) { }
+/** + * serial_initfunc() - Forward declare of driver registration routine + * @name: Name of the real driver registration routine. + * + * This macro expands onto forward declaration of a driver registration + * routine, which is then used below in serial_initialize() function. + * The declaration is made weak and aliases to serial_null() so in case + * the driver is not compiled in, the function is still declared and can + * be used, but aliases to serial_null() and thus is optimized away. + */ #define serial_initfunc(name) \ void name(void) \ __attribute__((weak, alias("serial_null"))); @@ -94,6 +112,16 @@ serial_initfunc(s3c44b0_serial_initialize); serial_initfunc(sa1100_serial_initialize); serial_initfunc(sh_serial_initialize);
+/** + * serial_register() - Register serial driver with serial driver core + * @dev: Pointer to the serial driver structure + * + * This function registers the serial driver supplied via @dev with + * serial driver core, thus making U-Boot aware of it and making it + * available for U-Boot to use. On platforms that still require manual + * relocation of constant variables, relocation of the supplied structure + * is performed. + */ void serial_register(struct serial_device *dev) { #ifdef CONFIG_NEEDS_MANUAL_RELOC @@ -117,6 +145,15 @@ void serial_register(struct serial_device *dev) serial_devices = dev; }
+/** + * serial_initialize() - Register all compiled-in serial port drivers + * + * This function registers all serial port drivers that are compiled + * into the U-Boot binary with the serial core, thus making them + * available to U-Boot to use. Lastly, this function assigns a default + * serial port to the serial core. That serial port is then used as a + * default output. + */ void serial_initialize(void) { mpc8xx_serial_initialize(); @@ -175,6 +212,13 @@ void serial_initialize(void) serial_assign(default_serial_console()->name); }
+/** + * serial_stdio_init() - Register serial ports with STDIO core + * + * This function generates a proxy driver for each serial port driver. + * These proxy drivers then register with the STDIO core, making the + * serial drivers available as STDIO devices. + */ void serial_stdio_init(void) { struct stdio_dev dev; @@ -199,6 +243,18 @@ void serial_stdio_init(void) } }
+/** + * serial_assign() - Select the serial output device by name + * @name: Name of the serial driver to be used as default output + * + * This function configures the serial output multiplexing by + * selecting which serial device will be used as default. In case + * the STDIO "serial" device is selected as stdin/stdout/stderr, + * the serial device previously configured by this function will be + * used for the particular operation. + * + * Returns 0 on success, negative on error. + */ int serial_assign(const char *name) { struct serial_device *s; @@ -213,6 +269,12 @@ int serial_assign(const char *name) return -EINVAL; }
+/** + * serial_reinit_all() - Reinitialize all compiled-in serial ports + * + * This function reinitializes all serial ports that are compiled + * into U-Boot by calling their serial_start() functions. + */ void serial_reinit_all(void) { struct serial_device *s; @@ -221,6 +283,21 @@ void serial_reinit_all(void) s->start(); }
+/** + * get_current() - Return pointer to currently selected serial port + * + * This function returns a pointer to currently selected serial port. + * The currently selected serial port is altered by serial_assign() + * function. + * + * In case this function is called before relocation or before any serial + * port is configured, this function calls default_serial_console() to + * determine the serial port. Otherwise, the configured serial port is + * returned. + * + * Returns pointer to the currently selected serial port on success, + * NULL on error. + */ static struct serial_device *get_current(void) { struct serial_device *dev; @@ -245,36 +322,112 @@ static struct serial_device *get_current(void) return dev; }
+/** + * serial_init() - Initialize currently selected serial port + * + * This function initializes the currently selected serial port. This + * usually involves setting up the registers of that particular port, + * enabling clock and such. This function uses the get_current() call + * to determine which port is selected. + * + * Returns 0 on success, negative on error. + */ int serial_init(void) { return get_current()->start(); }
+/** + * serial_setbrg() - Configure baud-rate of currently selected serial port + * + * This function configures the baud-rate of the currently selected + * serial port. The baud-rate is retrieved from global data within + * the serial port driver. This function uses the get_current() call + * to determine which port is selected. + * + * Returns 0 on success, negative on error. + */ void serial_setbrg(void) { get_current()->setbrg(); }
+/** + * serial_getc() - Read single character from currently selected serial port + * + * This function retrieves a single character from currently selected + * serial port. In case there is no character in the serial port fifo, + * this function will block and wait for the character indefinitelly. + * This function uses the get_current() call to determine which port + * is selected. + * + * Returns the character on success, negative on error. + */ int serial_getc(void) { return get_current()->getc(); }
+/** + * serial_tstc() - Test if data is available on currently selected serial port + * + * This function tests if a single character is available on currently + * selected serial port. In case there is no character in the serial + * port buffer, this function will not block. This function uses the + * get_current() call to determine which port is selected. + * + * Returns positive if character is available, zero otherwise. + */ int serial_tstc(void) { return get_current()->tstc(); }
+/** + * serial_putc() - Output single character via currently selected serial port + * @c: Single character to be output from the serial port. + * + * This function outputs a single character via currently selected + * serial port. This character is written into the fifo of the port + * and the function waits for the character to be emitted, therefore + * this function may block for a short amount of time. This function + * uses the get_current() call to determine which port is selected. + */ void serial_putc(const char c) { get_current()->putc(c); }
+/** + * serial_puts() - Output string via currently selected serial port + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string via currently + * selected serial port. This function behaves as an accelerator + * in case the hardware has a larger fifo buffer. The whole string + * that is to be output is available to the function implementing + * the hardware manipulation. The function waits for the whole string + * to be emitted, therefore this function may block for a some amount + * of time. This function uses the get_current() call to determine + * which port is selected. + */ void serial_puts(const char *s) { get_current()->puts(s); }
+/** + * default_serial_puts() - Output string by calling serial_putc() in loop + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string by calling serial_putc() + * in a loop. Most drivers do not fifo larger than one byte, thus this + * function precisely implements their serial_puts(). + * + * To optimize the number of get_current() calls, this function only + * calls get_current() once and then directly accesses the putc() call + * of the &struct serial_device . + */ void default_serial_puts(const char *s) { struct serial_device *dev = get_current(); @@ -285,6 +438,17 @@ void default_serial_puts(const char *s) #if CONFIG_POST & CONFIG_SYS_POST_UART static const int bauds[] = CONFIG_SYS_BAUDRATE_TABLE;
+/** + * uart_post_test() - Test the currently selected serial port using POST + * @flags: POST framework flags + * + * Do a loopback test of the currently selected serial port. This + * function is only useful in the context of the POST testing framwork. + * The serial port is firstly configured into loopback mode and then + * characters are sent through it. + * + * Returns 0 on success, value otherwise. + */ /* Mark weak until post/cpu/.../uart.c migrate over */ __weak int uart_post_test(int flags)
Add kerneldoc annotations into serial core.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- drivers/serial/serial.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+)
V2: Rewrap the documentation. V3: Adjust the documentation to be more correct.
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 57e3b75..9f8c35b 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -33,10 +33,28 @@ DECLARE_GLOBAL_DATA_PTR; static struct serial_device *serial_devices; static struct serial_device *serial_current;
+/** + * serial_null() - Void registration routine of a serial driver + * + * This routine implements a void registration routine of a serial + * driver. The registration routine of a particular driver is aliased + * to this empty function in case the driver is not compiled into + * U-Boot. + */ static void serial_null(void) { }
+/** + * serial_initfunc() - Forward declare of driver registration routine + * @name: Name of the real driver registration routine. + * + * This macro expands onto forward declaration of a driver registration + * routine, which is then used below in serial_initialize() function. + * The declaration is made weak and aliases to serial_null() so in case + * the driver is not compiled in, the function is still declared and can + * be used, but aliases to serial_null() and thus is optimized away. + */ #define serial_initfunc(name) \ void name(void) \ __attribute__((weak, alias("serial_null"))); @@ -94,6 +112,16 @@ serial_initfunc(s3c44b0_serial_initialize); serial_initfunc(sa1100_serial_initialize); serial_initfunc(sh_serial_initialize);
+/** + * serial_register() - Register serial driver with serial driver core + * @dev: Pointer to the serial driver structure + * + * This function registers the serial driver supplied via @dev with + * serial driver core, thus making U-Boot aware of it and making it + * available for U-Boot to use. On platforms that still require manual + * relocation of constant variables, relocation of the supplied structure + * is performed. + */ void serial_register(struct serial_device *dev) { #ifdef CONFIG_NEEDS_MANUAL_RELOC @@ -117,6 +145,15 @@ void serial_register(struct serial_device *dev) serial_devices = dev; }
+/** + * serial_initialize() - Register all compiled-in serial port drivers + * + * This function registers all serial port drivers that are compiled + * into the U-Boot binary with the serial core, thus making them + * available to U-Boot to use. Lastly, this function assigns a default + * serial port to the serial core. That serial port is then used as a + * default output. + */ void serial_initialize(void) { mpc8xx_serial_initialize(); @@ -175,6 +212,13 @@ void serial_initialize(void) serial_assign(default_serial_console()->name); }
+/** + * serial_stdio_init() - Register serial ports with STDIO core + * + * This function generates a proxy driver for each serial port driver. + * These proxy drivers then register with the STDIO core, making the + * serial drivers available as STDIO devices. + */ void serial_stdio_init(void) { struct stdio_dev dev; @@ -199,6 +243,18 @@ void serial_stdio_init(void) } }
+/** + * serial_assign() - Select the serial output device by name + * @name: Name of the serial driver to be used as default output + * + * This function configures the serial output multiplexing by + * selecting which serial device will be used as default. In case + * the STDIO "serial" device is selected as stdin/stdout/stderr, + * the serial device previously configured by this function will be + * used for the particular operation. + * + * Returns 0 on success, negative on error. + */ int serial_assign(const char *name) { struct serial_device *s; @@ -213,6 +269,12 @@ int serial_assign(const char *name) return -EINVAL; }
+/** + * serial_reinit_all() - Reinitialize all compiled-in serial ports + * + * This function reinitializes all serial ports that are compiled + * into U-Boot by calling their serial_start() functions. + */ void serial_reinit_all(void) { struct serial_device *s; @@ -221,6 +283,21 @@ void serial_reinit_all(void) s->start(); }
+/** + * get_current() - Return pointer to currently selected serial port + * + * This function returns a pointer to currently selected serial port. + * The currently selected serial port is altered by serial_assign() + * function. + * + * In case this function is called before relocation or before any serial + * port is configured, this function calls default_serial_console() to + * determine the serial port. Otherwise, the configured serial port is + * returned. + * + * Returns pointer to the currently selected serial port on success, + * NULL on error. + */ static struct serial_device *get_current(void) { struct serial_device *dev; @@ -245,36 +322,113 @@ static struct serial_device *get_current(void) return dev; }
+/** + * serial_init() - Initialize currently selected serial port + * + * This function initializes the currently selected serial port. This + * usually involves setting up the registers of that particular port, + * enabling clock and such. This function uses the get_current() call + * to determine which port is selected. + * + * Returns 0 on success, negative on error. + */ int serial_init(void) { return get_current()->start(); }
+/** + * serial_setbrg() - Configure baud-rate of currently selected serial port + * + * This function configures the baud-rate of the currently selected + * serial port. The baud-rate is retrieved from global data within + * the serial port driver. This function uses the get_current() call + * to determine which port is selected. + * + * Returns 0 on success, negative on error. + */ void serial_setbrg(void) { get_current()->setbrg(); }
+/** + * serial_getc() - Read character from currently selected serial port + * + * This function retrieves a character from currently selected serial + * port. In case there is no character waiting on the serial port, + * this function will block and wait for the character to appear. This + * function uses the get_current() call to determine which port is + * selected. + * + * Returns the character on success, negative on error. + */ int serial_getc(void) { return get_current()->getc(); }
+/** + * serial_tstc() - Test if data is available on currently selected serial port + * + * This function tests if one or more characters are available on + * currently selected serial port. This function never blocks. This + * function uses the get_current() call to determine which port is + * selected. + * + * Returns positive if character is available, zero otherwise. + */ int serial_tstc(void) { return get_current()->tstc(); }
+/** + * serial_putc() - Output character via currently selected serial port + * @c: Single character to be output from the serial port. + * + * This function outputs a character via currently selected serial + * port. This character is passed to the serial port driver responsible + * for controlling the hardware. The hardware may still be in process + * of transmitting another character, therefore this function may block + * for a short amount of time. This function uses the get_current() + * call to determine which port is selected. + */ void serial_putc(const char c) { get_current()->putc(c); }
+/** + * serial_puts() - Output string via currently selected serial port + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string via currently + * selected serial port. This function behaves as an accelerator + * in case the hardware can queue multiple characters for transfer. + * The whole string that is to be output is available to the function + * implementing the hardware manipulation. Transmitting the whole + * string may take some time, thus this function may block for some + * amount of time. This function uses the get_current() call to + * determine which port is selected. + */ void serial_puts(const char *s) { get_current()->puts(s); }
+/** + * default_serial_puts() - Output string by calling serial_putc() in loop + * @s: Zero-terminated string to be output from the serial port. + * + * This function outputs a zero-terminated string by calling serial_putc() + * in a loop. Most drivers do not support queueing more than one byte for + * transfer, thus this function precisely implements their serial_puts(). + * + * To optimize the number of get_current() calls, this function only + * calls get_current() once and then directly accesses the putc() call + * of the &struct serial_device . + */ void default_serial_puts(const char *s) { struct serial_device *dev = get_current(); @@ -285,6 +439,17 @@ void default_serial_puts(const char *s) #if CONFIG_POST & CONFIG_SYS_POST_UART static const int bauds[] = CONFIG_SYS_BAUDRATE_TABLE;
+/** + * uart_post_test() - Test the currently selected serial port using POST + * @flags: POST framework flags + * + * Do a loopback test of the currently selected serial port. This + * function is only useful in the context of the POST testing framwork. + * The serial port is firstly configured into loopback mode and then + * characters are sent through it. + * + * Returns 0 on success, value otherwise. + */ /* Mark weak until post/cpu/.../uart.c migrate over */ __weak int uart_post_test(int flags)
Add STDIO documentation template.
Signed-off-by: Marek Vasut marex@denx.de Cc: Marek Vasut marek.vasut@gmail.com Cc: Tom Rini trini@ti.com --- doc/DocBook/Makefile | 2 +- doc/DocBook/stdio.tmpl | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 doc/DocBook/stdio.tmpl
diff --git a/doc/DocBook/Makefile b/doc/DocBook/Makefile index 5f13d3d..da88b32 100644 --- a/doc/DocBook/Makefile +++ b/doc/DocBook/Makefile @@ -8,7 +8,7 @@
include $(TOPDIR)/config.mk
-DOCBOOKS := linker_lists.xml +DOCBOOKS := linker_lists.xml stdio.xml
### # The build process is as follows (targets): diff --git a/doc/DocBook/stdio.tmpl b/doc/DocBook/stdio.tmpl new file mode 100644 index 0000000..4783abb --- /dev/null +++ b/doc/DocBook/stdio.tmpl @@ -0,0 +1,46 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN" + "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []> + +<book id="UBootSTDIO"> + <bookinfo> + <title>The U-Boot STDIO subsystem</title> + + <legalnotice> + <para> + This documentation is free software; you can redistribute + it and/or modify it under the terms of the GNU General Public + License as published by the Free Software Foundation; either + version 2 of the License, or (at your option) any later + version. + </para> + + <para> + This program is distributed in the hope that it will be + useful, but WITHOUT ANY WARRANTY; without even the implied + warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + See the GNU General Public License for more details. + </para> + + <para> + You should have received a copy of the GNU General Public + License along with this program; if not, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, + MA 02111-1307 USA + </para> + + <para> + For more details see the file COPYING in the source + distribution of U-Boot Bootloader. + </para> + </legalnotice> + </bookinfo> + +<toc></toc> + + <chapter id="adt"> + <title>U-Boot Serial subsystem</title> +!Idrivers/serial/serial.c + </chapter> + +</book>
participants (5)
-
Allen Martin -
Joe Hershberger -
Marek Vasut -
Simon Glass -
Tom Rini