* [PATCH BUGFIX] ALSA: pci: fix reading of swapped values from pcmreg in AC97 codec
@ 2022-03-22 20:06 Giacomo Guiduzzi
2022-03-22 20:19 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Giacomo Guiduzzi @ 2022-03-22 20:06 UTC (permalink / raw
To: perex, tiwai, alsa-devel; +Cc: Giacomo Guiduzzi, paolo.valente
Tests 72 and 78 for ALSA in kselftest fail due to reading
inconsistent values from some devices on a VirtualBox
Virtual Machine using the snd_intel8x0 driver for the AC'97
Audio Controller device.
Taking for example test number 72, this is what the test reports:
"Surround Playback Volume.0 expected 1 but read 0, is_volatile 0"
"Surround Playback Volume.1 expected 0 but read 1, is_volatile 0"
These errors repeat for each value from 0 to 31.
Taking a look at these error messages it is possible to notice
that the written values are read back swapped.
When the write is performed, these values are initially stored in
an array used to sanity-check them and write them in the pcmreg
array. To write them, the two one-byte values are packed together
in a two-byte variable through bitwise operations: the first
value is shifted left by one byte and the second value is stored in the
right byte through a bitwise OR. When reading the values back,
right shifts are performed to retrieve the previously stored
bytes. These shifts are executed in the wrong order, thus
reporting the values swapped as shown above.
This patch fixes this mistake by reversing the read
operations' order.
Signed-off-by: Giacomo Guiduzzi <guiduzzi.giacomo@gmail.com>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
sound/pci/ac97/ac97_codec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
index 01f296d524ce..cb60a07d39a8 100644
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -938,8 +938,8 @@ static int snd_ac97_ad18xx_pcm_get_volume(struct snd_kcontrol *kcontrol, struct
int codec = kcontrol->private_value & 3;
mutex_lock(&ac97->page_mutex);
- ucontrol->value.integer.value[0] = 31 - ((ac97->spec.ad18xx.pcmreg[codec] >> 0) & 31);
- ucontrol->value.integer.value[1] = 31 - ((ac97->spec.ad18xx.pcmreg[codec] >> 8) & 31);
+ ucontrol->value.integer.value[0] = 31 - ((ac97->spec.ad18xx.pcmreg[codec] >> 8) & 31);
+ ucontrol->value.integer.value[1] = 31 - ((ac97->spec.ad18xx.pcmreg[codec] >> 0) & 31);
mutex_unlock(&ac97->page_mutex);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH BUGFIX] ALSA: pci: fix reading of swapped values from pcmreg in AC97 codec
2022-03-22 20:06 [PATCH BUGFIX] ALSA: pci: fix reading of swapped values from pcmreg in AC97 codec Giacomo Guiduzzi
@ 2022-03-22 20:19 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2022-03-22 20:19 UTC (permalink / raw
To: Giacomo Guiduzzi; +Cc: alsa-devel, paolo.valente, tiwai
On Tue, 22 Mar 2022 21:06:54 +0100,
Giacomo Guiduzzi wrote:
>
> Tests 72 and 78 for ALSA in kselftest fail due to reading
> inconsistent values from some devices on a VirtualBox
> Virtual Machine using the snd_intel8x0 driver for the AC'97
> Audio Controller device.
> Taking for example test number 72, this is what the test reports:
> "Surround Playback Volume.0 expected 1 but read 0, is_volatile 0"
> "Surround Playback Volume.1 expected 0 but read 1, is_volatile 0"
> These errors repeat for each value from 0 to 31.
>
> Taking a look at these error messages it is possible to notice
> that the written values are read back swapped.
> When the write is performed, these values are initially stored in
> an array used to sanity-check them and write them in the pcmreg
> array. To write them, the two one-byte values are packed together
> in a two-byte variable through bitwise operations: the first
> value is shifted left by one byte and the second value is stored in the
> right byte through a bitwise OR. When reading the values back,
> right shifts are performed to retrieve the previously stored
> bytes. These shifts are executed in the wrong order, thus
> reporting the values swapped as shown above.
>
> This patch fixes this mistake by reversing the read
> operations' order.
>
> Signed-off-by: Giacomo Guiduzzi <guiduzzi.giacomo@gmail.com>
> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Thanks, applied now.
Takashi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-22 20:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-22 20:06 [PATCH BUGFIX] ALSA: pci: fix reading of swapped values from pcmreg in AC97 codec Giacomo Guiduzzi
2022-03-22 20:19 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.