Re: [PATCH v8 7/7] test: dm: Add test for FW Loader feature

[Christian Marangi <ansuelsmth@gmail.com>]

On Fri, Oct 23, 2026 at 06:12:38AM -0700, Simon Glass wrote:
> Hi Christian,
> 
> On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> > test: dm: Add test for FW Loader feature
> >
> > Add basic test for FW Loader feature for the Filesystem way and FIP.
> >
> > Additional MMC testing image are added that loads a dummy firmware.bin in
> > a fat partition or a simple FIP blob.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >
> >  arch/sandbox/dts/test.dts          |  38 +++++++
> >  configs/sandbox64_defconfig        |   2 +
> >  configs/sandbox_defconfig          |   2 +
> >  configs/sandbox_flattree_defconfig |   2 +
> >  configs/sandbox_noinst_defconfig   |   1 +
> >  configs/sandbox_spl_defconfig      |   2 +
> >  configs/sandbox_vpl_defconfig      |   2 +
> >  test/dm/Makefile                   |   1 +
> >  test/dm/fw_loader.c                | 218 +++++++++++++++++++++++++++++++++++++
> >  test/py/tests/fip/test.fip         | Bin 0 -> 101 bytes
> >  test/py/tests/test_ut.py           |  26 +++++
> >  11 files changed, 294 insertions(+)
> 
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > +	ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
> > +					buf, firmware_size, firmware_size + 1);
> > +	ut_asserteq(0, ret);
> 
> These tests show that the two loaders behave differently when the
> offset is past the end: FS returns 0 here, but FIP returns -EINVAL for
> the same request (see below). Callers go through the uclass and should
> not need to know which loader is behind the phandle, so please can you
> pick one behaviour for both drivers? I think -EINVAL is better, and
> the cover letter says FIP is meant to match FS anyway.
>

Yes exactly... the problem is that I don't want to cause regression in the
use of FS loader with this change of error handling.

Ok I will change FS loader to return -EINVAL as a separate commit.
 
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > +	ret = request_firmware_into_buf(fw_loader_dev,
> > +					"45a113ac-c7f2-30b0-90a5-a399ab912716",
> > +					buf, firmware_size, firmware_size + 1);
> > +	ut_asserteq(-EINVAL, ret);
> 
> This is the FIP case mentioned above.
> 
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > +	/* Enable the mmc12 node for this test */
> > +	root = oftree_root(oftree_default());
> 
> This helper is used for mmc13 too, so the comment should not name a
> specific node.
> 
> > diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> > @@ -685,6 +685,31 @@ def test_ut_dm_init(ubman):
> > +    fipfile = os.path.join(ubman.config.source_dir,
> > +                            'test/py/tests/fip/test.fip')
> 
> Please can you explain in the commit message how test.fip was created
> (e.g. the fiptool command line), so the next person can update it?
> Better still, since the format is just a header plus one ToC entry,
> you could build the 101 bytes in Python here and avoid a binary blob
> in the tree. What do you think?

Ehhhhh I checked and it's a lot of effort on creating a python shim to
simulate the format of a FIP blob. I will check it again but providing the
3kb blob was more practical than polluting the python script with something
used for just one task.

> 
> Binman has a atf_fip.py but you can't use that here, unfortunately.
> 
> Regards,
> Simon

-- 
	Ansuel