diff mbox series

[v2] kselftest: alsa: Add simplistic test for ALSA mixer controls kselftest

Message ID 20211206160305.194011-1-broonie@kernel.org
State Superseded
Headers show
Series [v2] kselftest: alsa: Add simplistic test for ALSA mixer controls kselftest | expand

Commit Message

Mark Brown Dec. 6, 2021, 4:03 p.m. UTC
Add a basic test for the mixer control interface. For every control on
every sound card in the system it checks that it can read and write the
default value where the control supports that and for writeable controls
attempts to write all valid values, restoring the default values after
each test to minimise disruption for users.

There are quite a few areas for improvement - currently no coverage of the
generation of notifications, several of the control types don't have any
coverage for the values and we don't have any testing of error handling
when we attempt to write out of range values - but this provides some basic
coverage.

This is added as a kselftest since unlike other ALSA test programs it does
not require either physical setup of the device or interactive monitoring
by users and kselftest is one of the test suites that is frequently run by
people doing general automated testing so should increase coverage. It is
written in terms of alsa-lib since tinyalsa is not generally packaged for
distributions which makes things harder for general users interested in
kselftest as a whole but it will be a barrier to people with Android.

Signed-off-by: Mark Brown <broonie@kernel.org>
---

v2: Use pkg-config to get CFLAGS and LDLIBS for alsa-lib.

 MAINTAINERS                               |   7 +
 tools/testing/selftests/Makefile          |   3 +-
 tools/testing/selftests/alsa/.gitignore   |   1 +
 tools/testing/selftests/alsa/Makefile     |   9 +
 tools/testing/selftests/alsa/mixer-test.c | 616 ++++++++++++++++++++++
 5 files changed, 635 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/alsa/.gitignore
 create mode 100644 tools/testing/selftests/alsa/Makefile
 create mode 100644 tools/testing/selftests/alsa/mixer-test.c

Comments

Takashi Sakamoto Dec. 8, 2021, 2:26 p.m. UTC | #1
On Tue, Dec 7, 2021, at 23:25, Mark Brown wrote:
> On Tue, Dec 07, 2021 at 12:20:32PM +0900, Takashi Sakamoto wrote:
>> On Mon, Dec 06, 2021 at 04:03:05PM +0000, Mark Brown wrote:
>
>> I think it safer to take care of volatile attribute when comparing read
>> value to written value. I'm glad if you review below patch.
>
> Yes, that's a good spot, it was an oversight to not take care of
> volatile controls - I'll roll that in if I send a new version or
> I guess Takashi could apply on top of my v2?  If people are mostly happy
> and at Jaroslav is also preparing patches on top of this it might make
> sense to get it into git sooner.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Feel free to append it for your new version with or without my sign-off.
If keeping it, I could review your respun one by receiving according to To
or Cc.

>> As another topic, the runtime of alsa-lib application largely differs
>> between process user due to the result of parsing text files for
>> configuration space. I can easily imagine that developers unfamiliar to
>> alsa-lib carelessly adds invalid or inadequate configurations to files
>> under target path of alsa-lib configuration space, and they are puzzled
>> since they are unaware of the fact that the kselftest is affected by
>> userspace stuffs for the runtime.
>
>> If we respect the basic theory of test (idempotence), we can use ioctl(2)
>> with requests for ALSA control interface since it's not so complicated
>> (at least it is easier than ALSA PCM interface). The purpose of
>> kselftest is to test kernel stuffs, not to test userspace stuffs
>> including alsa-lib implementation and variety of plugins.
>
> Right, I was originally thinking of implementing this in terms of
> tinyalsa which is much more direct (though I was amused to see that's
> gained userspace plugins at some point!) partly for this reason but the
> lack of widespread packaging for it was a bit of a blocker and it didn't
> feel like a great idea to essentially do yet another userspace ALSA
> library even if as you say it can be pretty trivial.  Jaroslav's
> suggestion of using a custom configuration to override the default seems
> like it addresses everything though.
>
> I do think there's an advantage for test comprehensibility in having the
> test written in terms of similar APIs to a normal userspace application
> - it makes it easier to relate what the test is doing to normal usage
> which is helpful when trying to understand what the test is trying to
> tell you.

In my opinion, test is merely test. It's not a sample program.

What important is what is tested. and how to assist developers if failed.
If more suitable for the direction, we should do it, even if using raw ioctl
in the case.

For your information, `check_event()` in `test/user-ctl-element-set.c`, my
rough implementation of test for event triggered by tlv operation, might
be helpful to you or start point t to discuss about event check.


Regards

Takashi Sakamoto
Mark Brown Dec. 8, 2021, 4:07 p.m. UTC | #2
On Wed, Dec 08, 2021 at 11:26:59PM +0900, Takashi Sakamoto wrote:
> On Tue, Dec 7, 2021, at 23:25, Mark Brown wrote:

> > I do think there's an advantage for test comprehensibility in having the
> > test written in terms of similar APIs to a normal userspace application
> > - it makes it easier to relate what the test is doing to normal usage
> > which is helpful when trying to understand what the test is trying to
> > tell you.

> In my opinion, test is merely test. It's not a sample program.

> What important is what is tested. and how to assist developers if failed.
> If more suitable for the direction, we should do it, even if using raw ioctl
> in the case.

Sure, but it's also important that if someone sees a test failure they
can figure out what was being tested and if the test makes sense - there
are plenty of testsuites out there with tests where what the problems
are in the test, or it's hard to tell what the test is even supposed to
be verifying.  Putting barriers in the way of understanding the test
means it's that bit harder to get people to put in the work required to
figure out what the test is trying to tell them and pay attention to it.

> For your information, `check_event()` in `test/user-ctl-element-set.c`, my
> rough implementation of test for event triggered by tlv operation, might
> be helpful to you or start point t to discuss about event check.

Thanks.  The main issue here is finding time to look at stuff rather
than figuring out the APIs, they're reasonably easy to work with
fortunately.
Mark Brown Dec. 8, 2021, 6:39 p.m. UTC | #3
On Wed, Dec 08, 2021 at 10:42:35AM -0700, Shuah Khan wrote:
> On 12/6/21 9:03 AM, Mark Brown wrote:

> > +SOUND - ALSA SELFTESTS
> > +M:	Mark Brown <broonie@kernel.org>
> > +L:	alsa-devel@alsa-project.org (moderated for non-subscribers)

> Please add linux-kselftest list as well here.

get_maintainers pulls it in from the wider entry (the mention of
alsa-devel is reudnant too).

> > +int num_cards = 0;
> > +int num_controls = 0;
> > +struct card_data *card_list = NULL;
> > +struct ctl_data *ctl_list = NULL;

> No need to initailize the above globals.

They're not declared static so the initial value is undefined.

> > +void find_controls(void)
> > +{
> > +	char name[32];

> Use SYSFS_PATH_MAX = 255 like other tools do?

This isn't a path, it's an ALSA limit for a name that is embedded in a
struct (snd_ctl_card_info->name).  There's no magic define for these
lengths.

> > +
> > +			ctl_data->next = ctl_list;
> > +			ctl_list = ctl_data;
> > +		}
> > +
> > +	next_card:

> No need to indent the label

No need but it looks wrong otherwise - it's certainly what I'd expect
for normal kernel code.

> > +	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
> > +		ksft_print_msg("%s is inactive\n", ctl->name);
> > +		ksft_test_result_skip("get_value.%d.%d\n",
> > +				      ctl->card->card, ctl->elem);
> 
> The two messages could be combined?

The user facing control names almost always have spaces in them so while
it's useful to have them for diagnostic purposes I don't think it's a
good idea to put them in the KTAP test names, that's likely to confuse
things trying to work with the KTAP output.  The general pattern I'm
following for these tests is to print one or more diagnostic lines
explaining why a tests was skipped or failed with the human readable
control name so people can hopefully figure out what's going on and have
the KTAP facing name that tooling will deal with be a consistent
test.card.control format for parsers and databases dealing with test
results en masse.

> > +bool test_ctl_write_valid_boolean(struct ctl_data *ctl)
> > +{
> > +	int err, i, j;
> > +	bool fail = false;
> > +	snd_ctl_elem_value_t *val;
> 
> Add blank line after declarations.
> 
> > +	snd_ctl_elem_value_alloca(&val);

This is idiomatic for alsa-lib code.

> > +int main(void)
> > +{
> > +	struct ctl_data *ctl;
> > +
> > +	ksft_print_header();

> Add a check for root and skil the test.

There is no need for this test to run as root in most configurations,
it is common to provide direct access to the sound cards to some or all
users - for example with desktop distros the entire userspace audio
subsystem normally runs as the logged in user by default.  alsa-lib's
card enumeration should deal with any permissions problems accessing
cards in the system cleanly.  If the user running the test can't access
any cards or the cards that can be accessed don't have any controls to
test then we will find no controls during enumeration, report a plan to
do zero tests and then exit cleanly which seems reasonable to me.  If we
do need to explicitly bomb out rather than report zero tests we should
be doing it based on the number of controls we find rather than the user
we're running as.
Mark Brown Dec. 8, 2021, 8:12 p.m. UTC | #4
On Wed, Dec 08, 2021 at 11:59:18AM -0700, Shuah Khan wrote:
> On 12/8/21 11:39 AM, Mark Brown wrote:
> > On Wed, Dec 08, 2021 at 10:42:35AM -0700, Shuah Khan wrote:

> > > > +	snd_ctl_elem_value_alloca(&val);

> > This is idiomatic for alsa-lib code.

> This is kernel code that is going into kernel sources. Why follow
> alsa-lib convention?

Well, the kernel doesn't generally use alloca() as a pattern given the
relatively small stack sizes we have and doesn't define helpers like
these for it...  it's a toss up here between the conventions for use of
the library we're using and the conventions of the kernel.

> > > > +	ksft_print_header();

> > > Add a check for root and skil the test.

> > There is no need for this test to run as root in most configurations,
> > it is common to provide direct access to the sound cards to some or all
> > users - for example with desktop distros the entire userspace audio
> > subsystem normally runs as the logged in user by default.  alsa-lib's

> On my system, I don't see any output if run as root. Are there some tests
> that work as non-root?

All of them work as non-root if the user they're running as has access
to a card, if they do or not is system dependent - there may not be any
cards at all in a given system to find.  Running as root will punch
through most permission problems but it's not a requirement and a system
could use a security module like SELinux to restrict what root can do.
The sound devices are usually in /dev/snd, though userspace can place
them where it wants - if run as a user that can access the relevant
devices for the mixer interface (usually /dev/snd/controlC* though again
userspace can rename them) then the tests will run on those devices.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 79ef55bf2ca7..ba25b33e2f96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17946,6 +17946,7 @@  F:	Documentation/sound/
 F:	include/sound/
 F:	include/uapi/sound/
 F:	sound/
+F:	tools/testing/selftests/alsa
 
 SOUND - COMPRESSED AUDIO
 M:	Vinod Koul <vkoul@kernel.org>
@@ -17965,6 +17966,12 @@  F:	include/sound/dmaengine_pcm.h
 F:	sound/core/pcm_dmaengine.c
 F:	sound/soc/soc-generic-dmaengine-pcm.c
 
+SOUND - ALSA SELFTESTS
+M:	Mark Brown <broonie@kernel.org>
+L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
+S:	Supported
+F:	tools/testing/selftests/alsa
+
 SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)
 M:	Liam Girdwood <lgirdwood@gmail.com>
 M:	Mark Brown <broonie@kernel.org>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index c852eb40c4f7..d08fe4cfe811 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,5 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
-TARGETS = arm64
+TARGETS += alsa
+TARGETS += arm64
 TARGETS += bpf
 TARGETS += breakpoints
 TARGETS += capabilities
diff --git a/tools/testing/selftests/alsa/.gitignore b/tools/testing/selftests/alsa/.gitignore
new file mode 100644
index 000000000000..3bb7c41266a8
--- /dev/null
+++ b/tools/testing/selftests/alsa/.gitignore
@@ -0,0 +1 @@ 
+mixer-test
diff --git a/tools/testing/selftests/alsa/Makefile b/tools/testing/selftests/alsa/Makefile
new file mode 100644
index 000000000000..f64d9090426d
--- /dev/null
+++ b/tools/testing/selftests/alsa/Makefile
@@ -0,0 +1,9 @@ 
+# SPDX-License-Identifier: GPL-2.0
+#
+
+CFLAGS += $(shell pkg-config --cflags alsa)
+LDLIBS += $(shell pkg-config --libs alsa)
+
+TEST_GEN_PROGS := mixer-test
+
+include ../lib.mk
diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c
new file mode 100644
index 000000000000..6082efa0b426
--- /dev/null
+++ b/tools/testing/selftests/alsa/mixer-test.c
@@ -0,0 +1,616 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// kselftest for the ALSA mixer API
+//
+// Original author: Mark Brown <broonie@kernel.org>
+// Copyright (c) 2021 Arm Limited
+
+// This test will iterate over all cards detected in the system, exercising
+// every mixer control it can find.  This may conflict with other system
+// software if there is audio activity so is best run on a system with a
+// minimal active userspace.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+#include <getopt.h>
+#include <stdarg.h>
+#include <ctype.h>
+#include <math.h>
+#include <errno.h>
+#include <assert.h>
+#include <alsa/asoundlib.h>
+#include <poll.h>
+#include <stdint.h>
+
+#include "../kselftest.h"
+
+#define TESTS_PER_CONTROL 3
+
+struct card_data {
+	snd_ctl_t *handle;
+	int card;
+	int num_ctls;
+	snd_ctl_elem_list_t *ctls;
+	struct card_data *next;
+};
+
+struct ctl_data {
+	const char *name;
+	snd_ctl_elem_id_t *id;
+	snd_ctl_elem_info_t *info;
+	snd_ctl_elem_value_t *def_val;
+	int elem;
+	struct card_data *card;
+	struct ctl_data *next;
+};
+
+int num_cards = 0;
+int num_controls = 0;
+struct card_data *card_list = NULL;
+struct ctl_data *ctl_list = NULL;
+
+void find_controls(void)
+{
+	char name[32];
+	int card, ctl, err;
+	struct card_data *card_data;
+	struct ctl_data *ctl_data;
+
+	card = -1;
+	if (snd_card_next(&card) < 0 || card < 0)
+		return;
+
+	while (card >= 0) {
+		sprintf(name, "hw:%d", card);
+
+		card_data = malloc(sizeof(*card_data));
+		if (!card_data) {
+			ksft_print_msg("Out of memory\n");
+			ksft_exit_fail();
+		}
+
+		err = snd_ctl_open(&card_data->handle, name, 0);
+		if (err < 0) {
+			ksft_print_msg("Failed to get hctl for card %d: %s\n",
+				       card, snd_strerror(err));
+			goto next_card;
+		}
+
+		/* Count controls */
+		snd_ctl_elem_list_malloc(&card_data->ctls);
+		snd_ctl_elem_list(card_data->handle, card_data->ctls);
+		card_data->num_ctls = snd_ctl_elem_list_get_count(card_data->ctls);
+
+		/* Enumerate control information */
+		snd_ctl_elem_list_alloc_space(card_data->ctls, card_data->num_ctls);
+		snd_ctl_elem_list(card_data->handle, card_data->ctls);
+
+		card_data->card = num_cards++;
+		card_data->next = card_list;
+		card_list = card_data;
+
+		num_controls += card_data->num_ctls;
+
+		for (ctl = 0; ctl < card_data->num_ctls; ctl++) {
+			ctl_data = malloc(sizeof(*ctl_data));
+			if (!ctl_data) {
+				ksft_print_msg("Out of memory\n");
+				ksft_exit_fail();
+			}
+
+			ctl_data->card = card_data;
+			ctl_data->elem = ctl;
+			ctl_data->name = snd_ctl_elem_list_get_name(card_data->ctls,
+								    ctl);
+
+			err = snd_ctl_elem_id_malloc(&ctl_data->id);
+			if (err < 0) {
+				ksft_print_msg("Out of memory\n");
+				ksft_exit_fail();
+			}
+
+			err = snd_ctl_elem_info_malloc(&ctl_data->info);
+			if (err < 0) {
+				ksft_print_msg("Out of memory\n");
+				ksft_exit_fail();
+			}
+
+			err = snd_ctl_elem_value_malloc(&ctl_data->def_val);
+			if (err < 0) {
+				ksft_print_msg("Out of memory\n");
+				ksft_exit_fail();
+			}
+
+			snd_ctl_elem_list_get_id(card_data->ctls, ctl,
+						 ctl_data->id);
+			snd_ctl_elem_info_set_id(ctl_data->info, ctl_data->id);
+			err = snd_ctl_elem_info(card_data->handle,
+						ctl_data->info);
+			if (err < 0) {
+				ksft_print_msg("%s getting info for %d\n",
+					       snd_strerror(err),
+					       ctl_data->name);
+			}
+
+			snd_ctl_elem_value_set_id(ctl_data->def_val,
+						  ctl_data->id);
+
+			ctl_data->next = ctl_list;
+			ctl_list = ctl_data;
+		}
+
+	next_card:
+		if (snd_card_next(&card) < 0) {
+			ksft_print_msg("snd_card_next");
+			break;
+		}
+	}
+}
+
+/*
+ * Check that we can read the default value and it is valid. Write
+ * tests use the read value to restore the default.
+ */
+void test_ctl_get_value(struct ctl_data *ctl)
+{
+	int err;
+	long int_val;
+	long long int64_val;
+
+	/* If the control is turned off let's be polite */
+	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
+		ksft_print_msg("%s is inactive\n", ctl->name);
+		ksft_test_result_skip("get_value.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	/* Can't test reading on an unreadable control */
+	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
+		ksft_print_msg("%s is not readable\n", ctl->name);
+		ksft_test_result_skip("get_value.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	err = snd_ctl_elem_read(ctl->card->handle, ctl->def_val);
+	if (err < 0) {
+		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
+			       snd_strerror(err));
+		goto out;
+	}
+
+	switch (snd_ctl_elem_info_get_type(ctl->info)) {
+	case SND_CTL_ELEM_TYPE_NONE:
+		ksft_print_msg("%s Invalid control type NONE\n", ctl->name);
+		err = -1;
+		break;
+
+	case SND_CTL_ELEM_TYPE_BOOLEAN:
+		int_val = snd_ctl_elem_value_get_boolean(ctl->def_val, 0);
+		switch (int_val) {
+		case 0:
+		case 1:
+			break;
+		default:
+			ksft_print_msg("%s Invalid boolean value %ld\n",
+				       ctl->name, int_val);
+			err = -1;
+			break;
+		}
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER:
+		int_val = snd_ctl_elem_value_get_integer(ctl->def_val, 0);
+
+		if (int_val < snd_ctl_elem_info_get_min(ctl->info)) {
+			ksft_print_msg("%s value %ld less than minimum %ld\n",
+				       ctl->name, int_val,
+				       snd_ctl_elem_info_get_min(ctl->info));
+			err = -1;
+		}
+
+		if (int_val > snd_ctl_elem_info_get_max(ctl->info)) {
+			ksft_print_msg("%s value %ld more than maximum %ld\n",
+				       ctl->name, int_val,
+				       snd_ctl_elem_info_get_max(ctl->info));
+			err = -1;
+		}
+
+		/* Only check step size if there is one and we're in bounds */
+		if (err >= 0 && snd_ctl_elem_info_get_step(ctl->info) &&
+		    (int_val - snd_ctl_elem_info_get_min(ctl->info) %
+		     snd_ctl_elem_info_get_step(ctl->info))) {
+			ksft_print_msg("%s value %ld invalid for step %ld minimum %ld\n",
+				       ctl->name, int_val,
+				       snd_ctl_elem_info_get_step(ctl->info),
+				       snd_ctl_elem_info_get_min(ctl->info));
+			err = -1;
+		}
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER64:
+		int64_val = snd_ctl_elem_value_get_integer64(ctl->def_val, 0);
+
+		if (int64_val < snd_ctl_elem_info_get_min64(ctl->info)) {
+			ksft_print_msg("%s value %lld less than minimum %lld\n",
+				       ctl->name, int64_val,
+				       snd_ctl_elem_info_get_min64(ctl->info));
+			err = -1;
+		}
+
+		if (int64_val > snd_ctl_elem_info_get_max64(ctl->info)) {
+			ksft_print_msg("%s value %lld more than maximum %lld\n",
+				       ctl->name, int64_val,
+				       snd_ctl_elem_info_get_max(ctl->info));
+			err = -1;
+		}
+
+		/* Only check step size if there is one and we're in bounds */
+		if (err >= 0 && snd_ctl_elem_info_get_step64(ctl->info) &&
+		    (int64_val - snd_ctl_elem_info_get_min64(ctl->info)) %
+		    snd_ctl_elem_info_get_step64(ctl->info)) {
+			ksft_print_msg("%s value %lld invalid for step %lld minimum %lld\n",
+				       ctl->name, int64_val,
+				       snd_ctl_elem_info_get_step64(ctl->info),
+				       snd_ctl_elem_info_get_min64(ctl->info));
+			err = -1;
+		}
+		break;
+
+	default:
+		/* No tests for other types */
+		ksft_test_result_skip("get_value.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+out:
+	ksft_test_result(err >= 0, "get_value.%d.%d\n",
+			 ctl->card->card, ctl->elem);
+}
+
+bool show_mismatch(struct ctl_data *ctl, int index,
+		   snd_ctl_elem_value_t *read_val,
+		   snd_ctl_elem_value_t *expected_val)
+{
+	long long expected_int, read_int;
+
+	/*
+	 * We factor out the code to compare values representable as
+	 * integers, ensure that check doesn't log otherwise.
+	 */
+	expected_int = 0;
+	read_int = 0;
+
+	switch (snd_ctl_elem_info_get_type(ctl->info)) {
+	case SND_CTL_ELEM_TYPE_BOOLEAN:
+		expected_int = snd_ctl_elem_value_get_boolean(expected_val,
+							      index);
+		read_int = snd_ctl_elem_value_get_boolean(read_val, index);
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER:
+		expected_int = snd_ctl_elem_value_get_integer(expected_val,
+							      index);
+		read_int = snd_ctl_elem_value_get_integer(read_val, index);
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER64:
+		expected_int = snd_ctl_elem_value_get_integer64(expected_val,
+								index);
+		read_int = snd_ctl_elem_value_get_integer64(read_val,
+							    index);
+		break;
+
+	case SND_CTL_ELEM_TYPE_ENUMERATED:
+		expected_int = snd_ctl_elem_value_get_enumerated(expected_val,
+								 index);
+		read_int = snd_ctl_elem_value_get_enumerated(read_val,
+							     index);
+		break;
+
+	default:
+		break;
+	}
+
+	if (expected_int != read_int) {
+		ksft_print_msg("%s.%d expected %lld but read %lld\n",
+			       ctl->name, index, expected_int, read_int);
+		return true;
+	} else {
+		return false;
+	}
+}
+
+/*
+ * Write a value then if possible verify that we get the expected
+ * result.  An optional expected value can be provided if we expect
+ * the write to fail, for verifying that invalid writes don't corrupt
+ * anything.
+ */
+int write_and_verify(struct ctl_data *ctl,
+		     snd_ctl_elem_value_t *write_val,
+		     snd_ctl_elem_value_t *expected_val)
+{
+	int err, i;
+	bool error_expected, mismatch_shown;
+	snd_ctl_elem_value_t *read_val, *w_val;
+	snd_ctl_elem_value_alloca(&read_val);
+	snd_ctl_elem_value_alloca(&w_val);
+
+	/*
+	 * We need to copy the write value since writing can modify
+	 * the value which causes surprises, and allocate an expected
+	 * value if we expect to read back what we wrote.
+	 */
+	snd_ctl_elem_value_copy(w_val, write_val);
+	if (expected_val) {
+		error_expected = true;
+	} else {
+		error_expected = false;
+		snd_ctl_elem_value_alloca(&expected_val);
+		snd_ctl_elem_value_copy(expected_val, write_val);
+	}
+
+	/*
+	 * Do the write, if we have an expected value ignore the error
+	 * and carry on to validate the expected value.
+	 */
+	err = snd_ctl_elem_write(ctl->card->handle, w_val);
+	if (err < 0 && !error_expected) {
+		ksft_print_msg("snd_ctl_elem_write() failed: %s\n",
+			       snd_strerror(err));
+		return err;
+	}
+
+	/* Can we do the verification part? */
+	if (!snd_ctl_elem_info_is_readable(ctl->info))
+		return err;
+
+	snd_ctl_elem_value_set_id(read_val, ctl->id);
+
+	err = snd_ctl_elem_read(ctl->card->handle, read_val);
+	if (err < 0) {
+		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
+			       snd_strerror(err));
+		return err;
+	}
+
+	/*
+	 * Use the libray to compare values, if there's a mismatch
+	 * carry on and try to provide a more useful diagnostic than
+	 * just "mismatch".
+	 */
+	if (!snd_ctl_elem_value_compare(expected_val, read_val))
+		return 0;
+
+	mismatch_shown = false;
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++)
+		if (show_mismatch(ctl, i, read_val, expected_val))
+			mismatch_shown = true;
+
+	if (!mismatch_shown)
+		ksft_print_msg("%s read and written values differ\n",
+			       ctl->name);
+
+	return -1;
+}
+
+/*
+ * Make sure we can write the default value back to the control, this
+ * should validate that at least some write works.
+ */
+void test_ctl_write_default(struct ctl_data *ctl)
+{
+	int err;
+
+	/* If the control is turned off let's be polite */
+	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
+		ksft_print_msg("%s is inactive\n", ctl->name);
+		ksft_test_result_skip("write_default.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	if (!snd_ctl_elem_info_is_writable(ctl->info)) {
+		ksft_print_msg("%s is not writeable\n", ctl->name);
+		ksft_test_result_skip("write_default.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	/* No idea what the default was for unreadable controls */
+	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
+		ksft_print_msg("%s couldn't read default\n", ctl->name);
+		ksft_test_result_skip("write_default.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	err = write_and_verify(ctl, ctl->def_val, NULL);
+
+	ksft_test_result(err >= 0, "write_default.%d.%d\n",
+			 ctl->card->card, ctl->elem);
+}
+
+bool test_ctl_write_valid_boolean(struct ctl_data *ctl)
+{
+	int err, i, j;
+	bool fail = false;
+	snd_ctl_elem_value_t *val;
+	snd_ctl_elem_value_alloca(&val);
+
+	snd_ctl_elem_value_set_id(val, ctl->id);
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++) {
+		for (j = 0; j < 2; j++) {
+			snd_ctl_elem_value_set_boolean(val, i, j);
+			err = write_and_verify(ctl, val, NULL);
+			if (err != 0)
+				fail = true;
+		}
+	}
+
+	return !fail;
+}
+
+bool test_ctl_write_valid_integer(struct ctl_data *ctl)
+{
+	int err;
+	int i;
+	long j, step;
+	bool fail = false;
+	snd_ctl_elem_value_t *val;
+	snd_ctl_elem_value_alloca(&val);
+
+	snd_ctl_elem_value_set_id(val, ctl->id);
+
+	step = snd_ctl_elem_info_get_step(ctl->info);
+	if (!step)
+		step = 1;
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++) {
+		for (j = snd_ctl_elem_info_get_min(ctl->info);
+		     j <= snd_ctl_elem_info_get_max(ctl->info); j += step) {
+
+			snd_ctl_elem_value_set_integer(val, i, j);
+			err = write_and_verify(ctl, val, NULL);
+			if (err != 0)
+				fail = true;
+		}
+	}
+
+
+	return !fail;
+}
+
+bool test_ctl_write_valid_integer64(struct ctl_data *ctl)
+{
+	int err, i;
+	long long j, step;
+	bool fail = false;
+	snd_ctl_elem_value_t *val;
+	snd_ctl_elem_value_alloca(&val);
+
+	snd_ctl_elem_value_set_id(val, ctl->id);
+
+	step = snd_ctl_elem_info_get_step64(ctl->info);
+	if (!step)
+		step = 1;
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++) {
+		for (j = snd_ctl_elem_info_get_min64(ctl->info);
+		     j <= snd_ctl_elem_info_get_max64(ctl->info); j += step) {
+
+			snd_ctl_elem_value_set_integer64(val, i, j);
+			err = write_and_verify(ctl, val, NULL);
+			if (err != 0)
+				fail = true;
+		}
+	}
+
+
+	return !fail;
+}
+
+bool test_ctl_write_valid_enumerated(struct ctl_data *ctl)
+{
+	int err, i, j;
+	bool fail = false;
+	snd_ctl_elem_value_t *val;
+	snd_ctl_elem_value_alloca(&val);
+
+	snd_ctl_elem_value_set_id(val, ctl->id);
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++) {
+		for (j = 0; j < snd_ctl_elem_info_get_items(ctl->info); j++) {
+			snd_ctl_elem_value_set_enumerated(val, i, j);
+			err = write_and_verify(ctl, val, NULL);
+			if (err != 0)
+				fail = true;
+		}
+	}
+
+	return !fail;
+}
+
+void test_ctl_write_valid(struct ctl_data *ctl)
+{
+	bool pass;
+	int err;
+
+	/* If the control is turned off let's be polite */
+	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
+		ksft_print_msg("%s is inactive\n", ctl->name);
+		ksft_test_result_skip("write_valid.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	if (!snd_ctl_elem_info_is_writable(ctl->info)) {
+		ksft_print_msg("%s is not writeable\n", ctl->name);
+		ksft_test_result_skip("write_valid.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	switch (snd_ctl_elem_info_get_type(ctl->info)) {
+	case SND_CTL_ELEM_TYPE_BOOLEAN:
+		pass = test_ctl_write_valid_boolean(ctl);
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER:
+		pass = test_ctl_write_valid_integer(ctl);
+		break;
+
+	case SND_CTL_ELEM_TYPE_INTEGER64:
+		pass = test_ctl_write_valid_integer64(ctl);
+		break;
+
+	case SND_CTL_ELEM_TYPE_ENUMERATED:
+		pass = test_ctl_write_valid_enumerated(ctl);
+		break;
+
+	default:
+		/* No tests for this yet */
+		ksft_test_result_skip("write_valid.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	/* Restore the default value to minimise disruption */
+	err = write_and_verify(ctl, ctl->def_val, NULL);
+	if (err < 0)
+		pass = false;
+
+	ksft_test_result(pass, "write_valid.%d.%d\n",
+			 ctl->card->card, ctl->elem);
+}
+
+int main(void)
+{
+	struct ctl_data *ctl;
+
+	ksft_print_header();
+
+	find_controls();
+
+	ksft_set_plan(num_controls * TESTS_PER_CONTROL);
+
+	for (ctl = ctl_list; ctl != NULL; ctl = ctl->next) {
+		/* 
+		 * Must test get_value() before we write anything, the
+		 * test stores the default value for later cleanup.
+		 */
+		test_ctl_get_value(ctl);
+		test_ctl_write_default(ctl);
+		test_ctl_write_valid(ctl);
+	}
+
+	ksft_exit_pass();
+
+	return 0;
+}