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

login
register
mail settings
Submitter Simon Glass
Date Nov. 3, 2012, 12:27 a.m.
Message ID <1351902453-27956-17-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/196783/
State Superseded, archived
Delegated to: Tom Rini
Headers show

Comments

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

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