On 07/28/2017 09:57 PM, Simon Glass wrote:
Hi Stephen,
On 27 July 2017 at 15:27, Stephen Warren swarren@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:
- If SPL will print a sign-on: Wait for SPL to print sign-on
- Wait for main U-Boot to print a sign-on
- 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.
But it's doing it in a much more "after-the-fact" and brittle way.
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:
- If SPL will print a sign-on: Wait for SPL to print sign-on
- Wait for main U-Boot to print a sign-on
- If a duplicate sign-on message will be printed: Wait for duplicate sign-on from main U-Boot
- 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.
Yes, if patches 2/3 fix the root-cause, then I'd agree let's just apply those instead.