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?
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).
--
Tom
signature.asc
Description: PGP signature
