| Message ID | 20240614153717.30143-1-tiwai@suse.de |
|---|---|
| Headers | show |
| Series | ALSA: some driver fixes for control input validations | expand |
On 14. 06. 24 17:37, Takashi Iwai wrote: > The control elements with volatile flag don't guarantee that the > written values are actually saved for the next reads, hence we can't > verify the written values reliably. Skip the verification after 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> Reviewed-by: Jaroslav Kysela <perex@perex.cz>
On Fri, 14 Jun 2024 17:57:37 +0200, Mark Brown wrote: > > On Fri, Jun 14, 2024 at 05:37:13PM +0200, Takashi Iwai wrote: > > > @@ -616,6 +616,10 @@ static int write_and_verify(struct ctl_data *ctl, > > if (!snd_ctl_elem_info_is_readable(ctl->info)) > > return err; > > > > + /* Skip the verification for volatile controls, too */ > > + if (snd_ctl_elem_info_is_volatile(ctl->info)) > > + return err; > > + > > I think we should do the checks in test_ctl_get_value() still - a read > and then ctl_value_is_valid() on whatever we read. It doesn't need to > match the value we wrote but it should still be valid for the control. So something like below? Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH v3] kselftest/alsa: mixer-test: Allow value mismatch for volatile controls The control elements with volatile flag don't guarantee that the written values are actually saved for the next reads, hence we can't verify the written values reliably. Return as success for volatile controls even if the value verification after writes fails, in order to avoid false-positive. 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 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index 1c04e5f638a0..62b77737f0de 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -668,6 +668,10 @@ static int write_and_verify(struct ctl_data *ctl, ksft_print_msg("%s read and written values differ\n", ctl->name); + /* Allow difference for volatile controls */ + if (snd_ctl_elem_info_is_volatile(ctl->info)) + return 0; + return -1; }
Hi, this is a revised patch set as a follow up of the thread about the errors reported by kselftest mixer-test. It changes HD-audio and vmaster control behavior to return -EINVAL for invalid input values. There is a change in kselftest itself to skip the verification after write tests for volatile controls, too. It's for the channel map controls that can't hold the stable values. v1->v2: * Skip only verification after write in kselftest * Add sanity check to HDMI chmap write, too Takashi === Takashi Iwai (6): ALSA: vmaster: Return error for invalid input values ALSA: hda: Return -EINVAL for invalid volume/switch inputs ALSA: control: Apply sanity check of input values for user elements kselftest/alsa: mixer-test: Skip write verification for volatile controls ALSA: chmap: Mark Channel Map controls as volatile ALSA: hda: Add input value sanity checks to HDMI channel map controls sound/core/control.c | 3 ++- sound/core/pcm_lib.c | 1 + sound/core/vmaster.c | 8 ++++++++ sound/hda/hdmi_chmap.c | 18 ++++++++++++++++++ sound/pci/hda/hda_codec.c | 23 ++++++++++++++++++----- tools/testing/selftests/alsa/mixer-test.c | 4 ++++ 6 files changed, 51 insertions(+), 6 deletions(-)