Hi Tom, On Tue, 25 Jun 2024 at 15:27, Tom Rini <tr...@konsulko.com> wrote: > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <tr...@konsulko.com> wrote: > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote: > > > > > > > In Labgrid there is the concept of a 'role', which is similar to the > > > > U-Boot board ID in U-Boot's pytest subsystem. > > > > > > > > The role indicates both the target and information about the U-Boot > > > > build to use. It can also provide any amount of other configuration. > > > > The information is obtained using the 'labgrid-client query' operation. > > > > > > > > Make use of this in tests, so that only the role is required in gitlab > > > > and other situations. The board type and other things can be queried > > > > as needed. > > > > > > > > Use a new 'u-boot-test-getrole' script to obtain the requested > > > > information. > > > > > > > > With this it is possible to run lab tests in gitlab with just a single > > > > 'ROLE' variable for each board. > > > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > > --- > > > > > > > > (no changes since v1) > > > > > > > > test/py/conftest.py | 31 +++++++++++++++++++++++++++---- > > > > 1 file changed, 27 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/test/py/conftest.py b/test/py/conftest.py > > > > index 6547c6922c6..5de8d7b0e23 100644 > > > > --- a/test/py/conftest.py > > > > +++ b/test/py/conftest.py > > > > @@ -23,6 +23,7 @@ from pathlib import Path > > > > import pytest > > > > import re > > > > from _pytest.runner import runtestprotocol > > > > +import subprocess > > > > import sys > > > > > > > > # Globals: The HTML log file, and the connection to the U-Boot console. > > > > @@ -79,6 +80,7 @@ def pytest_addoption(parser): > > > > parser.addoption('--gdbserver', default=None, > > > > help='Run sandbox under gdbserver. The argument is the channel > > > > '+ > > > > 'over which gdbserver should communicate, e.g. localhost:1234') > > > > + parser.addoption('--role', help='U-Boot board role (for Labgrid)') > > > > parser.addoption('--no-prompt-wait', default=False, > > > > action='store_true', > > > > help="Assume that U-Boot is ready and don't wait for a prompt") > > > > > > > > @@ -130,12 +132,33 @@ def get_details(config): > > > > str: Build directory > > > > str: Source directory > > > > """ > > > > - board_type = config.getoption('board_type') > > > > - board_identity = config.getoption('board_identity') > > > > + role = config.getoption('role') > > > > build_dir = config.getoption('build_dir') > > > > + if role: > > > > + board_identity = role > > > > + cmd = ['u-boot-test-getrole', role, '--configure'] > > > > + env = os.environ.copy() > > > > + if build_dir: > > > > + env['U_BOOT_BUILD_DIR'] = build_dir > > > > + proc = subprocess.run(cmd, capture_output=True, > > > > encoding='utf-8', > > > > + env=env) > > > > + if proc.returncode: > > > > + raise ValueError(proc.stderr) > > > > + print('conftest: lab:', proc.stdout) > > > > + vals = {} > > > > + for line in proc.stdout.splitlines(): > > > > + item, value = line.split(' ', maxsplit=1) > > > > + k = item.split(':')[-1] > > > > + vals[k] = value > > > > + print('conftest: lab info:', vals) > > > > + board_type, default_build_dir, source_dir = (vals['board'], > > > > + vals['build_dir'], vals['source_dir']) > > > > + else: > > > > + board_type = config.getoption('board_type') > > > > + board_identity = config.getoption('board_identity') > > > > > > > > - source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) > > > > - default_build_dir = source_dir + '/build-' + board_type > > > > + source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) > > > > + default_build_dir = source_dir + '/build-' + board_type > > > > if not build_dir: > > > > build_dir = default_build_dir > > > > > > I'm a little confused here. Why can't we construct "role" from > > > board_type+board_identity and then we have the board > > > conf.${board_type}_${board_identity} file set whatever needs setting to > > > be "labgrid" ? > > > > The role is equivalent to the board identity, not the combination of > > the U-Boot board type and the board identity. I went this way to avoid > > having to type long and complicated roles when connecting to boards. > > It is similar to how things work today, except that the board type is > > implied by the 'role'. > > > > For boards which have multiple identities (e.g. can support two > > different board types), Labgrid handles acquiring and releasing the > > shares resources, to avoid any problems. > > I guess I have two sets of questions. First, if it's basically the > board identity why can't we just use that as the role name, in your > setup?
Yes, that's what I am doing. If you look in console.labgrid you can see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument. > > But second, I'm not sure why we need this. The labgrid support we worked > up here (but, sigh, it's not really clean enough to post) has: > $ cat bin/lootbox/conf.rpi_4_na > console_impl=labgrid > reset_impl=labgrid > flash_impl=labgrid.rpi_4 > flash_writer=labgrid.rpi_4 > > For example and: > $ cat bin/writer.labgrid.rpi_4 > set -e > > build=${U_BOOT_BUILD_DIR} > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img > > So I don't know what the "role" part you have is for. And yes, there end > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms > (such as beagleplay) we have: > $ cat bin/writer.labgrid.ti-k3 > set -e > build=${U_BOOT_BUILD_DIR} > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw" > echo "per the board documentation." > exit 1 > fi > echo "Writing build at ${build}" > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin > echo "Done writing build" > > In all cases we first build U-Boot and then pass the build directory to > test.py, in something like: > export LG_PLACE=rpi4 > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \ > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \ > --exitfirst > > The only U-Boot side changes I needed to make were for counting the SPL > banner instances, and that was regardless of framework (a particularly > fun platform will show it 3 times). Yes it is possible to build U-Boot separately. Of course that involved various blobs and so on, so every board is different. The approach I have taken here is to have Labgrid call buildman to build what is needed, with the blobs defined in the environment. You can use the -B flag to use a pre-built U-Boot, with the scripts I've included. Regards, Simon