[PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman

[Tom Rini]

On Sun, Aug 06, 2023 at 04:43:06PM +0530, Sughosh Ganu wrote:
> On Sun, 6 Aug 2023 at 03:48, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote:
> > > Hi Sughosh,
> > >
> > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > >
> > > > The EFI capsule files can now be generated as part of u-boot
> > > > build through binman. Add capsule entry nodes in the u-boot.dtsi for
> > > > the sandbox architecture for generating the capsules. These capsules
> > > > are then used for testing the EFI capsule update functionality on the
> > > > sandbox platforms. Also add binman nodes for generating the input
> > > > files used for these capsules.
> > > >
> > > > Remove the corresponding logic which was used for generation of these
> > > > input files which is now superfluous.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > ---
> > > > Changes since V6:
> > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> > > >   input filenames.
> > > > * Generate the capsule input files through binman text entries.
> > > >
> > > >  arch/sandbox/dts/u-boot.dtsi                  | 363 ++++++++++++++++++
> > > >  include/sandbox_efi_capsule.h                 |  11 +
> > > >  test/py/tests/test_efi_capsule/conftest.py    | 168 +-------
> > > >  test/py/tests/test_efi_capsule/signature.dts  |  10 -
> > > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> > > >  5 files changed, 385 insertions(+), 203 deletions(-)
> > > >  delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> > > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> > >
> > > I think you are still getting confused with using filenames when you
> > > don't need to. See below...
> > >
> > >
> > > >
> > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > index 60bd004937..f1b16b41c2 100644
> > > > --- a/arch/sandbox/dts/u-boot.dtsi
> > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> 
> <snip>
> 
> > >
> > > > +               };
> > > > +       };
> > > > +
> > > > +       capsule2 {
> > > > +               filename = "Test02";
> > > > +               capsule {
> > > > +                       type = "efi-capsule";
> > > > +                       image-index = <0x2>;
> > > > +                       image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID;
> > > > +
> > > > +                       blob {
> > > > +                               filename = UBOOT_ENV_IMAGE_NEW;
> > > > +                       };
> > > > +               };
> > > > +       };
> > > > +
> > > > +       capsule3 {
> > > > +               filename = "Test03";
> > > > +               capsule {
> > > > +                       type = "efi-capsule";
> > > > +                       image-index = <0x1>;
> > > > +                       image-type-id = SANDBOX_INCORRECT_GUID;
> > > > +
> > > > +                       blob {
> > > > +                               filename = UBOOT_BIN_IMAGE_NEW;
> > > > +                       };
> > > > +               };
> > > > +       };
> > > > +
> > > > +       capsule4 {
> > > > +               filename = "Test04";
> > > > +               capsule {
> > > > +                       type = "efi-capsule";
> > > > +                       image-index = <0x1>;
> > > > +                       image-type-id = SANDBOX_FIT_IMAGE_GUID;
> > > > +
> > > > +                       blob {
> > > > +                               filename =
> > > [..]
> > >
> > > > +       capsule19 {
> > > > +               filename = "Test115";
> > >
> > > [..]
> > >
> > > We really need to talk about these tests. So many cases! Can you not
> > > reduce this?
> >
> > And why are these tests in the generic looking file and not one of the
> > test dts files? This looks like something that would make implementation
> > in real life confusing and non-obvious.
> 
> These are not capsules that are being generated for binman tests.
> Those dts files are indeed under tools/binman/test/. These capsules
> are the ones used for testing the capsule update feature in the CI
> run. The idea of having these capsule nodes defined in this file is to
> have them generated as part of the standard build.

The high level concern I have (so it applies to a few of these patches)
is that it's not going to be clear and straightforward on how to use
this regularly (for example, if I follow all of this right, I should be
able to do a capsule update to push a build to one of my boards, and
then run our pytest suite on it, instead of playing games with SD mux
boards and so forth) or production (configure product-board so that we
can deliver an updated firmware via $mechanism).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230807/f3b12924/attachment.sig>