alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add KUNIT tests for ASoC topology
@ 2021-01-20 15:28 Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 1/5] ASoC: topology: Properly unregister DAI on removal Amadeusz Sławiński
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

This series adds unit tests for ASoC topology.

First fix problems found when developing and running test cases and
then add tests implementation.

Tests themselves are quite simple and just call
snd_soc_tplg_component_load() with various parameters and check the
result. Tests themselves are described in more detail in commits
adding them.

Goal is to expand the amount of test cases in following patches.

Prerequisity for this patchset are 2 patches which have already been
sent:
https://lore.kernel.org/alsa-devel/20210114163602.911205-1-amadeuszx.slawinski@linux.intel.com/T/#t

Description on how typical test case itself works:

In order to load topology we need to have 3 things:
card, codec component & platform component.

In typical test case we register card and platform component and bind
to dummy codec. There are of course execeptions, when we want to
test behaviour of topology API when component or card is missing.
Note that this is bit different from typical scenario (in SOF and skylake
drivers) where card is registered by machine driver and component by
platform driver, as we register both when setting up test.

If you check the test case most of them have similar architecture of:
1.
	/* run test */
	ret = snd_soc_register_card(&kunit_comp->card);
	if (ret != 0 && ret != -EPROBE_DEFER)
		KUNIT_FAIL(test, "Failed to register card");

2.
	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
	KUNIT_EXPECT_EQ(test, 0, ret);

3.
	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
	KUNIT_EXPECT_EQ(test, 0, ret);

Ad. 1.
First we register card, which in most tests returns -EPROBE_DEFER
(from snd_soc_bind_card()), as platform component is not yet created.
I test for both 0 and -EPROBE_DEFER, as it makes it easier to reshuffle
this code around if needed and there is one test case which does it in
different order.

Ad. 2.
Then we initialize platform component with structure pointing at proper
probe function, which calls snd_soc_tplg_component_load() with test
parameters and checks expected result.

Ad. 3.
And then in follow up we call snd_soc_add_component() which creates
platform component for us and calls snd_soc_try_rebind_card() which
if everything is bound properly calls previously set probe function.

Amadeusz Sławiński (5):
  ASoC: topology: Properly unregister DAI on removal
  Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()"
  ASoC: topology: KUnit: Add KUnit tests passing various arguments to
    snd_soc_tplg_component_load
  ASoC: topology: KUnit: Add KUnit tests passing empty topology with
    variants to snd_soc_tplg_component_load
  ASoC: topology: KUnit: Add KUnit tests passing topology with PCM to
    snd_soc_tplg_component_load

 include/sound/soc.h           |   4 -
 sound/soc/Kconfig             |  17 +
 sound/soc/Makefile            |   5 +
 sound/soc/soc-devres.c        |  37 --
 sound/soc/soc-topology-test.c | 843 ++++++++++++++++++++++++++++++++++
 sound/soc/soc-topology.c      |   9 +-
 6 files changed, 870 insertions(+), 45 deletions(-)
 create mode 100644 sound/soc/soc-topology-test.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] ASoC: topology: Properly unregister DAI on removal
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
@ 2021-01-20 15:28 ` Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()" Amadeusz Sławiński
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

DAIs need to be removed when topology unload function is called (usually
done when component is being removed). We can't do this when device is
being removed, as structures we operate on when removing DAI can already
be freed.

Fixes: 6ae4902f2f34 ("ASoC: soc-topology: use devm_snd_soc_register_dai()")
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-topology.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 5476854c12b0..94a162b884c9 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -447,7 +447,7 @@ static void remove_dai(struct snd_soc_component *comp,
 {
 	struct snd_soc_dai_driver *dai_drv =
 		container_of(dobj, struct snd_soc_dai_driver, dobj);
-	struct snd_soc_dai *dai;
+	struct snd_soc_dai *dai, *_dai;
 
 	if (pass != SOC_TPLG_PASS_PCM_DAI)
 		return;
@@ -455,9 +455,9 @@ static void remove_dai(struct snd_soc_component *comp,
 	if (dobj->ops && dobj->ops->dai_unload)
 		dobj->ops->dai_unload(comp, dobj);
 
-	for_each_component_dais(comp, dai)
+	for_each_component_dais_safe(comp, dai, _dai)
 		if (dai->driver == dai_drv)
-			dai->driver = NULL;
+			snd_soc_unregister_dai(dai);
 
 	list_del(&dobj->list);
 }
@@ -1742,7 +1742,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
-	dai = devm_snd_soc_register_dai(tplg->dev, tplg->comp, dai_drv, false);
+	dai = snd_soc_register_dai(tplg->comp, dai_drv, false);
 	if (!dai)
 		return -ENOMEM;
 
@@ -1750,6 +1750,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
 	if (ret != 0) {
 		dev_err(dai->dev, "Failed to create DAI widgets %d\n", ret);
+		snd_soc_unregister_dai(dai);
 		return ret;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()"
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 1/5] ASoC: topology: Properly unregister DAI on removal Amadeusz Sławiński
@ 2021-01-20 15:28 ` Amadeusz Sławiński
  2021-01-20 17:29   ` Mark Brown
  2021-01-20 15:28 ` [PATCH 3/5] ASoC: topology: KUnit: Add KUnit tests passing various arguments to snd_soc_tplg_component_load Amadeusz Sławiński
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

As described in preceding patch, waiting with DAI removal until device
is being unregistered can lead to working on freed memory, hence having
DAIs managed by resource managed memory is a bad idea, revert commit
creating devm_ API for this, so no one makes a mistake of using it.

This reverts commit 0fae253af563cf5d1f5dc651d520c3eafd74f183.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/soc.h    |  4 ----
 sound/soc/soc-devres.c | 37 -------------------------------------
 2 files changed, 41 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 3fa6c40a63b7..69cb19a93765 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1236,10 +1236,6 @@ void snd_soc_remove_pcm_runtime(struct snd_soc_card *card,
 struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 					 struct snd_soc_dai_driver *dai_drv,
 					 bool legacy_dai_naming);
-struct snd_soc_dai *devm_snd_soc_register_dai(struct device *dev,
-					      struct snd_soc_component *component,
-					      struct snd_soc_dai_driver *dai_drv,
-					      bool legacy_dai_naming);
 void snd_soc_unregister_dai(struct snd_soc_dai *dai);
 
 struct snd_soc_dai *snd_soc_find_dai(
diff --git a/sound/soc/soc-devres.c b/sound/soc/soc-devres.c
index 4534a1c03e8e..c6364caabc0e 100644
--- a/sound/soc/soc-devres.c
+++ b/sound/soc/soc-devres.c
@@ -9,43 +9,6 @@
 #include <sound/soc.h>
 #include <sound/dmaengine_pcm.h>
 
-static void devm_dai_release(struct device *dev, void *res)
-{
-	snd_soc_unregister_dai(*(struct snd_soc_dai **)res);
-}
-
-/**
- * devm_snd_soc_register_dai - resource-managed dai registration
- * @dev: Device used to manage component
- * @component: The component the DAIs are registered for
- * @dai_drv: DAI driver to use for the DAI
- * @legacy_dai_naming: if %true, use legacy single-name format;
- *	if %false, use multiple-name format;
- */
-struct snd_soc_dai *devm_snd_soc_register_dai(struct device *dev,
-					      struct snd_soc_component *component,
-					      struct snd_soc_dai_driver *dai_drv,
-					      bool legacy_dai_naming)
-{
-	struct snd_soc_dai **ptr;
-	struct snd_soc_dai *dai;
-
-	ptr = devres_alloc(devm_dai_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	dai = snd_soc_register_dai(component, dai_drv, legacy_dai_naming);
-	if (dai) {
-		*ptr = dai;
-		devres_add(dev, ptr);
-	} else {
-		devres_free(ptr);
-	}
-
-	return dai;
-}
-EXPORT_SYMBOL_GPL(devm_snd_soc_register_dai);
-
 static void devm_component_release(struct device *dev, void *res)
 {
 	const struct snd_soc_component_driver **cmpnt_drv = res;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] ASoC: topology: KUnit: Add KUnit tests passing various arguments to snd_soc_tplg_component_load
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 1/5] ASoC: topology: Properly unregister DAI on removal Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()" Amadeusz Sławiński
@ 2021-01-20 15:28 ` Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 4/5] ASoC: topology: KUnit: Add KUnit tests passing empty topology with variants " Amadeusz Sławiński
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

In order to ensure correct behaviour of topology API, add unit tests
exercising topology functionality.

Start with adding cases for passing various arguments to
snd_soc_tplg_component_load as it is part of exposed topology API.

First test case adds test passing NULL component as argument.
Following one adds test case for passing NULL ops as argument.
Finally add test case passing NULL fw as argument.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/Kconfig             |  17 ++
 sound/soc/Makefile            |   5 +
 sound/soc/soc-topology-test.c | 303 ++++++++++++++++++++++++++++++++++
 3 files changed, 325 insertions(+)
 create mode 100644 sound/soc/soc-topology-test.c

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 47da9ce17693..010328069256 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -37,6 +37,23 @@ config SND_SOC_COMPRESS
 config SND_SOC_TOPOLOGY
 	bool
 
+config SND_SOC_TOPOLOGY_KUNIT_TESTS
+	tristate "KUnit tests for SoC topology"
+	depends on KUNIT
+	depends on SND_SOC_TOPOLOGY
+	default KUNIT_ALL_TESTS
+	help
+	  If you want to perform tests on ALSA SoC topology support say Y here.
+
+	  This builds a module which can be later manually loaded to run KUNIT
+	  test cases against soc-topology.c API. This should be primarily used
+	  by developers to test their changes to ASoC.
+
+	  Do note that it creates fake playback devices which do not interact
+	  well with userspace. When running tests one may want to disable
+	  userspace applications such as pulseaudio, to prevent unnecessary
+	  problems.
+
 config SND_SOC_ACPI
 	tristate
 
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 508dbaa1814e..c93affe81094 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -7,6 +7,11 @@ ifneq ($(CONFIG_SND_SOC_TOPOLOGY),)
 snd-soc-core-objs += soc-topology.o
 endif
 
+ifneq ($(CONFIG_SND_SOC_TOPOLOGY_KUNIT_TESTS),)
+# snd-soc-test-objs := soc-topology-test.o
+obj-$(CONFIG_SND_SOC_TOPOLOGY_KUNIT_TESTS) := soc-topology-test.o
+endif
+
 ifneq ($(CONFIG_SND_SOC_GENERIC_DMAENGINE_PCM),)
 snd-soc-core-objs += soc-generic-dmaengine-pcm.o
 endif
diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c
new file mode 100644
index 000000000000..7276ed95d83d
--- /dev/null
+++ b/sound/soc/soc-topology-test.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * soc-topology-test.c  --  ALSA SoC Topology Kernel Unit Tests
+ *
+ * Copyright(c) 2021 Intel Corporation. All rights reserved.
+ */
+
+#include <linux/firmware.h>
+#include <linux/random.h>
+#include <sound/core.h>
+#include <sound/soc.h>
+#include <sound/soc-topology.h>
+#include <kunit/test.h>
+
+/* ===== HELPER FUNCTIONS =================================================== */
+
+/*
+ * snd_soc_component needs device to operate on (primarily for prints), create
+ * fake one, as we don't register with PCI or anything else
+ * device_driver name is used in some of the prints (fmt_single_name) so
+ * we also mock up minimal one
+ */
+static struct device *test_dev;
+
+static struct device_driver test_drv = {
+	.name = "sound-soc-topology-test-driver",
+};
+
+static int snd_soc_tplg_test_init(struct kunit *test)
+{
+	test_dev = root_device_register("sound-soc-topology-test");
+	test_dev = get_device(test_dev);
+	if (!test_dev)
+		return -ENODEV;
+
+	test_dev->driver = &test_drv;
+
+	return 0;
+}
+
+static void snd_soc_tplg_test_exit(struct kunit *test)
+{
+	put_device(test_dev);
+	root_device_unregister(test_dev);
+}
+
+/*
+ * helper struct we use when registering component, as we load topology during
+ * component probe, we need to pass struct kunit somehow to probe function, so
+ * we can report test result
+ */
+struct kunit_soc_component {
+	struct kunit *kunit;
+	int expect; /* what result we expect when loading topology */
+	struct snd_soc_component comp;
+	struct snd_soc_card card;
+	struct firmware fw;
+};
+
+static int d_probe(struct snd_soc_component *component)
+{
+	struct kunit_soc_component *kunit_comp =
+			container_of(component, struct kunit_soc_component, comp);
+	int ret;
+
+	ret = snd_soc_tplg_component_load(component, NULL, &kunit_comp->fw);
+	KUNIT_EXPECT_EQ_MSG(kunit_comp->kunit, kunit_comp->expect, ret,
+			    "Failed topology load");
+
+	return 0;
+}
+
+static void d_remove(struct snd_soc_component *component)
+{
+	struct kunit_soc_component *kunit_comp =
+			container_of(component, struct kunit_soc_component, comp);
+	int ret;
+
+	ret = snd_soc_tplg_component_remove(component);
+	KUNIT_EXPECT_EQ(kunit_comp->kunit, 0, ret);
+}
+
+/*
+ * ASoC minimal boiler plate
+ */
+SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
+
+SND_SOC_DAILINK_DEF(platform, DAILINK_COMP_ARRAY(COMP_PLATFORM("sound-soc-topology-test")));
+
+static struct snd_soc_dai_link kunit_dai_links[] = {
+	{
+		.name = "KUNIT Audio Port",
+		.id = 0,
+		.stream_name = "Audio Playback/Capture",
+		.nonatomic = 1,
+		.dynamic = 1,
+		.trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.dpcm_playback = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(dummy, dummy, platform),
+	},
+};
+
+static const struct snd_soc_component_driver test_component = {
+	.name = "sound-soc-topology-test",
+	.probe = d_probe,
+	.remove = d_remove,
+	.non_legacy_dai_naming = 1,
+};
+
+/* ===== TEST CASES ========================================================= */
+
+// TEST CASE
+// Test passing NULL component as parameter to snd_soc_tplg_component_load
+
+/*
+ * need to override generic probe function with one using NULL when calling
+ * topology load during component initialization, we don't need .remove
+ * handler as load should fail
+ */
+static int d_probe_null_comp(struct snd_soc_component *component)
+{
+	struct kunit_soc_component *kunit_comp =
+			container_of(component, struct kunit_soc_component, comp);
+	int ret;
+
+	/* instead of passing component pointer as first argument, pass NULL here */
+	ret = snd_soc_tplg_component_load(NULL, NULL, &kunit_comp->fw);
+	KUNIT_EXPECT_EQ_MSG(kunit_comp->kunit, kunit_comp->expect, ret,
+			    "Failed topology load");
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver test_component_null_comp = {
+	.name = "sound-soc-topology-test",
+	.probe = d_probe_null_comp,
+	.non_legacy_dai_naming = 1,
+};
+
+static void snd_soc_tplg_test_load_with_null_comp(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component_null_comp, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test passing NULL ops as parameter to snd_soc_tplg_component_load
+
+/*
+ * NULL ops is default case, we pass empty topology (fw), so we don't have
+ * anything to parse and just do nothing, which results in return 0; from
+ * calling soc_tplg_dapm_complete in soc_tplg_process_headers
+ */
+static void snd_soc_tplg_test_load_with_null_ops(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = 0; /* expect success */
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test passing NULL fw as parameter to snd_soc_tplg_component_load
+
+/*
+ * need to override generic probe function with one using NULL pointer to fw
+ * when calling topology load during component initialization, we don't need
+ * .remove handler as load should fail
+ */
+static int d_probe_null_fw(struct snd_soc_component *component)
+{
+	struct kunit_soc_component *kunit_comp =
+			container_of(component, struct kunit_soc_component, comp);
+	int ret;
+
+	/* instead of passing fw pointer as third argument, pass NULL here */
+	ret = snd_soc_tplg_component_load(component, NULL, NULL);
+	KUNIT_EXPECT_EQ_MSG(kunit_comp->kunit, kunit_comp->expect, ret,
+			    "Failed topology load");
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver test_component_null_fw = {
+	.name = "sound-soc-topology-test",
+	.probe = d_probe_null_fw,
+	.non_legacy_dai_naming = 1,
+};
+
+static void snd_soc_tplg_test_load_with_null_fw(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component_null_fw, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+/* ===== KUNIT MODULE DEFINITIONS =========================================== */
+
+static struct kunit_case snd_soc_tplg_test_cases[] = {
+	KUNIT_CASE(snd_soc_tplg_test_load_with_null_comp),
+	KUNIT_CASE(snd_soc_tplg_test_load_with_null_ops),
+	KUNIT_CASE(snd_soc_tplg_test_load_with_null_fw),
+	{}
+};
+
+static struct kunit_suite snd_soc_tplg_test_suite = {
+	.name = "snd_soc_tplg_test",
+	.init = snd_soc_tplg_test_init,
+	.exit = snd_soc_tplg_test_exit,
+	.test_cases = snd_soc_tplg_test_cases,
+};
+
+kunit_test_suites(&snd_soc_tplg_test_suite);
+
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] ASoC: topology: KUnit: Add KUnit tests passing empty topology with variants to snd_soc_tplg_component_load
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
                   ` (2 preceding siblings ...)
  2021-01-20 15:28 ` [PATCH 3/5] ASoC: topology: KUnit: Add KUnit tests passing various arguments to snd_soc_tplg_component_load Amadeusz Sławiński
@ 2021-01-20 15:28 ` Amadeusz Sławiński
  2021-01-20 15:28 ` [PATCH 5/5] ASoC: topology: KUnit: Add KUnit tests passing topology with PCM " Amadeusz Sławiński
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

In order to ensure correct behaviour of topology API, add unit tests
exercising topology functionality.

Add "empty" topology template and tests for parsing it. Also adds few
variants with bad magic numbers.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-topology-test.c | 311 +++++++++++++++++++++++++++++++++-
 1 file changed, 310 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c
index 7276ed95d83d..bae40c111f9b 100644
--- a/sound/soc/soc-topology-test.c
+++ b/sound/soc/soc-topology-test.c
@@ -6,7 +6,6 @@
  */
 
 #include <linux/firmware.h>
-#include <linux/random.h>
 #include <sound/core.h>
 #include <sound/soc.h>
 #include <sound/soc-topology.h>
@@ -108,6 +107,37 @@ static const struct snd_soc_component_driver test_component = {
 	.non_legacy_dai_naming = 1,
 };
 
+/* ===== TOPOLOGY TEMPLATES ================================================= */
+
+// Structural representation of topology which can be generated with:
+// $ touch empty
+// $ alsatplg -c empty -o empty.tplg
+// $ xxd -i empty.tplg
+
+struct tplg_tmpl_001 {
+	struct snd_soc_tplg_hdr header;
+	struct snd_soc_tplg_manifest manifest;
+} __packed;
+
+static struct tplg_tmpl_001 tplg_tmpl_empty = {
+	.header = {
+		.magic = SND_SOC_TPLG_MAGIC,
+		.abi = 5,
+		.version = 0,
+		.type = SND_SOC_TPLG_TYPE_MANIFEST,
+		.size = sizeof(struct snd_soc_tplg_hdr),
+		.vendor_type = 0,
+		.payload_size = sizeof(struct snd_soc_tplg_manifest),
+		.index = 0,
+		.count = 1,
+	},
+
+	.manifest = {
+		.size = sizeof(struct snd_soc_tplg_manifest),
+		/* rest of fields is 0 */
+	},
+};
+
 /* ===== TEST CASES ========================================================= */
 
 // TEST CASE
@@ -282,12 +312,291 @@ static void snd_soc_tplg_test_load_with_null_fw(struct kunit *test)
 	snd_soc_unregister_component(test_dev);
 }
 
+// TEST CASE
+// Test passing "empty" topology file
+static void snd_soc_tplg_test_load_empty_tplg(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	struct tplg_tmpl_001 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = 0; /* expect success */
+
+	size = sizeof(tplg_tmpl_empty);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_empty, sizeof(tplg_tmpl_empty));
+
+	kunit_comp->fw.data = (u8 *)data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test "empty" topology file, but with bad "magic"
+// In theory we could loop through all possible bad values, but it takes too
+// long, so just use SND_SOC_TPLG_MAGIC + 1
+static void snd_soc_tplg_test_load_empty_tplg_bad_magic(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	struct tplg_tmpl_001 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	size = sizeof(tplg_tmpl_empty);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_empty, sizeof(tplg_tmpl_empty));
+	/*
+	 * override abi
+	 * any value != magic number is wrong
+	 */
+	data->header.magic = SND_SOC_TPLG_MAGIC + 1;
+
+	kunit_comp->fw.data = (u8 *)data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test "empty" topology file, but with bad "abi"
+// In theory we could loop through all possible bad values, but it takes too
+// long, so just use SND_SOC_TPLG_ABI_VERSION + 1
+static void snd_soc_tplg_test_load_empty_tplg_bad_abi(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	struct tplg_tmpl_001 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	size = sizeof(tplg_tmpl_empty);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_empty, sizeof(tplg_tmpl_empty));
+	/*
+	 * override abi
+	 * any value != accepted range is wrong
+	 */
+	data->header.abi = SND_SOC_TPLG_ABI_VERSION + 1;
+
+	kunit_comp->fw.data = (u8 *)data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test "empty" topology file, but with bad "size"
+// In theory we could loop through all possible bad values, but it takes too
+// long, so just use sizeof(struct snd_soc_tplg_hdr) + 1
+static void snd_soc_tplg_test_load_empty_tplg_bad_size(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	struct tplg_tmpl_001 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	size = sizeof(tplg_tmpl_empty);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_empty, sizeof(tplg_tmpl_empty));
+	/*
+	 * override size
+	 * any value != struct size is wrong
+	 */
+	data->header.size = sizeof(struct snd_soc_tplg_hdr) + 1;
+
+	kunit_comp->fw.data = (u8 *)data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+}
+
+// TEST CASE
+// Test "empty" topology file, but with bad "payload_size"
+// In theory we could loop through all possible bad values, but it takes too
+// long, so just use the known wrong one
+static void snd_soc_tplg_test_load_empty_tplg_bad_payload_size(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	struct tplg_tmpl_001 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = -EINVAL; /* expect failure */
+
+	size = sizeof(tplg_tmpl_empty);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_empty, sizeof(tplg_tmpl_empty));
+	/*
+	 * override payload size
+	 * there is only explicit check for 0, so check with it, other values
+	 * are handled by just not reading behind EOF
+	 */
+	data->header.payload_size = 0;
+
+	kunit_comp->fw.data = (u8 *)data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	/* cleanup */
+	snd_soc_unregister_component(test_dev);
+
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+}
+
 /* ===== KUNIT MODULE DEFINITIONS =========================================== */
 
 static struct kunit_case snd_soc_tplg_test_cases[] = {
 	KUNIT_CASE(snd_soc_tplg_test_load_with_null_comp),
 	KUNIT_CASE(snd_soc_tplg_test_load_with_null_ops),
 	KUNIT_CASE(snd_soc_tplg_test_load_with_null_fw),
+	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg),
+	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_magic),
+	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_abi),
+	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_size),
+	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_payload_size),
 	{}
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] ASoC: topology: KUnit: Add KUnit tests passing topology with PCM to snd_soc_tplg_component_load
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
                   ` (3 preceding siblings ...)
  2021-01-20 15:28 ` [PATCH 4/5] ASoC: topology: KUnit: Add KUnit tests passing empty topology with variants " Amadeusz Sławiński
@ 2021-01-20 15:28 ` Amadeusz Sławiński
  2021-01-21  0:05 ` (subset) [PATCH 0/5] Add KUNIT tests for ASoC topology Mark Brown
  2021-01-21 19:39 ` Mark Brown
  6 siblings, 0 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-20 15:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Takashi Iwai
  Cc: Cezary Rojewski, Amadeusz Sławiński,
	Pierre-Louis Bossart, alsa-devel

In order to ensure correct behaviour of topology API, add unit tests
exercising topology functionality.

Add topology containing PCM template and tests for parsing it. Also
adds test cases simulating modules reloads in case of separate drivers
for card and component.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-topology-test.c | 231 ++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)

diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c
index bae40c111f9b..36e2a3486dbf 100644
--- a/sound/soc/soc-topology-test.c
+++ b/sound/soc/soc-topology-test.c
@@ -138,6 +138,79 @@ static struct tplg_tmpl_001 tplg_tmpl_empty = {
 	},
 };
 
+// Structural representation of topology containing SectionPCM
+
+struct tplg_tmpl_002 {
+	struct snd_soc_tplg_hdr header;
+	struct snd_soc_tplg_manifest manifest;
+	struct snd_soc_tplg_hdr pcm_header;
+	struct snd_soc_tplg_pcm pcm;
+} __packed;
+
+static struct tplg_tmpl_002 tplg_tmpl_with_pcm = {
+	.header = {
+		.magic = SND_SOC_TPLG_MAGIC,
+		.abi = 5,
+		.version = 0,
+		.type = SND_SOC_TPLG_TYPE_MANIFEST,
+		.size = sizeof(struct snd_soc_tplg_hdr),
+		.vendor_type = 0,
+		.payload_size = sizeof(struct snd_soc_tplg_manifest),
+		.index = 0,
+		.count = 1,
+	},
+	.manifest = {
+		.size = sizeof(struct snd_soc_tplg_manifest),
+		.pcm_elems = 1,
+		/* rest of fields is 0 */
+	},
+	.pcm_header = {
+		.magic = SND_SOC_TPLG_MAGIC,
+		.abi = 5,
+		.version = 0,
+		.type = SND_SOC_TPLG_TYPE_PCM,
+		.size = sizeof(struct snd_soc_tplg_hdr),
+		.vendor_type = 0,
+		.payload_size = sizeof(struct snd_soc_tplg_pcm),
+		.index = 0,
+		.count = 1,
+	},
+	.pcm = {
+		.size = sizeof(struct snd_soc_tplg_pcm),
+		.pcm_name = "KUNIT Audio",
+		.dai_name = "kunit-audio-dai",
+		.pcm_id = 0,
+		.dai_id = 0,
+		.playback = 1,
+		.capture = 1,
+		.compress = 0,
+		.stream = {
+			[0] = {
+				.channels = 2,
+			},
+			[1] = {
+				.channels = 2,
+			},
+		},
+		.num_streams = 0,
+		.caps = {
+			[0] = {
+				.name = "kunit-audio-playback",
+				.channels_min = 2,
+				.channels_max = 2,
+			},
+			[1] = {
+				.name = "kunit-audio-capture",
+				.channels_min = 2,
+				.channels_max = 2,
+			},
+		},
+		.flag_mask = 0,
+		.flags = 0,
+		.priv = { 0 },
+	},
+};
+
 /* ===== TEST CASES ========================================================= */
 
 // TEST CASE
@@ -586,6 +659,161 @@ static void snd_soc_tplg_test_load_empty_tplg_bad_payload_size(struct kunit *tes
 	KUNIT_EXPECT_EQ(test, 0, ret);
 }
 
+// TEST CASE
+// Test passing topology file with PCM definition
+static void snd_soc_tplg_test_load_pcm_tplg(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	u8 *data;
+	int size;
+	int ret;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = 0; /* expect success */
+
+	size = sizeof(tplg_tmpl_with_pcm);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_with_pcm, sizeof(tplg_tmpl_with_pcm));
+
+	kunit_comp->fw.data = data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	snd_soc_unregister_component(test_dev);
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+}
+
+// TEST CASE
+// Test passing topology file with PCM definition
+// with component reload
+static void snd_soc_tplg_test_load_pcm_tplg_reload_comp(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	u8 *data;
+	int size;
+	int ret;
+	int i;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = 0; /* expect success */
+
+	size = sizeof(tplg_tmpl_with_pcm);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_with_pcm, sizeof(tplg_tmpl_with_pcm));
+
+	kunit_comp->fw.data = data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_register_card(&kunit_comp->card);
+	if (ret != 0 && ret != -EPROBE_DEFER)
+		KUNIT_FAIL(test, "Failed to register card");
+
+	for (i = 0; i < 100; i++) {
+		ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+		KUNIT_EXPECT_EQ(test, 0, ret);
+
+		ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+		KUNIT_EXPECT_EQ(test, 0, ret);
+
+		snd_soc_unregister_component(test_dev);
+	}
+
+	/* cleanup */
+	ret = snd_soc_unregister_card(&kunit_comp->card);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+}
+
+// TEST CASE
+// Test passing topology file with PCM definition
+// with card reload
+static void snd_soc_tplg_test_load_pcm_tplg_reload_card(struct kunit *test)
+{
+	struct kunit_soc_component *kunit_comp;
+	u8 *data;
+	int size;
+	int ret;
+	int i;
+
+	/* prepare */
+	kunit_comp = kunit_kzalloc(test, sizeof(*kunit_comp), GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, kunit_comp);
+	kunit_comp->kunit = test;
+	kunit_comp->expect = 0; /* expect success */
+
+	size = sizeof(tplg_tmpl_with_pcm);
+	data = kunit_kzalloc(kunit_comp->kunit, size, GFP_KERNEL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(kunit_comp->kunit, data);
+
+	memcpy(data, &tplg_tmpl_with_pcm, sizeof(tplg_tmpl_with_pcm));
+
+	kunit_comp->fw.data = data;
+	kunit_comp->fw.size = size;
+
+	kunit_comp->card.dev = test_dev,
+	kunit_comp->card.name = "kunit-card",
+	kunit_comp->card.owner = THIS_MODULE,
+	kunit_comp->card.dai_link = kunit_dai_links,
+	kunit_comp->card.num_links = ARRAY_SIZE(kunit_dai_links),
+	kunit_comp->card.fully_routed = true,
+
+	/* run test */
+	ret = snd_soc_component_initialize(&kunit_comp->comp, &test_component, test_dev);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	ret = snd_soc_add_component(&kunit_comp->comp, NULL, 0);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+
+	for (i = 0; i < 100; i++) {
+		ret = snd_soc_register_card(&kunit_comp->card);
+		if (ret != 0 && ret != -EPROBE_DEFER)
+			KUNIT_FAIL(test, "Failed to register card");
+
+		ret = snd_soc_unregister_card(&kunit_comp->card);
+		KUNIT_EXPECT_EQ(test, 0, ret);
+	}
+
+	/* cleanup */
+	snd_soc_unregister_component(test_dev);
+}
+
 /* ===== KUNIT MODULE DEFINITIONS =========================================== */
 
 static struct kunit_case snd_soc_tplg_test_cases[] = {
@@ -597,6 +825,9 @@ static struct kunit_case snd_soc_tplg_test_cases[] = {
 	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_abi),
 	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_size),
 	KUNIT_CASE(snd_soc_tplg_test_load_empty_tplg_bad_payload_size),
+	KUNIT_CASE(snd_soc_tplg_test_load_pcm_tplg),
+	KUNIT_CASE(snd_soc_tplg_test_load_pcm_tplg_reload_comp),
+	KUNIT_CASE(snd_soc_tplg_test_load_pcm_tplg_reload_card),
 	{}
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()"
  2021-01-20 15:28 ` [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()" Amadeusz Sławiński
@ 2021-01-20 17:29   ` Mark Brown
  2021-01-21 15:08     ` Amadeusz Sławiński
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2021-01-20 17:29 UTC (permalink / raw)
  To: Amadeusz Sławiński
  Cc: Cezary Rojewski, alsa-devel, Liam Girdwood, Takashi Iwai,
	Pierre-Louis Bossart

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On Wed, Jan 20, 2021 at 04:28:43PM +0100, Amadeusz Sławiński wrote:
> As described in preceding patch, waiting with DAI removal until device
> is being unregistered can lead to working on freed memory, hence having

There is a potential for misuse with essentially all devm functions -
why is this one so special?  

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> DAIs managed by resource managed memory is a bad idea, revert commit
> creating devm_ API for this, so no one makes a mistake of using it.
> 
> This reverts commit 0fae253af563cf5d1f5dc651d520c3eafd74f183.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: (subset) [PATCH 0/5] Add KUNIT tests for ASoC topology
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
                   ` (4 preceding siblings ...)
  2021-01-20 15:28 ` [PATCH 5/5] ASoC: topology: KUnit: Add KUnit tests passing topology with PCM " Amadeusz Sławiński
@ 2021-01-21  0:05 ` Mark Brown
  2021-01-21 19:39 ` Mark Brown
  6 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2021-01-21  0:05 UTC (permalink / raw)
  To: Amadeusz Sławiński, Liam Girdwood, Takashi Iwai
  Cc: Cezary Rojewski, Pierre-Louis Bossart, alsa-devel

On Wed, 20 Jan 2021 16:28:41 +0100, Amadeusz Sławiński wrote:
> This series adds unit tests for ASoC topology.
> 
> First fix problems found when developing and running test cases and
> then add tests implementation.
> 
> Tests themselves are quite simple and just call
> snd_soc_tplg_component_load() with various parameters and check the
> result. Tests themselves are described in more detail in commits
> adding them.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: topology: Properly unregister DAI on removal
      commit: fc4cb1e15f0c66f2e37314349dc4a82bd946fbb1

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()"
  2021-01-20 17:29   ` Mark Brown
@ 2021-01-21 15:08     ` Amadeusz Sławiński
  0 siblings, 0 replies; 10+ messages in thread
From: Amadeusz Sławiński @ 2021-01-21 15:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: Pierre-Louis Bossart, Cezary Rojewski, Takashi Iwai, alsa-devel,
	Liam Girdwood

On 1/20/2021 6:29 PM, Mark Brown wrote:
> On Wed, Jan 20, 2021 at 04:28:43PM +0100, Amadeusz Sławiński wrote:
>> As described in preceding patch, waiting with DAI removal until device
>> is being unregistered can lead to working on freed memory, hence having
> 
> There is a potential for misuse with essentially all devm functions -
> why is this one so special?

Hm... right, I blinded myself thinking too much about specific use case 
where it was used in topology. As topology is used with drivers split 
into multiple modules where it caused problem. It should work fine if 
someone writes monolithic module and needs to register DAI. I guess we 
can skip this patch then.

> 
> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.
> 
>> DAIs managed by resource managed memory is a bad idea, revert commit
>> creating devm_ API for this, so no one makes a mistake of using it.
>>
>> This reverts commit 0fae253af563cf5d1f5dc651d520c3eafd74f183.
> 
> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: (subset) [PATCH 0/5] Add KUNIT tests for ASoC topology
  2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
                   ` (5 preceding siblings ...)
  2021-01-21  0:05 ` (subset) [PATCH 0/5] Add KUNIT tests for ASoC topology Mark Brown
@ 2021-01-21 19:39 ` Mark Brown
  6 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2021-01-21 19:39 UTC (permalink / raw)
  To: Amadeusz Sławiński, Liam Girdwood, Takashi Iwai
  Cc: Cezary Rojewski, alsa-devel, Pierre-Louis Bossart

On Wed, 20 Jan 2021 16:28:41 +0100, Amadeusz Sławiński wrote:
> This series adds unit tests for ASoC topology.
> 
> First fix problems found when developing and running test cases and
> then add tests implementation.
> 
> Tests themselves are quite simple and just call
> snd_soc_tplg_component_load() with various parameters and check the
> result. Tests themselves are described in more detail in commits
> adding them.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[3/5] ASoC: topology: KUnit: Add KUnit tests passing various arguments to snd_soc_tplg_component_load
      commit: d52bbf747cfa8a2988289009241214a84982cc7d
[4/5] ASoC: topology: KUnit: Add KUnit tests passing empty topology with variants to snd_soc_tplg_component_load
      commit: cec9128dfcf9101f903470e43d46278e5b07ef24
[5/5] ASoC: topology: KUnit: Add KUnit tests passing topology with PCM to snd_soc_tplg_component_load
      commit: 3ad8c8e9efc53d14d928b84aabe1a27dd5d3171b

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-21 19:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 15:28 [PATCH 0/5] Add KUNIT tests for ASoC topology Amadeusz Sławiński
2021-01-20 15:28 ` [PATCH 1/5] ASoC: topology: Properly unregister DAI on removal Amadeusz Sławiński
2021-01-20 15:28 ` [PATCH 2/5] Revert "ASoC: soc-devres: add devm_snd_soc_register_dai()" Amadeusz Sławiński
2021-01-20 17:29   ` Mark Brown
2021-01-21 15:08     ` Amadeusz Sławiński
2021-01-20 15:28 ` [PATCH 3/5] ASoC: topology: KUnit: Add KUnit tests passing various arguments to snd_soc_tplg_component_load Amadeusz Sławiński
2021-01-20 15:28 ` [PATCH 4/5] ASoC: topology: KUnit: Add KUnit tests passing empty topology with variants " Amadeusz Sławiński
2021-01-20 15:28 ` [PATCH 5/5] ASoC: topology: KUnit: Add KUnit tests passing topology with PCM " Amadeusz Sławiński
2021-01-21  0:05 ` (subset) [PATCH 0/5] Add KUNIT tests for ASoC topology Mark Brown
2021-01-21 19:39 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).