[U-Boot] [PATCH 1/3] test/py: Handle the banner being printed after relocation

[Simon Glass]

Hi Stephen,

On 27 July 2017 at 15:27, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 07/27/2017 09:31 AM, Simon Glass wrote:
>>
>> If CONFIG_DISPLAY_BOARDINFO_LATE is enabled, U-Boot displays the banner
>> again after relocation so that it is visible on the video display. Detect
>> this and allow it.
>>
>> Note: This patch is only an interim fix. If it is applied we should
>> consider reverting it later since it is not needed if U-Boot is working
>> correctly.
>
>
> It'd be useful if that paragraph could be expanded a bit; what exactly is
> the bug and how will it be fixed? The situation described in the first
> paragraph doesn't seem like a bug. Perhaps the fix be to delay the sign-on
> message until video output is available and only print it once, or not print
> the sign-on to serial when printing it a second time so that it's seen on
> video?

The bug is described /fixed in patches 2 & 3 of this series. I sent
this patch in case there was urgency in fixing it.

>
>> diff --git a/test/py/u_boot_console_base.py
>> b/test/py/u_boot_console_base.py
>
>
>> @@ -355,12 +355,31 @@ class ConsoleBase(object):
>>               self.u_boot_version_string = self.p.after
>>               while True:
>>                   m = self.p.expect([self.prompt_compiled,
>> -                    pattern_stop_autoboot_prompt] + self.bad_patterns)
>> +                    pattern_stop_autoboot_prompt,
>> pattern_u_boot_main_signon] +
>> +                    self.bad_patterns)
>
>
> This doesn't seem quite right. At a very high level, the current code does
> this:
>
> 1) If SPL will print a sign-on:
>      Wait for SPL to print sign-on
> 2) Wait for main U-Boot to print a sign-on
> 3) Wait for auto-boot prompt and interrupt it.
>
> This change modifies (3) to alternatively wait for another sign-on prompt,

Well actually the existing code already does this if you look closely.
If it sees second sign-on it detects this and fails the test
(presumably because it indicates there was an unexpected reboot).
Otherwise I would not have bothered with this patch.

> which means it's doing 2 unrelated things. Better would be to just add a new
> standalone step to wait for the extra sign-on message:
>
> 1) If SPL will print a sign-on:
>      Wait for SPL to print sign-on
> 2) Wait for main U-Boot to print a sign-on
> 3) If a duplicate sign-on message will be printed:
>      Wait for duplicate sign-on from main U-Boot
> 4) Wait for auto-boot prompt and interrupt it.
>
> Judging by the code quoted below, new step (3) above can use the value of
> bcfg.get('config_display_boardinfo_late', 'n') == 'y' to make the decision.
>
> That has the advantage of not requiring the following heuristic to
> differentiate between expected sign-on messages and unexpected sign-on
> messages due to U-Boot crashing and restarting:

Yes I think that would work. I did consider making it explicit on that
option, which is indeed what causes the problem.

But if there are no objections I think it is best to skip this interim
patch and apply the real fix (patches 2, 3) once they get some review.

>
>
>> +                # If CONFIG_DISPLAY_BOARDINFO_LATE is defined then we
>> will
>> +                # display the banner again after relocation. See
>> +                # console_announce_r() for the implementation. The
>> original
>> +                # banner has two blank lines before it but this one has
>> none.
>> +                # We can use this to distinguish a repeat of the banner
>> from
>> +                # a reset of the board. Here we fail if we see even a
>> single
>> +                # empty line in the characters leading up to the banner.

Regards,
Simon