Hi Bin,
On 12 June 2017 at 19:10, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Jun 12, 2017 at 11:17 PM, Simon Glass <s...@chromium.org> wrote:
>> Hi Bin,
>>
>> On 12 June 2017 at 09:13, Bin Meng <bmeng...@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Sun, Jun 11, 2017 at 1:59 AM, Simon Glass <s...@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 <s...@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.
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.
>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> /* Called before relocation - use serial functions */
>>>> int console_init_f(void)
>>>> {
>>>> diff --git a/include/console.h b/include/console.h
>>>> index 3d37f6a53b..cea29ed6dc 100644
>>>> --- a/include/console.h
>>>> +++ b/include/console.h
>>>> @@ -42,6 +42,18 @@ void console_record_reset(void);
>>>> */
>>>> void console_record_reset_enable(void);
>>>>
>>>> +/**
>>>> + * console_announce_r() - print a U-Boot console on non-serial consoles
>>>> + *
>>>> + * When U-Boot starts up with a display it generally does not announce
>>>> itself
>>>> + * on the display. The banner is instead emitted on the UART before
>>>> relocation.
>>>> + * This function prints a banner on devices which (we assume) did not
>>>> receive
>>>> + * it before relocation.
>>>> + *
>>>> + * @return 0 (meaning no errors)
>>>> + */
>>>> +int console_announce_r(void);
>>>> +
>>>> /*
>>>> * CONSOLE multiplexing.
>>>> */
>>>> --
>>>
>>> And I see another (same) patch @
>>> https://patchwork.ozlabs.org/patch/769426/ which got applied, but this
>>> one was sent later than the applied one. I am confused..
>>
>> Another patch in this series caused a breakage using a long BUILD_TAG
>> due to a bug in how the snprintf() calls were done. So I dropped two
>> patches from that series and sent this series instead.
>>
>> It wasn't noticed since I don't use BUILD_TAG and no tests were in
>> place. It just happened that Stephen Warren has automated testing that
>> caught it before it was pulled to mainline.
>
> OK, thanks for the clarification.
>
> Regards,
> Bin
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
