[U-Boot] [PATCH 1/2] IOMUX: Add console multiplexing support.
See doc/README.iomux for a general description of what this does.
This is the first of two commits. The second commit touches net/eth.c and has to go through the custodian, so I split it out for simplicity.
Tested with MAKEALL 8xx.
Signed-off-by: Gary Jennejohn garyj@denx.de --- common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++ common/iomux.c | 133 +++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 90 ++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 48 ++++++++++++++ 7 files changed, 458 insertions(+), 0 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..9138742 100644 --- a/common/Makefile +++ b/common/Makefile @@ -155,6 +155,7 @@ COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_IO_MUX) += iomux.o
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c) diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 637d6c9..6fc9313 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -213,6 +213,11 @@ int _do_setenv (int flag, int argc, char *argv[]) return 1; }
+#ifdef CONFIG_IO_MUX + i = iomux_doenv(console, argv[2]); + if (i) + return i; +#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_IO_MUX */ }
/* diff --git a/common/console.c b/common/console.c index 56d9118..6158af9 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,78 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/ + +static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS]; + +/* + * This depends on tstc() always being called before getc(). + * No attempt is made to demultiplex multiple input sources. + */ +static int iomux_getc(void) +{ + unsigned char ret; + + ret = tstcdev->getc(); + tstcdev = NULL; + return ret; +} + +static int iomux_tstc(int file) +{ + int i, ret; + device_t *dev; + + disable_ctrlc(1); + for (i = 0; i < MAX_CONSARGS; i++) { + dev = console_devices[file][i]; + if (dev == NULL) + break; + if (dev->tstc != NULL) { + ret = dev->tstc(); + if (ret > 0) { + tstcdev = dev; + disable_ctrlc(0); + return ret; + } + } + } + disable_ctrlc(0); + + return 0; +} + +static void iomux_putc(int file, const char c) +{ + int i; + device_t *dev; + + for (i = 0; i < MAX_CONSARGS; i++) { + dev = console_devices[file][i]; + if (dev == NULL) + break; + if (dev->putc != NULL) + dev->putc(c); + } +} + +static void iomux_puts(int file, const char *s) +{ + int i; + device_t *dev; + + for (i = 0; i < MAX_CONSARGS; i++) { + dev = console_devices[file][i]; + if (dev == NULL) + break; + if (dev->puts != NULL) + dev->puts(s); + } +} +#endif /* defined(CONFIG_IO_MUX) */ + /** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
void serial_printf (const char *fmt, ...) @@ -115,7 +187,41 @@ void serial_printf (const char *fmt, ...) int fgetc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX) + { + int cntr = 0; + + /* + * Effectively poll for input wherever it may be available. + */ + for (;;) { + /* + * Upper layer may have already called tstc() so + * check for that first. + */ + if (tstcdev != NULL) + return iomux_getc(); + iomux_tstc(file); + /* + * Even this is too slow for serial cut&paste due + * to the overhead of calling tstc() then getc(). + * It gets worse if nc is set as a console because + * nc_tstc() is really slow. + * + * The idea behind this counter is to avoid calling + * the watchdog via udelay() too often. + * COUNT_TIL_UDELAY is defined in iomux.h and is just + * a guesstimate. + */ + if (cntr++ > COUNT_TIL_UDELAY) { + udelay(1); + cntr = 0; + } + } + } +#else return stdio_devices[file]->getc (); +#endif
return -1; } @@ -123,7 +229,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX) + return iomux_tstc(file); +#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +241,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX) + iomux_putc(file, c); +#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX) + iomux_puts(file, s); +#else stdio_devices[file]->puts (s); +#endif }
void fprintf (int file, const char *fmt, ...) @@ -407,6 +525,9 @@ int console_init_r (void) #ifdef CFG_CONSOLE_ENV_OVERWRITE int i; #endif /* CFG_CONSOLE_ENV_OVERWRITE */ +#ifdef CONFIG_IO_MUX + int iomux_err = 0; +#endif
/* set default handlers at first */ gd->jt[XF_getc] = serial_getc; @@ -425,6 +546,14 @@ int console_init_r (void) inputdev = search_device (DEV_FLAGS_INPUT, stdinname); outputdev = search_device (DEV_FLAGS_OUTPUT, stdoutname); errdev = search_device (DEV_FLAGS_OUTPUT, stderrname); +#ifdef CONFIG_IO_MUX + iomux_err = iomux_doenv(stdin, stdinname); + iomux_err += iomux_doenv(stdout, stdoutname); + iomux_err += iomux_doenv(stderr, stderrname); + if (!iomux_err) + /* Successful, so skip all the code below. */ + goto done; +#endif } /* if the devices are overwritten or not found, use default device */ if (inputdev == NULL) { @@ -439,15 +568,40 @@ int console_init_r (void) /* Initializes output console first */ if (outputdev != NULL) { console_setfile (stdout, outputdev); +#ifdef CONFIG_IO_MUX + /* need to set a console if not done above. */ + console_devices[stdout][0] = outputdev; +#endif } if (errdev != NULL) { console_setfile (stderr, errdev); +#ifdef CONFIG_IO_MUX + /* need to set a console if not done above. */ + console_devices[stderr][0] = errdev; +#endif } if (inputdev != NULL) { console_setfile (stdin, inputdev); +#ifdef CONFIG_IO_MUX + /* need to set a console if not done above. */ + console_devices[stdin][0] = inputdev; +#endif }
+#ifdef CONFIG_IO_MUX +#ifdef CONFIG_NETCONSOLE + /* + * Must do this very late in net/eth.c because a network device may + * be set as a console at boot. + */ + gd->flags |= 0; +#else + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif /* CONFIG_NETCONSOLE */ +done: +#else gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif /* CONFIG_IO_MUX */
#ifndef CFG_CONSOLE_INFO_QUIET /* Print information */ @@ -455,21 +609,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_IO_MUX + iomux_printdevs(stdin); +#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_IO_MUX + iomux_printdevs(stdout); +#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_IO_MUX + iomux_printdevs(stderr); +#else printf ("%s\n", stdio_devices[stderr]->name); +#endif } #endif /* CFG_CONSOLE_INFO_QUIET */
@@ -524,11 +690,18 @@ int console_init_r (void) if (outputdev != NULL) { console_setfile (stdout, outputdev); console_setfile (stderr, outputdev); +#ifdef CONFIG_IO_MUX + console_devices[stdout][0] = outputdev; + console_devices[stderr][0] = outputdev; +#endif }
/* Initializes input console */ if (inputdev != NULL) { console_setfile (stdin, inputdev); +#ifdef CONFIG_IO_MUX + console_devices[stdin][0] = inputdev; +#endif }
gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ diff --git a/common/iomux.c b/common/iomux.c new file mode 100644 index 0000000..d62f7e4 --- /dev/null +++ b/common/iomux.c @@ -0,0 +1,133 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +#include <common.h> +#include <serial.h> +#include <malloc.h> + +#ifdef CONFIG_IO_MUX +void iomux_printdevs(const int console) +{ + int i; + device_t *dev; + + for (i = 0; i < MAX_CONSARGS; i++) { + dev = console_devices[console][i]; + if (dev == NULL) + break; + serial_printf("%s ", dev->name); + } + serial_printf("\n"); +} + +int iomux_doenv(const int console, const char *arg) +{ + char *console_args, *temp, *start[MAX_CONSARGS]; + int i, j, io_flag, cs_idx; + device_t *dev; + device_t *cons_set[MAX_CONSARGS]; + +#ifdef CFG_CONSOLE_IS_IN_ENV + if (arg == NULL) + return 1; +#endif + + i = 0; + console_args = strdup(arg); + if (console_args == NULL) + return 1; + start[0] = console_args; + /* + * Check whether a comma separated list of devices was + * entered and count how many devices were entered. + * The array start[] has pointers to the beginning of + * each device name (up to MAX_CONSARGS devices). + */ + for (;;) { + temp = strchr(start[i], ','); + if (temp != NULL) { + i++; + *temp = '\0'; + if (i == MAX_CONSARGS) + break; + start[i] = temp + 1; + continue; + } + break; + } + if (i != MAX_CONSARGS) + i++; + cs_idx = 0; + memset((void *)cons_set, 0, CONSDEVS_LINE_SIZE); + + switch (console) { + case stdin: + io_flag = DEV_FLAGS_INPUT; + break; + case stdout: + case stderr: + io_flag = DEV_FLAGS_OUTPUT; + break; + default: + return 1; + } + + for (j = 0; j < i; j++) { + /* + * Check whether the device exists and is valid. + * console_assign() also calls search_device(), + * but I need the pointer to the device. + */ + dev = search_device(io_flag, start[j]); + if (dev == NULL) + continue; + /* + * Try assigning the specified device. + * This could screw up the console settings for apps. + */ + if (console_assign(console, start[j]) < 0) + continue; +#ifdef CONFIG_SERIAL_MULTI + /* + * This was taken from common/cmd_nvedit.c. + * This will never work because serial_assign() returns + * 1 upon error, not -1. + * This would almost always return an error anyway because + * serial_assign() expects the name of a serial device, like + * serial_smc, but the user generally only wants to set serial. + */ + if (serial_assign(start[j]) < 0) + continue; +#endif + cons_set[cs_idx++] = dev; + } + free(console_args); + /* failed to set any console */ + if (cs_idx == 0) + return 1; + else + memcpy(console_devices[console], cons_set, + CONSDEVS_LINE_SIZE);; + return 0; +} +#endif /* CONFIG_IO_MUX */ diff --git a/doc/README.iomux b/doc/README.iomux new file mode 100644 index 0000000..222843c --- /dev/null +++ b/doc/README.iomux @@ -0,0 +1,90 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH garyj@denx.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +U-Boot console multiplexing +=========================== + +HOW CONSOLE MULTIPLEXING WORKS +------------------------------ + +This functionality is controlled with CONFIG_IO_MUX in the board +configuration file. + +Two new files, common/iomux.c and include/iomux.h, contain the heart +of the environment setting implementation. + +The execution is then in common/cmd_nvedit.c and common/console.c. + +A user can use a comma-separated list of devices to set stdin, stdout +and stderr. For example: setenv stdin serial,nc. NOTE: No spaces +are allowed around the comma(s)! + +The length of the list is limited to MAX_CONSARGS entries, which is +defined in include/iomux.h and presently set to 6. + +It should be possible to specify any device which console_assign() +finds acceptable, but the code has only been tested with serial and +nc. + +The major change in common/console.c was to modify fgetc() to call +the iomux_tstc() routine in a for-loop. iomux_tstc() in turn calls +the tstc() routine for every registered device, but exits immediately +when one of them returns true. fgetc() then calls iomux_getc(), +which calls the correpsonding getc() routine. fgetc() hangs in +the for-loop until iomux_tstc() returns true and the input can be +retrieved. + +Thus, a user can type into any device registered for stdin. No effort +has been made to demulitplex simultaneous input from multiple stdin +devices. + +fputc() and fputs() have been modified to call iomux_putc() and +iomux_puts() respectively, which call the corresponding output +routines for every registered device. + +Thus, a user can see the ouput for any device registered for stdout +or stderr on all devices registered for stdout or stderr. As an +example, if stdin=serial,nc and stdout=serial,nc then all output +for serial, e.g. echos of input on serial, will appear on serial and nc. + +Just as with the old console code, this statement is still true: +If not defined in the environment, the first input device is assigned +to the 'stdin' file, the first output one to 'stdout' and 'stderr'. + +If CONFIG_CONSOLE_IS_IN_ENV is defined then multiple input/output +devices will be set at boot time. + +CAVEATS +------- + +Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list. + +The overhead associated with calling tstc() and then getc() means that +cut&paste will not work, even if serial is set as the only device for +stdin, stdout and stderr. + +Using nc as a stdin device results in even more overhead because nc_tstc() +is extremely slow. The slowdown is so extreme that it negatively impacts +timers such as the autoboot countdown. diff --git a/include/common.h b/include/common.h index 33c6e10..8cc7e3a 100644 --- a/include/common.h +++ b/include/common.h @@ -675,6 +675,13 @@ void fputc(int file, const char c); int ftstc(int file); int fgetc(int file);
+/* + * CONSOLE multiplexing. + */ +#ifdef CONFIG_IO_MUX +#include <iomux.h> +#endif + int pcmcia_init (void);
#ifdef CONFIG_STATUS_LED diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..1310a75 --- /dev/null +++ b/include/iomux.h @@ -0,0 +1,48 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * * 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. + * + * 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 + */ + +#ifndef _IO_MUX_H +#define _IO_MUX_H + +#include <devices.h> + +/* + * Stuff required to support console multiplexing. + */ +/* Only allow this many devices in the comma separated list of consoles */ +#define MAX_CONSARGS 6 + +/* + * Avoid invoking WATCHDOG via udelayy() on every pass through the loop + * in fgetc(). Note this is all based on guesswork. + */ +#define COUNT_TIL_UDELAY 10000 + +/* + * Pointers to devices used for each file type. Defined in console.c. + */ +extern device_t *console_devices[MAX_FILES][MAX_CONSARGS]; +#define CONSDEVS_LINE_SIZE (sizeof(device_t *) * MAX_CONSARGS) + +int iomux_doenv(const int, const char *); +void iomux_printdevs(const int); +device_t *search_device(int, char *); + +#endif /* _IO_MUX_H */
Dear Gary Jennejohn,
In message 20080914164107.32a9fce1@peedub.jennejohn.org you wrote:
See doc/README.iomux for a general description of what this does.
Sorry, but this is not really a good commit message. Please explain at least in a shoprt summary what the patch is supposed to implement.
This is the first of two commits. The second commit touches net/eth.c and has to go through the custodian, so I split it out for simplicity.
Comments like this that are not suppoesed to be come part of the commit message should go below the '---' line.
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 637d6c9..6fc9313 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -213,6 +213,11 @@ int _do_setenv (int flag, int argc, char *argv[]) return 1; }
+#ifdef CONFIG_IO_MUX
i = iomux_doenv(console, argv[2]);if (i)return i;+#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_IO_MUX */ }
/*diff --git a/common/console.c b/common/console.c index 56d9118..6158af9 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,78 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
+/*
- This depends on tstc() always being called before getc().
- No attempt is made to demultiplex multiple input sources.
- */
+static int iomux_getc(void) +{
- unsigned char ret;
- ret = tstcdev->getc();
- tstcdev = NULL;
- return ret;
+}
+static int iomux_tstc(int file) +{
- int i, ret;
- device_t *dev;
- disable_ctrlc(1);
- for (i = 0; i < MAX_CONSARGS; i++) {
dev = console_devices[file][i];if (dev == NULL)break;if (dev->tstc != NULL) {ret = dev->tstc();if (ret > 0) {tstcdev = dev;disable_ctrlc(0);return ret;}}- }
- disable_ctrlc(0);
- return 0;
+}
+static void iomux_putc(int file, const char c) +{
- int i;
- device_t *dev;
- for (i = 0; i < MAX_CONSARGS; i++) {
dev = console_devices[file][i];if (dev == NULL)break;if (dev->putc != NULL)dev->putc(c);- }
+}
+static void iomux_puts(int file, const char *s) +{
- int i;
- device_t *dev;
- for (i = 0; i < MAX_CONSARGS; i++) {
dev = console_devices[file][i];if (dev == NULL)break;if (dev->puts != NULL)dev->puts(s);- }
+} +#endif /* defined(CONFIG_IO_MUX) */
/** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
void serial_printf (const char *fmt, ...) @@ -115,7 +187,41 @@ void serial_printf (const char *fmt, ...) int fgetc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX)
- {
int cntr = 0;/** Effectively poll for input wherever it may be available.*/for (;;) {/** Upper layer may have already called tstc() so* check for that first.*/if (tstcdev != NULL)return iomux_getc();iomux_tstc(file);/** Even this is too slow for serial cut&paste due* to the overhead of calling tstc() then getc().* It gets worse if nc is set as a console because* nc_tstc() is really slow.** The idea behind this counter is to avoid calling* the watchdog via udelay() too often.* COUNT_TIL_UDELAY is defined in iomux.h and is just* a guesstimate.*/if (cntr++ > COUNT_TIL_UDELAY) {udelay(1);cntr = 0;}}- }
+#else return stdio_devices[file]->getc (); +#endif
return -1; } @@ -123,7 +229,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX)
return iomux_tstc(file);+#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +241,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX)
iomux_putc(file, c);+#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_IO_MUX)
iomux_puts(file, s);+#else stdio_devices[file]->puts (s); +#endif }
void fprintf (int file, const char *fmt, ...) @@ -407,6 +525,9 @@ int console_init_r (void) #ifdef CFG_CONSOLE_ENV_OVERWRITE int i; #endif /* CFG_CONSOLE_ENV_OVERWRITE */ +#ifdef CONFIG_IO_MUX
- int iomux_err = 0;
+#endif
/* set default handlers at first */ gd->jt[XF_getc] = serial_getc; @@ -425,6 +546,14 @@ int console_init_r (void) inputdev = search_device (DEV_FLAGS_INPUT, stdinname); outputdev = search_device (DEV_FLAGS_OUTPUT, stdoutname); errdev = search_device (DEV_FLAGS_OUTPUT, stderrname); +#ifdef CONFIG_IO_MUX
iomux_err = iomux_doenv(stdin, stdinname);iomux_err += iomux_doenv(stdout, stdoutname);iomux_err += iomux_doenv(stderr, stderrname);if (!iomux_err)/* Successful, so skip all the code below. */goto done;+#endif } /* if the devices are overwritten or not found, use default device */ if (inputdev == NULL) { @@ -439,15 +568,40 @@ int console_init_r (void) /* Initializes output console first */ if (outputdev != NULL) { console_setfile (stdout, outputdev); +#ifdef CONFIG_IO_MUX
/* need to set a console if not done above. */console_devices[stdout][0] = outputdev;+#endif
Could you please explain the "if not done above" part? I don't see where this would be done or not be done, i. e. don;t we always have to do this here?
} if (errdev != NULL) { console_setfile (stderr, errdev); +#ifdef CONFIG_IO_MUX
/* need to set a console if not done above. */console_devices[stderr][0] = errdev;+#endif } if (inputdev != NULL) { console_setfile (stdin, inputdev); +#ifdef CONFIG_IO_MUX
/* need to set a console if not done above. */console_devices[stdin][0] = inputdev;+#endif }
+#ifdef CONFIG_IO_MUX +#ifdef CONFIG_NETCONSOLE
- /*
* Must do this very late in net/eth.c because a network device may* be set as a console at boot.*/- gd->flags |= 0;
+#else
- gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
+#endif /* CONFIG_NETCONSOLE */ +done: +#else gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif /* CONFIG_IO_MUX */
Wouldn't this be easier to read if written as:
#if !(defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ #endif done: ;
?
But then, I don't see why the netconsole initialization depends in any way on the I/O mux feature - I'd expect that the behaviour is the same with or without that.
#ifndef CFG_CONSOLE_INFO_QUIET /* Print information */ @@ -455,21 +609,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_IO_MUX
iomux_printdevs(stdin);+#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_IO_MUX
iomux_printdevs(stdout);+#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_IO_MUX
iomux_printdevs(stderr);+#else printf ("%s\n", stdio_devices[stderr]->name); +#endif
Instead of repeating this sequence 3 times, we should probably make it a function?
diff --git a/common/iomux.c b/common/iomux.c new file mode 100644 index 0000000..d62f7e4 --- /dev/null +++ b/common/iomux.c @@ -0,0 +1,133 @@ +/*
- (C) Copyright 2008
- Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de.
- See file CREDITS for list of people who contributed to this
- project.
- This program 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.
- 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.
- 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
- */
+#include <common.h> +#include <serial.h> +#include <malloc.h>
+#ifdef CONFIG_IO_MUX +void iomux_printdevs(const int console) +{
- int i;
- device_t *dev;
- for (i = 0; i < MAX_CONSARGS; i++) {
dev = console_devices[console][i];if (dev == NULL)break;serial_printf("%s ", dev->name);- }
- serial_printf("\n");
+}
+int iomux_doenv(const int console, const char *arg) +{
- char *console_args, *temp, *start[MAX_CONSARGS];
- int i, j, io_flag, cs_idx;
- device_t *dev;
- device_t *cons_set[MAX_CONSARGS];
+#ifdef CFG_CONSOLE_IS_IN_ENV
- if (arg == NULL)
return 1;+#endif
- i = 0;
- console_args = strdup(arg);
- if (console_args == NULL)
return 1;- start[0] = console_args;
- /*
* Check whether a comma separated list of devices was* entered and count how many devices were entered.* The array start[] has pointers to the beginning of* each device name (up to MAX_CONSARGS devices).*/- for (;;) {
temp = strchr(start[i], ',');if (temp != NULL) {i++;*temp = '\0';if (i == MAX_CONSARGS)break;start[i] = temp + 1;continue;}break;- }
- if (i != MAX_CONSARGS)
i++;
One could rewrite the whole part as
start[0] = temp = strdup(arg); if (temp == NULL) return 1; for (i=0; i<MAX_CONSARGS-1; ) { temp = strchr(start[i], ','); if (temp == NULL) break; *temp = '\0'; start[++i] = ++temp; }
What do you think?
- cs_idx = 0;
- memset((void *)cons_set, 0, CONSDEVS_LINE_SIZE);
- switch (console) {
- case stdin:
io_flag = DEV_FLAGS_INPUT;break;- case stdout:
- case stderr:
io_flag = DEV_FLAGS_OUTPUT;break;- default:
"free(console_args);" missing here.
return 1;- }
- for (j = 0; j < i; j++) {
/** Check whether the device exists and is valid.* console_assign() also calls search_device(),* but I need the pointer to the device.*/dev = search_device(io_flag, start[j]);if (dev == NULL)continue;/** Try assigning the specified device.* This could screw up the console settings for apps.*/if (console_assign(console, start[j]) < 0)continue;+#ifdef CONFIG_SERIAL_MULTI
/** This was taken from common/cmd_nvedit.c.* This will never work because serial_assign() returns* 1 upon error, not -1.
Ummm... if you know this doesn't work, why don't you fix it and make it work?
* This would almost always return an error anyway because* serial_assign() expects the name of a serial device, like* serial_smc, but the user generally only wants to set serial.
With CONFIG_SERIAL_MULTI enabled, the user probably does want to select specifically between several serial ports - that's the whole idea of defining CONFIG_SERIAL_MULTI, isn't it?
*/if (serial_assign(start[j]) < 0)continue;+#endif
cons_set[cs_idx++] = dev;- }
- free(console_args);
- /* failed to set any console */
- if (cs_idx == 0)
return 1;- else
No "else" needed here; this saves a level of indentation.
+CAVEATS +-------
+Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list.
+The overhead associated with calling tstc() and then getc() means that +cut&paste will not work, even if serial is set as the only device for +stdin, stdout and stderr.
You mentioned that you tested this on MPC8xx systems. for these, the tstc() implementation (smc_tstc() resp. scc_tstc() in "cpu/mpc8xx/serial.c") essentially consists of 3 lines of code and boils down to testing a bit; there are no delays or so. Then why does the new code become so slow? The overhead of calling tstc() alone can not be the only reason?
+Using nc as a stdin device results in even more overhead because nc_tstc() +is extremely slow. The slowdown is so extreme that it negatively impacts +timers such as the autoboot countdown.
That means the code is unusable as is and needs to be fixed first?
Please repost after fixing.
Best regards,
Wolfgang Denk
return i;+#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_IO_MUX */ }
/*diff --git a/common/console.c b/common/console.c index 56d9118..6158af9 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,78 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
please use a list_head instead
+/*
- This depends on tstc() always being called before getc().
- No attempt is made to demultiplex multiple input sources.
- */
+static int iomux_getc(void) +{
- unsigned char ret;
- ret = tstcdev->getc();
- tstcdev = NULL;
- return ret;
+}
#ifdef CONFIG_STATUS_LED
diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..1310a75 --- /dev/null +++ b/include/iomux.h @@ -0,0 +1,48 @@ +/*
- (C) Copyright 2008
- Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de.
- See file CREDITS for list of people who contributed to this
- project.
- This program 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.
* 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.
please fix
- 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
- */
+#ifndef _IO_MUX_H +#define _IO_MUX_H
+#include <devices.h>
+/*
- Stuff required to support console multiplexing.
- */
+/* Only allow this many devices in the comma separated list of consoles */ +#define MAX_CONSARGS 6
No more need if you use list_head
+/*
- Avoid invoking WATCHDOG via udelayy() on every pass through the loop
- in fgetc(). Note this is all based on guesswork.
- */
+#define COUNT_TIL_UDELAY 10000
+/*
- Pointers to devices used for each file type. Defined in console.c.
- */
+extern device_t *console_devices[MAX_FILES][MAX_CONSARGS];
ditto
+#define CONSDEVS_LINE_SIZE (sizeof(device_t *) * MAX_CONSARGS)
ditto
Best Regards, J.
On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
return i;+#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_IO_MUX */ }
/*diff --git a/common/console.c b/common/console.c index 56d9118..6158af9 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,78 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
please use a list_head instead
This is an excellent suggestion. Thanks!
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************
Dear Gary,
In message 20080915110101.6b5e0a5b@peedub.jennejohn.org you wrote:
On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
please use a list_head instead
This is an excellent suggestion. Thanks!
I understood that you would repost a new, modified patch which uses this suggestion, but I haven't seen any repost sice.
You know that the merge window is open right now - do you plan to repost this stuff any time soon?
Best regards,
Wolfgang Denk
On Fri, 24 Oct 2008 13:18:42 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Wolfgang,
In message 20080915110101.6b5e0a5b@peedub.jennejohn.org you wrote:
On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
please use a list_head instead
This is an excellent suggestion. Thanks!
I understood that you would repost a new, modified patch which uses this suggestion, but I haven't seen any repost sice.
You know that the merge window is open right now - do you plan to repost this stuff any time soon?
I actually solved the problem by using realloc() rather than using a list_head.
I've merged the console multiplexing stuff into HEAD but hesitate to post it because my previous patch (setting GD_FLG_DEVINIT) is still unresolved.
But if the window is now open I'll post the new patch without resolution of the other one. However, this approach may well lead to an unusable U-Boot if the user wants to set nc as a console in the environment.
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************
On 14:25 Fri 24 Oct , Gary Jennejohn wrote:
On Fri, 24 Oct 2008 13:18:42 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Wolfgang,
In message 20080915110101.6b5e0a5b@peedub.jennejohn.org you wrote:
On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
+#if defined(CONFIG_IO_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t *console_devices[MAX_FILES][MAX_CONSARGS];
please use a list_head instead
This is an excellent suggestion. Thanks!
I understood that you would repost a new, modified patch which uses this suggestion, but I haven't seen any repost sice.
You know that the merge window is open right now - do you plan to repost this stuff any time soon?
I actually solved the problem by using realloc() rather than using a list_head.
I've merged the console multiplexing stuff into HEAD but hesitate to post it because my previous patch (setting GD_FLG_DEVINIT) is still unresolved.
But if the window is now open I'll post the new patch without resolution of the other one. However, this approach may well lead to an unusable U-Boot if the user wants to set nc as a console in the environment.
I think it's really important to use list_head in order to have no limitation and more important not increse the u-boot.bin size.
Best Regards, J.
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
This allows a user to specify multiple console devices in the environment with a command like this: setenv stdin serial,nc. As a result, the user can enter text on both the serial and netconsole interfaces.
All devices - stdin, stdout and stderr - can be set in this manner.
1) common/iomux.c and include/iomux.h contain the environment setting implementation. 2) doc/README.iomux contains a somewhat more detailed description. 3) The implementation in (1) is called from common/cmd_nvedit.c to handle setenv and from common/console.c to handle initialization of input/output devices at boot time. 4) common/console.c also contains the code needed to poll multiple console devices for input and send output to all devices registered for output. 5) include/common.h includes iomux.h and common/Makefile generates iomux.o when CONFIG_SYS_CONSOLE_MUX is set.
Signed-off-by: Gary Jennejohn garyj@denx.de --- common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++ common/iomux.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 89 +++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 54 ++++++++++++++++ 7 files changed, 489 insertions(+), 0 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
diff --git a/common/Makefile b/common/Makefile index f00cbd9..0296a8a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -156,6 +156,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_UPDATE_TFTP) += update.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_SYS_CONSOLE_MUX) += iomux.o
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c) diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index d280cb0..7a49296 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -213,6 +213,11 @@ int _do_setenv (int flag, int argc, char *argv[]) return 1; }
+#ifdef CONFIG_SYS_CONSOLE_MUX + i = iomux_doenv(console, argv[2]); + if (i) + return i; +#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_SYS_CONSOLE_MUX */ }
/* diff --git a/common/console.c b/common/console.c index 6f0846f..c89020d 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,73 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_SYS_CONSOLE_MUX) +/** Console I/O multiplexing *******************************************/ + +static device_t *tstcdev; +device_t **console_devices[MAX_FILES]; +int cd_count[MAX_FILES]; + +/* + * This depends on tstc() always being called before getc(). + * No attempt is made to demultiplex multiple input sources. + */ +static int iomux_getc(void) +{ + unsigned char ret; + + ret = tstcdev->getc(); + tstcdev = NULL; + return ret; +} + +static int iomux_tstc(int file) +{ + int i, ret; + device_t *dev; + + disable_ctrlc(1); + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->tstc != NULL) { + ret = dev->tstc(); + if (ret > 0) { + tstcdev = dev; + disable_ctrlc(0); + return ret; + } + } + } + disable_ctrlc(0); + + return 0; +} + +static void iomux_putc(int file, const char c) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->putc != NULL) + dev->putc(c); + } +} + +static void iomux_puts(int file, const char *s) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->puts != NULL) + dev->puts(s); + } +} +#endif /* defined(CONFIG_SYS_CONSOLE_MUX) */ + /** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
void serial_printf (const char *fmt, ...) @@ -115,7 +182,41 @@ void serial_printf (const char *fmt, ...) int fgetc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX) + { + int cntr = 0; + + /* + * Effectively poll for input wherever it may be available. + */ + for (;;) { + /* + * Upper layer may have already called tstc() so + * check for that first. + */ + if (tstcdev != NULL) + return iomux_getc(); + iomux_tstc(file); + /* + * Even this is too slow for serial cut&paste due + * to the overhead of calling tstc() then getc(). + * It gets worse if nc is set as a console because + * nc_tstc() is rather slow. + * + * The idea behind this counter is to avoid calling + * the watchdog via udelay() too often. + * COUNT_TIL_UDELAY is defined in iomux.h and is just + * a guesstimate. + */ + if (cntr++ > COUNT_TIL_UDELAY) { + udelay(1); + cntr = 0; + } + } + } +#else return stdio_devices[file]->getc (); +#endif
return -1; } @@ -123,7 +224,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX) + return iomux_tstc(file); +#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +236,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX) + iomux_putc(file, c); +#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX) + iomux_puts(file, s); +#else stdio_devices[file]->puts (s); +#endif }
void fprintf (int file, const char *fmt, ...) @@ -407,6 +520,9 @@ int console_init_r (void) #ifdef CONFIG_SYS_CONSOLE_ENV_OVERWRITE int i; #endif /* CONFIG_SYS_CONSOLE_ENV_OVERWRITE */ +#ifdef CONFIG_SYS_CONSOLE_MUX + int iomux_err = 0; +#endif
/* set default handlers at first */ gd->jt[XF_getc] = serial_getc; @@ -425,6 +541,14 @@ int console_init_r (void) inputdev = search_device (DEV_FLAGS_INPUT, stdinname); outputdev = search_device (DEV_FLAGS_OUTPUT, stdoutname); errdev = search_device (DEV_FLAGS_OUTPUT, stderrname); +#ifdef CONFIG_SYS_CONSOLE_MUX + iomux_err = iomux_doenv(stdin, stdinname); + iomux_err += iomux_doenv(stdout, stdoutname); + iomux_err += iomux_doenv(stderr, stderrname); + if (!iomux_err) + /* Successful, so skip all the code below. */ + goto done; +#endif } /* if the devices are overwritten or not found, use default device */ if (inputdev == NULL) { @@ -438,15 +562,34 @@ int console_init_r (void) } /* Initializes output console first */ if (outputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stdout, outputdev->name); +#else console_setfile (stdout, outputdev); +#endif } if (errdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stderr, errdev->name); +#else console_setfile (stderr, errdev); +#endif } if (inputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stdin, inputdev->name); +#else console_setfile (stdin, inputdev); +#endif }
+#ifdef CONFIG_SYS_CONSOLE_MUX +done: +#endif + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET @@ -455,21 +598,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX + iomux_printdevs(stdin); +#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX + iomux_printdevs(stdout); +#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX + iomux_printdevs(stderr); +#else printf ("%s\n", stdio_devices[stderr]->name); +#endif } #endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
@@ -524,11 +679,18 @@ int console_init_r (void) if (outputdev != NULL) { console_setfile (stdout, outputdev); console_setfile (stderr, outputdev); +#ifdef CONFIG_SYS_CONSOLE_MUX + console_devices[stdout][0] = outputdev; + console_devices[stderr][0] = outputdev; +#endif }
/* Initializes input console */ if (inputdev != NULL) { console_setfile (stdin, inputdev); +#ifdef CONFIG_SYS_CONSOLE_MUX + console_devices[stdin][0] = inputdev; +#endif }
gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ diff --git a/common/iomux.c b/common/iomux.c new file mode 100644 index 0000000..b83bdb5 --- /dev/null +++ b/common/iomux.c @@ -0,0 +1,170 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +#include <common.h> +#include <serial.h> +#include <malloc.h> + +#ifdef CONFIG_SYS_CONSOLE_MUX +void iomux_printdevs(const int console) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[console]; i++) { + dev = console_devices[console][i]; + serial_printf("%s ", dev->name); + } + serial_printf("\n"); +} + +/* This tries to preserve the old list if an error occurs. */ +int iomux_doenv(const int console, const char *arg) +{ + char *console_args, *temp, **start; + int i, j, io_flag, cs_idx; + device_t *dev; + device_t **cons_set; + +#ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV + if (arg == NULL) + return 1; +#endif + + console_args = strdup(arg); + if (console_args == NULL) + return 1; + /* + * Check whether a comma separated list of devices was + * entered and count how many devices were entered. + * The array start[] has pointers to the beginning of + * each device name (up to MAX_CONSARGS devices). + * + * Have to do this twice - once to count the number of + * commas and then again to populate start. + */ + i = 0; + temp = console_args; + for (;;) { + temp = strchr(temp, ','); + if (temp != NULL) { + i++; + temp++; + continue; + } + /* There's always one entry more than the number of commas. */ + i++; + break; + } + start = (char **)malloc(i * sizeof(char *)); + if (start == NULL) { + free(console_args); + return 1; + } + i = 0; + start[0] = console_args; + for (;;) { + temp = strchr(start[i], ','); + if (temp != NULL) { + i++; + *temp = '\0'; + start[i] = temp + 1; + continue; + } + /* There's always one entry more than the number of commas. */ + i++; + break; + } + cons_set = (device_t **)calloc(1, i * sizeof(device_t *)); + if (cons_set == NULL) { + free(start); + free(console_args); + return 1; + } + + switch (console) { + case stdin: + io_flag = DEV_FLAGS_INPUT; + break; + case stdout: + case stderr: + io_flag = DEV_FLAGS_OUTPUT; + break; + default: + return 1; + } + + cs_idx = 0; + for (j = 0; j < i; j++) { + /* + * Check whether the device exists and is valid. + * console_assign() also calls search_device(), + * but I need the pointer to the device. + */ + dev = search_device(io_flag, start[j]); + if (dev == NULL) + continue; + /* + * Try assigning the specified device. + * This could screw up the console settings for apps. + */ + if (console_assign(console, start[j]) < 0) + continue; +#ifdef CONFIG_SERIAL_MULTI + /* + * This was taken from common/cmd_nvedit.c. + * This will never work because serial_assign() returns + * 1 upon error, not -1. + * This would almost always return an error anyway because + * serial_assign() expects the name of a serial device, like + * serial_smc, but the user generally only wants to set serial. + */ + if (serial_assign(start[j]) < 0) + continue; +#endif + cons_set[cs_idx++] = dev; + } + free(console_args); + free(start); + /* failed to set any console */ + if (cs_idx == 0) { + free(cons_set); + return 1; + } else { + /* Works even if console_devices[console] is NULL. */ + console_devices[console] = + (device_t **)realloc(console_devices[console], + cs_idx * sizeof(device_t *)); + if (console_devices[console] == NULL) { + free(cons_set); + return 1; + } + memcpy(console_devices[console], cons_set, cs_idx * + sizeof(device_t *)); + + cd_count[console] = cs_idx; + } + free(cons_set); + return 0; +} +#endif /* CONFIG_SYS_CONSOLE_MUX */ diff --git a/doc/README.iomux b/doc/README.iomux new file mode 100644 index 0000000..bff75fa --- /dev/null +++ b/doc/README.iomux @@ -0,0 +1,89 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH garyj@denx.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +U-Boot console multiplexing +=========================== + +HOW CONSOLE MULTIPLEXING WORKS +------------------------------ + +This functionality is controlled with CONFIG_SYS_CONSOLE_MUX in the board +configuration file. + +Two new files, common/iomux.c and include/iomux.h, contain the heart +of the environment setting implementation. + +The execution is then in common/cmd_nvedit.c and common/console.c. + +A user can use a comma-separated list of devices to set stdin, stdout +and stderr. For example: setenv stdin serial,nc. NOTE: No spaces +are allowed around the comma(s)! + +The length of the list is limited by malloc(), since the array used +is allocated and freed dynamically. + +It should be possible to specify any device which console_assign() +finds acceptable, but the code has only been tested with serial and +nc. + +The major change in common/console.c was to modify fgetc() to call +the iomux_tstc() routine in a for-loop. iomux_tstc() in turn calls +the tstc() routine for every registered device, but exits immediately +when one of them returns true. fgetc() then calls iomux_getc(), +which calls the correpsonding getc() routine. fgetc() hangs in +the for-loop until iomux_tstc() returns true and the input can be +retrieved. + +Thus, a user can type into any device registered for stdin. No effort +has been made to demulitplex simultaneous input from multiple stdin +devices. + +fputc() and fputs() have been modified to call iomux_putc() and +iomux_puts() respectively, which call the corresponding output +routines for every registered device. + +Thus, a user can see the ouput for any device registered for stdout +or stderr on all devices registered for stdout or stderr. As an +example, if stdin=serial,nc and stdout=serial,nc then all output +for serial, e.g. echos of input on serial, will appear on serial and nc. + +Just as with the old console code, this statement is still true: +If not defined in the environment, the first input device is assigned +to the 'stdin' file, the first output one to 'stdout' and 'stderr'. + +If CONFIG_SYS_CONSOLE_IS_IN_ENV is defined then multiple input/output +devices can be set at boot time if defined in the environment. + +CAVEATS +------- + +Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list. + +The overhead associated with calling tstc() and then getc() means that +copy&paste will normally not work, even if serial is set as the only device +for stdin, stdout and stderr. + +Using nc as a stdin device results in even more overhead because nc_tstc() +is quite slow. diff --git a/include/common.h b/include/common.h index b8a654a..53cc297 100644 --- a/include/common.h +++ b/include/common.h @@ -675,6 +675,13 @@ void fputc(int file, const char c); int ftstc(int file); int fgetc(int file);
+/* + * CONSOLE multiplexing. + */ +#ifdef CONFIG_SYS_CONSOLE_MUX +#include <iomux.h> +#endif + int pcmcia_init (void);
#ifdef CONFIG_STATUS_LED diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..75b66e2 --- /dev/null +++ b/include/iomux.h @@ -0,0 +1,54 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + *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. + * + * 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 + */ + +#ifndef _IO_MUX_H +#define _IO_MUX_H + +#include <devices.h> + +/* + * Stuff required to support console multiplexing. + */ + +/* + * Avoid invoking WATCHDOG via udelay() on every pass through the loop + * in fgetc(). Note this is all based on guesswork. + */ +#define COUNT_TIL_UDELAY 10000 + +/* + * Pointers to devices used for each file type. Defined in console.c + * but storage is allocated in iomux.c. + */ +extern device_t **console_devices[MAX_FILES]; +/* + * The count of devices assigned to each FILE. Defined in console.c + * and populated in iomux.c. + */ +extern int cd_count[MAX_FILES]; + +int iomux_doenv(const int, const char *); +void iomux_printdevs(const int); +device_t *search_device(int, char *); + +#endif /* _IO_MUX_H */
Dear Gary Jennejohn,
In message 20081030112119.59036e03@ernst.jennejohn.org you wrote:
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
Is it correct to assume that this patch version does not yet include any changes resultiung from the discussion we had yesterday how to optimize timing behaviour?
diff --git a/common/Makefile b/common/Makefile index f00cbd9..0296a8a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -156,6 +156,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_UPDATE_TFTP) += update.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_SYS_CONSOLE_MUX) += iomux.o
As this is a user configurable option, it should be named CONFIG_CONSOLE_MUX. Please remove the "SYS_" part. And please keep the list sorted.
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index d280cb0..7a49296 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -213,6 +213,11 @@ int _do_setenv (int flag, int argc, char *argv[]) return 1; }
+#ifdef CONFIG_SYS_CONSOLE_MUX
i = iomux_doenv(console, argv[2]);if (i)return i;+#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_SYS_CONSOLE_MUX */ }
Just to be sure - does your new iomux_doenv() code handle the situation that is usually handled by serial_assign(), too (if CONFIG_SERIAL_MULTI is set) ?
diff --git a/common/console.c b/common/console.c index 6f0846f..c89020d 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,73 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_SYS_CONSOLE_MUX) +/** Console I/O multiplexing *******************************************/
+static device_t *tstcdev; +device_t **console_devices[MAX_FILES]; +int cd_count[MAX_FILES];
+/*
- This depends on tstc() always being called before getc().
I don't think this is a valid assumption.
- No attempt is made to demultiplex multiple input sources.
- */
+static int iomux_getc(void) +{
- unsigned char ret;
- ret = tstcdev->getc();
- tstcdev = NULL;
- return ret;
+}
If iomux_getc() gets called twice we crash U-Boot because of a NULL pointer dereference?
+static int iomux_tstc(int file) +{
- int i, ret;
- device_t *dev;
- disable_ctrlc(1);
- for (i = 0; i < cd_count[file]; i++) {
dev = console_devices[file][i];if (dev->tstc != NULL) {ret = dev->tstc();if (ret > 0) {tstcdev = dev;disable_ctrlc(0);return ret;}}- }
- disable_ctrlc(0);
- return 0;
+}
If I'm reading this correctly, then there is not even a guarantee that tstcdev gets set in this function, so the protection against a NULL pointer in iomux_getc() seems even more vital to me ?
@@ -115,7 +182,41 @@ void serial_printf (const char *fmt, ...) int fgetc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
- {
Please move the braces outside the #ifdef.
int cntr = 0;/** Effectively poll for input wherever it may be available.*/for (;;) {/** Upper layer may have already called tstc() so* check for that first.*/if (tstcdev != NULL)return iomux_getc();iomux_tstc(file);/** Even this is too slow for serial cut&paste due* to the overhead of calling tstc() then getc().* It gets worse if nc is set as a console because* nc_tstc() is rather slow.
We discussed a potential solution to this problem yesterday. Do you want to try this out in your implementation?
* The idea behind this counter is to avoid calling* the watchdog via udelay() too often.* COUNT_TIL_UDELAY is defined in iomux.h and is just* a guesstimate.*/if (cntr++ > COUNT_TIL_UDELAY) {udelay(1);cntr = 0;}
Please do not re-invent the wheel. If the watchdog triggering rate needs to be limited then please use the CONFIG_WD_MAX_RATE parameter as it was done in commit d32a874b "lwmon5 watchdog: limit trigger rate".
@@ -123,7 +224,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
return iomux_tstc(file);+#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +236,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
iomux_putc(file, c);+#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
iomux_puts(file, s);+#else stdio_devices[file]->puts (s); +#endif }
Would it be possible to just set
stdio_devices[file]->tstc = iomux_tstc; stdio_devices[file]->putc = iomux_putc; stdio_devices[file]->puts = iomux_puts;
?
This would eliminate the need for these #ifdef's (and possibly a few more) ?
@@ -438,15 +562,34 @@ int console_init_r (void) } /* Initializes output console first */ if (outputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stdout, outputdev->name);+#else console_setfile (stdout, outputdev); +#endif } if (errdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stderr, errdev->name);+#else console_setfile (stderr, errdev); +#endif } if (inputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stdin, inputdev->name);+#else console_setfile (stdin, inputdev); +#endif }
See above. Maybe these #ifdef's could be eliminated as well?
@@ -455,21 +598,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stdin);+#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stdout);+#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stderr);+#else printf ("%s\n", stdio_devices[stderr]->name); +#endif }
And these, too?
diff --git a/common/iomux.c b/common/iomux.c new file mode 100644 index 0000000..b83bdb5 --- /dev/null +++ b/common/iomux.c @@ -0,0 +1,170 @@ +/*
- (C) Copyright 2008
- Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de.
- See file CREDITS for list of people who contributed to this
- project.
- This program 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.
- 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.
- 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
- */
+#include <common.h> +#include <serial.h> +#include <malloc.h>
+#ifdef CONFIG_SYS_CONSOLE_MUX +void iomux_printdevs(const int console) +{
- int i;
- device_t *dev;
- for (i = 0; i < cd_count[console]; i++) {
dev = console_devices[console][i];serial_printf("%s ", dev->name);- }
- serial_printf("\n");
+}
The old code uses plain printf(), while your new version explicitely calls serial_printf(). I gues sthis is intentional? What is the rationale behind it?
+/* This tries to preserve the old list if an error occurs. */ +int iomux_doenv(const int console, const char *arg) +{
- char *console_args, *temp, **start;
- int i, j, io_flag, cs_idx;
- device_t *dev;
- device_t **cons_set;
+#ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
- if (arg == NULL)
return 1;+#endif
Do we need this special case test?
- console_args = strdup(arg);
- if (console_args == NULL)
return 1;
I think this test here would catch it as well?
- /*
* Check whether a comma separated list of devices was* entered and count how many devices were entered.* The array start[] has pointers to the beginning of* each device name (up to MAX_CONSARGS devices).** Have to do this twice - once to count the number of* commas and then again to populate start.*/- i = 0;
- temp = console_args;
- for (;;) {
temp = strchr(temp, ',');if (temp != NULL) {i++;temp++;continue;}/* There's always one entry more than the number of commas. */i++;break;- }
- start = (char **)malloc(i * sizeof(char *));
- if (start == NULL) {
free(console_args);return 1;- }
- i = 0;
- start[0] = console_args;
- for (;;) {
temp = strchr(start[i], ',');if (temp != NULL) {i++;*temp = '\0';start[i] = temp + 1;continue;}/* There's always one entry more than the number of commas. */i++;break;- }
I suggest to swap logic around as this allows for easier to read code:
for (;;) { temp = strchr(start[i++], ','); if (temp == NULL) break; *temp = '\0'; start[i] = temp + 1; }
- cons_set = (device_t **)calloc(1, i * sizeof(device_t *));
Why not:
cons_set = (device_t **)calloc(i, sizeof(device_t *));
?
- if (cons_set == NULL) {
free(start);free(console_args);return 1;- }
- switch (console) {
- case stdin:
io_flag = DEV_FLAGS_INPUT;break;- case stdout:
- case stderr:
io_flag = DEV_FLAGS_OUTPUT;break;- default:
return 1;
Memory leak? free start & console_args & cons_set ??
diff --git a/doc/README.iomux b/doc/README.iomux new file mode 100644 index 0000000..bff75fa --- /dev/null +++ b/doc/README.iomux @@ -0,0 +1,89 @@ +/*
- (C) Copyright 2008
- Gary Jennejohn, DENX Software Engineering GmbH garyj@denx.de
- See file CREDITS for list of people who contributed to this
- project.
- This program 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.
- 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.
- 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
- */
+U-Boot console multiplexing +===========================
+HOW CONSOLE MULTIPLEXING WORKS +------------------------------
+This functionality is controlled with CONFIG_SYS_CONSOLE_MUX in the board +configuration file.
+Two new files, common/iomux.c and include/iomux.h, contain the heart +of the environment setting implementation.
It seems to me that they do much more than just setting environment variables?
+The execution is then in common/cmd_nvedit.c and common/console.c.
Hm... but cmd_nvedit.c really is for environment variables handling only?
+A user can use a comma-separated list of devices to set stdin, stdout +and stderr. For example: setenv stdin serial,nc. NOTE: No spaces
Maybe it's better to quote the example to make it stand out:
...and stderr. For example: "setenv stdin serial,nc". NOTE: No spaces
?
+Thus, a user can see the ouput for any device registered for stdout +or stderr on all devices registered for stdout or stderr. As an +example, if stdin=serial,nc and stdout=serial,nc then all output
Quote examples ?
+CAVEATS +-------
+Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list.
+The overhead associated with calling tstc() and then getc() means that +copy&paste will normally not work, even if serial is set as the only device +for stdin, stdout and stderr.
You should probably mention the conditions where you tested this with such results. It probably depends on the system performance and the console baud rate, doesn't it?
+Using nc as a stdin device results in even more overhead because nc_tstc() +is quite slow.
There was discussion before how that could be accelerated. Has any work spent on actually implementing / testing such measures?
diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..75b66e2 --- /dev/null +++ b/include/iomux.h
...
+/*
- Avoid invoking WATCHDOG via udelay() on every pass through the loop
- in fgetc(). Note this is all based on guesswork.
- */
+#define COUNT_TIL_UDELAY 10000
I think we should use a more determinitic approach. See comment above.
Thanks.
Best regards,
Wolfgang Denk
On Thu, 30 Oct 2008 12:59:49 +0100 Wolfgang Denk wd@denx.de wrote:
In message 20081030112119.59036e03@ernst.jennejohn.org you wrote:
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
Is it correct to assume that this patch version does not yet include any changes resultiung from the discussion we had yesterday how to optimize timing behaviour?
Yes. It also doesn't appear to me that this has been resolved yet.
diff --git a/common/Makefile b/common/Makefile index f00cbd9..0296a8a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -156,6 +156,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_UPDATE_TFTP) += update.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_SYS_CONSOLE_MUX) += iomux.o
As this is a user configurable option, it should be named CONFIG_CONSOLE_MUX. Please remove the "SYS_" part. And please keep the list sorted.
I chose this analogously to CONFIG_SYS_CONSOLE_IS_IN_ENV. Obviously I misunderstand something about how these names are chosen. Please enlighten me.
@@ -115,7 +182,41 @@ void serial_printf (const char *fmt, ...) int fgetc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
- {
Please move the braces outside the #ifdef.
OK
int cntr = 0;/** Effectively poll for input wherever it may be available.*/for (;;) {/** Upper layer may have already called tstc() so* check for that first.*/if (tstcdev != NULL)return iomux_getc();iomux_tstc(file);/** Even this is too slow for serial cut&paste due* to the overhead of calling tstc() then getc().* It gets worse if nc is set as a console because* nc_tstc() is rather slow.We discussed a potential solution to this problem yesterday. Do you want to try this out in your implementation?
See my comment above.
* The idea behind this counter is to avoid calling* the watchdog via udelay() too often.* COUNT_TIL_UDELAY is defined in iomux.h and is just* a guesstimate.*/if (cntr++ > COUNT_TIL_UDELAY) {udelay(1);cntr = 0;}Please do not re-invent the wheel. If the watchdog triggering rate needs to be limited then please use the CONFIG_WD_MAX_RATE parameter as it was done in commit d32a874b "lwmon5 watchdog: limit trigger rate".
Sounds good to me. I wasn't aware of CONFIG_WD_MAX_RATE. There are way too many undocumented options in u-boot.
@@ -123,7 +224,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
return iomux_tstc(file);+#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +236,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
iomux_putc(file, c);+#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_SYS_CONSOLE_MUX)
iomux_puts(file, s);+#else stdio_devices[file]->puts (s); +#endif }
Would it be possible to just set
stdio_devices[file]->tstc = iomux_tstc; stdio_devices[file]->putc = iomux_putc; stdio_devices[file]->puts = iomux_puts;
?
This would eliminate the need for these #ifdef's (and possibly a few more) ?
@@ -438,15 +562,34 @@ int console_init_r (void) } /* Initializes output console first */ if (outputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stdout, outputdev->name);+#else console_setfile (stdout, outputdev); +#endif } if (errdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stderr, errdev->name);+#else console_setfile (stderr, errdev); +#endif } if (inputdev != NULL) { +#ifdef CONFIG_SYS_CONSOLE_MUX
/* need to set a console if not done above. */iomux_doenv(stdin, inputdev->name);+#else console_setfile (stdin, inputdev); +#endif }
See above. Maybe these #ifdef's could be eliminated as well?
@@ -455,21 +598,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stdin);+#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stdout);+#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_SYS_CONSOLE_MUX
iomux_printdevs(stderr);+#else printf ("%s\n", stdio_devices[stderr]->name); +#endif }
And these, too?
Seems like a good idea. I'll look at it.
- int i;
- device_t *dev;
- for (i = 0; i < cd_count[console]; i++) {
dev = console_devices[console][i];serial_printf("%s ", dev->name);- }
- serial_printf("\n");
+}
The old code uses plain printf(), while your new version explicitely calls serial_printf(). I gues sthis is intentional? What is the rationale behind it?
To avoid writing to e.g. nc before it's ready for output.
+/* This tries to preserve the old list if an error occurs. */ +int iomux_doenv(const int console, const char *arg) +{
- char *console_args, *temp, **start;
- int i, j, io_flag, cs_idx;
- device_t *dev;
- device_t **cons_set;
+#ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
- if (arg == NULL)
return 1;+#endif
Do we need this special case test?
- console_args = strdup(arg);
- if (console_args == NULL)
return 1;I think this test here would catch it as well?
Ah, I see that it's safe to pass NULL to strdup(). OK, I can simplify the code knowing that.
- /*
* Check whether a comma separated list of devices was* entered and count how many devices were entered.* The array start[] has pointers to the beginning of* each device name (up to MAX_CONSARGS devices).** Have to do this twice - once to count the number of* commas and then again to populate start.*/- i = 0;
- temp = console_args;
- for (;;) {
temp = strchr(temp, ',');if (temp != NULL) {i++;temp++;continue;}/* There's always one entry more than the number of commas. */i++;break;- }
- start = (char **)malloc(i * sizeof(char *));
- if (start == NULL) {
free(console_args);return 1;- }
- i = 0;
- start[0] = console_args;
- for (;;) {
temp = strchr(start[i], ',');if (temp != NULL) {i++;*temp = '\0';start[i] = temp + 1;continue;}/* There's always one entry more than the number of commas. */i++;break;- }
I suggest to swap logic around as this allows for easier to read code:
for (;;) { temp = strchr(start[i++], ','); if (temp == NULL) break; *temp = '\0'; start[i] = temp + 1; }
Seems reasonable, but I have to consider it in more detail.
- cons_set = (device_t **)calloc(1, i * sizeof(device_t *));
Why not:
cons_set = (device_t **)calloc(i, sizeof(device_t *));
?
Hadn't thought of that. A good idea.
- if (cons_set == NULL) {
free(start);free(console_args);return 1;- }
- switch (console) {
- case stdin:
io_flag = DEV_FLAGS_INPUT;break;- case stdout:
- case stderr:
io_flag = DEV_FLAGS_OUTPUT;break;- default:
return 1;Memory leak? free start & console_args & cons_set ??
Yes, could be. I added default as an afterthought and didn't consider all its implications.
+Two new files, common/iomux.c and include/iomux.h, contain the heart +of the environment setting implementation.
It seems to me that they do much more than just setting environment variables?
Their whole reason for their existence is to handle setenv.
+The execution is then in common/cmd_nvedit.c and common/console.c.
Hm... but cmd_nvedit.c really is for environment variables handling only?
Yeah, the wording could be improved.
+A user can use a comma-separated list of devices to set stdin, stdout +and stderr. For example: setenv stdin serial,nc. NOTE: No spaces
Maybe it's better to quote the example to make it stand out:
...and stderr. For example: "setenv stdin serial,nc". NOTE: No spaces
?
I thought about that myself but didn't do it.
+CAVEATS +-------
+Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list.
+The overhead associated with calling tstc() and then getc() means that +copy&paste will normally not work, even if serial is set as the only device +for stdin, stdout and stderr.
You should probably mention the conditions where you tested this with such results. It probably depends on the system performance and the console baud rate, doesn't it?
True.
+Using nc as a stdin device results in even more overhead because nc_tstc() +is quite slow.
There was discussion before how that could be accelerated. Has any work spent on actually implementing / testing such measures?
Not by me.
diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..75b66e2 --- /dev/null +++ b/include/iomux.h
...
+/*
- Avoid invoking WATCHDOG via udelay() on every pass through the loop
- in fgetc(). Note this is all based on guesswork.
- */
+#define COUNT_TIL_UDELAY 10000
I think we should use a more determinitic approach. See comment above.
I agree.
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************
Dear Gary Jennejohn,
In message 20081030142153.1761d6fa@ernst.jennejohn.org you wrote:
Is it correct to assume that this patch version does not yet include any changes resultiung from the discussion we had yesterday how to optimize timing behaviour?
Yes. It also doesn't appear to me that this has been resolved yet.
Well, you're in a position to try it out. I ain't.
As this is a user configurable option, it should be named CONFIG_CONSOLE_MUX. Please remove the "SYS_" part. And please keep the list sorted.
I chose this analogously to CONFIG_SYS_CONSOLE_IS_IN_ENV. Obviously I misunderstand something about how these names are chosen. Please enlighten me.
CONFIG_SYS_CONSOLE_IS_IN_ENV was incorrectly chosen. It should be CONFIG_CONSOLE_IS_IN_ENV as well.
Sounds good to me. I wasn't aware of CONFIG_WD_MAX_RATE. There are way too many undocumented options in u-boot.
Please feel free to send patches to improve the documentation.
- int i;
- device_t *dev;
- for (i = 0; i < cd_count[console]; i++) {
dev = console_devices[console][i];serial_printf("%s ", dev->name);- }
- serial_printf("\n");
+}
The old code uses plain printf(), while your new version explicitely calls serial_printf(). I gues sthis is intentional? What is the rationale behind it?
To avoid writing to e.g. nc before it's ready for output.
I'm not sure if this is a good idea. It changes existing behaviour, and thisis always a bad thing. Assume systems with LCD display, where output usually goes there. I think it should be up to the nc driver to handle tyhis and, for example, ignore such calls until it has been sufficiently initialized.
Thanks.
Best regards,
Wolfgang Denk
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
This allows a user to specify multiple console devices in the environment with a command like this: setenv stdin serial,nc. As a result, the user can enter text on both the serial and netconsole interfaces.
All devices - stdin, stdout and stderr - can be set in this manner.
1) common/iomux.c and include/iomux.h contain the environment setting implementation. 2) doc/README.iomux contains a somewhat more detailed description. 3) The implementation in (1) is called from common/cmd_nvedit.c to handle setenv and from common/console.c to handle initialization of input/output devices at boot time. 4) common/console.c also contains the code needed to poll multiple console devices for input and send output to all devices registered for output. 5) include/common.h includes iomux.h and common/Makefile generates iomux.o when CONFIG_SYS_CONSOLE_MUX is set.
Signed-off-by: Gary Jennejohn garyj@denx.de ---
V3 - handle comments from the ML - iomux_doenv() now removes repeated device entries
common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 156 +++++++++++++++++++++++++++++++++++++++++++++- common/iomux.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 106 +++++++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 48 ++++++++++++++ 7 files changed, 498 insertions(+), 1 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
diff --git a/common/Makefile b/common/Makefile index f00cbd9..b1d2297 100644 --- a/common/Makefile +++ b/common/Makefile @@ -149,6 +149,7 @@ COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o COBJS-$(CONFIG_VFD) += cmd_vfd.o COBJS-$(CONFIG_CMD_DOC) += docecc.o +COBJS-$(CONFIG_CONSOLE_MUX) += iomux.o COBJS-y += flash.o COBJS-y += kgdb.o COBJS-$(CONFIG_LCD) += lcd.o diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index d280cb0..85025da 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -213,6 +213,11 @@ int _do_setenv (int flag, int argc, char *argv[]) return 1; }
+#ifdef CONFIG_CONSOLE_MUX + i = iomux_doenv(console, argv[2]); + if (i) + return i; +#else /* Try assigning specified device */ if (console_assign (console, argv[2]) < 0) return 1; @@ -221,6 +226,7 @@ int _do_setenv (int flag, int argc, char *argv[]) if (serial_assign (argv[2]) < 0) return 1; #endif +#endif /* CONFIG_CONSOLE_MUX */ }
/* diff --git a/common/console.c b/common/console.c index 6f0846f..89aeab6 100644 --- a/common/console.c +++ b/common/console.c @@ -93,6 +93,76 @@ static int console_setfile (int file, device_t * dev) return error; }
+#if defined(CONFIG_CONSOLE_MUX) +/** Console I/O multiplexing *******************************************/ + +static device_t *tstcdev; +device_t **console_devices[MAX_FILES]; +int cd_count[MAX_FILES]; + +/* + * This depends on tstc() always being called before getc(). + * This is guaranteed to be true because this routine is called + * only from fgetc() which assures it. + * No attempt is made to demultiplex multiple input sources. + */ +static int iomux_getc(void) +{ + unsigned char ret; + + /* This is never called with testcdev == NULL */ + ret = tstcdev->getc(); + tstcdev = NULL; + return ret; +} + +static int iomux_tstc(int file) +{ + int i, ret; + device_t *dev; + + disable_ctrlc(1); + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->tstc != NULL) { + ret = dev->tstc(); + if (ret > 0) { + tstcdev = dev; + disable_ctrlc(0); + return ret; + } + } + } + disable_ctrlc(0); + + return 0; +} + +static void iomux_putc(int file, const char c) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->putc != NULL) + dev->putc(c); + } +} + +static void iomux_puts(int file, const char *s) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[file]; i++) { + dev = console_devices[file][i]; + if (dev->puts != NULL) + dev->puts(s); + } +} +#endif /* defined(CONFIG_CONSOLE_MUX) */ + /** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
void serial_printf (const char *fmt, ...) @@ -114,8 +184,31 @@ void serial_printf (const char *fmt, ...)
int fgetc (int file) { - if (file < MAX_FILES) + if (file < MAX_FILES) { +#if defined(CONFIG_CONSOLE_MUX) + /* + * Effectively poll for input wherever it may be available. + */ + for (;;) { + /* + * Upper layer may have already called tstc() so + * check for that first. + */ + if (tstcdev != NULL) + return iomux_getc(); + iomux_tstc(file); +#ifdef CONFIG_WATCHDOG + /* + * If the watchdog must be rate-limited then it should + * already be handled in board-specific code. + */ + udelay(1); +#endif + } +#else return stdio_devices[file]->getc (); +#endif + }
return -1; } @@ -123,7 +216,11 @@ int fgetc (int file) int ftstc (int file) { if (file < MAX_FILES) +#if defined(CONFIG_CONSOLE_MUX) + return iomux_tstc(file); +#else return stdio_devices[file]->tstc (); +#endif
return -1; } @@ -131,13 +228,21 @@ int ftstc (int file) void fputc (int file, const char c) { if (file < MAX_FILES) +#if defined(CONFIG_CONSOLE_MUX) + iomux_putc(file, c); +#else stdio_devices[file]->putc (c); +#endif }
void fputs (int file, const char *s) { if (file < MAX_FILES) +#if defined(CONFIG_CONSOLE_MUX) + iomux_puts(file, s); +#else stdio_devices[file]->puts (s); +#endif }
void fprintf (int file, const char *fmt, ...) @@ -407,6 +512,9 @@ int console_init_r (void) #ifdef CONFIG_SYS_CONSOLE_ENV_OVERWRITE int i; #endif /* CONFIG_SYS_CONSOLE_ENV_OVERWRITE */ +#ifdef CONFIG_CONSOLE_MUX + int iomux_err = 0; +#endif
/* set default handlers at first */ gd->jt[XF_getc] = serial_getc; @@ -425,6 +533,14 @@ int console_init_r (void) inputdev = search_device (DEV_FLAGS_INPUT, stdinname); outputdev = search_device (DEV_FLAGS_OUTPUT, stdoutname); errdev = search_device (DEV_FLAGS_OUTPUT, stderrname); +#ifdef CONFIG_CONSOLE_MUX + iomux_err = iomux_doenv(stdin, stdinname); + iomux_err += iomux_doenv(stdout, stdoutname); + iomux_err += iomux_doenv(stderr, stderrname); + if (!iomux_err) + /* Successful, so skip all the code below. */ + goto done; +#endif } /* if the devices are overwritten or not found, use default device */ if (inputdev == NULL) { @@ -438,15 +554,34 @@ int console_init_r (void) } /* Initializes output console first */ if (outputdev != NULL) { +#ifdef CONFIG_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stdout, outputdev->name); +#else console_setfile (stdout, outputdev); +#endif } if (errdev != NULL) { +#ifdef CONFIG_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stderr, errdev->name); +#else console_setfile (stderr, errdev); +#endif } if (inputdev != NULL) { +#ifdef CONFIG_CONSOLE_MUX + /* need to set a console if not done above. */ + iomux_doenv(stdin, inputdev->name); +#else console_setfile (stdin, inputdev); +#endif }
+#ifdef CONFIG_CONSOLE_MUX +done: +#endif + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET @@ -455,21 +590,33 @@ int console_init_r (void) if (stdio_devices[stdin] == NULL) { puts ("No input devices available!\n"); } else { +#ifdef CONFIG_CONSOLE_MUX + iomux_printdevs(stdin); +#else printf ("%s\n", stdio_devices[stdin]->name); +#endif }
puts ("Out: "); if (stdio_devices[stdout] == NULL) { puts ("No output devices available!\n"); } else { +#ifdef CONFIG_CONSOLE_MUX + iomux_printdevs(stdout); +#else printf ("%s\n", stdio_devices[stdout]->name); +#endif }
puts ("Err: "); if (stdio_devices[stderr] == NULL) { puts ("No error devices available!\n"); } else { +#ifdef CONFIG_CONSOLE_MUX + iomux_printdevs(stderr); +#else printf ("%s\n", stdio_devices[stderr]->name); +#endif } #endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
@@ -524,11 +671,18 @@ int console_init_r (void) if (outputdev != NULL) { console_setfile (stdout, outputdev); console_setfile (stderr, outputdev); +#ifdef CONFIG_CONSOLE_MUX + console_devices[stdout][0] = outputdev; + console_devices[stderr][0] = outputdev; +#endif }
/* Initializes input console */ if (inputdev != NULL) { console_setfile (stdin, inputdev); +#ifdef CONFIG_CONSOLE_MUX + console_devices[stdin][0] = inputdev; +#endif }
gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ diff --git a/common/iomux.c b/common/iomux.c new file mode 100644 index 0000000..bdcc853 --- /dev/null +++ b/common/iomux.c @@ -0,0 +1,175 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +#include <common.h> +#include <serial.h> +#include <malloc.h> + +#ifdef CONFIG_CONSOLE_MUX +void iomux_printdevs(const int console) +{ + int i; + device_t *dev; + + for (i = 0; i < cd_count[console]; i++) { + dev = console_devices[console][i]; + printf("%s ", dev->name); + } + printf("\n"); +} + +/* This tries to preserve the old list if an error occurs. */ +int iomux_doenv(const int console, const char *arg) +{ + char *console_args, *temp, **start; + int i, j, k, io_flag, cs_idx, repeat; + device_t *dev; + device_t **cons_set; + + console_args = strdup(arg); + if (console_args == NULL) + return 1; + /* + * Check whether a comma separated list of devices was + * entered and count how many devices were entered. + * The array start[] has pointers to the beginning of + * each device name (up to MAX_CONSARGS devices). + * + * Have to do this twice - once to count the number of + * commas and then again to populate start. + */ + i = 0; + temp = console_args; + for (;;) { + temp = strchr(temp, ','); + if (temp != NULL) { + i++; + temp++; + continue; + } + /* There's always one entry more than the number of commas. */ + i++; + break; + } + start = (char **)malloc(i * sizeof(char *)); + if (start == NULL) { + free(console_args); + return 1; + } + i = 0; + start[0] = console_args; + for (;;) { + temp = strchr(start[i++], ','); + if (temp == NULL) + break; + *temp = '\0'; + start[i] = temp + 1; + } + cons_set = (device_t **)calloc(i, sizeof(device_t *)); + if (cons_set == NULL) { + free(start); + free(console_args); + return 1; + } + + switch (console) { + case stdin: + io_flag = DEV_FLAGS_INPUT; + break; + case stdout: + case stderr: + io_flag = DEV_FLAGS_OUTPUT; + break; + default: + free(start); + free(console_args); + free(cons_set); + return 1; + } + + cs_idx = 0; + for (j = 0; j < i; j++) { + /* + * Check whether the device exists and is valid. + * console_assign() also calls search_device(), + * but I need the pointer to the device. + */ + dev = search_device(io_flag, start[j]); + if (dev == NULL) + continue; + /* + * Prevent multiple entries for a device. + */ + repeat = 0; + for (k = 0; k < cs_idx; k++) { + if (dev == cons_set[k]) { + repeat++; + break; + } + } + if (repeat) + continue; + /* + * Try assigning the specified device. + * This could screw up the console settings for apps. + */ + if (console_assign(console, start[j]) < 0) + continue; +#ifdef CONFIG_SERIAL_MULTI + /* + * This was taken from common/cmd_nvedit.c. + * This will never work because serial_assign() returns + * 1 upon error, not -1. + * This would almost always return an error anyway because + * serial_assign() expects the name of a serial device, like + * serial_smc, but the user generally only wants to set serial. + */ + if (serial_assign(start[j]) < 0) + continue; +#endif + cons_set[cs_idx++] = dev; + } + free(console_args); + free(start); + /* failed to set any console */ + if (cs_idx == 0) { + free(cons_set); + return 1; + } else { + /* Works even if console_devices[console] is NULL. */ + console_devices[console] = + (device_t **)realloc(console_devices[console], + cs_idx * sizeof(device_t *)); + if (console_devices[console] == NULL) { + free(cons_set); + return 1; + } + memcpy(console_devices[console], cons_set, cs_idx * + sizeof(device_t *)); + + cd_count[console] = cs_idx; + } + free(cons_set); + return 0; +} +#endif /* CONFIG_CONSOLE_MUX */ diff --git a/doc/README.iomux b/doc/README.iomux new file mode 100644 index 0000000..5b82a86 --- /dev/null +++ b/doc/README.iomux @@ -0,0 +1,106 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH garyj@denx.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + * 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. + * + * 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 + */ + +U-Boot console multiplexing +=========================== + +HOW CONSOLE MULTIPLEXING WORKS +------------------------------ + +This functionality is controlled with CONFIG_CONSOLE_MUX in the board +configuration file. + +Two new files, common/iomux.c and include/iomux.h, contain the heart +(iomux_doenv()) of the environment setting implementation. + +iomux_doenv() is called in common/cmd_nvedit.c to handle setenv and in +common/console.c in console_init_r() during bootup to initialize +stdio_devices[]. + +A user can use a comma-separated list of devices to set stdin, stdout +and stderr. For example: "setenv stdin serial,nc". NOTE: No spaces +are allowed around the comma(s)! + +The length of the list is limited by malloc(), since the array used +is allocated and freed dynamically. + +It should be possible to specify any device which console_assign() +finds acceptable, but the code has only been tested with serial and +nc. + +iomux_doenv() prevents multiple use of the same device, e.g. "setenv +stdin nc,nc,serial" will discard the second nc. iomux_doenv() is +not able to modify the environment, however, so that "pri stdin" still +shows "nc,nc,serial". + +The major change in common/console.c was to modify fgetc() to call +the iomux_tstc() routine in a for-loop. iomux_tstc() in turn calls +the tstc() routine for every registered device, but exits immediately +when one of them returns true. fgetc() then calls iomux_getc(), +which calls the corresponding getc() routine. fgetc() hangs in +the for-loop until iomux_tstc() returns true and the input can be +retrieved. + +Thus, a user can type into any device registered for stdin. No effort +has been made to demulitplex simultaneous input from multiple stdin +devices. + +fputc() and fputs() have been modified to call iomux_putc() and +iomux_puts() respectively, which call the corresponding output +routines for every registered device. + +Thus, a user can see the ouput for any device registered for stdout +or stderr on all devices registered for stdout or stderr. As an +example, if stdin=serial,nc and stdout=serial,nc then all output +for serial, e.g. echos of input on serial, will appear on serial and nc. + +Just as with the old console code, this statement is still true: +If not defined in the environment, the first input device is assigned +to the 'stdin' file, the first output one to 'stdout' and 'stderr'. + +If CONFIG_SYS_CONSOLE_IS_IN_ENV is defined then multiple input/output +devices can be set at boot time if defined in the environment. + +CAVEATS +------- + +Note that common/iomux.c calls console_assign() for every registered +device as it is discovered. This means that the environment settings +for application consoles will be set to the last device in the list. + +On a slow machine, such as MPC852T clocked at 66MHz, the overhead associated +with calling tstc() and then getc() means that copy&paste will normally not +work, even when stdin=stdout=stderr=serial. +On a faster machine, such as a sequoia, cut&paste of longer (about 80 +characters) lines works fine when serial is the only device used. + +Using nc as a stdin device results in even more overhead because nc_tstc() +is quite slow. Even on a sequoia cut&paste does not work on the serial +interface when nc is added to stdin, although there is no character loss using +the ethernet interface for input. In this test case stdin=serial,nc and +stdout=serial. + +In addition, the overhead associated with sending to two devices, when one of +them is nc, also causes problems. Even on a sequoia cut&paste does not work +on the serial interface (stdin=serial) when nc is added to stdout (stdout= +serial,nc). diff --git a/include/common.h b/include/common.h index b8a654a..a005bb7 100644 --- a/include/common.h +++ b/include/common.h @@ -675,6 +675,13 @@ void fputc(int file, const char c); int ftstc(int file); int fgetc(int file);
+/* + * CONSOLE multiplexing. + */ +#ifdef CONFIG_CONSOLE_MUX +#include <iomux.h> +#endif + int pcmcia_init (void);
#ifdef CONFIG_STATUS_LED diff --git a/include/iomux.h b/include/iomux.h new file mode 100644 index 0000000..257c1f7 --- /dev/null +++ b/include/iomux.h @@ -0,0 +1,48 @@ +/* + * (C) Copyright 2008 + * Gary Jennejohn, DENX Software Engineering GmbH, garyj@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program 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. + * + *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. + * + * 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 + */ + +#ifndef _IO_MUX_H +#define _IO_MUX_H + +#include <devices.h> + +/* + * Stuff required to support console multiplexing. + */ + +/* + * Pointers to devices used for each file type. Defined in console.c + * but storage is allocated in iomux.c. + */ +extern device_t **console_devices[MAX_FILES]; +/* + * The count of devices assigned to each FILE. Defined in console.c + * and populated in iomux.c. + */ +extern int cd_count[MAX_FILES]; + +int iomux_doenv(const int, const char *); +void iomux_printdevs(const int); +device_t *search_device(int, char *); + +#endif /* _IO_MUX_H */
On 15:04 Thu 06 Nov , Gary Jennejohn wrote:
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
This allows a user to specify multiple console devices in the environment with a command like this: setenv stdin serial,nc. As a result, the user can enter text on both the serial and netconsole interfaces.
All devices - stdin, stdout and stderr - can be set in this manner.
- common/iomux.c and include/iomux.h contain the environment setting
implementation. 2) doc/README.iomux contains a somewhat more detailed description. 3) The implementation in (1) is called from common/cmd_nvedit.c to handle setenv and from common/console.c to handle initialization of input/output devices at boot time. 4) common/console.c also contains the code needed to poll multiple console devices for input and send output to all devices registered for output. 5) include/common.h includes iomux.h and common/Makefile generates iomux.o when CONFIG_SYS_CONSOLE_MUX is set.
Signed-off-by: Gary Jennejohn garyj@denx.de
V3
- handle comments from the ML
- iomux_doenv() now removes repeated device entries
common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 156 +++++++++++++++++++++++++++++++++++++++++++++- common/iomux.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 106 +++++++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 48 ++++++++++++++ 7 files changed, 498 insertions(+), 1 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
you steel do not use list_head.
You agree about the idea.
is there any special reason?
Best Regards, J.
On Thu, 6 Nov 2008 15:30:38 +0100 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
On 15:04 Thu 06 Nov , Gary Jennejohn wrote:
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
This allows a user to specify multiple console devices in the environment with a command like this: setenv stdin serial,nc. As a result, the user can enter text on both the serial and netconsole interfaces.
All devices - stdin, stdout and stderr - can be set in this manner.
- common/iomux.c and include/iomux.h contain the environment setting
implementation. 2) doc/README.iomux contains a somewhat more detailed description. 3) The implementation in (1) is called from common/cmd_nvedit.c to handle setenv and from common/console.c to handle initialization of input/output devices at boot time. 4) common/console.c also contains the code needed to poll multiple console devices for input and send output to all devices registered for output. 5) include/common.h includes iomux.h and common/Makefile generates iomux.o when CONFIG_SYS_CONSOLE_MUX is set.
Signed-off-by: Gary Jennejohn garyj@denx.de
V3
- handle comments from the ML
- iomux_doenv() now removes repeated device entries
common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 156 +++++++++++++++++++++++++++++++++++++++++++++- common/iomux.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 106 +++++++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 48 ++++++++++++++ 7 files changed, 498 insertions(+), 1 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
you steel do not use list_head.
You agree about the idea.
is there any special reason?
There are actually two a) I *did* look at using list_head, but I couldn't wrap my head around it in a reasonable length of time and it wasn't clear to me how to implement the functionality I wanted b) It was much easier for me to modify the code to what I needed using realloc since it was already using an array
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************
Dear Gary Jennejohn,
In message 20081106150423.3abb36d6@ernst.jennejohn.org you wrote:
Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.
This allows a user to specify multiple console devices in the environment with a command like this: setenv stdin serial,nc. As a result, the user can enter text on both the serial and netconsole interfaces.
All devices - stdin, stdout and stderr - can be set in this manner.
- common/iomux.c and include/iomux.h contain the environment setting
implementation. 2) doc/README.iomux contains a somewhat more detailed description. 3) The implementation in (1) is called from common/cmd_nvedit.c to handle setenv and from common/console.c to handle initialization of input/output devices at boot time. 4) common/console.c also contains the code needed to poll multiple console devices for input and send output to all devices registered for output. 5) include/common.h includes iomux.h and common/Makefile generates iomux.o when CONFIG_SYS_CONSOLE_MUX is set.
Signed-off-by: Gary Jennejohn garyj@denx.de
V3
- handle comments from the ML
- iomux_doenv() now removes repeated device entries
common/Makefile | 1 + common/cmd_nvedit.c | 6 ++ common/console.c | 156 +++++++++++++++++++++++++++++++++++++++++++++- common/iomux.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.iomux | 106 +++++++++++++++++++++++++++++++ include/common.h | 7 ++ include/iomux.h | 48 ++++++++++++++ 7 files changed, 498 insertions(+), 1 deletions(-) create mode 100644 common/iomux.c create mode 100644 doc/README.iomux create mode 100644 include/iomux.h
Applied, thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Gary Jennejohn -
Jean-Christophe PLAGNIOL-VILLARD -
Wolfgang Denk