diff mbox series

kselftest/alsa: Fix validation of writes to volatile controls

Message ID 20240614-alsa-selftest-volatile-v1-1-3874f02964b1@kernel.org (mailing list archive)
State New, archived
Headers show
Series kselftest/alsa: Fix validation of writes to volatile controls | expand

Commit Message

Mark Brown June 14, 2024, 4:40 p.m. UTC 
When validating writes to controls we check that whatever value we wrote
actually appears in the control.  For volatile controls we cannot assume
that this will be the case, the value may be changed at any time
including between our write and read.  Handle this by moving the check
for volatile controls that we currently do for events to a separate
block and just verifying that whatever value we read is valid for the
control.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/mixer-test.c | 45 ++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 16 deletions(-)


---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240614-alsa-selftest-volatile-d6f3e8e28c08

Best regards,

Comments

Takashi Iwai June 16, 2024, 7:32 a.m. UTC | #1 
On Fri, 14 Jun 2024 18:40:37 +0200,
Mark Brown wrote:
> 
> When validating writes to controls we check that whatever value we wrote
> actually appears in the control.  For volatile controls we cannot assume
> that this will be the case, the value may be changed at any time
> including between our write and read.  Handle this by moving the check
> for volatile controls that we currently do for events to a separate
> block and just verifying that whatever value we read is valid for the
> control.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

I'll pick this in v3 patch set.


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..dd74f8cc7ece 100644
--- a/tools/testing/selftests/alsa/mixer-test.c
+++ b/tools/testing/selftests/alsa/mixer-test.c
@@ -625,6 +625,21 @@  static int write_and_verify(struct ctl_data *ctl,
 		return err;
 	}
 
+	/*
+	 * We can't verify any specific value for volatile controls
+	 * but we should still check that whatever we read is a valid
+	 * vale for the control.
+	 */
+	if (snd_ctl_elem_info_is_volatile(ctl->info)) {
+		if (!ctl_value_valid(ctl, read_val)) {
+			ksft_print_msg("Volatile control %s has invalid value\n",
+				       ctl->name);
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
 	/*
 	 * Check for an event if the value changed, or confirm that
 	 * there was none if it didn't.  We rely on the kernel
@@ -632,22 +647,20 @@  static int write_and_verify(struct ctl_data *ctl,
 	 * write, this is currently true, should that ever change this
 	 * will most likely break and need updating.
 	 */
-	if (!snd_ctl_elem_info_is_volatile(ctl->info)) {
-		err = wait_for_event(ctl, 0);
-		if (snd_ctl_elem_value_compare(initial_val, read_val)) {
-			if (err < 1) {
-				ksft_print_msg("No event generated for %s\n",
-					       ctl->name);
-				show_values(ctl, initial_val, read_val);
-				ctl->event_missing++;
-			}
-		} else {
-			if (err != 0) {
-				ksft_print_msg("Spurious event generated for %s\n",
-					       ctl->name);
-				show_values(ctl, initial_val, read_val);
-				ctl->event_spurious++;
-			}
+	err = wait_for_event(ctl, 0);
+	if (snd_ctl_elem_value_compare(initial_val, read_val)) {
+		if (err < 1) {
+			ksft_print_msg("No event generated for %s\n",
+				       ctl->name);
+			show_values(ctl, initial_val, read_val);
+			ctl->event_missing++;
+		}
+	} else {
+		if (err != 0) {
+			ksft_print_msg("Spurious event generated for %s\n",
+				       ctl->name);
+			show_values(ctl, initial_val, read_val);
+			ctl->event_spurious++;
 		}
 	}