[PATCH 1/5] ALSA: snd-usb: don't fail hard if mixer creation fails

[PATCH 1/5] ALSA: snd-usb: don't fail hard if mixer creation fails

[Daniel Mack]

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);
 		}
 	}
 
-- 
1.7.5.4

[PATCH 2/5] ALSA: snd-usb: use snd_printk() instead of printk()

[Daniel Mack]

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;
-- 
1.7.5.4

[PATCH 3/5] ALSA: snd-usb: be more verbose about bogus UAC_FORMAT_TYPE descriptors

[Daniel Mack]

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;
 		}
 
-- 
1.7.5.4

[PATCH 4/5] ALSA: snd-usb: ignore mixer units of type UAC2_EXTENSION_UNIT_V2

[Daniel Mack]

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;
-- 
1.7.5.4

_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable

[PATCH 5/5] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

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;
-- 
1.7.5.4

_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable

Re: [PATCH 1/5] ALSA: snd-usb: don't fail hard if mixer creation fails

[Takashi Iwai]

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);
>  		} 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);
>  		}
>  	}
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 2/5] ALSA: snd-usb: use snd_printk() instead of printk()

[Takashi Iwai]

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");
>  		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;
> -- 
> 1.7.5.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 1/5] ALSA: snd-usb: don't fail hard if mixer creation fails

[Daniel Mack]

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