On Thu, Oct 31, 2024 at 07:03:19PM +0100, Simon Glass wrote:
Hi Tom,
On Fri, 27 Sept 2024 at 04:52, Tom Rini trini@konsulko.com wrote:
On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
Hi Tom,
On Wed, 25 Sept 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
Hi Tom,
On Mon, 23 Sept 2024 at 22:35, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> When Labgrid is used, it can get U-Boot ready for running tests. It > prints a message when it has done so. > > Add logic to detect this message and accept it. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > (no changes since v1) > > test/py/u_boot_console_base.py | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-)
What happens is that labgrid can also be told to look for and then interrupt autoboot, just like our pytests can do, and the system is at the prompt. But this is also what it's like for a system with autoboot disabled. Do we actually need this patch to achieve the functionality you want? Doesn't that already just happen?
The point of this patch is actually to remove code in pytest, by allowing it to skip all the banner-detection stuff. It does not affect things in Labgrid, since it still needs to watch for banners, etc.
But you can't remove code from pytest, people can and will run the suite outside of labgrid.
Yes, that's right. I meant that with Labgrid the code is not used.
OK. I still think this (and I assume the related flag to tell whichever helper that was that the platform is ready to go) are too implementation specific.
OK, but it does have the virtue of working properly on all the boards I have, rather than just a subset, without this patch.
Without this patch, we have to tell Labgrid's U-Boot driver to do nothing, so that pytest does it. But that is not a good idea, since Labgrid has a lot more info about the board than pytest has. For example, look at all the SPL-banner-count stuff.
Why do you have to tell it to do nothing? The pytest suite works fine, today, if the board stops at the prompt automatically. To be clear, the labgrid yaml file I'm using with my scripts has the information to stop autoboot in it and it's not causing a problem.
But if it wasn't causing a problem I would not have invented this annoying scheme.
Well, I honestly thought it might be a remnant from development that wasn't needed once everything else was done. I do that from time to time myself.
OK.
For several boards, by the time pytest gets to see the output it is too late to press a key and stop.
Why / how? Do we perhaps need to adjust the timeout upwards, slightly? Especially in light of the changes you also did to detect a "dead" board and so we don't wait 30 minutes for a test run to fail.
Why / how - because the board prints its SPL banner as soon as reset is released, then waits for U-Boot proper to be sent, at which case the U-Boot banner is sent .Then labgrid disconnects from the console and runs microcom, which connects to the console over ser2net and misses some of what has already been sent.
This is actually a fundamental problem, not something we can adjust with timeouts.
Wait, what? *Labgrid* is the thing that's the problem here with dropping the connection to the board and re-establishing it at some point?
That is the kind of detail which (a) needs to be in the commit message (b) perhaps an issue filed (or if already exists Link:'d in the commit message) and (c) would have made it clear why we need this work-around and then I'd have been a lot happier to accept it as a work-around back on v1 or so.
Also, some boards have a different prompt which is not detected by pytest.
For example: configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n"
Yeah, I had forgotten that as I turn that off along with turning on other tests via config fragment, on that platform. That really is a deficiency in our pytest version of this.
OK
Basically, without this patch we cannot use '-s uboot' to tell the Labgrid strategy to take us to a U-Boot prompt. We must just use a raw console with no strategy, relying on pytest to do all the work.
I hope that helps explain the problem?
I think I see what you're saying, and it's based on the assumption that we'll make everyone either use labgrid?
Not at all. I tested this version of the series again with my pre-Labgrid lab and it works fine. But I do believe that the hooks that pytest has are not ideal for use with Labgrid.
OK. But, why can't you make use of the labgrid functionality to pause the board? It's the state flag for labgrid-client yes? Dropping that in with my labgrid support would be just a tweak to the console script to check if the board set labgrid_strategy or something, and I think you could adapt yours to that too?
I am really not sure what to say here. I have explained the reality of the situation. I have spent hours debugging it and figuring out the root cause. There is no other tweak I can think of that will solve this problem.
Well, I hate to get on a tangent here, but I think a common thread is that after you've spent hours debugging a problem and working out a solution you then omit much of the details. I'd much rather see a much longer commit message here describing the problem, or a longer description under the "---" where you explain that labgrid uses minicom, gets things up, drops the connection and uses ser2net and so there's a chance we miss messages and so need something like this. Or, that you've been debugging on *x86_64* a bunch of EFI_LOADER things and off the shelf distributions aren't working (and ExitBootServices() is where you had a problem with Ubuntu I gather still on x86_64).
Regards, Simon