[alsa-devel] [PATCH v2] ALSA: snd-usb-audio: fixed mixer range reading during probing, when compiled with CONFIG_PM defined
During probing, snd_usb_autoresume() fails with -ENODEV. This brakes get_ctl_value() in mixer.c when reading the range values. In the new version snd_usb_autoresume() returns 0 in this case which brakes not the following query. The following snd_usb_suspend() fails silently during probing anyway. The patch also makes the error messages more significant. The original error messages where identified as misleading during debug.
Signed-off-by: Daniel Schürmann daschuer@mixxx.org --- sound/usb/card.c | 7 +++++-- sound/usb/mixer.c | 11 +++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 5254b18..2f55d00 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -629,9 +629,12 @@ static void usb_audio_disconnect(struct usb_interface *intf) int snd_usb_autoresume(struct snd_usb_audio *chip) { int err = -ENODEV; - + + if (chip->probing) { + return 0; + } down_read(&chip->shutdown_rwsem); - if (!chip->shutdown && !chip->probing) + if (!chip->shutdown) err = usb_autopm_get_interface(chip->pm_intf); up_read(&chip->shutdown_rwsem);
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..5d4f3c6 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -333,9 +333,12 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
memset(buf, 0, sizeof(buf));
- ret = snd_usb_autoresume(chip) ? -EIO : 0; - if (ret) + ret = snd_usb_autoresume(chip); + if (ret) { + snd_printk(KERN_ERR "snd_usb_autoresume() failed with %d\n", ret); + ret = -EIO; goto error; + }
down_read(&chip->shutdown_rwsem); if (chip->shutdown) @@ -351,8 +354,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
if (ret < 0) { error: - snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n", - request, validx, idx, cval->val_type); + snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d, ret = %d\n", + bRequest, validx, idx, cval->val_type, ret); return ret; }
Hi Takashi,
I have tested you Idea, and It works fine as expected. It is include a in the patch below. I have also debugged deeper, and found out that there is still a scaling issue of the Class 2 Mixer. I will try to fix it in a separate patch.
Kind regards,
Daniel
2013/4/24 Daniel Schürmann daschuer@mixxx.org
During probing, snd_usb_autoresume() fails with -ENODEV. This brakes get_ctl_value() in mixer.c when reading the range values. In the new version snd_usb_autoresume() returns 0 in this case which brakes not the following query. The following snd_usb_suspend() fails silently during probing anyway. The patch also makes the error messages more significant. The original error messages where identified as misleading during debug.
Signed-off-by: Daniel Schürmann daschuer@mixxx.org
sound/usb/card.c | 7 +++++-- sound/usb/mixer.c | 11 +++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 5254b18..2f55d00 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -629,9 +629,12 @@ static void usb_audio_disconnect(struct usb_interface *intf) int snd_usb_autoresume(struct snd_usb_audio *chip) { int err = -ENODEV;
if (chip->probing) {
return 0;
} down_read(&chip->shutdown_rwsem);
if (!chip->shutdown && !chip->probing)
if (!chip->shutdown) err = usb_autopm_get_interface(chip->pm_intf); up_read(&chip->shutdown_rwsem);
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..5d4f3c6 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -333,9 +333,12 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
memset(buf, 0, sizeof(buf));
ret = snd_usb_autoresume(chip) ? -EIO : 0;
if (ret)
ret = snd_usb_autoresume(chip);
if (ret) {
snd_printk(KERN_ERR "snd_usb_autoresume() failed with
%d\n", ret);
ret = -EIO; goto error;
} down_read(&chip->shutdown_rwsem); if (chip->shutdown)
@@ -351,8 +354,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
if (ret < 0) {
error:
snd_printk(KERN_ERR "cannot get ctl value: req = %#x,
wValue = %#x, wIndex = %#x, type = %d\n",
request, validx, idx, cval->val_type);
snd_printk(KERN_ERR "cannot get ctl value: req = %#x,
wValue = %#x, wIndex = %#x, type = %d, ret = %d\n",
bRequest, validx, idx, cval->val_type, ret); return ret; }
-- 1.7.9.5
At Wed, 24 Apr 2013 20:29:23 +0200, Daniel Schürmann wrote:
During probing, snd_usb_autoresume() fails with -ENODEV. This brakes get_ctl_value() in mixer.c when reading the range values. In the new version snd_usb_autoresume() returns 0 in this case which brakes not the following query. The following snd_usb_suspend() fails silently during probing anyway. The patch also makes the error messages more significant. The original error messages where identified as misleading during debug.
I applied my original version to make symmetry with the suspend side.
Also, the changes of the error messages aren't applied because they have no direct relation with the fix itself. Please repost the patch if it's really needed.
(BTW, it doesn't brakes but breaks.)
thanks,
Takashi
Signed-off-by: Daniel Schürmann daschuer@mixxx.org
sound/usb/card.c | 7 +++++-- sound/usb/mixer.c | 11 +++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 5254b18..2f55d00 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -629,9 +629,12 @@ static void usb_audio_disconnect(struct usb_interface *intf) int snd_usb_autoresume(struct snd_usb_audio *chip) { int err = -ENODEV;
- if (chip->probing) {
return 0;
- } down_read(&chip->shutdown_rwsem);
- if (!chip->shutdown && !chip->probing)
- if (!chip->shutdown) err = usb_autopm_get_interface(chip->pm_intf); up_read(&chip->shutdown_rwsem);
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..5d4f3c6 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -333,9 +333,12 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
memset(buf, 0, sizeof(buf));
- ret = snd_usb_autoresume(chip) ? -EIO : 0;
- if (ret)
ret = snd_usb_autoresume(chip);
if (ret) {
snd_printk(KERN_ERR "snd_usb_autoresume() failed with %d\n", ret);
ret = -EIO;
goto error;
}
down_read(&chip->shutdown_rwsem); if (chip->shutdown)
@@ -351,8 +354,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
if (ret < 0) { error:
snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",
request, validx, idx, cval->val_type);
snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d, ret = %d\n",
return ret; }bRequest, validx, idx, cval->val_type, ret);
-- 1.7.9.5
participants (2)
-
Daniel Schürmann
-
Takashi Iwai