diff mbox series

[4/5] kselftest/alsa: mixer-test: Skip write tests for volatile controls

Message ID 20240614124728.27901-5-tiwai@suse.de (mailing list archive)
State New
Headers show
Series ALSA: some driver fixes for control input validations | expand

Commit Message

Takashi Iwai June 14, 2024, 12:47 p.m. UTC
The control elements with volatile flag don't guarantee that the
written values are actually saved for the next reads, hence they
aren't suitable for the standard mixer tests.  Skip the write tests
for those volatile controls for avoiding confusion.

Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Closes: https://lore.kernel.org/r/1d44be36-9bb9-4d82-8953-5ae2a4f09405@molgen.mpg.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 tools/testing/selftests/alsa/mixer-test.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Mark Brown June 14, 2024, 2:29 p.m. UTC | #1
On Fri, Jun 14, 2024 at 02:47:26PM +0200, Takashi Iwai wrote:

> The control elements with volatile flag don't guarantee that the
> written values are actually saved for the next reads, hence they
> aren't suitable for the standard mixer tests.  Skip the write tests
> for those volatile controls for avoiding confusion.

We should still verify that you can actually write I think...

> +	if (!snd_ctl_elem_info_is_volatile(ctl->info)) {
> +		ksft_print_msg("%s is volatile\n", ctl->name);
> +		ksft_test_result_skip("write_invalid.%d.%d\n",
> +				      ctl->card->card, ctl->elem);
> +		return;
> +	}

...and that you don't read back invalid values after a write like this
for example.  I think any change for this should be in the validation of
the read but we should still try the writes we think we can do.
Takashi Iwai June 14, 2024, 2:40 p.m. UTC | #2
On Fri, 14 Jun 2024 16:29:03 +0200,
Mark Brown wrote:
> 
> On Fri, Jun 14, 2024 at 02:47:26PM +0200, Takashi Iwai wrote:
> 
> > The control elements with volatile flag don't guarantee that the
> > written values are actually saved for the next reads, hence they
> > aren't suitable for the standard mixer tests.  Skip the write tests
> > for those volatile controls for avoiding confusion.
> 
> We should still verify that you can actually write I think...
> 
> > +	if (!snd_ctl_elem_info_is_volatile(ctl->info)) {
> > +		ksft_print_msg("%s is volatile\n", ctl->name);
> > +		ksft_test_result_skip("write_invalid.%d.%d\n",
> > +				      ctl->card->card, ctl->elem);
> > +		return;
> > +	}
> 
> ...and that you don't read back invalid values after a write like this
> for example.  I think any change for this should be in the validation of
> the read but we should still try the writes we think we can do.

OK, makes sense.  Will respin this one.


thanks,

Takashi
diff mbox series

Patch

diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c
index 1c04e5f638a0..3c9a45fb5a29 100644
--- a/tools/testing/selftests/alsa/mixer-test.c
+++ b/tools/testing/selftests/alsa/mixer-test.c
@@ -702,6 +702,13 @@  static void test_ctl_write_default(struct ctl_data *ctl)
 		return;
 	}
 
+	if (snd_ctl_elem_info_is_volatile(ctl->info)) {
+		ksft_print_msg("%s is volatile\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",
@@ -827,6 +834,13 @@  static void test_ctl_write_valid(struct ctl_data *ctl)
 		return;
 	}
 
+	if (snd_ctl_elem_info_is_volatile(ctl->info)) {
+		ksft_print_msg("%s is volatile\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);
@@ -1039,6 +1053,13 @@  static void test_ctl_write_invalid(struct ctl_data *ctl)
 		return;
 	}
 
+	if (!snd_ctl_elem_info_is_volatile(ctl->info)) {
+		ksft_print_msg("%s is volatile\n", ctl->name);
+		ksft_test_result_skip("write_invalid.%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_invalid_boolean(ctl);