Hi Sughosh, On Tue, 1 Aug 2023 at 06:29, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Wed, 26 Jul 2023 at 04:06, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 20 Jul 2023 at 03:20, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > hi Simon, > > > > > > On Thu, 20 Jul 2023 at 00:41, Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Wed, 19 Jul 2023 at 02:42, Sughosh Ganu <sughosh.g...@linaro.org> > > > > wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Wed, 19 Jul 2023 at 06:41, Simon Glass <s...@chromium.org> wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > On Mon, 17 Jul 2023 at 04:44, Sughosh Ganu > > > > > > <sughosh.g...@linaro.org> wrote: > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > On Sun, 16 Jul 2023 at 05:12, Simon Glass <s...@chromium.org> > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu > > > > > > > > <sughosh.g...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Add support in binman for generating capsules. The capsule > > > > > > > > > parameters > > > > > > > > > can be specified either through a config file or through the > > > > > > > > > capsule > > > > > > > > > binman entry. Also add test cases in binman for capsule > > > > > > > > > generation, > > > > > > > > > and enable this testing on the sandbox_spl variant. > > > > > > > > > > > > > > > > Can you use sandbox instead, or perhaps sandbox_spl? SPL is > > > > > > > > really for > > > > > > > > SPL testing. > > > > > > > > > > > > > > Er, I am actually using the sandbox_spl variant. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > > > > > > > > --- > > > > > > > > > Changes since V3: > > > > > > > > > * Add test cases for covering the various capsule generation > > > > > > > > > scenarios. > > > > > > > > > * Add function comments in the mkeficapsule bintool. > > > > > > > > > * Fix the fetch method of the mkeficapsule bintool to enable > > > > > > > > > building > > > > > > > > > the tool. > > > > > > > > > * Add more details about the capsule parameters in the > > > > > > > > > documentation > > > > > > > > > as well as the code. > > > > > > > > > * Fix order of module imports, and addition of blank lines in > > > > > > > > > the > > > > > > > > > capsule.py file. > > > > > > > > > * Use SetContents in the ObtainContents method. > > > > > > > > > > > > > > > > > > configs/sandbox_spl_defconfig | 1 + > > > > > > > > > tools/binman/btool/mkeficapsule.py | 158 > > > > > > > > > ++++++++++++++++++ > > > > > > > > > tools/binman/entries.rst | 37 ++++ > > > > > > > > > tools/binman/etype/capsule.py | 132 > > > > > > > > > +++++++++++++++ > > > > > > > > > tools/binman/ftest.py | 127 > > > > > > > > > ++++++++++++++ > > > > > > > > > tools/binman/test/282_capsule.dts | 18 ++ > > > > > > > > > tools/binman/test/283_capsule_signed.dts | 20 +++ > > > > > > > > > tools/binman/test/284_capsule_conf.dts | 14 ++ > > > > > > > > > tools/binman/test/285_capsule_missing_key.dts | 19 +++ > > > > > > > > > .../binman/test/286_capsule_missing_index.dts | 17 ++ > > > > > > > > > .../binman/test/287_capsule_missing_guid.dts | 17 ++ > > > > > > > > > .../test/288_capsule_missing_payload.dts | 17 ++ > > > > > > > > > tools/binman/test/289_capsule_missing.dts | 17 ++ > > > > > > > > > tools/binman/test/290_capsule_version.dts | 19 +++ > > > > > > > > > tools/binman/test/capsule_cfg.txt | 6 + > > > > > > > > > 15 files changed, 619 insertions(+) > > > > > > > > > create mode 100644 tools/binman/btool/mkeficapsule.py > > > > > > > > > create mode 100644 tools/binman/etype/capsule.py > > > > > > > > > create mode 100644 tools/binman/test/282_capsule.dts > > > > > > > > > create mode 100644 tools/binman/test/283_capsule_signed.dts > > > > > > > > > create mode 100644 tools/binman/test/284_capsule_conf.dts > > > > > > > > > create mode 100644 > > > > > > > > > tools/binman/test/285_capsule_missing_key.dts > > > > > > > > > create mode 100644 > > > > > > > > > tools/binman/test/286_capsule_missing_index.dts > > > > > > > > > create mode 100644 > > > > > > > > > tools/binman/test/287_capsule_missing_guid.dts > > > > > > > > > create mode 100644 > > > > > > > > > tools/binman/test/288_capsule_missing_payload.dts > > > > > > > > > create mode 100644 tools/binman/test/289_capsule_missing.dts > > > > > > > > > create mode 100644 tools/binman/test/290_capsule_version.dts > > > > > > > > > create mode 100644 tools/binman/test/capsule_cfg.txt > > > > > > > > > > > > > > > > This looks pretty good to me. Some nits below > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/configs/sandbox_spl_defconfig > > > > > > > > > b/configs/sandbox_spl_defconfig > > > > > > > > > index dd848c57c6..2fcc789347 100644 > > > > > > > > > --- a/configs/sandbox_spl_defconfig > > > > > > > > > +++ b/configs/sandbox_spl_defconfig > > > > > > > > > @@ -248,3 +248,4 @@ CONFIG_UNIT_TEST=y > > > > > > > > > CONFIG_SPL_UNIT_TEST=y > > > > > > > > > CONFIG_UT_TIME=y > > > > > > > > > CONFIG_UT_DM=y > > > > > > > > > +CONFIG_TOOLS_MKEFICAPSULE=y > > > > > > > > > > > > > > > > Why enabling this here? I don't think it is needed in > > > > > > > > sandbox_spl, but > > > > > > > > in any case it should be in a different patch if needed. > > > > > > > > > > > > > > The binman tests run on the sandbox_spl variant. When running the > > > > > > > capsule generation tests, the mkeficapsule tool should be present > > > > > > > on > > > > > > > the board variant no? > > > > > > > > > > > > Can we run this on the 'sandbox' variant instead? > > > > > > > > > > The capsule tests can be run on sandbox. But the change in the > > > > > sandbox_spl_defconfig is for adding support for the capsule tests > > > > > which are run as part of the binman test suite in CI. So it would be a > > > > > > > > The binman tests are run separately, in 'Run binman, buildman, dtoc, > > > > Kconfig and patman testsuites' in gitlab. > > > > > > Yes, and that uses the sandbox_spl variant build for running the > > > tests. Which is why I added building the mkeficapsule tool for the > > > variant. > > > > Not really. It uses sandbox_spl to obtain the pylibfdt library, that's all. > > > > The tests should use sandbox. By enabling CONFIG_BINMAN you will get > > pylibfdt anyway. > > I am not sure if I am missing something. But if I do not build the > mkeficapsule tool as part of the sandbox_spl variant, I get errors in > CI on the binman tests. And I believe that is because binman is being > run with the '--toolpath /tmp/sandbox_spl/tools', which is built for > the sandbox_spl variant. So if the mkeficapsule tool is not built for > the sandbox_spl variant, this results in the binman capsule tests to > fail in CI. I see this at least with the azure pipeline.
Yes, alll sandbox variants should build all tools, I believe. The mkeficapsule tool should really be built by all boards, right? It also needs to appear in the u-boot-tools debian package too, for example. [..] Regards, Simon