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.

common/cmd_nvedit.c +6 -0

index 637d6c9..6fc9313 100644
@@ -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 */
 		}
 
 		/*

common/console.c +129 -0

index 56d9118..6158af9 100644
@@ -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) {

common/console.c +12 -0

 	}
 	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

common/console.c +8 -0

 #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
 	}

common/iomux.c new file +69 -0

new file mode 100644
index 0000000..d62f7e4
@@ -0,0 +1,133 @@
+/*
+ * (C) Copyright 2008
+ * Gary Jennejohn, DENX Software Engineering GmbH, garyj at 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++;

common/iomux.c new file +13 -0

+		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;
+		/*

common/cmd_nvedit.c +3 -0

+				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 */
 		}
 
 		/*

common/console.c +0 -0

index 56d9118..6158af9 100644

common/console.c +4 -0

+
+/*
+ * This depends on tstc() always being called before getc().
+ * No attempt is made to demultiplex multiple input sources.

include/iomux.h new file +3 -0

new file mode 100644
index 0000000..1310a75
@@ -0,0 +1,48 @@
+/*
+ * (C) Copyright 2008
+ * Gary Jennejohn, DENX Software Engineering GmbH, garyj at denx.de.

include/iomux.h new file +7 -0

+ *
+ * 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
+ */
+

Quoted 21 lines

common/cmd_nvedit.c +3 -0

+				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 */
 		}
 
 		/*

common/console.c +0 -0

index 56d9118..6158af9 100644
@@ -93,6 +93,78 @@ static int console_setfile (int file, device_t * dev)
 	return error;
 }
 

common/console.c static int console_setfile (int file, device_t * dev) · line 100

Quoted 5 lines

common/console.c +5 -0

+#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

Quoted 8 lines · On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD…

common/console.c static int console_setfile (int file, device_t * dev) · line 100

Quoted 5 lines

common/console.c +5 -0

+#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!

Quoted 12 lines · In message <[email protected]> you wrote:

Quoted 8 lines · On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD…

common/console.c static int console_setfile (int file, device_t * dev) · line 100

Quoted 5 lines

common/console.c +5 -0

+#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

Quoted 1 line

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?

On Fri, 24 Oct 2008 13:18:42 +0200
Wolfgang Denk <[email protected]> wrote:

Dear Wolfgang,

Quoted 13 lines

Quoted 12 lines · In message <[email protected]> you wrote:

Quoted 8 lines · On Sun, 14 Sep 2008 23:59:39 +0200 Jean-Christophe PLAGNIOL-VILLARD…

common/console.c static int console_setfile (int file, device_t * dev) · line 100

Quoted 5 lines

common/console.c +5 -0

+#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

Quoted 8 lines

Quoted 1 line

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.

Modifications to support console multiplexing. This is controlled using CONFIG_SYS_CONSOLE_MUX in the board configuration file.

common/cmd_nvedit.c +5 -0

index d280cb0..7a49296 100644
@@ -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;

common/console.c +0 -0

index 6f0846f..c89020d 100644
@@ -93,6 +93,73 @@ static int console_setfile (int file, device_t * dev)
 	return error;
 }

common/console.c +11 -0

+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) {

common/console.c +8 -0

+		int cntr = 0;
+
+		/*
+		 * Effectively poll for input wherever it may be available.
+		 */
+		for (;;) {
+			/*
+			 * Upper layer may have already called tstc() so

common/console.c +8 -0

@@ -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
 }

common/console.c +9 -0

@@ -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);

common/console.c +8 -0

@@ -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
 	}
 

common/iomux.c new file +29 -0

new file mode 100644
index 0000000..b83bdb5
@@ -0,0 +1,170 @@
+/*
+ * (C) Copyright 2008
+ * Gary Jennejohn, DENX Software Engineering GmbH, garyj at 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)

common/iomux.c new file +31 -0

+	/*
+	 * 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], ',');

common/iomux.c new file +6 -0

+	if (cons_set == NULL) {
+		free(start);
+		free(console_args);
+		return 1;
+	}
+

doc/README.iomux new file +24 -0

new file mode 100644
index 0000000..bff75fa
@@ -0,0 +1,89 @@
+/*
+ * (C) Copyright 2008
+ * Gary Jennejohn, DENX Software Engineering GmbH <[email protected]>
+ *
+ * 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

include/iomux.h new file +0 -0

new file mode 100644
index 0000000..75b66e2

Quoted 3 lines · In message <[email protected]> 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?

common/Makefile COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o · line 159

Quoted 10 lines

common/Makefile +1 -0

index f00cbd9..0296a8a 100644
@@ -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.

common/Makefile void serial_printf (const char *fmt, ...) · line 186

Quoted 6 lines

common/Makefile +2 -0

@@ -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.

Quoted 13 lines

common/Makefile +13 -0

+		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);

common/Makefile void serial_printf (const char *fmt, ...) · line 204

Quoted 5 lines

common/Makefile +5 -0

+			/*
+			 * 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?

Quoted 6 lines

common/Makefile +6 -0

+			 * 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) {

common/Makefile void serial_printf (const char *fmt, ...) · line 213

Quoted 3 lines

common/Makefile +3 -0

+				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".

Quoted 22 lines

common/Makefile +8 -0

@@ -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
 }

common/Makefile int ftstc (int file) · line 254

Quoted 10 lines

common/Makefile +4 -0

 
 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) ?

Quoted 17 lines

common/Makefile +9 -0

@@ -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);

common/Makefile int console_init_r (void) · line 587

Quoted 10 lines

common/Makefile +6 -0

+#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?

Quoted 22 lines

common/Makefile +8 -0

@@ -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
 	}
 

common/Makefile int console_init_r (void) · line 628

Quoted 10 lines

common/Makefile +4 -0

 	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?

Quoted 5 lines

common/Makefile +5 -0

+	int i;
+	device_t *dev;
+
+	for (i = 0; i < cd_count[console]; i++) {
+		dev = console_devices[console][i];

common/Makefile

Quoted 4 lines

common/Makefile +4 -0

+		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?

common/Makefile

Quoted 12 lines

common/Makefile +12 -0

+/* 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?

common/Makefile

Quoted 3 lines

common/Makefile +3 -0

+	console_args = strdup(arg);
+	if (console_args == NULL)
+		return 1;

I think this test here would catch it as well?

Quoted 31 lines

common/Makefile +31 -0

+	/*
+	 * 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], ',');

common/Makefile

Quoted 10 lines

common/Makefile +10 -0

+		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;
	}

common/Makefile

Quoted 1 line

common/Makefile +1 -0

+	cons_set = (device_t **)calloc(1, i * sizeof(device_t *));

Why not:

	cons_set = (device_t **)calloc(i, sizeof(device_t *));

?

Quoted 10 lines

common/Makefile +10 -0

+	if (cons_set == NULL) {
+		free(start);
+		free(console_args);
+		return 1;
+	}
+
+	switch (console) {
+	case stdin:
+		io_flag = DEV_FLAGS_INPUT;
+		break;

common/Makefile

Quoted 6 lines

common/Makefile +6 -0

+	case stdout:
+	case stderr:
+		io_flag = DEV_FLAGS_OUTPUT;
+		break;
+	default:
+		return 1;

Memory leak? free start & console_args & cons_set ??

common/Makefile

Quoted 2 lines

common/Makefile +2 -0

+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?

Quoted 1 line

+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?

common/Makefile

Quoted 2 lines

common/Makefile +2 -0

+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

?

Quoted 6 lines

common/Makefile +6 -0

+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.

common/Makefile

Quoted 4 lines

common/Makefile +4 -0

+
+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?

common/Makefile

Quoted 2 lines

common/Makefile +2 -0

+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?

Quoted 5 lines

include/iomux.h new file +0 -0

new file mode 100644
index 0000000..75b66e2

...

include/iomux.h line 37

Quoted 5 lines

include/iomux.h new file +5 -0

+/*
+ * 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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Sounds good to me. I wasn't aware of CONFIG_WD_MAX_RATE. There are way too many undocumented options in u-boot.

Quoted 5 lines

Quoted 5 lines

common/iomux.c new file +5 -0

+	int i;
+	device_t *dev;
+
+	for (i = 0; i < cd_count[console]; i++) {
+		dev = console_devices[console][i];

Quoted 8 lines

common/iomux.c line 39

Quoted 4 lines

common/iomux.c new file +4 -0

+		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.

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 <[email protected]>

---

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

Quoted 28 lines · Gary Jennejohn

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 <[email protected]>

---

V3
- handle comments from the ML
- iomux_doenv() now removes repeated device entries

Quoted 11 lines

 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?

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 <[email protected]>

---

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