The first two patches change snd_soc_card_get_kcontrol() to use the
core snd_ctl_find_id_mixer() functionality instead of open-coding its
own list walk.
The last patch adds a KUnit test for this, which was tested on the
original and modified code.
Changes in V2:
Only change is in the KUnit test (patch #3) to make the const strings
more consty.
Richard Fitzgerald (3):
ALSA: control: Introduce snd_ctl_find_id_mixer_locked()
ASoC: soc-card: Use snd_ctl_find_id_mixer() instead of open-coding
ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol
include/sound/control.h | 23 +++++
sound/soc/Kconfig | 8 ++
sound/soc/Makefile | 4 +
sound/soc/soc-card-test.c | 184 ++++++++++++++++++++++++++++++++++++++
sound/soc/soc-card.c | 21 +----
5 files changed, 223 insertions(+), 17 deletions(-)
create mode 100644 sound/soc/soc-card-test.c
--
2.39.2
Use the snd_ctl_find_id_mixer[_locked]() wrapper in
snd_soc_card_get_kcontrol[_locked]() instead of open-coding a custom
list walk of the card controls list.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/soc/soc-card.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/sound/soc/soc-card.c b/sound/soc/soc-card.c
index 8a2f163da6bc..0a3104d4ad23 100644
--- a/sound/soc/soc-card.c
+++ b/sound/soc/soc-card.c
@@ -32,33 +32,20 @@ static inline int _soc_card_ret(struct snd_soc_card *card,
struct snd_kcontrol *snd_soc_card_get_kcontrol_locked(struct snd_soc_card *soc_card,
const char *name)
{
- struct snd_card *card = soc_card->snd_card;
- struct snd_kcontrol *kctl;
-
- /* must be held read or write */
- lockdep_assert_held(&card->controls_rwsem);
-
if (unlikely(!name))
return NULL;
- list_for_each_entry(kctl, &card->controls, list)
- if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name)))
- return kctl;
- return NULL;
+ return snd_ctl_find_id_mixer_locked(soc_card->snd_card, name);
}
EXPORT_SYMBOL_GPL(snd_soc_card_get_kcontrol_locked);
struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
const char *name)
{
- struct snd_card *card = soc_card->snd_card;
- struct snd_kcontrol *kctl;
+ if (unlikely(!name))
+ return NULL;
- down_read(&card->controls_rwsem);
- kctl = snd_soc_card_get_kcontrol_locked(soc_card, name);
- up_read(&card->controls_rwsem);
-
- return kctl;
+ return snd_ctl_find_id_mixer(soc_card->snd_card, name);
}
EXPORT_SYMBOL_GPL(snd_soc_card_get_kcontrol);
--
2.39.2
Add a new snd-soc-card KUnit test with a simple test case for
snd_soc_card_get_kcontrol() and snd_soc_card_get_kcontrol_locked().
Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/soc/Kconfig | 8 ++
sound/soc/Makefile | 4 +
sound/soc/soc-card-test.c | 184 ++++++++++++++++++++++++++++++++++++++
3 files changed, 196 insertions(+)
create mode 100644 sound/soc/soc-card-test.c
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 439fa631c342..a52afb423b46 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -66,6 +66,14 @@ config SND_SOC_TOPOLOGY_KUNIT_TEST
userspace applications such as pulseaudio, to prevent unnecessary
problems.
+config SND_SOC_CARD_KUNIT_TEST
+ tristate "KUnit tests for SoC card"
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ If you want to perform tests on ALSA SoC card functions say Y here.
+ If unsure, say N.
+
config SND_SOC_UTILS_KUNIT_TEST
tristate "KUnit tests for SoC utils"
depends on KUNIT
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 8376fdb217ed..f90f5300b36e 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -12,6 +12,10 @@ ifneq ($(CONFIG_SND_SOC_TOPOLOGY_KUNIT_TEST),)
obj-$(CONFIG_SND_SOC_TOPOLOGY_KUNIT_TEST) += soc-topology-test.o
endif
+ifneq ($(CONFIG_SND_SOC_CARD_KUNIT_TEST),)
+obj-$(CONFIG_SND_SOC_CARD_KUNIT_TEST) += soc-card-test.o
+endif
+
ifneq ($(CONFIG_SND_SOC_UTILS_KUNIT_TEST),)
# snd-soc-test-objs := soc-utils-test.o
obj-$(CONFIG_SND_SOC_UTILS_KUNIT_TEST) += soc-utils-test.o
diff --git a/sound/soc/soc-card-test.c b/sound/soc/soc-card-test.c
new file mode 100644
index 000000000000..075c52fe82e5
--- /dev/null
+++ b/sound/soc/soc-card-test.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (C) 2024 Cirrus Logic, Inc. and
+// Cirrus Logic International Semiconductor Ltd.
+
+#include <kunit/device.h>
+#include <kunit/test.h>
+#include <linux/module.h>
+#include <sound/control.h>
+#include <sound/soc.h>
+#include <sound/soc-card.h>
+
+struct soc_card_test_priv {
+ struct device *card_dev;
+ struct snd_soc_card *card;
+};
+
+static const struct snd_kcontrol_new test_card_controls[] = {
+ SOC_SINGLE("Fee", SND_SOC_NOPM, 0, 1, 0),
+ SOC_SINGLE("Fi", SND_SOC_NOPM, 1, 1, 0),
+ SOC_SINGLE("Fo", SND_SOC_NOPM, 2, 1, 0),
+ SOC_SINGLE("Fum", SND_SOC_NOPM, 3, 1, 0),
+ SOC_SINGLE("Left Fee", SND_SOC_NOPM, 4, 1, 0),
+ SOC_SINGLE("Right Fee", SND_SOC_NOPM, 5, 1, 0),
+ SOC_SINGLE("Left Fi", SND_SOC_NOPM, 6, 1, 0),
+ SOC_SINGLE("Right Fi", SND_SOC_NOPM, 7, 1, 0),
+ SOC_SINGLE("Left Fo", SND_SOC_NOPM, 8, 1, 0),
+ SOC_SINGLE("Right Fo", SND_SOC_NOPM, 9, 1, 0),
+ SOC_SINGLE("Left Fum", SND_SOC_NOPM, 10, 1, 0),
+ SOC_SINGLE("Right Fum", SND_SOC_NOPM, 11, 1, 0),
+};
+
+static void test_snd_soc_card_get_kcontrol(struct kunit *test)
+{
+ struct soc_card_test_priv *priv = test->priv;
+ struct snd_soc_card *card = priv->card;
+ struct snd_kcontrol *kc;
+ struct soc_mixer_control *mc;
+ int i, ret;
+
+ ret = snd_soc_add_card_controls(card, test_card_controls, ARRAY_SIZE(test_card_controls));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ /* Look up every control */
+ for (i = 0; i < ARRAY_SIZE(test_card_controls); ++i) {
+ kc = snd_soc_card_get_kcontrol(card, test_card_controls[i].name);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, kc, "Failed to find '%s'\n",
+ test_card_controls[i].name);
+ if (!kc)
+ continue;
+
+ /* Test that it is the correct control */
+ mc = (struct soc_mixer_control *)kc->private_value;
+ KUNIT_EXPECT_EQ_MSG(test, mc->shift, i, "For '%s'\n", test_card_controls[i].name);
+ }
+
+ /* Test some names that should not be found */
+ kc = snd_soc_card_get_kcontrol(card, "None");
+ KUNIT_EXPECT_NULL(test, kc);
+
+ kc = snd_soc_card_get_kcontrol(card, "Left None");
+ KUNIT_EXPECT_NULL(test, kc);
+
+ kc = snd_soc_card_get_kcontrol(card, "Left");
+ KUNIT_EXPECT_NULL(test, kc);
+
+ kc = snd_soc_card_get_kcontrol(card, NULL);
+ KUNIT_EXPECT_NULL(test, kc);
+}
+
+static void test_snd_soc_card_get_kcontrol_locked(struct kunit *test)
+{
+ struct soc_card_test_priv *priv = test->priv;
+ struct snd_soc_card *card = priv->card;
+ struct snd_kcontrol *kc, *kcw;
+ struct soc_mixer_control *mc;
+ int i, ret;
+
+ ret = snd_soc_add_card_controls(card, test_card_controls, ARRAY_SIZE(test_card_controls));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ /* Look up every control */
+ for (i = 0; i < ARRAY_SIZE(test_card_controls); ++i) {
+ down_read(&card->snd_card->controls_rwsem);
+ kc = snd_soc_card_get_kcontrol_locked(card, test_card_controls[i].name);
+ up_read(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, kc, "Failed to find '%s'\n",
+ test_card_controls[i].name);
+ if (!kc)
+ continue;
+
+ /* Test that it is the correct control */
+ mc = (struct soc_mixer_control *)kc->private_value;
+ KUNIT_EXPECT_EQ_MSG(test, mc->shift, i, "For '%s'\n", test_card_controls[i].name);
+
+ down_write(&card->snd_card->controls_rwsem);
+ kcw = snd_soc_card_get_kcontrol_locked(card, test_card_controls[i].name);
+ up_write(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, kcw, "Failed to find '%s'\n",
+ test_card_controls[i].name);
+
+ KUNIT_EXPECT_PTR_EQ(test, kc, kcw);
+ }
+
+ /* Test some names that should not be found */
+ down_read(&card->snd_card->controls_rwsem);
+ kc = snd_soc_card_get_kcontrol_locked(card, "None");
+ up_read(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NULL(test, kc);
+
+ down_read(&card->snd_card->controls_rwsem);
+ kc = snd_soc_card_get_kcontrol_locked(card, "Left None");
+ up_read(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NULL(test, kc);
+
+ down_read(&card->snd_card->controls_rwsem);
+ kc = snd_soc_card_get_kcontrol_locked(card, "Left");
+ up_read(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NULL(test, kc);
+
+ down_read(&card->snd_card->controls_rwsem);
+ kc = snd_soc_card_get_kcontrol_locked(card, NULL);
+ up_read(&card->snd_card->controls_rwsem);
+ KUNIT_EXPECT_NULL(test, kc);
+}
+
+static int soc_card_test_case_init(struct kunit *test)
+{
+ struct soc_card_test_priv *priv;
+ int ret;
+
+ priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ test->priv = priv;
+
+ priv->card_dev = kunit_device_register(test, "sound-soc-card-test");
+ priv->card_dev = get_device(priv->card_dev);
+ if (!priv->card_dev)
+ return -ENODEV;
+
+ priv->card = kunit_kzalloc(test, sizeof(*priv->card), GFP_KERNEL);
+ if (!priv->card)
+ return -ENOMEM;
+
+ priv->card->name = "soc-card-test";
+ priv->card->dev = priv->card_dev;
+ priv->card->owner = THIS_MODULE;
+
+ ret = snd_soc_register_card(priv->card);
+ if (!ret)
+ return ret;
+
+ return 0;
+}
+
+static void soc_card_test_case_exit(struct kunit *test)
+{
+ struct soc_card_test_priv *priv = test->priv;
+
+ if (priv->card)
+ snd_soc_unregister_card(priv->card);
+
+ if (priv->card_dev)
+ put_device(priv->card_dev);
+}
+
+static struct kunit_case soc_card_test_cases[] = {
+ KUNIT_CASE(test_snd_soc_card_get_kcontrol),
+ KUNIT_CASE(test_snd_soc_card_get_kcontrol_locked),
+ {}
+};
+
+static struct kunit_suite soc_card_test_suite = {
+ .name = "soc-card",
+ .test_cases = soc_card_test_cases,
+ .init = soc_card_test_case_init,
+ .exit = soc_card_test_case_exit,
+};
+
+kunit_test_suites(&soc_card_test_suite);
+
+MODULE_DESCRIPTION("ASoC soc-card KUnit test");
+MODULE_LICENSE("GPL");
--
2.39.2
On Mon, 01 Apr 2024 12:02:07 +0200,
Richard Fitzgerald wrote:
>
> The first two patches change snd_soc_card_get_kcontrol() to use the
> core snd_ctl_find_id_mixer() functionality instead of open-coding its
> own list walk.
>
> The last patch adds a KUnit test for this, which was tested on the
> original and modified code.
>
> Changes in V2:
> Only change is in the KUnit test (patch #3) to make the const strings
> more consty.
>
> Richard Fitzgerald (3):
> ALSA: control: Introduce snd_ctl_find_id_mixer_locked()
> ASoC: soc-card: Use snd_ctl_find_id_mixer() instead of open-coding
> ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol
I suppose this goes via Mark's tree. Feel free to take my ack:
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
On Mon, 01 Apr 2024 10:02:07 +0000, Richard Fitzgerald wrote:
> The first two patches change snd_soc_card_get_kcontrol() to use the
> core snd_ctl_find_id_mixer() functionality instead of open-coding its
> own list walk.
>
> The last patch adds a KUnit test for this, which was tested on the
> original and modified code.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ALSA: control: Introduce snd_ctl_find_id_mixer_locked()
commit: 08ea486a61451189b190c7b89e406b889cf693fa
[2/3] ASoC: soc-card: Use snd_ctl_find_id_mixer() instead of open-coding
commit: 897cc72b08374c1224a9ded03c82dfc8e41f80c2
[3/3] ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol
commit: ef7784e41db73f3d31ce545227ebba4483479a26
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark