mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: Nico Pache <npache@redhat.com>,
	broonie@kernel.org, patches@opensource.cirrus.com,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	kunit-dev@googlegroups.com
Cc: simont@opensource.cirrus.com, ckeepax@opensource.cirrus.com,
	brendan.higgins@linux.dev, davidgow@google.com, rmoar@google.com,
	johannes.berg@intel.com, sj@kernel.org
Subject: Re: [PATCH] kunit: cs_dsp: Depend on FW_CS_DSP rather then enabling it
Date: Tue, 8 Apr 2025 10:25:46 +0100	[thread overview]
Message-ID: <9024776c-7028-4522-a773-8d53d233dabf@opensource.cirrus.com> (raw)
In-Reply-To: <20250319230539.140869-1-npache@redhat.com>

On 19/03/2025 11:05 pm, Nico Pache wrote:
> FW_CS_DSP gets enabled if KUNIT is enabled. The test should rather
> depend on if the feature is enabled. Fix this by moving FW_CS_DSP to the
> depends on clause, and set CONFIG_FW_CS_DSP=y in the kunit tooling.
> 
> Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
> Signed-off-by: Nico Pache <npache@redhat.com>

This patch doesn't actually work and breaks kunit.py.

> ---
>   drivers/firmware/cirrus/Kconfig              | 3 +--
>   tools/testing/kunit/configs/all_tests.config | 2 ++
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/cirrus/Kconfig b/drivers/firmware/cirrus/Kconfig
> index 0a883091259a..989568ab5712 100644
> --- a/drivers/firmware/cirrus/Kconfig
> +++ b/drivers/firmware/cirrus/Kconfig
> @@ -11,9 +11,8 @@ config FW_CS_DSP_KUNIT_TEST_UTILS
>   
>   config FW_CS_DSP_KUNIT_TEST
>   	tristate "KUnit tests for Cirrus Logic cs_dsp" if !KUNIT_ALL_TESTS
> -	depends on KUNIT && REGMAP
> +	depends on KUNIT && REGMAP && FW_CS_DSP
>   	default KUNIT_ALL_TESTS
> -	select FW_CS_DSP

These changes result in a circular reference:

error: recursive dependency detected!
	symbol FW_CS_DSP is selected by FW_CS_DSP_KUNIT_TEST_UTILS
	symbol FW_CS_DSP_KUNIT_TEST_UTILS is selected by
FW_CS_DSP_KUNIT_TEST
	symbol FW_CS_DSP_KUNIT_TEST depends on FW_CS_DSP

>   	select FW_CS_DSP_KUNIT_TEST_UTILS
>   	help
>   	  This builds KUnit tests for cs_dsp.
> diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
> index b0049be00c70..96c6b4aca87d 100644
> --- a/tools/testing/kunit/configs/all_tests.config
> +++ b/tools/testing/kunit/configs/all_tests.config
> @@ -49,3 +49,5 @@ CONFIG_SOUND=y
>   CONFIG_SND=y
>   CONFIG_SND_SOC=y
>   CONFIG_SND_SOC_TOPOLOGY_BUILD=y
> +
> +CONFIG_FW_CS_DSP=y

This won't work because CONFIG_FW_CS_DSP isn't a visible option so it
breaks kunit.py:

ERROR:root:Not all Kconfig options selected in kunitconfig were in the
generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_FW_CS_DSP=y

I suggest dropping this change to all_tests.config and only fixing
the Kconfig so that CONFIG_KUNIT_ALL_TESTS doesn't force
CONFIG_FW_CS_DSP.

  parent reply	other threads:[~2025-04-08  9:26 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-19 23:05 Nico Pache
2025-03-19 23:11 ` Nico Pache
2025-03-20 12:20   ` Richard Fitzgerald
2025-03-20 17:33     ` Nico Pache
2025-03-20 17:35       ` Nico Pache
2025-03-22 10:11         ` Richard Fitzgerald
2025-03-22 19:02           ` Richard Fitzgerald
2025-03-22 20:28             ` Richard Fitzgerald
2025-03-25 17:10             ` Mark Brown
2025-03-24 18:34           ` Nico Pache
2025-03-20 19:13 ` Mark Brown
2025-03-20 22:21   ` Nico Pache
2025-03-20 22:48     ` Mark Brown
2025-03-21 11:37       ` Nico Pache
2025-03-21 14:51         ` Mark Brown
2025-03-21 15:01           ` Nico Pache
2025-03-21 15:51             ` Mark Brown
2025-03-24 18:31               ` Nico Pache
2025-03-22 10:23             ` Richard Fitzgerald
2025-04-08  9:25 ` Richard Fitzgerald [this message]
2025-04-11 12:39   ` Richard Fitzgerald
2025-04-11 14:54     ` Nico Pache

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9024776c-7028-4522-a773-8d53d233dabf@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=brendan.higgins@linux.dev \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=davidgow@google.com \
    --cc=johannes.berg@intel.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=npache@redhat.com \
    --cc=patches@opensource.cirrus.com \
    --cc=rmoar@google.com \
    --cc=simont@opensource.cirrus.com \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®