| Message ID | 20240614124728.27901-5-tiwai@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ALSA: some driver fixes for control input validations | expand |
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.
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 --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);
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(+)