Re: [PATCH] kunit: cs_dsp: Depend on FW_CS_DSP rather then enabling it

[Richard Fitzgerald]

On 22/3/25 19:02, Richard Fitzgerald wrote:
> On 22/3/25 10:11, Richard Fitzgerald wrote:
> > On 20/3/25 17:35, Nico Pache wrote:
> > > Sorry links got mangled
> > Thanks. I'm on vacation right now but I'll take a look through
> > all those when I have time.
> >
> > The unterminated string bugfix is this:
> > https://lore.kernel.org/all/20250211-cs_dsp-kunit-strings-v1-1- 
> > d9bc2035d154@xxxxxxxxxxxxx/
> >
> > I got lucky on all the UM, X86 and ARM builds I tested.
> It looks like that bugfix has got lost.
> Mark sent an email on 20 Feb to say it has been merged into his
> sound tree. But that patch isn't in torvalds/master.
>
> It's possible that the unterminated strings are causing the problems
> you have seen.

I've reproduced this, and it's not a bug in the test. The test is
correctly finding a change in behavior that was introduced into cs_dsp.
This doesn't affect normal firmware files that have real content. But
the tests create an "empty" firmware file that has only the header. This
is useful, so we can have a dummy firmware file that doesn't create
side-effects when it is loaded.

A normal file contains data blocks, which will set ret = 0 when they are
processed. An "empty" file does not have any data block so ret is never
set to 0. The end of the function used to do this:

ret = regmap_async_complete(regmap);

which would make ret == 0 on exit from the function. But a recent commit
removed the async regmap writes and so removed this line. A real
firmware file will already have set ret == 0 when it processed a data
block. The dummy files don't.

So... we have a kunit test, but KUNIT_ALL_TESTS doesn't mean "run all
tests". It means "run test for modules that are already selected."
Modules must be manually selected to get a KUNIT_ALL_TESTS run to
actually run the test code. So it didn't get a kunit test run.

I'll send a patch to fix this.