[PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[Mark Hills]

Some interfaces reference endpoints which do not exists. To
accomodate these, do not fail completely, but skip over them.

This allows the Electrix Ebox-44 with earlier firmware to be
detected and used for audio.

Signed-off-by: Mark Hills <mark@pogo.org.uk>
---
 sound/usb/mixer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index ab23869..c374c72 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1388,7 +1388,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, void *r
 	for (pin = 0; pin < input_pins; pin++) {
 		err = parse_audio_unit(state, desc->baSourceID[pin]);
 		if (err < 0)
-			return err;
+			continue;
 		err = check_input_term(state, desc->baSourceID[pin], &iterm);
 		if (err < 0)
 			return err;
-- 
1.7.4.4

[PATCH 2/2] snd-usb-audio: Replace mixer for Electrix Ebox-44

[Mark Hills]

The mixer units from the firmware are corrupt, and even where they
are valid they presents mono controls as L and R channels of
stereo.

Signed-off-by: Mark Hills <mark@pogo.org.uk>
---
 sound/usb/mixer_maps.c   |   13 +++++++++
 sound/usb/mixer_quirks.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
index f1324c4..41daaa2 100644
--- a/sound/usb/mixer_maps.c
+++ b/sound/usb/mixer_maps.c
@@ -288,6 +288,15 @@ static struct usbmix_name_map scratch_live_map[] = {
 	{ 0 } /* terminator */
 };
 
+static struct usbmix_name_map ebox44_map[] = {
+	{ 4, NULL }, /* FU */
+	{ 6, NULL }, /* MU */
+	{ 7, NULL }, /* FU */
+	{ 10, NULL }, /* FU */
+	{ 11, NULL }, /* MU */
+	{ 0 }
+};
+
 /* "Gamesurround Muse Pocket LT" looks same like "Sound Blaster MP3+"
  *  most importand difference is SU[8], it should be set to "Capture Source"
  *  to make alsamixer and PA working properly.
@@ -371,6 +380,10 @@ static struct usbmix_ctl_map usbmix_ctl_maps[] = {
 		.map = scratch_live_map,
 		.ignore_ctl_error = 1,
 	},
+	{
+		.id = USB_ID(0x200c, 0x1018),
+		.map = ebox44_map,
+	},
 	{ 0 } /* terminator */
 };
 
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index ab125ee..e2072ed 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -557,6 +557,69 @@ static int snd_maudio_ftu_create_mixer(struct usb_mixer_interface *mixer)
 	return 0;
 }
 
+static int snd_ebox44_create_ctl(struct usb_mixer_interface *mixer,
+				int unitid, int control, int cmask,
+				int val_type, const char *name)
+{
+	struct usb_mixer_elem_info *cval;
+	struct snd_kcontrol *kctl;
+
+	cval = kzalloc(sizeof(*cval), GFP_KERNEL);
+	if (!cval)
+		return -ENOMEM;
+
+	cval->id = unitid;
+	cval->mixer = mixer;
+
+	cval->val_type = val_type;
+	cval->channels = 1;
+	cval->control = control;
+	cval->cmask = cmask;
+
+	/* Volume controls will override these values */
+	cval->min = 0;
+	cval->max = 1;
+	cval->res = 0;
+
+	cval->dBmin = 0;
+	cval->dBmax = 0;
+
+	kctl = snd_ctl_new1(snd_usb_feature_unit_ctl, cval);
+	if (!kctl) {
+		kfree(cval);
+		return -ENOMEM;
+	}
+
+	snprintf(kctl->id.name, sizeof(kctl->id.name), name);
+	kctl->private_free = usb_mixer_elem_free;
+	return snd_usb_mixer_add_control(mixer, kctl);
+}
+
+/*
+ * Create mixer for Electrix Ebox-44
+ *
+ * The mixer units from this device are corrupt, and even where they
+ * are valid they presents mono controls as L and R channels of
+ * stereo. So we create a good mixer in code.
+ */
+
+static int snd_ebox44_create_mixer(struct usb_mixer_interface *mixer)
+{
+	snd_ebox44_create_ctl(mixer, 4, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Headphone Playback Switch");
+	snd_ebox44_create_ctl(mixer, 4, 2, 0x1, USB_MIXER_S16, "Headphone A Mix Playback Volume");
+	snd_ebox44_create_ctl(mixer, 4, 2, 0x2, USB_MIXER_S16, "Headphone B Mix Playback Volume");
+
+	snd_ebox44_create_ctl(mixer, 7, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Output Playback Switch");
+	snd_ebox44_create_ctl(mixer, 7, 2, 0x1, USB_MIXER_S16, "Output A Playback Volume");
+	snd_ebox44_create_ctl(mixer, 7, 2, 0x2, USB_MIXER_S16, "Output B Playback Volume");
+
+	snd_ebox44_create_ctl(mixer, 10, 1, 0x0, USB_MIXER_INV_BOOLEAN, "Input Capture Switch");
+	snd_ebox44_create_ctl(mixer, 10, 2, 0x1, USB_MIXER_S16, "Input A Capture Volume");
+	snd_ebox44_create_ctl(mixer, 10, 2, 0x2, USB_MIXER_S16, "Input B Capture Volume");
+
+	return 0;
+}
+
 void snd_emuusb_set_samplerate(struct snd_usb_audio *chip,
 			       unsigned char samplerate_id)
 {
@@ -619,6 +682,10 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer)
 				snd_nativeinstruments_ta10_mixers,
 				ARRAY_SIZE(snd_nativeinstruments_ta10_mixers));
 		break;
+
+	case USB_ID(0x200c, 0x1018): /* Electrix Ebox-44 */
+		err = snd_ebox44_create_mixer(mixer);
+		break;
 	}
 
 	return err;
-- 
1.7.4.4

Re: [PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[Takashi Iwai]

At Sat, 14 Apr 2012 17:19:23 +0100,
Mark Hills wrote:
> 
> Some interfaces reference endpoints which do not exists. To
> accomodate these, do not fail completely, but skip over them.
> 
> This allows the Electrix Ebox-44 with earlier firmware to be
> detected and used for audio.

Does the driver warn something?  Ignoring silently doesn't sound
right...


Takashi

> 
> Signed-off-by: Mark Hills <mark@pogo.org.uk>
> ---
>  sound/usb/mixer.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index ab23869..c374c72 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -1388,7 +1388,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, void *r
>  	for (pin = 0; pin < input_pins; pin++) {
>  		err = parse_audio_unit(state, desc->baSourceID[pin]);
>  		if (err < 0)
> -			return err;
> +			continue;
>  		err = check_input_term(state, desc->baSourceID[pin], &iterm);
>  		if (err < 0)
>  			return err;
> -- 
> 1.7.4.4
> 

Re: [PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[Mark Hills]

On Sat, 14 Apr 2012, Takashi Iwai wrote:

> At Sat, 14 Apr 2012 17:19:23 +0100,
> Mark Hills wrote:
> > 
> > Some interfaces reference endpoints which do not exists. To
> > accomodate these, do not fail completely, but skip over them.
> > 
> > This allows the Electrix Ebox-44 with earlier firmware to be
> > detected and used for audio.
> 
> Does the driver warn something?  Ignoring silently doesn't sound
> right...

Yes, parse_audio_unit already reports the error. Though I didn't do an 
exhaustive check for all cases.

The suggestion to do this came from Clemens (see thread "usb-audio: 
Correct way to do a mixer quirk?"). Originally I was hoping to just quirk 
this for the specific device, but the rest of the driver does also seem to 
be fairly tolerant of errors if it means more devices can be used.

> 
> Takashi
> 
> > 
> > Signed-off-by: Mark Hills <mark@pogo.org.uk>
> > ---
> >  sound/usb/mixer.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index ab23869..c374c72 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -1388,7 +1388,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, void *r
> >  	for (pin = 0; pin < input_pins; pin++) {
> >  		err = parse_audio_unit(state, desc->baSourceID[pin]);
> >  		if (err < 0)
> > -			return err;
> > +			continue;
> >  		err = check_input_term(state, desc->baSourceID[pin], &iterm);
> >  		if (err < 0)
> >  			return err;
> > -- 
> > 1.7.4.4
> > 
> 

-- 
Mark

Re: [PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[Takashi Iwai]

At Sat, 14 Apr 2012 21:38:42 +0100 (BST),
Mark Hills wrote:
> 
> On Sat, 14 Apr 2012, Takashi Iwai wrote:
> 
> > At Sat, 14 Apr 2012 17:19:23 +0100,
> > Mark Hills wrote:
> > > 
> > > Some interfaces reference endpoints which do not exists. To
> > > accomodate these, do not fail completely, but skip over them.
> > > 
> > > This allows the Electrix Ebox-44 with earlier firmware to be
> > > detected and used for audio.
> > 
> > Does the driver warn something?  Ignoring silently doesn't sound
> > right...
> 
> Yes, parse_audio_unit already reports the error. Though I didn't do an 
> exhaustive check for all cases.
> 
> The suggestion to do this came from Clemens (see thread "usb-audio: 
> Correct way to do a mixer quirk?"). Originally I was hoping to just quirk 
> this for the specific device, but the rest of the driver does also seem to 
> be fairly tolerant of errors if it means more devices can be used.

OK, I merged your patches now to sound git tree topic/misc branch.
Thanks!


Takashi

> 
> > 
> > Takashi
> > 
> > > 
> > > Signed-off-by: Mark Hills <mark@pogo.org.uk>
> > > ---
> > >  sound/usb/mixer.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > > index ab23869..c374c72 100644
> > > --- a/sound/usb/mixer.c
> > > +++ b/sound/usb/mixer.c
> > > @@ -1388,7 +1388,7 @@ static int parse_audio_mixer_unit(struct mixer_build *state, int unitid, void *r
> > >  	for (pin = 0; pin < input_pins; pin++) {
> > >  		err = parse_audio_unit(state, desc->baSourceID[pin]);
> > >  		if (err < 0)
> > > -			return err;
> > > +			continue;
> > >  		err = check_input_term(state, desc->baSourceID[pin], &iterm);
> > >  		if (err < 0)
> > >  			return err;
> > > -- 
> > > 1.7.4.4
> > > 
> > 
> 
> -- 
> Mark
> 

Re: [PATCH 1/2] snd-usb-audio: Skip un-parseable mixer units instead of erroring

[Jassi Brar]

On 15 April 2012 00:54, Takashi Iwai <tiwai@suse.de> wrote:
> At Sat, 14 Apr 2012 17:19:23 +0100,
> Mark Hills wrote:
>>
>> Some interfaces reference endpoints which do not exists. To
>> accomodate these, do not fail completely, but skip over them.
>>
>> This allows the Electrix Ebox-44 with earlier firmware to be
>> detected and used for audio.
>
> Does the driver warn something?  Ignoring silently doesn't sound
> right...
>
I had been looking into UAC2 device side driver. Debugging
it, enlightened me of the mess the USB audio implementations are in.
IMHO ALSA support usb audio better than AppleUSBAudio ... guess they
are too embarrassed of the code that they no longer keep it public :D

Of course, major blame is on buggy implementation/reporting of configuration
by the devices.

I was wondering if it would make sense for ALSA to first 'mend' any
discrepancies in the reported configuration (say by having optional
callbacks registered against vid/pid), and then instantiate the sound card
which only refers to the cached sanitized configuration ?

That way, we could write usb-audio core to UAC specification, while
keeping vendor specific hacks out separately. Or so do I think.

Regards.
-Jassi