linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] docs: admin-guide: add valsa driver documentation
@ 2023-05-13 20:20 Ivan Orlov
  2023-05-13 20:20 ` [PATCH 2/3] ALSA: Implement the new virtual driver Ivan Orlov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ivan Orlov @ 2023-05-13 20:20 UTC (permalink / raw)
  To: corbet, akpm, perex, tiwai, broonie, skhan
  Cc: Ivan Orlov, linux-doc, linux-kernel, alsa-devel, linux-kselftest,
	gregkh, himadrispandya, linux-kernel-mentees

Add documentation for the new Virtual ALSA driver. It covers all possible
usage cases: errors and delay injections, random and pattern-based data
generation, playback and ioctl redefinition functionalities testing.

We have a lot of different virtual media drivers, which can be used for
testing of the userspace applications and media subsystem middle layer.
However, all of them are aimed at testing the video functionality and
simulating the video devices. For audio devices we have only snd-dummy
module, which is good in simulating the correct behavior of an ALSA device.
I decided to write a tool, which would help to test the userspace ALSA
programs (and the PCM middle layer as well) under unusual circumstances
to figure out how they would behave. So I came up with this Virtual ALSA
Driver.

This new Virtual ALSA Driver has several features which can be useful
during the userspace ALSA applications testing/fuzzing, or testing/fuzzing
of the PCM middle layer. Not all of them can be implemented using the
existing virtual drivers (like dummy or loopback). Here is what can this
driver do:

- Simulate both capture and playback processes
- Check the playback stream for containing the looped pattern
- Generate random or pattern-based capture data
- Inject delays into the playback and capturing processes
- Inject errors during the PCM callbacks

Also, this driver can check the playback stream for containing the
predefined pattern, which is used in the corresponding selftest to check
the PCM middle layer data transferring functionality. Additionally, this
driver redefines the default RESET ioctl, and the selftest covers this PCM
API functionality as well.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 Documentation/admin-guide/index.rst |   1 +
 Documentation/admin-guide/valsa.rst | 114 ++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)
 create mode 100644 Documentation/admin-guide/valsa.rst

diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
index 43ea35613dfc..328cc59275a1 100644
--- a/Documentation/admin-guide/index.rst
+++ b/Documentation/admin-guide/index.rst
@@ -131,6 +131,7 @@ configure specific aspects of kernel behavior to your liking.
    thunderbolt
    ufs
    unicode
+   valsa
    vga-softcursor
    video-output
    xfs
diff --git a/Documentation/admin-guide/valsa.rst b/Documentation/admin-guide/valsa.rst
new file mode 100644
index 000000000000..64ffc130fb4c
--- /dev/null
+++ b/Documentation/admin-guide/valsa.rst
@@ -0,0 +1,114 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+The Virtual ALSA Driver
+=======================
+
+The Virtual ALSA Driver emulates a generic ALSA device, and can be used for
+testing/fuzzing of the userspace ALSA applications, as well as for testing/fuzzing of
+the ALSA middle layer. Additionally, it can be used for simulating hard to reproduce
+problems with PCM devices.
+
+What can this driver do?
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+At this moment the driver can do the following things:
+	* Simulate both capture and playback processes
+	* Generate random or pattern-based capturing data
+	* Inject delays into the playback and capturing processes
+	* Inject errors during the PCM callbacks
+
+Also, this driver can check the playback stream for containing the
+predefined pattern, which is used in the corresponding selftest (alsa/valsa-test.sh)
+to check the PCM middle layer data transferring functionality. Additionally, this
+driver redefines the default RESET ioctl, and the selftest covers this PCM
+API functionality as well.
+
+Configuration
+-------------
+
+The driver has several parameters besides the common ALSA module parameters:
+
+	* fill_mode (bool) - Buffer fill mode (see below)
+	* inject_delay (int)
+	* inject_hwpars_err (bool)
+	* inject_prepare_err (bool)
+	* inject_trigger_err (bool)
+
+
+Capture Data Generation
+-----------------------
+
+The driver has two modes of data generation: the first (0 in the fill_mode parameter)
+means random data generation, the second (1 in the fill_mode) - pattern-based
+data generation. Let's look at the second mode.
+
+First of all, you may want to specify the pattern for data generation. You can do it
+by writing the pattern to the debugfs file (/sys/kernel/debug/valsa/fill_pattern).
+Like that:
+
+.. code-block:: bash
+
+	echo -n mycoolpattern > /sys/kernel/debug/valsa/fill_pattern
+
+After this, every capture action performed on the 'valsa' device will return
+'mycoolpatternmycoolpatternmycoolpatternmy...' in the capturing buffer.
+
+The pattern itself can be up to 4096 bytes long.
+
+Delay injection
+---------------
+
+The driver has 'inject_delay' parameter, which has very self-descriptive name and
+can be used for time delay/speedup simulations. The parameter has integer type, and
+it means the delay added between module's internal timer ticks.
+
+If the 'inject_delay' value is positive, the buffer will be filled slower, if it is
+negative - faster. You can try it yourself by starting a recording in any
+audiorecording application (like Audacity) and selecting the 'valsa' device as a
+source.
+
+This parameter can be also used for generating a huge amount of sound data in a very
+short period of time (with the negative 'inject_delay' value).
+
+Errors injection
+----------------
+
+This module can be used for injecting errors into the PCM communication process. This
+action can help you to figure out how the userspace ALSA program behaves under unusual
+circumstances.
+
+For example, you can make all 'hw_params' PCM callback calls return EBUSY error by
+writing '1' to the 'inject_hwpars_err' module parameter:
+
+.. code-block:: bash
+
+	echo 1 > /sys/module/snd_valsa/parameters/inject_hwpars_err
+
+Errors can be injected into the following PCM callbacks:
+
+	* hw_params (EBUSY)
+	* prepare (EINVAL)
+	* trigger (EINVAL)
+
+
+Playback test
+-------------
+
+This driver can be also used for the playback functionality testing - every time you
+write the playback data to the 'valsa' PCM device and close it, the driver checks the
+buffer for containing the looped pattern (which is specified in the fill_pattern
+debugfs file). If the playback buffer content represents the looped pattern, 'pc_test'
+debugfs entry is set into '1'. Otherwise, the driver sets it to '0'.
+
+ioctl redefinition test
+-----------------------
+
+The driver redefines the 'reset' ioctl, which is default for all PCM devices. To test
+this functionality, we can trigger the reset ioctl and check the 'ioctl_test' debugfs
+entry:
+
+.. code-block:: bash
+
+	cat /sys/kernel/debug/valsa/ioctl_test
+
+If the ioctl is triggered successfully, this file will contain '1', and '0' otherwise.
-- 
2.34.1


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

* [PATCH 2/3] ALSA: Implement the new virtual driver
  2023-05-13 20:20 [PATCH 1/3] docs: admin-guide: add valsa driver documentation Ivan Orlov
@ 2023-05-13 20:20 ` Ivan Orlov
  2023-05-14  9:18   ` Takashi Iwai
  2023-05-13 20:20 ` [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver Ivan Orlov
  2023-05-14  9:21 ` [PATCH 1/3] docs: admin-guide: add valsa driver documentation Takashi Iwai
  2 siblings, 1 reply; 9+ messages in thread
From: Ivan Orlov @ 2023-05-13 20:20 UTC (permalink / raw)
  To: corbet, akpm, perex, tiwai, broonie, skhan
  Cc: Ivan Orlov, linux-doc, linux-kernel, alsa-devel, linux-kselftest,
	gregkh, himadrispandya, linux-kernel-mentees

We have a lot of different virtual media drivers, which can be used for
testing of the userspace applications and media subsystem middle layer.
However, all of them are aimed at testing the video functionality and
simulating the video devices. For audio devices we have only snd-dummy
module, which is good in simulating the correct behavior of an ALSA device.
I decided to write a tool, which would help to test the userspace ALSA
programs (and the PCM middle layer as well) under unusual circumstances
to figure out how they would behave. So I came up with this Virtual ALSA
Driver.

This new Virtual ALSA Driver has several features which can be useful
during the userspace ALSA applications testing/fuzzing, or testing/fuzzing
of the PCM middle layer. Not all of them can be implemented using the
existing virtual drivers (like dummy or loopback). Here is what can this
driver do:

- Simulate both capture and playback processes
- Generate random or pattern-based capture data
- Inject delays into the playback and capturing processes
- Inject errors during the PCM callbacks

Also, this driver can check the playback stream for containing the
predefined pattern, which is used in the corresponding selftest to check
the PCM middle layer data transferring functionality. Additionally, this
driver redefines the default RESET ioctl, and the selftest covers this PCM
API functionality as well.

Pattern-based capture stream data generation works in the following way:
user can set the pattern by writing to the 'fill_pattern' debugfs file.
After that, the capture stream in case of reading will be filled with this
pattern (for example, if the pattern is 'abc', the capture stream will
contain 'abcabcabc...'). The pattern itself can be up to 4096 bytes long.

After all, I think this driver would be a good start, and I believe in the
future we will see more virtual sound drivers.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 MAINTAINERS            |   7 +
 sound/drivers/Kconfig  |  15 ++
 sound/drivers/Makefile |   2 +
 sound/drivers/valsa.c  | 508 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 532 insertions(+)
 create mode 100644 sound/drivers/valsa.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7e0b87d5aa2e..6dcb09d5f3d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22411,6 +22411,13 @@ S:	Maintained
 F:	drivers/nvdimm/virtio_pmem.c
 F:	drivers/nvdimm/nd_virtio.c
 
+VIRTUAL ALSA DRIVER
+M:	Ivan Orlov <ivan.orlov0322@gmail.com>
+S:	Maintained
+F:	Documentation/admin-guide/valsa.rst
+F:	sound/drivers/valsa.c
+F:	tools/testing/selftests/alsa/valsa-test.sh
+
 VIRTUAL BOX GUEST DEVICE DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 M:	Arnd Bergmann <arnd@arndb.de>
diff --git a/sound/drivers/Kconfig b/sound/drivers/Kconfig
index be3009746f3a..fa7d6f22ff06 100644
--- a/sound/drivers/Kconfig
+++ b/sound/drivers/Kconfig
@@ -109,6 +109,21 @@ config SND_ALOOP
 	  To compile this driver as a module, choose M here: the module
 	  will be called snd-aloop.
 
+config SND_VALSA
+	tristate "Virtual ALSA driver"
+	select SND_PCM
+	help
+	  Say 'Y' or 'M' to include support for the Virtual ALSA driver. This
+	  driver is aimed at extended testing of the userspace applications
+	  which use the ALSA API, as well as the ALSA middle layer testing.
+
+	  It can generate random or pattern-based data into the capture stream,
+	  check the playback stream for containing the selected pattern, inject
+	  time delays during capture/playback, inject errors into the PCM
+	  callbacks, redefine the RESET ioctl operation to perform the PCM middle
+	  layer testing and inject errors during the PCM callbacks. You can find
+	  the corresponding selftest in the 'alsa' selftests folder.
+
 config SND_VIRMIDI
 	tristate "Virtual MIDI soundcard"
 	depends on SND_SEQUENCER
diff --git a/sound/drivers/Makefile b/sound/drivers/Makefile
index b60303180a1b..229eeb788995 100644
--- a/sound/drivers/Makefile
+++ b/sound/drivers/Makefile
@@ -8,6 +8,7 @@ snd-dummy-objs := dummy.o
 snd-aloop-objs := aloop.o
 snd-mtpav-objs := mtpav.o
 snd-mts64-objs := mts64.o
+snd-valsa-objs := valsa.o
 snd-portman2x4-objs := portman2x4.o
 snd-serial-u16550-objs := serial-u16550.o
 snd-serial-generic-objs := serial-generic.o
@@ -17,6 +18,7 @@ snd-virmidi-objs := virmidi.o
 obj-$(CONFIG_SND_DUMMY) += snd-dummy.o
 obj-$(CONFIG_SND_ALOOP) += snd-aloop.o
 obj-$(CONFIG_SND_VIRMIDI) += snd-virmidi.o
+obj-$(CONFIG_SND_VALSA) += snd-valsa.o
 obj-$(CONFIG_SND_SERIAL_U16550) += snd-serial-u16550.o
 obj-$(CONFIG_SND_SERIAL_GENERIC) += snd-serial-generic.o
 obj-$(CONFIG_SND_MTPAV) += snd-mtpav.o
diff --git a/sound/drivers/valsa.c b/sound/drivers/valsa.c
new file mode 100644
index 000000000000..62f05271cbad
--- /dev/null
+++ b/sound/drivers/valsa.c
@@ -0,0 +1,508 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ALSA virtual driver
+ *
+ * Copyright 2023 Ivan Orlov <ivan.orlov0322@gmail.com>
+ *
+ * This is a simple virtual ALSA driver, which can be used for audio applications/ALSA middle layer
+ * testing or fuzzing.
+ * It can:
+ *	- Simulate 'playback' and 'capture' actions
+ *	- Generate random or pattern-based capture data
+ *	- Check playback buffer for containing looped template, and notify about the results
+ *	through the debugfs entry
+ *	- Inject delays into the playback and capturing processes. See 'inject_delay' parameter.
+ *	- Inject errors during the PCM callbacks.
+ *	- Register custom RESET ioctl and notify when it is called through the debugfs entry
+ *
+ * The driver supports framerates from 8 kHz to 48 kHz. At the moment, only one substream
+ * is supported.
+ *
+ * You can find the corresponding selftest in the 'alsa' selftests folder.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <sound/pcm.h>
+#include <sound/core.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/timer.h>
+#include <linux/random.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+
+#define DEVNAME "valsad"
+#define CARD_NAME "virtualcard"
+#define TIMER_PER_SEC 5
+#define TIMER_INTERVAL (HZ / TIMER_PER_SEC)
+#define DELAY_JIFFIES HZ
+
+#define FILL_MODE_RAND	0
+#define FILL_MODE_PAT	1
+
+#define MAX_PATTERN_LEN 4096
+
+static int index = -1;
+static char *id = "valsa";
+static bool enable = true;
+static int inject_delay;
+static bool inject_hwpars_err;
+static bool inject_prepare_err;
+static bool inject_trigger_err;
+
+static short fill_mode = FILL_MODE_PAT;
+static char fill_pattern[MAX_PATTERN_LEN] = "abacaba";
+static ssize_t pattern_len = 7;
+static struct timer_list timer;
+
+static u8 playback_capture_test;
+static u8 ioctl_reset_test;
+static struct dentry *driver_debug_dir;
+
+module_param(index, int, 0444);
+MODULE_PARM_DESC(index, "Index value for " CARD_NAME " soundcard");
+module_param(id, charp, 0444);
+MODULE_PARM_DESC(id, "ID string for " CARD_NAME " soundcard");
+module_param(enable, bool, 0444);
+MODULE_PARM_DESC(enable, "Enable " CARD_NAME " soundcard.");
+module_param(fill_mode, short, 0600);
+MODULE_PARM_DESC(fill_mode, "Buffer fill mode: rand(0) or pattern(1)");
+module_param(inject_delay, int, 0600);
+MODULE_PARM_DESC(inject_delay, "Inject delays during playback/capture (in jiffies)");
+module_param(inject_hwpars_err, bool, 0600);
+MODULE_PARM_DESC(inject_hwpars_err, "Inject EBUSY error in the 'hw_params' callback");
+module_param(inject_prepare_err, bool, 0600);
+MODULE_PARM_DESC(inject_prepare_err, "Inject EINVAL error in the 'prepare' callback");
+module_param(inject_trigger_err, bool, 0600);
+MODULE_PARM_DESC(inject_trigger_err, "Inject EINVAL error in the 'trigger' callback");
+
+struct valsa {
+	struct snd_pcm_substream *substream;
+	struct snd_pcm *pcm;
+	size_t buf_pos;
+	size_t period_pos;
+	size_t b_rw;
+	bool is_buf_corrupted;
+	size_t period_bytes;
+	size_t total_bytes;
+	struct platform_device *pdev;
+	struct snd_card *card;
+};
+
+static struct valsa *valsa;
+
+static struct snd_pcm_hardware snd_valsa_hw = {
+	.info = (SNDRV_PCM_INFO_INTERLEAVED |
+		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
+		 SNDRV_PCM_INFO_MMAP_VALID),
+	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
+	.rates =		SNDRV_PCM_RATE_8000_48000,
+	.rate_min =		8000,
+	.rate_max =		48000,
+	.channels_min =		1,
+	.channels_max =		2,
+	.buffer_bytes_max =	32768,
+	.period_bytes_min =	4096,
+	.period_bytes_max =	32768,
+	.periods_min =		1,
+	.periods_max =		1024,
+};
+
+static inline void inc_buf_pos(size_t by, size_t bytes)
+{
+	valsa->total_bytes += by;
+	valsa->buf_pos += by;
+	valsa->buf_pos %= bytes;
+}
+
+/*
+ * Check one block of the buffer. Here we iterate the buffer until we find '0'. This condition is
+ * necessary because we need to detect when the reading/writing ends, so we assume that the pattern
+ * doesn't contain zeros.
+ */
+static void check_buf_block(struct valsa *valsa, struct snd_pcm_runtime *runtime)
+{
+	size_t i;
+	u8 current_byte;
+
+	for (i = 0; i < valsa->b_rw; i++) {
+		current_byte = runtime->dma_area[valsa->buf_pos];
+		if (!current_byte)
+			break;
+		if (current_byte != fill_pattern[valsa->total_bytes % pattern_len]) {
+			valsa->is_buf_corrupted = true;
+			break;
+		}
+		inc_buf_pos(1, runtime->dma_bytes);
+	}
+	inc_buf_pos(valsa->b_rw - i, runtime->dma_bytes);
+}
+
+static void fill_block_pattern(struct valsa *valsa, struct snd_pcm_runtime *runtime)
+{
+	size_t i;
+
+	for (i = 0; i < valsa->b_rw; i++) {
+		runtime->dma_area[valsa->buf_pos] = fill_pattern[valsa->total_bytes % pattern_len];
+		inc_buf_pos(1, runtime->dma_bytes);
+	}
+}
+
+static void fill_block_random(struct valsa *valsa, struct snd_pcm_runtime *runtime)
+{
+	size_t in_cur_block = runtime->dma_bytes - valsa->buf_pos;
+
+	if (valsa->b_rw <= in_cur_block) {
+		get_random_bytes(&runtime->dma_area[valsa->buf_pos], valsa->b_rw);
+	} else {
+		get_random_bytes(&runtime->dma_area[valsa->buf_pos], in_cur_block);
+		get_random_bytes(runtime->dma_area, valsa->b_rw - in_cur_block);
+	}
+	inc_buf_pos(valsa->b_rw, runtime->dma_bytes);
+}
+
+static void fill_block(struct valsa *valsa, struct snd_pcm_runtime *runtime)
+{
+	switch (fill_mode) {
+	case FILL_MODE_RAND:
+		fill_block_random(valsa, runtime);
+		break;
+	case FILL_MODE_PAT:
+		fill_block_pattern(valsa, runtime);
+		break;
+	}
+}
+
+/*
+ * Here we iterate through the buffer by (buffer_size / iterates_per_second) bytes.
+ * The driver uses timer to simulate the hardware pointer moving, and notify the PCM middle layer
+ * about period elapsed.
+ */
+static void timer_timeout(struct timer_list *data)
+{
+	struct snd_pcm_runtime *runtime;
+
+	if (!valsa->substream)
+		return;
+	runtime = valsa->substream->runtime;
+
+	if (valsa->substream->stream == SNDRV_PCM_STREAM_PLAYBACK && !valsa->is_buf_corrupted)
+		check_buf_block(valsa, runtime);
+	else if (valsa->substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		fill_block(valsa, runtime);
+	else
+		inc_buf_pos(valsa->b_rw, runtime->dma_bytes);
+
+	valsa->period_pos += valsa->b_rw;
+	if (valsa->period_pos >= valsa->period_bytes) {
+		valsa->period_pos %= valsa->period_bytes;
+		snd_pcm_period_elapsed(valsa->substream);
+	}
+
+	mod_timer(&timer, jiffies + TIMER_INTERVAL + inject_delay);
+}
+
+static int snd_valsa_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct valsa *valsa = snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	runtime->hw = snd_valsa_hw;
+	valsa->substream = substream;
+	valsa->buf_pos = 0;
+	valsa->is_buf_corrupted = false;
+	valsa->period_pos = 0;
+	valsa->total_bytes = 0;
+
+	playback_capture_test = 0;
+	ioctl_reset_test = 0;
+
+	timer_shutdown_sync(&timer);
+	timer_setup(&timer, timer_timeout, 0);
+	mod_timer(&timer, jiffies + TIMER_INTERVAL);
+	return 0;
+}
+
+static int snd_valsa_pcm_close(struct snd_pcm_substream *substream)
+{
+	struct valsa *valsa = snd_pcm_substream_chip(substream);
+
+	timer_shutdown_sync(&timer);
+	valsa->substream = NULL;
+	playback_capture_test = !valsa->is_buf_corrupted;
+	return 0;
+}
+
+static int snd_valsa_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	if (inject_trigger_err)
+		return -EINVAL;
+
+	valsa->period_bytes = frames_to_bytes(runtime, runtime->period_size);
+	// We want to record RATE samples per sec, it is rate * sample_bytes bytes
+	valsa->b_rw = runtime->rate * runtime->sample_bits / 8 / TIMER_PER_SEC;
+	return 0;
+}
+
+static snd_pcm_uframes_t snd_valsa_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	return bytes_to_frames(substream->runtime, valsa->buf_pos);
+}
+
+static int snd_valsa_free(struct valsa *valsa)
+{
+	if (!valsa)
+		return 0;
+	kfree(valsa);
+	return 0;
+}
+
+// These callbacks are required, but empty - all freeing occurs in pdev_remove
+static int snd_valsa_dev_free(struct snd_device *device)
+{
+	return 0;
+}
+
+static void valsa_pdev_release(struct device *dev)
+{
+}
+
+static int snd_valsa_pcm_prepare(struct snd_pcm_substream *substream)
+{
+	if (inject_prepare_err)
+		return -EINVAL;
+	return 0;
+}
+
+static int snd_valsa_pcm_hw_params(struct snd_pcm_substream *substream,
+				   struct snd_pcm_hw_params *params)
+{
+	if (inject_hwpars_err)
+		return -EBUSY;
+	return 0;
+}
+
+static int snd_valsa_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int snd_valsa_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg)
+{
+	switch (cmd) {
+	case SNDRV_PCM_IOCTL1_RESET:
+		ioctl_reset_test = 1;
+		break;
+	}
+	return snd_pcm_lib_ioctl(substream, cmd, arg);
+}
+
+static const struct snd_pcm_ops snd_valsa_playback_ops = {
+	.open =		snd_valsa_pcm_open,
+	.close =	snd_valsa_pcm_close,
+	.trigger =	snd_valsa_pcm_trigger,
+	.hw_params =	snd_valsa_pcm_hw_params,
+	.ioctl =	snd_valsa_ioctl,
+	.hw_free =	snd_valsa_pcm_hw_free,
+	.prepare =	snd_valsa_pcm_prepare,
+	.pointer =	snd_valsa_pcm_pointer,
+};
+
+static const struct snd_pcm_ops snd_valsa_capture_ops = {
+	.open =		snd_valsa_pcm_open,
+	.close =	snd_valsa_pcm_close,
+	.trigger =	snd_valsa_pcm_trigger,
+	.hw_params =	snd_valsa_pcm_hw_params,
+	.hw_free =	snd_valsa_pcm_hw_free,
+	.ioctl =	snd_valsa_ioctl,
+	.prepare =	snd_valsa_pcm_prepare,
+	.pointer =	snd_valsa_pcm_pointer,
+};
+
+static int snd_valsa_new_pcm(struct valsa *valsa)
+{
+	struct snd_pcm *pcm;
+	int err;
+
+	err = snd_pcm_new(valsa->card, "VirtualAlsa", 0, 1, 1, &pcm);
+	if (err < 0)
+		return err;
+	pcm->private_data = valsa;
+	strcpy(pcm->name, "VirtualAlsa");
+	valsa->pcm = pcm;
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_valsa_playback_ops);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &snd_valsa_capture_ops);
+
+	err = snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, &valsa->pdev->dev,
+					     64 * 1024, 64 * 1024);
+	return err;
+}
+
+static int snd_valsa_create(struct snd_card *card, struct platform_device *pdev,
+			    struct valsa **r_valsa)
+{
+	struct valsa *valsa;
+	int err;
+	static const struct snd_device_ops ops = {
+		.dev_free = snd_valsa_dev_free,
+	};
+
+	valsa = kzalloc(sizeof(*valsa), GFP_KERNEL);
+	if (!valsa)
+		return -ENOMEM;
+	valsa->card = card;
+	valsa->pdev = pdev;
+	valsa->buf_pos = 0;
+	valsa->period_pos = 0;
+
+	err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, valsa, &ops);
+	if (err < 0)
+		goto _err_free_chip;
+
+	err = snd_valsa_new_pcm(valsa);
+	if (err < 0)
+		goto _err_free_chip;
+
+	*r_valsa = valsa;
+	return 0;
+
+_err_free_chip:
+	snd_valsa_free(valsa);
+	return err;
+}
+
+static int valsa_probe(struct platform_device *pdev)
+{
+	struct snd_card *card;
+	int err;
+
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (err)
+		return err;
+
+	err = snd_devm_card_new(&pdev->dev, index, id, THIS_MODULE, 0, &card);
+	if (err < 0)
+		return err;
+	err = snd_valsa_create(card, pdev, &valsa);
+	if (err < 0)
+		return err;
+
+	strcpy(card->driver, "VirtualALSA");
+	strcpy(card->shortname, "VirtualALSA");
+	strcpy(card->longname, "Virtual ALSA card");
+
+	err = snd_card_register(card);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int pdev_remove(struct platform_device *dev)
+{
+	snd_valsa_free(valsa);
+	return 0;
+}
+
+static struct platform_device valsa_pdev = {
+	.name =		"valsa",
+	.dev.release =	valsa_pdev_release,
+};
+
+static struct platform_driver valsa_pdrv = {
+	.probe =	valsa_probe,
+	.remove =	pdev_remove,
+	.driver =	{
+		.name = "valsa",
+	},
+};
+
+static ssize_t pattern_write(struct file *file, const char __user *buff, size_t len, loff_t *off)
+{
+	ssize_t to_write = len;
+
+	if (*off + to_write > MAX_PATTERN_LEN)
+		to_write = MAX_PATTERN_LEN - *off;
+
+	// Crop silently everything over the buffer
+	if (to_write <= 0)
+		return len;
+
+	if (copy_from_user(fill_pattern + *off, buff, to_write))
+		return -EFAULT;
+	pattern_len = *off + to_write;
+	*off += to_write;
+
+	return to_write;
+}
+
+static ssize_t pattern_read(struct file *file, char __user *buff, size_t len, loff_t *off)
+{
+	ssize_t to_read = len;
+
+	if (*off + to_read >= MAX_PATTERN_LEN)
+		to_read = MAX_PATTERN_LEN - *off;
+	if (to_read <= 0)
+		return 0;
+
+	if (copy_to_user(buff, fill_pattern + *off, to_read))
+		to_read = 0;
+	else
+		*off += to_read;
+
+	return to_read;
+}
+
+static const struct file_operations fill_pattern_fops = {
+	.read = pattern_read,
+	.write = pattern_write,
+};
+
+static int init_debug_files(void)
+{
+	driver_debug_dir = debugfs_create_dir("valsa", NULL);
+	if (IS_ERR(driver_debug_dir))
+		return PTR_ERR(driver_debug_dir);
+	debugfs_create_u8("pc_test", 0444, driver_debug_dir, &playback_capture_test);
+	debugfs_create_u8("ioctl_test", 0444, driver_debug_dir, &ioctl_reset_test);
+	debugfs_create_file("fill_pattern", 0600, driver_debug_dir, NULL, &fill_pattern_fops);
+
+	return 0;
+}
+
+static void clear_debug_files(void)
+{
+	debugfs_remove_recursive(driver_debug_dir);
+}
+
+static int __init mod_init(void)
+{
+	int err = 0;
+
+	err = init_debug_files();
+	if (err)
+		return err;
+	err = platform_device_register(&valsa_pdev);
+	if (err)
+		return err;
+	err = platform_driver_register(&valsa_pdrv);
+	if (err)
+		platform_device_unregister(&valsa_pdev);
+	return err;
+}
+
+static void __exit mod_exit(void)
+{
+	clear_debug_files();
+
+	platform_driver_unregister(&valsa_pdrv);
+	platform_device_unregister(&valsa_pdev);
+}
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ivan Orlov");
+module_init(mod_init);
+module_exit(mod_exit);
-- 
2.34.1


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

* [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver
  2023-05-13 20:20 [PATCH 1/3] docs: admin-guide: add valsa driver documentation Ivan Orlov
  2023-05-13 20:20 ` [PATCH 2/3] ALSA: Implement the new virtual driver Ivan Orlov
@ 2023-05-13 20:20 ` Ivan Orlov
  2023-05-15  1:28   ` Mark Brown
  2023-05-14  9:21 ` [PATCH 1/3] docs: admin-guide: add valsa driver documentation Takashi Iwai
  2 siblings, 1 reply; 9+ messages in thread
From: Ivan Orlov @ 2023-05-13 20:20 UTC (permalink / raw)
  To: corbet, akpm, perex, tiwai, broonie, skhan
  Cc: Ivan Orlov, linux-doc, linux-kernel, alsa-devel, linux-kselftest,
	gregkh, himadrispandya, linux-kernel-mentees

This test covers the new Virtual ALSA driver, including the capturing,
playback and ioctl redefinition functionalities. This test is also helpful
as an usage example of the 'valsa' driver.

We have a lot of different virtual media drivers, which can be used for
testing of the userspace applications and media subsystem middle layer.
However, all of them are aimed at testing the video functionality and
simulating the video devices. For audio devices we have only snd-dummy
module, which is good in simulating the correct behavior of an ALSA device.
I decided to write a tool, which would help to test the userspace ALSA
programs (and the PCM middle layer as well) under unusual circumstances
to figure out how they would behave. So I came up with this Virtual ALSA
Driver.

This new Virtual ALSA Driver has several features which can be useful
during the userspace ALSA applications testing/fuzzing, or testing/fuzzing
of the PCM middle layer. Not all of them can be implemented using the
existing virtual drivers (like dummy or loopback). Here is what can this
driver do:

- Simulate both capture and playback processes
- Generate random or pattern-based capture data
- Check the playback stream for containing the looped pattern
- Inject delays into the playback and capturing processes
- Inject errors during the PCM callbacks

Also, this driver can check the playback stream for containing the
predefined pattern, which is used in the corresponding selftest to check
the PCM middle layer data transferring functionality. Additionally, this
driver redefines the default RESET ioctl, and the selftest covers this PCM
API functionality as well.

After all, I think this driver would be a good start, and I believe in the
future we will see more virtual sound drivers.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 tools/testing/selftests/alsa/valsa-test.sh | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100755 tools/testing/selftests/alsa/valsa-test.sh

diff --git a/tools/testing/selftests/alsa/valsa-test.sh b/tools/testing/selftests/alsa/valsa-test.sh
new file mode 100755
index 000000000000..872dfac138c2
--- /dev/null
+++ b/tools/testing/selftests/alsa/valsa-test.sh
@@ -0,0 +1,55 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test for the 'valsa' virtual driver. It tests capture and playback functionalities,
+# as well as the 'reset' ioctl redefinition.
+
+uid=$(id -u)
+if [ $uid -ne 0 ]; then
+	echo "$0: Must be run as root"
+	exit 1
+fi
+
+if ! which modprobe > /dev/null 2>&1; then
+	echo "$0: You need modprobe installed"
+        exit 4
+fi
+
+if ! modinfo snd-valsa > /dev/null 2>&1; then
+	echo "$0: You must have the following enabled in your kernel:"
+	echo "CONFIG_SND_ALSAV=m"
+	exit 4
+fi
+
+modprobe snd-valsa
+
+if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
+	mount -t debugfs none /sys/kernel/debug
+
+	if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
+		echo "$0: Error mounting debugfs"
+		exit 4
+	fi
+fi
+
+success="1"
+arecord -D hw:CARD=valsa,DEV=0 -c 1 --buffer-size=8192 -f S16_LE -r 8000 --duration=4 out.wav
+
+if [[ $(< /sys/kernel/debug/valsa/ioctl_test) == "$success" ]]; then
+	echo "$0: ioctl test: success"
+else
+	echo "$0: ioctl test: fail"
+fi
+
+aplay -D hw:CARD=valsa,DEV=0 -c 1 --buffer-size=8192 -f S16_LE -r 8000 out.wav
+
+if [[ $(< /sys/kernel/debug/valsa/pc_test) == "$success" ]]; then
+	echo "$0: playback test: success"
+else
+	echo "$0: playback test: fail"
+fi
+
+rm -rf out.wav
+if ! rmmod snd-valsa > /dev/null 2>&1; then
+	echo "$0: Can't remove 'valsa' module: disable PulseAudio and unload it manually"
+fi
-- 
2.34.1


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

* Re: [PATCH 2/3] ALSA: Implement the new virtual driver
  2023-05-13 20:20 ` [PATCH 2/3] ALSA: Implement the new virtual driver Ivan Orlov
@ 2023-05-14  9:18   ` Takashi Iwai
  2023-05-14 10:08     ` Ivan Orlov
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2023-05-14  9:18 UTC (permalink / raw)
  To: Ivan Orlov
  Cc: corbet, akpm, perex, tiwai, broonie, skhan, linux-doc,
	linux-kernel, alsa-devel, linux-kselftest, gregkh,
	himadrispandya, linux-kernel-mentees

On Sat, 13 May 2023 22:20:36 +0200,
Ivan Orlov wrote:
> 
> We have a lot of different virtual media drivers, which can be used for
> testing of the userspace applications and media subsystem middle layer.
> However, all of them are aimed at testing the video functionality and
> simulating the video devices. For audio devices we have only snd-dummy
> module, which is good in simulating the correct behavior of an ALSA device.
> I decided to write a tool, which would help to test the userspace ALSA
> programs (and the PCM middle layer as well) under unusual circumstances
> to figure out how they would behave. So I came up with this Virtual ALSA
> Driver.
> 
> This new Virtual ALSA Driver has several features which can be useful
> during the userspace ALSA applications testing/fuzzing, or testing/fuzzing
> of the PCM middle layer. Not all of them can be implemented using the
> existing virtual drivers (like dummy or loopback). Here is what can this
> driver do:
> 
> - Simulate both capture and playback processes
> - Generate random or pattern-based capture data
> - Inject delays into the playback and capturing processes
> - Inject errors during the PCM callbacks
> 
> Also, this driver can check the playback stream for containing the
> predefined pattern, which is used in the corresponding selftest to check
> the PCM middle layer data transferring functionality. Additionally, this
> driver redefines the default RESET ioctl, and the selftest covers this PCM
> API functionality as well.
> 
> Pattern-based capture stream data generation works in the following way:
> user can set the pattern by writing to the 'fill_pattern' debugfs file.
> After that, the capture stream in case of reading will be filled with this
> pattern (for example, if the pattern is 'abc', the capture stream will
> contain 'abcabcabc...'). The pattern itself can be up to 4096 bytes long.
> 
> After all, I think this driver would be a good start, and I believe in the
> future we will see more virtual sound drivers.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>

The idea is interesting, and it's a definitely good thing to have.

I wonder, though, whether it could be better provided as an extention
to the existing snd-dummy driver.  The advantage of extending
snd-dummy driver would be that it already supports different formats,
etc.  OTOH, if we create an individual driver, the pro side is the
simpleness of the code.

I'm inclined to go with a separate driver, but I'm open about this.
Maybe Jaroslav and Mark have some opinions?

About this patch set: the driver name should be a bit more specific,
as this isn't a generic virtual driver that is used for general
purpose but it's only for testing.  And it's only for testing PCM.
So, a name like snd-test-pcm would be more appropriate, IMO.

And, we want the coverage of different formats, channels, rates and
accesses (interleaved vs non-interleaved).  How can we extend somehow
more for that?


thanks,

Takashi

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

* Re: [PATCH 1/3] docs: admin-guide: add valsa driver documentation
  2023-05-13 20:20 [PATCH 1/3] docs: admin-guide: add valsa driver documentation Ivan Orlov
  2023-05-13 20:20 ` [PATCH 2/3] ALSA: Implement the new virtual driver Ivan Orlov
  2023-05-13 20:20 ` [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver Ivan Orlov
@ 2023-05-14  9:21 ` Takashi Iwai
  2023-05-14 10:12   ` Ivan Orlov
  2 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2023-05-14  9:21 UTC (permalink / raw)
  To: Ivan Orlov
  Cc: corbet, akpm, perex, tiwai, broonie, skhan, linux-doc,
	linux-kernel, alsa-devel, linux-kselftest, gregkh,
	himadrispandya, linux-kernel-mentees

On Sat, 13 May 2023 22:20:35 +0200,
Ivan Orlov wrote:
> 
> Add documentation for the new Virtual ALSA driver. It covers all possible
> usage cases: errors and delay injections, random and pattern-based data
> generation, playback and ioctl redefinition functionalities testing.
> 
> We have a lot of different virtual media drivers, which can be used for
> testing of the userspace applications and media subsystem middle layer.
> However, all of them are aimed at testing the video functionality and
> simulating the video devices. For audio devices we have only snd-dummy
> module, which is good in simulating the correct behavior of an ALSA device.
> I decided to write a tool, which would help to test the userspace ALSA
> programs (and the PCM middle layer as well) under unusual circumstances
> to figure out how they would behave. So I came up with this Virtual ALSA
> Driver.
> 
> This new Virtual ALSA Driver has several features which can be useful
> during the userspace ALSA applications testing/fuzzing, or testing/fuzzing
> of the PCM middle layer. Not all of them can be implemented using the
> existing virtual drivers (like dummy or loopback). Here is what can this
> driver do:
> 
> - Simulate both capture and playback processes
> - Check the playback stream for containing the looped pattern
> - Generate random or pattern-based capture data
> - Inject delays into the playback and capturing processes
> - Inject errors during the PCM callbacks
> 
> Also, this driver can check the playback stream for containing the
> predefined pattern, which is used in the corresponding selftest to check
> the PCM middle layer data transferring functionality. Additionally, this
> driver redefines the default RESET ioctl, and the selftest covers this PCM
> API functionality as well.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
>  Documentation/admin-guide/index.rst |   1 +
>  Documentation/admin-guide/valsa.rst | 114 ++++++++++++++++++++++++++++

We have already subdirectories for the sound stuff
(Documentation/sound/*), and this should go to there, I suppose
(unless there is somewhere dedicated for each selftest scenario).


thanks,

Takashi

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

* Re: [PATCH 2/3] ALSA: Implement the new virtual driver
  2023-05-14  9:18   ` Takashi Iwai
@ 2023-05-14 10:08     ` Ivan Orlov
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Orlov @ 2023-05-14 10:08 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: corbet, akpm, perex, tiwai, broonie, skhan, linux-doc,
	linux-kernel, alsa-devel, linux-kselftest, gregkh,
	himadrispandya, linux-kernel-mentees

On 5/14/23 13:18, Takashi Iwai wrote:
> The idea is interesting, and it's a definitely good thing to have.
> 
> I wonder, though, whether it could be better provided as an extention
> to the existing snd-dummy driver.  The advantage of extending
> snd-dummy driver would be that it already supports different formats,
> etc.  OTOH, if we create an individual driver, the pro side is the
> simpleness of the code.
> 
> I'm inclined to go with a separate driver, but I'm open about this.
> Maybe Jaroslav and Mark have some opinions?
> 
> About this patch set: the driver name should be a bit more specific,
> as this isn't a generic virtual driver that is used for general
> purpose but it's only for testing.  And it's only for testing PCM.
> So, a name like snd-test-pcm would be more appropriate, IMO.
> 
> And, we want the coverage of different formats, channels, rates and
> accesses (interleaved vs non-interleaved).  How can we extend somehow
> more for that?
> 
> 
> thanks,
> 
> Takashi

Hello Takashi! Thank you for your reply. I fully agree with the naming 
issue, and I'll change it in the future versions of the patch set in 
case we choose to have it as a separate driver. I also prefer this 
option because in my opinion the use cases of these drivers are a little 
bit different. Also, I believe I can extend the driver to support 
different formats, channels and accesses in the near future.

Additionally, implementing these changes would be a perfect task for the 
end of the Linux Kernel Mentorship program I'm going through :) However, 
I'm open to other views on this, and I'm ready to move the functionality 
from my driver to the snd-dummy in case we prefer this option.

Thanks again for considering my changes!

Kind regards,
Ivan Orlov.


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

* Re: [PATCH 1/3] docs: admin-guide: add valsa driver documentation
  2023-05-14  9:21 ` [PATCH 1/3] docs: admin-guide: add valsa driver documentation Takashi Iwai
@ 2023-05-14 10:12   ` Ivan Orlov
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Orlov @ 2023-05-14 10:12 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: corbet, akpm, perex, tiwai, broonie, skhan, linux-doc,
	linux-kernel, alsa-devel, linux-kselftest, gregkh,
	himadrispandya, linux-kernel-mentees

On 5/14/23 13:21, Takashi Iwai wrote:
> We have already subdirectories for the sound stuff
> (Documentation/sound/*), and this should go to there, I suppose
> (unless there is somewhere dedicated for each selftest scenario).

Alright, I'll move the documentation to the sound subfolder. Thank you!

Kind regards,
Ivan Orlov.

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

* Re: [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver
  2023-05-13 20:20 ` [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver Ivan Orlov
@ 2023-05-15  1:28   ` Mark Brown
  2023-05-15  5:58     ` Ivan Orlov
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2023-05-15  1:28 UTC (permalink / raw)
  To: Ivan Orlov
  Cc: corbet, akpm, perex, tiwai, skhan, linux-doc, linux-kernel,
	alsa-devel, linux-kselftest, gregkh, himadrispandya,
	linux-kernel-mentees

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

On Sun, May 14, 2023 at 12:20:37AM +0400, Ivan Orlov wrote:

> +uid=$(id -u)
> +if [ $uid -ne 0 ]; then
> +	echo "$0: Must be run as root"
> +	exit 1
> +fi

It is not an error to run the selftest as a non-root user, the test
should be skipped.

> +modprobe snd-valsa

We don't check if the module was already loaded.

> +if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
> +	mount -t debugfs none /sys/kernel/debug
> +
> +	if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
> +		echo "$0: Error mounting debugfs"
> +		exit 4
> +	fi
> +fi

This will unconditionally attempt to mount debugfs in a standard
location and won't clean up after itself, if the system didn't have
debugfs mounted for some reason then this will leave it sitting there.

> +success="1"
> +arecord -D hw:CARD=valsa,DEV=0 -c 1 --buffer-size=8192 -f S16_LE -r 8000 --duration=4 out.wav
> +
> +if [[ $(< /sys/kernel/debug/valsa/ioctl_test) == "$success" ]]; then
> +	echo "$0: ioctl test: success"
> +else
> +	echo "$0: ioctl test: fail"
> +fi

Would it not be better to have a C program that actually calls the ioctl
rather than a custom debugfs thing that may or may not be wired up to do
the same thing as an ioctl would?  It seems like other than whatever
this ioctl test actually does this is all just a simplified version of
the existing pcm-test.

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

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

* Re: [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver
  2023-05-15  1:28   ` Mark Brown
@ 2023-05-15  5:58     ` Ivan Orlov
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Orlov @ 2023-05-15  5:58 UTC (permalink / raw)
  To: Mark Brown
  Cc: corbet, akpm, perex, tiwai, skhan, linux-doc, linux-kernel,
	alsa-devel, linux-kselftest, gregkh, himadrispandya,
	linux-kernel-mentees

On 5/15/23 05:28, Mark Brown wrote:
> On Sun, May 14, 2023 at 12:20:37AM +0400, Ivan Orlov wrote:
> 
>> +uid=$(id -u)
>> +if [ $uid -ne 0 ]; then
>> +	echo "$0: Must be run as root"
>> +	exit 1
>> +fi
> 
> It is not an error to run the selftest as a non-root user, the test
> should be skipped.

Alright, thanks!
>> +modprobe snd-valsa
> 
> We don't check if the module was already loaded.
> 
>> +if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
>> +	mount -t debugfs none /sys/kernel/debug
>> +
>> +	if [ ! -e /sys/kernel/debug/valsa/pc_test ]; then
>> +		echo "$0: Error mounting debugfs"
>> +		exit 4
>> +	fi
>> +fi
> 
> This will unconditionally attempt to mount debugfs in a standard
> location and won't clean up after itself, if the system didn't have
> debugfs mounted for some reason then this will leave it sitting there.

Yes, I agree... I'll remove this in the future versions, I think the 
approach with skipping in case of missing debugfs would be better.

By the way, I used the 'fpa' test as an example, and it looks like it 
should be fixed as well...

> Would it not be better to have a C program that actually calls the ioctl
> rather than a custom debugfs thing that may or may not be wired up to do
> the same thing as an ioctl would?  It seems like other than whatever
> this ioctl test actually does this is all just a simplified version of
> the existing pcm-test.

Well, the idea was to test the playback buffer consistency - the driver 
has the functionality of testing the playback buffer for containing the 
looped pattern (and if the buffer doesn't contain the looped pattern the 
test fails). So, firstly we get the capture buffer with this pattern 
(via arecord), and then send it to the driver for the test (via aplay).

The pcm-test (as I understand) performs only time checks, not the checks 
of the data played/captured, and I think it is pointless to test the 
time again. But I agree, that C test implementation would be better - 
using this approach I can perform additional buffer checks and cover 
more functionality of the driver.

Thank you very much for the review!

Kind regards,
Ivan Orlov.

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

end of thread, other threads:[~2023-05-15  6:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-13 20:20 [PATCH 1/3] docs: admin-guide: add valsa driver documentation Ivan Orlov
2023-05-13 20:20 ` [PATCH 2/3] ALSA: Implement the new virtual driver Ivan Orlov
2023-05-14  9:18   ` Takashi Iwai
2023-05-14 10:08     ` Ivan Orlov
2023-05-13 20:20 ` [PATCH 3/3] selftests: ALSA: Add test for the 'valsa' driver Ivan Orlov
2023-05-15  1:28   ` Mark Brown
2023-05-15  5:58     ` Ivan Orlov
2023-05-14  9:21 ` [PATCH 1/3] docs: admin-guide: add valsa driver documentation Takashi Iwai
2023-05-14 10:12   ` Ivan Orlov

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).