[U-Boot-Users] [PATCH] simplify silent console

[U-Boot-Users] [PATCH] simplify silent console

[Ladislav Michl]

Wolfgang,

silent console output is currently implemented by assigning nulldev
as output device. This is a bit overcomplicated, since there is
also GD_FLG_SILENT flag.

Patch bellow tries to simplify silencing console by honouring
GD_FLG_SILENT in console output functions, so there is no need to
mess with output device. Comments?

Best regards,
	ladis

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

CHANGELOG
* Silent console code simplification.
  Patch by Ladislav Michl, 25 Jul 2006


diff --git a/common/console.c b/common/console.c
index e9f23be..d8a0cb6 100644
--- a/common/console.c
+++ b/common/console.c
@@ -494,13 +494,7 @@ #ifdef CONFIG_SPLASH_SCREEN
 	/* suppress all output if splash screen is enabled and we have
 	   a bmp to display                                            */
 	if (getenv("splashimage") != NULL)
-		outputdev = search_device (DEV_FLAGS_OUTPUT, "nulldev");
-#endif
-
-#ifdef CONFIG_SILENT_CONSOLE
-	/* Suppress all output if "silent" mode requested		*/
-	if (gd->flags & GD_FLG_SILENT)
-		outputdev = search_device (DEV_FLAGS_OUTPUT, "nulldev");
+		gd->flags |= GD_FLG_SILENT;
 #endif
 
 	/* Scan devices looking for input and output devices */
diff --git a/common/main.c b/common/main.c
index ef28b3f..1ff3296 100644
--- a/common/main.c
+++ b/common/main.c
@@ -113,15 +113,17 @@ static __inline__ int abortboot(int boot
 	u_int i;
 
 #ifdef CONFIG_SILENT_CONSOLE
+	int silent = 0;
+
 	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
+		/* Restore console */
+		gd->flags &= ~GD_FLG_SILENT;
+		silent = 1;
 	}
 #endif
 
 #  ifdef CONFIG_AUTOBOOT_PROMPT
-	printf (CONFIG_AUTOBOOT_PROMPT, bootdelay);
+	serial_printf (CONFIG_AUTOBOOT_PROMPT, bootdelay);
 #  endif
 
 #  ifdef CONFIG_AUTOBOOT_DELAY_STR
@@ -199,14 +201,8 @@ #  if DEBUG_BOOTKEYS
 #  endif
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (!abort && silent)
+		gd->flags |= GD_FLG_SILENT;
 #endif
 
 	return abort;
@@ -223,10 +219,12 @@ static __inline__ int abortboot(int boot
 	int abort = 0;
 
 #ifdef CONFIG_SILENT_CONSOLE
+	int silent = 0;
+
 	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
+		/* Restore output */
+		silent = 1;
+		gd->flags &= ~GD_FLG_SILENT;
 	}
 #endif
 
@@ -275,14 +273,8 @@ # endif
 	putc ('\n');
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (!abort && silent)
+		gd->flags |= GD_FLG_SILENT;
 #endif
 
 	return abort;

[U-Boot-Users] [PATCH] simplify silent console

[Stefan Roese]

Hi Ladis,

On Tuesday 25 July 2006 17:30, Ladislav Michl wrote:
> silent console output is currently implemented by assigning nulldev
> as output device. This is a bit overcomplicated, since there is
> also GD_FLG_SILENT flag.
>
> Patch bellow tries to simplify silencing console by honouring
> GD_FLG_SILENT in console output functions, so there is no need to
> mess with output device. Comments?

Good idea. One thing though: After applying your patch the system _does_ print 
the bootcount messages "Hit any key to stop autoboot: ..." even if 
the "silent" variable is set. This was not done with the previous 
implementation and is not intended. Could you please rework your patch, that 
no message is printed here?

And please update the README.silent documentation too to match the updated 
implementation.

Thanks.

Best regards,
Stefan

[U-Boot-Users] [PATCH] simplify silent console

[Ladislav Michl]

On Mon, Oct 30, 2006 at 09:25:14AM +0100, Stefan Roese wrote:
> Hi Ladis,
> 
> On Tuesday 25 July 2006 17:30, Ladislav Michl wrote:
> > silent console output is currently implemented by assigning nulldev
> > as output device. This is a bit overcomplicated, since there is
> > also GD_FLG_SILENT flag.
> >
> > Patch bellow tries to simplify silencing console by honouring
> > GD_FLG_SILENT in console output functions, so there is no need to
> > mess with output device. Comments?
> 
> Good idea. One thing though: After applying your patch the system _does_ print 
> the bootcount messages "Hit any key to stop autoboot: ..." even if 
> the "silent" variable is set. This was not done with the previous 
> implementation and is not intended. Could you please rework your patch, that 
> no message is printed here?

It is intended. See a bit older thread here:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/17845/

On Thu, Jun 30, 2005 at 05:43:08PM -0400, Dave Ellis wrote:
| I think I see the problem. CONFIG_AUTOBOOT_PROMPT should be
| displayed, but it isn't because printf() is checking GD_FLG_SILENT.
| The cleanest solution I see is to check GD_FLG_SILENT only when
| sending directly to the handler, not when sending to the device.

Proposed change doesn't use nulldev at all and also doesn't set stdout
to nulldev and "back" [*] to serial.

[*] What is user wants to use usbtty? Safe way is not to deal with
output device at all. This way output that implies stdout is silenced - 
i.e. printf("Hello"); and output that specifies it would still work -
i.e. fprintf(stdout, "Hello"); This is used to get 'stop autoboot' prompt.

> And please update the README.silent documentation too to match the updated 
> implementation.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/common/main.c b/common/main.c
index cc4b50f..e637bbf 100644
--- a/common/main.c
+++ b/common/main.c
@@ -112,16 +112,8 @@ static __inline__ int abortboot(int bootdelay)
 	u_int presskey_max = 0;
 	u_int i;
 
-#ifdef CONFIG_SILENT_CONSOLE
-	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
-	}
-#endif
-
 #  ifdef CONFIG_AUTOBOOT_PROMPT
-	printf (CONFIG_AUTOBOOT_PROMPT, bootdelay);
+	fprintf(stdout, CONFIG_AUTOBOOT_PROMPT, bootdelay);
 #  endif
 
 #  ifdef CONFIG_AUTOBOOT_DELAY_STR
@@ -151,7 +143,7 @@ static __inline__ int abortboot(int bootdelay)
 				    presskey_max : delaykey[i].len;
 
 #  if DEBUG_BOOTKEYS
-		printf("%s key:<%s>\n",
+		fprintf(stdout, "%s key:<%s>\n",
 		       delaykey[i].retry ? "delay" : "stop",
 		       delaykey[i].str ? delaykey[i].str : "NULL");
 #  endif
@@ -168,7 +160,7 @@ static __inline__ int abortboot(int bootdelay)
 				    delaykey[i].str,
 				    delaykey[i].len) == 0) {
 #  if DEBUG_BOOTKEYS
-				printf("got %skey\n",
+				fprintf(stdout, "got %skey\n",
 				       delaykey[i].retry ? "delay" : "stop");
 #  endif
 
@@ -195,18 +187,12 @@ static __inline__ int abortboot(int bootdelay)
 	}
 #  if DEBUG_BOOTKEYS
 	if (!abort)
-		puts ("key timeout\n");
+		fputs(stdout, "key timeout\n");
 #  endif
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (abort)
+		gd->flags &= ~GD_FLG_SILENT;
 #endif
 
 	return abort;
@@ -222,18 +208,10 @@ static __inline__ int abortboot(int bootdelay)
 {
 	int abort = 0;
 
-#ifdef CONFIG_SILENT_CONSOLE
-	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
-	}
-#endif
-
 #ifdef CONFIG_MENUPROMPT
-	printf(CONFIG_MENUPROMPT, bootdelay);
+	fprintf(stdout, CONFIG_MENUPROMPT, bootdelay);
 #else
-	printf("Hit any key to stop autoboot: %2d ", bootdelay);
+	fprintf(stdout, "Hit any key to stop autoboot: %2d ", bootdelay);
 #endif
 
 #if defined CONFIG_ZERO_BOOTDELAY_CHECK
@@ -244,8 +222,8 @@ static __inline__ int abortboot(int bootdelay)
 	if (bootdelay >= 0) {
 		if (tstc()) {	/* we got a key press	*/
 			(void) getc();  /* consume input	*/
-			puts ("\b\b\b 0");
-			abort = 1; 	/* don't auto boot	*/
+			fputs(stdout, "\b\b\b 0");
+			abort = 1;	/* don't auto boot	*/
 		}
 	}
 #endif
@@ -269,20 +247,14 @@ static __inline__ int abortboot(int bootdelay)
 			udelay (10000);
 		}
 
-		printf ("\b\b\b%2d ", bootdelay);
+		fprintf(stdout, "\b\b\b%2d ", bootdelay);
 	}
 
-	putc ('\n');
+	fputc(stdout, '\n');
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (abort)
+		gd->flags &= ~GD_FLG_SILENT;
 #endif
 
 	return abort;

[U-Boot-Users] [PATCH] simplify silent console

[Wolfgang Denk]

In message <20070424120207.GA28852@michl.2n.cz> you wrote:
>
> > Good idea. One thing though: After applying your patch the system _does_ print 
> > the bootcount messages "Hit any key to stop autoboot: ..." even if 
> > the "silent" variable is set. This was not done with the previous 
> > implementation and is not intended. Could you please rework your patch, that 
> > no message is printed here?
> 
> It is intended. See a bit older thread here:
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/17845/

Please don't. "silent" means "silent" means "silent".

Assume there is amodem attached to the serial port  -  it  would  get
seriously confused by the countdown messages.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Generally speaking, there are other ways to accomplish whatever it is
that you think you need ...                               - Doug Gwyn

[U-Boot-Users] [PATCH] simplify silent console

[Ladislav Michl]

On Tue, Apr 24, 2007 at 02:10:19PM +0200, Wolfgang Denk wrote:
> > It is intended. See a bit older thread here:
> > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/17845/
> 
> Please don't. "silent" means "silent" means "silent".
> 
> Assume there is amodem attached to the serial port  -  it  would  get
> seriously confused by the countdown messages.

This is a reasonable assumption. Updated patch follows.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/common/console.c b/common/console.c
index e9f23be..d8a0cb6 100644
--- a/common/console.c
+++ b/common/console.c
@@ -494,13 +494,7 @@ int console_init_r (void)
 	/* suppress all output if splash screen is enabled and we have
 	   a bmp to display                                            */
 	if (getenv("splashimage") != NULL)
-		outputdev = search_device (DEV_FLAGS_OUTPUT, "nulldev");
-#endif
-
-#ifdef CONFIG_SILENT_CONSOLE
-	/* Suppress all output if "silent" mode requested		*/
-	if (gd->flags & GD_FLG_SILENT)
-		outputdev = search_device (DEV_FLAGS_OUTPUT, "nulldev");
+		gd->flags |= GD_FLG_SILENT;
 #endif
 
 	/* Scan devices looking for input and output devices */
diff --git a/common/main.c b/common/main.c
index cc4b50f..0003da2 100644
--- a/common/main.c
+++ b/common/main.c
@@ -112,16 +112,8 @@ static __inline__ int abortboot(int bootdelay)
 	u_int presskey_max = 0;
 	u_int i;
 
-#ifdef CONFIG_SILENT_CONSOLE
-	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
-	}
-#endif
-
 #  ifdef CONFIG_AUTOBOOT_PROMPT
-	printf (CONFIG_AUTOBOOT_PROMPT, bootdelay);
+	printf(CONFIG_AUTOBOOT_PROMPT, bootdelay);
 #  endif
 
 #  ifdef CONFIG_AUTOBOOT_DELAY_STR
@@ -195,18 +187,12 @@ static __inline__ int abortboot(int bootdelay)
 	}
 #  if DEBUG_BOOTKEYS
 	if (!abort)
-		puts ("key timeout\n");
+		puts("key timeout\n");
 #  endif
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (abort)
+		gd->flags &= ~GD_FLG_SILENT;
 #endif
 
 	return abort;
@@ -222,14 +208,6 @@ static __inline__ int abortboot(int bootdelay)
 {
 	int abort = 0;
 
-#ifdef CONFIG_SILENT_CONSOLE
-	if (gd->flags & GD_FLG_SILENT) {
-		/* Restore serial console */
-		console_assign (stdout, "serial");
-		console_assign (stderr, "serial");
-	}
-#endif
-
 #ifdef CONFIG_MENUPROMPT
 	printf(CONFIG_MENUPROMPT, bootdelay);
 #else
@@ -244,8 +222,8 @@ static __inline__ int abortboot(int bootdelay)
 	if (bootdelay >= 0) {
 		if (tstc()) {	/* we got a key press	*/
 			(void) getc();  /* consume input	*/
-			puts ("\b\b\b 0");
-			abort = 1; 	/* don't auto boot	*/
+			puts("\b\b\b 0");
+			abort = 1;	/* don't auto boot	*/
 		}
 	}
 #endif
@@ -266,23 +244,17 @@ static __inline__ int abortboot(int bootdelay)
 # endif
 				break;
 			}
-			udelay (10000);
+			udelay(10000);
 		}
 
-		printf ("\b\b\b%2d ", bootdelay);
+		printf("\b\b\b%2d ", bootdelay);
 	}
 
-	putc ('\n');
+	putc('\n');
 
 #ifdef CONFIG_SILENT_CONSOLE
-	if (abort) {
-		/* permanently enable normal console output */
-		gd->flags &= ~(GD_FLG_SILENT);
-	} else if (gd->flags & GD_FLG_SILENT) {
-		/* Restore silent console */
-		console_assign (stdout, "nulldev");
-		console_assign (stderr, "nulldev");
-	}
+	if (abort)
+		gd->flags &= ~GD_FLG_SILENT;
 #endif
 
 	return abort;