[U-Boot] [PATCH v2 2/3] Allow displaying the U-Boot banner on a video display

[Simon Glass]

Hi Bin,

On 15 June 2017 at 19:12, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Fri, Jun 16, 2017 at 12:09 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 12 June 2017 at 19:10, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Mon, Jun 12, 2017 at 11:17 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> Hi Bin,
>>>>
>>>> On 12 June 2017 at 09:13, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>> Hi Simon,
>>>>>
>>>>> On Sun, Jun 11, 2017 at 1:59 AM, Simon Glass <sjg at chromium.org> wrote:
>>>>>> At present the U-Boot banner is only displayed on the serial console. If
>>>>>> this is not visible to the user, the banner does not show. Some devices
>>>>>> have a video display which can usefully display this information.
>>>>>>
>>>>>> Add a banner which is printed after relocation only on non-serial devices
>>>>>> if CONFIG_DISPLAY_BOARDINFO_LATE is defined.
>>>>>>
>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Reword function comment for console_announce_r() slighty
>>>>>>
>>>>>>  common/board_r.c  |  1 +
>>>>>>  common/console.c  | 15 +++++++++++----
>>>>>>  include/console.h | 12 ++++++++++++
>>>>>>  3 files changed, 24 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/common/board_r.c b/common/board_r.c
>>>>>> index 15977e4bca..ff11eba5d3 100644
>>>>>> --- a/common/board_r.c
>>>>>> +++ b/common/board_r.c
>>>>>> @@ -846,6 +846,7 @@ static init_fnc_t init_sequence_r[] = {
>>>>>>  #endif
>>>>>>         console_init_r,         /* fully init console as a device */
>>>>>>  #ifdef CONFIG_DISPLAY_BOARDINFO_LATE
>>>>>> +       console_announce_r,
>>>>>>         show_board_info,
>>>>>>  #endif
>>>>>>  #ifdef CONFIG_ARCH_MISC_INIT
>>>>>> diff --git a/common/console.c b/common/console.c
>>>>>> index 1232808df5..3fcd7ce66b 100644
>>>>>> --- a/common/console.c
>>>>>> +++ b/common/console.c
>>>>>> @@ -202,7 +202,6 @@ static void console_putc(int file, const char c)
>>>>>>         }
>>>>>>  }
>>>>>>
>>>>>> -#if CONFIG_IS_ENABLED(PRE_CONSOLE_BUFFER)
>>>>>
>>>>> Why removing this? Without PRE_CONSOLE_BUFFER, I doubt it works.
>>>>
>>>> I want to be able to call the function below. It seems to work OK and
>>>> it doesn't rely on that option.
>>>>
>>>
>>> See comments below.
>>>
>>>>>
>>>>>>  static void console_puts_noserial(int file, const char *s)
>>>>>>  {
>>>>>>         int i;
>>>>>> @@ -214,7 +213,6 @@ static void console_puts_noserial(int file, const char *s)
>>>>>>                         dev->puts(dev, s);
>>>>>>         }
>>>>>>  }
>>>>>> -#endif
>>>>>>
>>>>>>  static void console_puts(int file, const char *s)
>>>>>>  {
>>>>>> @@ -248,13 +246,11 @@ static inline void console_putc(int file, const char c)
>>>>>>         stdio_devices[file]->putc(stdio_devices[file], c);
>>>>>>  }
>>>>>>
>>>>>> -#if CONFIG_IS_ENABLED(PRE_CONSOLE_BUFFER)
>>>>>>  static inline void console_puts_noserial(int file, const char *s)
>>>>>>  {
>>>>>>         if (strcmp(stdio_devices[file]->name, "serial") != 0)
>>>>>>                 stdio_devices[file]->puts(stdio_devices[file], s);
>>>>>>  }
>>>>>> -#endif
>>>>>>
>>>>>>  static inline void console_puts(int file, const char *s)
>>>>>>  {
>>>>>> @@ -699,6 +695,17 @@ static void console_update_silent(void)
>>>>>>  #endif
>>>>>>  }
>>>>>>
>>>>>> +int console_announce_r(void)
>>>>>> +{
>>>>>> +       char buf[DISPLAY_OPTIONS_BANNER_LENGTH];
>>>>>> +
>>>>>> +       display_options_get_banner(false, buf, sizeof(buf));
>>>>>> +
>>>>>> +       console_puts_noserial(stdout, buf);
>>>
>>> Then I think we should do something like this:
>>>
>>> int console_announce_r(void)
>>> {
>>> #ifndef CONFIG_PRE_CONSOLE_BUFFER
>>>         char buf[DISPLAY_OPTIONS_BANNER_LENGTH];
>>>
>>>         display_options_get_banner(false, buf, sizeof(buf));
>>>         console_puts_noserial(stdout, buf);
>>> #endif
>>> }
>>>
>>> If we don't do this, there will be duplicated U-Boot version string
>>> shown on the screen if CONFIG_PRE_CONSOLE_BUFFER is defined.
>>
>> I don't think so. This is a different thing. The pre-console buffer
>> only exists until console_init_f(), i.e. very early in pre-relocation
>> U-Boot. When console_init_f() is called the buffer is printed to the
>> (serial) console.
>>
>
> But in fact, the version string will be printed twice on the screen
> (not on the serial) if CONFIG_PRE_CONSOLE_BUFFER is defined. One
> existed in the pre-console buffer, as you mentioned. The other one is
> what you added here.

OK I see what you mean. I didn't know (or forgot) that the pre-console
buffer emitted its output later also.

I think I'll enable it for sandbox and try to rationalise the code a
little. Thanks for spotting this!

>
>> My patch is all about what happens after relocation. By this time the
>> pre-console buffer has been sent to the serial port. I want to display
>> the version number on the screen, and this should not affect the
>> pre-console buffer in any way.
>>
>
> [snip]
>
> Regards,
> Bin

Regards,
Simon