[alsa-devel] [PATCH 1/5] ALSA: snd-usb: don't fail hard if mixer creation fails
There are cards that the mixer code fails to create controls for, and currently, the whole sound device creation fails in such cases. This patch changes the code so that it just spits out warnings and continues looking at other functions of the descriptor set which might work just fine.
The worst thing that can happen now is that people see soundcards that don't have all funcions supported, but rejecting the whole device is not any better.
Signed-off-by: Daniel Mack zonque@gmail.com --- sound/usb/card.c | 5 ++--- sound/usb/mixer.c | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 220c616..8990b49 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -512,9 +512,8 @@ static void *snd_usb_audio_probe(struct usb_device *dev, if (err > 0) { /* create normal USB audio interfaces */ if (snd_usb_create_streams(chip, ifnum) < 0 || - snd_usb_create_mixer(chip, ifnum, ignore_ctl_error) < 0) { - goto __error; - } + snd_usb_create_mixer(chip, ifnum, ignore_ctl_error) < 0) + snd_printk(KERN_WARNING "snd_usb_create_mixer() failed\n"); }
/* we are allowed to call snd_card_register() many times */ diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index c22fa76..c43dbea 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1967,7 +1967,8 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer) state.oterm.name = desc->iTerminal; err = parse_audio_unit(&state, desc->bSourceID); if (err < 0) - return err; + snd_printk(KERN_WARNING "Unable to parse terminal with bSourceID %d\n", + desc->bSourceID); } else { /* UAC_VERSION_2 */ struct uac2_output_terminal_descriptor *desc = p;
@@ -1979,12 +1980,14 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer) state.oterm.name = desc->iTerminal; err = parse_audio_unit(&state, desc->bSourceID); if (err < 0) - return err; + snd_printk(KERN_WARNING "Unable to parse terminal with bSourceID %d\n", + desc->bSourceID);
/* for UAC2, use the same approach to also add the clock selectors */ err = parse_audio_unit(&state, desc->bCSourceID); if (err < 0) - return err; + snd_printk(KERN_WARNING "Unable to parse clock terminal with bCSourceID %d\n", + desc->bCSourceID); } }
Signed-off-by: Daniel Mack zonque@gmail.com --- sound/usb/card.c | 4 ++-- sound/usb/clock.c | 6 +++--- sound/usb/midi.c | 4 ++-- sound/usb/pcm.c | 12 ++++++------ 4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 8990b49..9a4f423 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -488,7 +488,7 @@ static void *snd_usb_audio_probe(struct usb_device *dev, break; } if (!chip) { - printk(KERN_ERR "no available usb audio device\n"); + snd_printk(KERN_ERR "no available usb audio device\n"); goto __error; } } @@ -709,7 +709,7 @@ static struct usb_driver usb_audio_driver = { static int __init snd_usb_audio_init(void) { if (nrpacks < 1 || nrpacks > MAX_PACKS) { - printk(KERN_WARNING "invalid nrpacks value.\n"); + snd_printk(KERN_WARNING "invalid nrpacks value.\n"); return -EINVAL; } return usb_register(&usb_audio_driver); diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 075195e..b319299 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -163,9 +163,9 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, /* Selector values are one-based */
if (ret > selector->bNrInPins || ret < 1) { - printk(KERN_ERR - "%s(): selector reported illegal value, id %d, ret %d\n", - __func__, selector->bClockID, ret); + snd_printk(KERN_ERR + "%s(): selector reported illegal value, id %d, ret %d\n", + __func__, selector->bClockID, ret);
return -EINVAL; } diff --git a/sound/usb/midi.c b/sound/usb/midi.c index f928910..a2ae089 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -237,8 +237,8 @@ static void dump_urb(const char *type, const u8 *data, int length) { snd_printk(KERN_DEBUG "%s packet: [", type); for (; length > 0; ++data, --length) - printk(" %02x", *data); - printk(" ]\n"); + snd_printk(" %02x", *data); + snd_printk(" ]\n"); } #else #define dump_urb(type, data, length) /* nothing */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b8dcbf4..24a7a404 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -300,12 +300,12 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) snd_usb_set_format_quirk(subs, fmt);
#if 0 - printk(KERN_DEBUG - "setting done: format = %d, rate = %d..%d, channels = %d\n", - fmt->format, fmt->rate_min, fmt->rate_max, fmt->channels); - printk(KERN_DEBUG - " datapipe = 0x%0x, syncpipe = 0x%0x\n", - subs->datapipe, subs->syncpipe); + snd_printk(KERN_DEBUG + "setting done: format = %d, rate = %d..%d, channels = %d\n", + fmt->format, fmt->rate_min, fmt->rate_max, fmt->channels); + snd_printk(KERN_DEBUG + " datapipe = 0x%0x, syncpipe = 0x%0x\n", + subs->datapipe, subs->syncpipe); #endif
return 0;
At Wed, 13 Jul 2011 02:13:26 +0200, Daniel Mack wrote:
Signed-off-by: Daniel Mack zonque@gmail.com
snd_printk() doesn't mean to give a prefix to output. Its behavior depends on CONFIG_SND_VERBOSE_PRINTK. Thus it'd be anyway better to put some prefix in each error/warning print.
thanks,
Takashi
sound/usb/card.c | 4 ++-- sound/usb/clock.c | 6 +++--- sound/usb/midi.c | 4 ++-- sound/usb/pcm.c | 12 ++++++------ 4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 8990b49..9a4f423 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -488,7 +488,7 @@ static void *snd_usb_audio_probe(struct usb_device *dev, break; } if (!chip) {
printk(KERN_ERR "no available usb audio device\n");
snd_printk(KERN_ERR "no available usb audio device\n"); goto __error;@@ -709,7 +709,7 @@ static struct usb_driver usb_audio_driver = { static int __init snd_usb_audio_init(void) { if (nrpacks < 1 || nrpacks > MAX_PACKS) {
printk(KERN_WARNING "invalid nrpacks value.\n");
snd_printk(KERN_WARNING "invalid nrpacks value.\n");diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 075195e..b319299 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -163,9 +163,9 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, /* Selector values are one-based */
if (ret > selector->bNrInPins || ret < 1) {
printk(KERN_ERR"%s(): selector reported illegal value, id %d, ret %d\n",__func__, selector->bClockID, ret);
snd_printk(KERN_ERR"%s(): selector reported illegal value, id %d, ret %d\n",__func__, selector->bClockID, ret); return -EINVAL;}
diff --git a/sound/usb/midi.c b/sound/usb/midi.c index f928910..a2ae089 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -237,8 +237,8 @@ static void dump_urb(const char *type, const u8 *data, int length) { snd_printk(KERN_DEBUG "%s packet: [", type); for (; length > 0; ++data, --length)
printk(" %02x", *data);- printk(" ]\n");
snd_printk(" %02x", *data);- snd_printk(" ]\n");
} #else #define dump_urb(type, data, length) /* nothing */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b8dcbf4..24a7a404 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -300,12 +300,12 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) snd_usb_set_format_quirk(subs, fmt);
#if 0
- printk(KERN_DEBUG
"setting done: format = %d, rate = %d..%d, channels = %d\n",fmt->format, fmt->rate_min, fmt->rate_max, fmt->channels);- printk(KERN_DEBUG
" datapipe = 0x%0x, syncpipe = 0x%0x\n",subs->datapipe, subs->syncpipe);
- snd_printk(KERN_DEBUG
"setting done: format = %d, rate = %d..%d, channels = %d\n",fmt->format, fmt->rate_min, fmt->rate_max, fmt->channels);- snd_printk(KERN_DEBUG
" datapipe = 0x%0x, syncpipe = 0x%0x\n",subs->datapipe, subs->syncpipe);#endif
return 0;
1.7.5.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Signed-off-by: Daniel Mack zonque@gmail.com --- sound/usb/endpoint.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index b0ef9f5..af59bd7 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -353,8 +353,8 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no) } if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) || ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) { - snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc\n", - dev->devnum, iface_no, altno); + snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n", + dev->devnum, iface_no, altno, protocol, fmt->bLength); continue; }
Failing with a hard error is unjustified. Simply ignore them for now so the rest of the device can be used.
Signed-off-by: Daniel Mack zonque@gmail.com Reported-by: Nicolai Krakowiak nicolai.krakowiak@gmail.com Cc: stable@kernel.org --- sound/usb/mixer.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index c43dbea..7f7dde8 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1899,6 +1899,9 @@ static int parse_audio_unit(struct mixer_build *state, int unitid) return parse_audio_extension_unit(state, unitid, p1); else /* UAC_VERSION_2 */ return parse_audio_processing_unit(state, unitid, p1); + case UAC2_EXTENSION_UNIT_V2: + snd_printdd(KERN_WARNING "Ignoring UAC2_EXTENSION_UNIT\n"); + return 0; /* FIXME - extension units are not implemented yet */ default: snd_printk(KERN_ERR "usbaudio: unit %u: unexpected type 0x%02x\n", unitid, p1[2]); return -EINVAL;
The Focusrite Scarlett 18i6 USB has them that way, which is probably a bug. Anyway, the driver should simply ignore this fact.
Signed-off-by: Daniel Mack zonque@gmail.com Reported-by: Nicolai Krakowiak nicolai.krakowiak@gmail.com Cc: stable@kernel.org --- sound/usb/endpoint.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index af59bd7..b558c67 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no) continue; } if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) || - ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) { + ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) { snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n", dev->devnum, iface_no, altno, protocol, fmt->bLength); continue;
At Wed, 13 Jul 2011 02:13:25 +0200, Daniel Mack wrote:
There are cards that the mixer code fails to create controls for, and currently, the whole sound device creation fails in such cases. This patch changes the code so that it just spits out warnings and continues looking at other functions of the descriptor set which might work just fine.
It depends. If a serious error like -ENOMEM, we should abort immediately. It means something is really bad.
The worst thing that can happen now is that people see soundcards that don't have all funcions supported, but rejecting the whole device is not any better.
Then it'd be better to check some "easy" errors to be ignored (or check serious errors to abort). I don't like ignoring all errors blindly.
thanks,
Takashi
Signed-off-by: Daniel Mack zonque@gmail.com
sound/usb/card.c | 5 ++--- sound/usb/mixer.c | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 220c616..8990b49 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -512,9 +512,8 @@ static void *snd_usb_audio_probe(struct usb_device *dev, if (err > 0) { /* create normal USB audio interfaces */ if (snd_usb_create_streams(chip, ifnum) < 0 ||
snd_usb_create_mixer(chip, ifnum, ignore_ctl_error) < 0) {goto __error;}
snd_usb_create_mixer(chip, ifnum, ignore_ctl_error) < 0)snd_printk(KERN_WARNING "snd_usb_create_mixer() failed\n");}
/* we are allowed to call snd_card_register() many times */
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index c22fa76..c43dbea 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1967,7 +1967,8 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer) state.oterm.name = desc->iTerminal; err = parse_audio_unit(&state, desc->bSourceID); if (err < 0)
return err;
snd_printk(KERN_WARNING "Unable to parse terminal with bSourceID %d\n",desc->bSourceID);@@ -1979,12 +1980,14 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer) state.oterm.name = desc->iTerminal; err = parse_audio_unit(&state, desc->bSourceID); if (err < 0)
return err;
snd_printk(KERN_WARNING "Unable to parse terminal with bSourceID %d\n",desc->bSourceID); /* for UAC2, use the same approach to also add the clock selectors */ err = parse_audio_unit(&state, desc->bCSourceID); if (err < 0)
return err;
snd_printk(KERN_WARNING "Unable to parse clock terminal with bCSourceID %d\n",desc->bCSourceID);-- 1.7.5.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Wed, Jul 13, 2011 at 7:29 AM, Takashi Iwai tiwai@suse.de wrote:
At Wed, 13 Jul 2011 02:13:25 +0200, Daniel Mack wrote:
There are cards that the mixer code fails to create controls for, and currently, the whole sound device creation fails in such cases. This patch changes the code so that it just spits out warnings and continues looking at other functions of the descriptor set which might work just fine.
It depends. If a serious error like -ENOMEM, we should abort immediately. It means something is really bad.
That's very true. I'll change this and resend the whole series.
Thanks, Daniel
participants (2)
-
Daniel Mack -
Takashi Iwai