[alsa-devel] [PATCH] fixed mixer range reading during probing when compiled with CONFIG_PM defined
During probing, snd_usb_autoresume() failes with -ENODEV. This brakes get_ctl_value() in mixer.c when rading the range values. The new code does not try to call snd_usb_autoresume() in this case. 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 missleading during debug.
Signed-off-by: Daniel Schürmann daschuer@mixxx.org --- sound/usb/mixer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..c2daa58 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -289,9 +289,11 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v int timeout = 10; int idx = 0, err;
- err = snd_usb_autoresume(cval->mixer->chip); - if (err < 0) - return -EIO; + if (!chip->probing) { + err = snd_usb_autoresume(cval->mixer->chip); + if (err < 0) + return -EIO; + } down_read(&chip->shutdown_rwsem); while (timeout-- > 0) { if (chip->shutdown) @@ -333,9 +335,14 @@ 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) - goto error; + if (!chip->probing) { + 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 +358,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; }
At Tue, 23 Apr 2013 23:38:49 +0200, Daniel Schürmann wrote:
During probing, snd_usb_autoresume() failes with -ENODEV. This brakes get_ctl_value() in mixer.c when rading the range values. The new code does not try to call snd_usb_autoresume() in this case. 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 missleading during debug.
Does this patch really fix the real bug (i.e. you got an error in get_ctl_value() during probe), or just hypothesis?
Also, better to correct many typos in the changelog :)
thanks,
Takashi
Signed-off-by: Daniel Schürmann daschuer@mixxx.org
sound/usb/mixer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..c2daa58 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -289,9 +289,11 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v int timeout = 10; int idx = 0, err;
- err = snd_usb_autoresume(cval->mixer->chip);
- if (err < 0)
return -EIO;
- if (!chip->probing) {
err = snd_usb_autoresume(cval->mixer->chip);if (err < 0)return -EIO;- } down_read(&chip->shutdown_rwsem); while (timeout-- > 0) { if (chip->shutdown)
@@ -333,9 +335,14 @@ 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)
goto error;
if (!chip->probing) {
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 +358,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);-- 1.7.9.5
Hi Takashi,
thank you for review.
The patch solves the following error messages when connecting the Hercules RMX2 DJ Controller:
[ 9.600161] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x201, wIndex = 0xa00, type = 4 [ 9.600487] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x200, wIndex = 0xa00, type = 4 [ 9.600984] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x201, wIndex = 0xb00, type = 4 [ 9.601236] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x200, wIndex = 0xb00, type = 4
But I am not entirely sure if this solves the real Bug. So I hope one with more experience can help out here.
Kind regards,
Daniel
2013/4/24 Takashi Iwai tiwai@suse.de
At Tue, 23 Apr 2013 23:38:49 +0200, Daniel Schürmann wrote:
During probing, snd_usb_autoresume() failes with -ENODEV. This brakes get_ctl_value() in mixer.c when rading the range values. The new code does not try to call snd_usb_autoresume() in this case. 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 missleading during debug.
Does this patch really fix the real bug (i.e. you got an error in get_ctl_value() during probe), or just hypothesis?
Also, better to correct many typos in the changelog :)
thanks,
Takashi
Signed-off-by: Daniel Schürmann daschuer@mixxx.org
sound/usb/mixer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index ca4739c..c2daa58 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -289,9 +289,11 @@ static int get_ctl_value_v1(struct
usb_mixer_elem_info *cval, int request, int v
int timeout = 10; int idx = 0, err;
err = snd_usb_autoresume(cval->mixer->chip);if (err < 0)return -EIO;
if (!chip->probing) {err = snd_usb_autoresume(cval->mixer->chip);if (err < 0)return -EIO;} down_read(&chip->shutdown_rwsem); while (timeout-- > 0) { if (chip->shutdown)@@ -333,9 +335,14 @@ 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)goto error;
if (!chip->probing) {ret = snd_usb_autoresume(chip);if (ret) {snd_printk(KERN_ERR "snd_usb_autoresume() failedwith %d\n", ret);
ret = -EIO;goto error;}} down_read(&chip->shutdown_rwsem); if (chip->shutdown)@@ -351,8 +358,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 09:15:17 +0200, Daniel Schürmann wrote:
Hi Takashi,
thank you for review.
The patch solves the following error messages when connecting the Hercules RMX2 DJ Controller:
[ 9.600161] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x201, wIndex = 0xa00, type = 4 [ 9.600487] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x200, wIndex = 0xa00, type = 4 [ 9.600984] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x201, wIndex = 0xb00, type = 4 [ 9.601236] ALSA mixer.c:352 cannot get ctl value: req = 0x83, wValue = 0x200, wIndex = 0xb00, type = 4
But I am not entirely sure if this solves the real Bug. So I hope one with more experience can help out here.
It's strange because snd_usb_autoresume() itself has already the check of chip->probing... Ah, wait, I see the culprit.
The reason is simple. snd_usb_autoresume() returns -ENODEV when chip->probing is set. This should be zero instead.
Does the patch below work instead of yours?
Takashi
--- diff --git a/sound/usb/card.c b/sound/usb/card.c index 5254b18..1a03317 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -631,7 +631,9 @@ int snd_usb_autoresume(struct snd_usb_audio *chip) int err = -ENODEV;
down_read(&chip->shutdown_rwsem); - if (!chip->shutdown && !chip->probing) + if (chip->probing) + err = 0; + else if (!chip->shutdown) err = usb_autopm_get_interface(chip->pm_intf); up_read(&chip->shutdown_rwsem);
participants (2)
-
Daniel Schürmann -
Takashi Iwai