Re: [PATCH v13 12/15] test: dm: Add test cases for FWU Metadata uclass

Etienne Carriere
2022-10-10T06:23:37Z

Hi Sughosh,

On Fri, 7 Oct 2022 at 05:16, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Etienne,
>
> On Thu, 6 Oct 2022 at 19:36, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 6 Oct 2022 at 11:07, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > Add test cases for accessing the FWU Metadata on the sandbox
> > > platform. The sandbox platform also uses the metadata access driver
> > > for GPT partitioned block devices.
> > >
> > > The FWU feature will be tested on the sandbox64 variant with a raw
> > > capsule. Remove the FIT capsule testing from sandbox64 defconfig --
> > > the FIT capsule test will be run on the sandbox_flattree variant.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > ---
> > > Changes since V12: None
> > >
> > > arch/sandbox/Kconfig | 6 +
> > > arch/sandbox/dts/test.dts | 7 +-
> > > board/sandbox/sandbox.c | 8 +
> > > configs/sandbox64_defconfig | 5 +-
> > > lib/fwu_updates/fwu.c | 6 +
> > > test/dm/Makefile | 1 +
> > > test/dm/fwu_mdata.c | 149 ++++++++++++++++++
> > > test/dm/fwu_mdata_disk_image.h | 112 +++++++++++++
> > > .../test_capsule_firmware_fit.py | 1 -
> > > .../test_capsule_firmware_signed_fit.py | 1 -
> > > tools/Makefile | 2 +-
> > > 11 files changed, 293 insertions(+), 5 deletions(-)
> > > create mode 100644 test/dm/fwu_mdata.c
> > > create mode 100644 test/dm/fwu_mdata_disk_image.h
>
> <snip>
>
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index 34a1aa7a8b..27687d5cd9 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -74,7 +74,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
> > > hostprogs-y += dumpimage mkimage
> > > hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
> > >
> > > -hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
> > > +hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST)$(CONFIG_FWU_MDATA_GPT_BLK) += file2include
> >
> > Late catch, my apologies. This won't work when both switches are enable.
>
> Okay, although this is not an issue in the current upstream, as
> CONFIG_CMD_BOOTEFI_SELFTEST is not enabled in the sandbox64 defconfig.
>
> > Prefer something like:
> >
> > +ifneq (,$(filter y,$(CONFIG_CMD_BOOTEFI_SELFTEST) $(CONFIG_FWU_MDATA_GPT_BLK)))
> > +hostprogs-y += file2include
> > +endif
>
> Okay. Btw, any issue with having something like this?
>
> ifneq ($(CONFIG_CMD_BOOTEFI_SELFTEST)$(CONFIG_FWU_MDATA_GPT_BLK),)
> hostprogs-y += file2include
> endif

I'm not shure. I prefer to ask maintainers whether they prefer
ifneq ($(CONFIG_FOO)$(CONFIG_BAR),)
or
ifneq (,$(filter y,$(CONFIG_FOO) $(CONFIG_BAR)))
for such boolean config switches.

br,
etienne

>
> -sughosh
>
> >
> > br,
> > etienne
> >
> > >
> > > FIT_OBJS-y := fit_common.o fit_image.o image-host.o boot/image-fit.o
> > > FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o boot/image-fit-sig.o
> > > --
> > > 2.34.1
> > >