[U-Boot,16/17] console: Enable function to display console info

Submitted by Simon Glass on Nov. 3, 2012, 12:27 a.m.

Details

Message ID 1351902453-27956-17-git-send-email-sjg@chromium.org
State Superseded, archived
Delegated to: Tom Rini
Headers show

Commit Message

Simon Glass Nov. 3, 2012, 12:27 a.m.
The CONFIG_SYS_CONSOLE_INFO_QUIET option should suppress the console
information, but allow boards to display it later if required. Adjust
the code to support this.

This is used to avoid printing the information while the LCD display
is not ready, since it only becomes ready when stdio init is complete.

BRANCH=snow
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/console.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Wolfgang Denk Nov. 3, 2012, 3:15 p.m.
Dear Simon Glass,

In message <1351902453-27956-17-git-send-email-sjg@chromium.org> you wrote:
> The CONFIG_SYS_CONSOLE_INFO_QUIET option should suppress the console
> information, but allow boards to display it later if required. Adjust
> the code to support this.
> 
> This is used to avoid printing the information while the LCD display
> is not ready, since it only becomes ready when stdio init is complete.
> 
> BRANCH=snow

Please get such comments out of the commit messages.

> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  common/console.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)

This is broken.  You miss the fact that stdio_print_current_devices()
gets called in a number of other places as well (you should have
asked yourself why it isn't a static function).  See at least
board/mpl/mip405/mip405.c

Best regards,

Wolfgang Denk
Simon Glass Nov. 15, 2012, 11:24 p.m.
HI Wolfgang,

On Sat, Nov 3, 2012 at 8:15 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1351902453-27956-17-git-send-email-sjg@chromium.org> you wrote:
>> The CONFIG_SYS_CONSOLE_INFO_QUIET option should suppress the console
>> information, but allow boards to display it later if required. Adjust
>> the code to support this.
>>
>> This is used to avoid printing the information while the LCD display
>> is not ready, since it only becomes ready when stdio init is complete.
>>
>> BRANCH=snow
>
> Please get such comments out of the commit messages.

Yes done, Tom has merged the patman patch for this.

>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  common/console.c |    6 ++++--
>>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> This is broken.  You miss the fact that stdio_print_current_devices()
> gets called in a number of other places as well (you should have
> asked yourself why it isn't a static function).  See at least
> board/mpl/mip405/mip405.c

Yes I realise that, but why would a board specifically call
stdio_print_current_devices() if the board's config has defined
CONFIG_SYS_CONSOLE_INFO_QUIET?

It seems to me that if we want to print it, we should be able to call
the function to do so, and that the option is only there to stop the
generic code from printing it when we don't want it.

The README says only:

- CONFIG_SYS_CONSOLE_INFO_QUIET
		Suppress display of console information at boot.

Or do I misunderstand this?

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> "In matrimony, to hesitate is sometimes to be saved."        - Butler

Patch hide | download patch | download mbox

diff --git a/common/console.c b/common/console.c
index 831897b..d3bbc26 100644
--- a/common/console.c
+++ b/common/console.c
@@ -593,7 +593,6 @@  int console_init_f(void)
 
 void stdio_print_current_devices(void)
 {
-#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET
 	/* Print information */
 	puts("In:    ");
 	if (stdio_devices[stdin] == NULL) {
@@ -615,7 +614,6 @@  void stdio_print_current_devices(void)
 	} else {
 		printf ("%s\n", stdio_devices[stderr]->name);
 	}
-#endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
 }
 
 #ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
@@ -694,7 +692,9 @@  done:
 
 	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
 
+#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET
 	stdio_print_current_devices();
+#endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
 
 #ifdef CONFIG_SYS_CONSOLE_ENV_OVERWRITE
 	/* set the environment variables (will overwrite previous env settings) */
@@ -769,7 +769,9 @@  int console_init_r(void)
 
 	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
 
+#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET
 	stdio_print_current_devices();
+#endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
 
 	/* Setting environment variables */
 	for (i = 0; i < 3; i++) {