simplify silent console

PATCH New Ladislav Michl 4 participants 5 messages Jul 25, 2006 – Apr 24, 2007

lore

Share & tools

lore

Ladislav Michl

author

2006-07-25 15:30 UTC #0 lore raw

PATCH 1/1 simplify silent console

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

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

common/console.c +1 -7

index e9f23be..d8a0cb6 100644
@@ -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 */

common/main.c +15 ~~-23~~

index ef28b3f..1ff3296 100644
@@ -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;

Stefan Roese

2006-10-30 08:25 UTC #1 lore raw

PATCH 1/1 simplify silent console

Hi Ladis,

Quoted 7 lines · Ladislav Michl

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

Ladislav Michl

2007-04-24 12:02 UTC #2 lore raw

PATCH 1/1 simplify silent console

Quoted 11 lines · Stefan Roese

Hi Ladis,
Quoted 7 lines · Ladislav Michl
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?

Quoted 5 lines

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.

Quoted 2 lines

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

Signed-off-by Ladislav Michl <[email protected]>

common/main.c +14 ~~-42~~

index cc4b50f..e637bbf 100644
@@ -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;

Wolfgang Denk

2007-04-24 12:10 UTC #3 lore raw

PATCH 1/1 simplify silent console

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

Quoted 6 lines
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

Signature · 5 lines

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

Ladislav Michl

2007-04-24 12:25 UTC #4 lore raw

PATCH 1/1 simplify silent console

Quoted 7 lines · Wolfgang Denk

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

common/console.c +1 -7

index e9f23be..d8a0cb6 100644
@@ -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 */

common/main.c +11 ~~-39~~

90 lines not on this page · raw message