[2/2] snd-usb-audio: Add duplex mode for Digidesign Mbox 1 and enable mixer

Commit Message

Takashi Iwai Nov. 10, 2014, 6:47 a.m. UTC

At Mon, 10 Nov 2014 10:45:19 +1100,
Damien Zammit wrote:
> 
> On 10/11/14 04:27, Takashi Iwai wrote:
> > At Sun, 09 Nov 2014 15:04:42 +0100,
> > Clemens Ladisch wrote:
> >>
> >> Takashi Iwai wrote:
> >>> I think the only point is the check in create_composite_quirk(), where
> >>> it marks the iface as claimed and skips the next entry that has been
> >>> already claimed.  However, the current code looks inconsistent -- it
> >>> allows multiple entries only if the iface matches with the current
> >>> one.  Fixing it like below would make things working.
> >>>
> >>> It's a quick idea, so a bit more reviews would be needed, though.
> >>> Clemens, what do you think?
> >>
> >> Reviewed-by: Clemens Ladisch <[e-mail]>
> 
> 
> >>> ---
> >>> --- a/sound/usb/quirks.c
> >>> +++ b/sound/usb/quirks.c
> >>> @@ -58,9 +58,17 @@ static int create_composite_quirk(struct snd_usb_audio *chip,
> >>>  		err = snd_usb_create_quirk(chip, iface, driver, quirk);
> >>>  		if (err < 0)
> >>>  			return err;
> >>> -		if (quirk->ifnum != probed_ifnum)
> >>> +	}
> >>> +
> >>> +	for (quirk = quirk->data; quirk->ifnum >= 0; ++quirk) {
> >>> +		iface = usb_ifnum_to_if(chip->dev, quirk->ifnum);
> >>> +		if (!iface)
> >>> +			continue;
> >>> +		if (quirk->ifnum != probed_ifnum &&
> >>> +		    !usb_interface_claimed(iface))
> >>>  			usb_driver_claim_interface(driver, iface, (void *)-1L);
> >>>  	}
> >>> +
> >>>  	return 0;
> >>>  }
> >>>
> >>
> 
> When I tried the patch on the hardware with my quirk, I got a kernel oops.

My bad, take the additional fix patch below.


Takashi

-- 8< --
From: Takashi Iwai <[e-mail]>
Subject: [PATCH] ALSA: usb-audio: Fix Oops by composite quirk enhancement

The quirk argument itself was used as iterator, so it cannot be taken
back to the original value, obviously.

Fixes: d4b8fc66f770 ('ALSA: usb-audio: Allow multiple entries for the same iface in composite quirk')
Signed-off-by: Takashi Iwai <[e-mail]>
---
 sound/usb/quirks.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Damien Zammit Nov. 10, 2014, 7:34 a.m. UTC | #1

On 10/11/14 17:47, Takashi Iwai wrote:
> My bad, take the additional fix patch below.
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <[e-mail]>
> Subject: [PATCH] ALSA: usb-audio: Fix Oops by composite quirk enhancement
> 
> The quirk argument itself was used as iterator, so it cannot be taken
> back to the original value, obviously.
> 
> Fixes: d4b8fc66f770 ('ALSA: usb-audio: Allow multiple entries for the same iface in composite quirk')
> Signed-off-by: Takashi Iwai <[e-mail]>
> ---
>  sound/usb/quirks.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

I confirm that this fixes the kernel oops, and the logic works.  See
next patch series for working patch!

Damien

Patch

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index e9ff3a6c60e4..809d7fab4633 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -43,12 +43,13 @@ 
 static int create_composite_quirk(struct snd_usb_audio *chip,
 				  struct usb_interface *iface,
 				  struct usb_driver *driver,
-				  const struct snd_usb_audio_quirk *quirk)
+				  const struct snd_usb_audio_quirk *quirk_comp)
 {
 	int probed_ifnum = get_iface_desc(iface->altsetting)->bInterfaceNumber;
+	const struct snd_usb_audio_quirk *quirk;
 	int err;
 
-	for (quirk = quirk->data; quirk->ifnum >= 0; ++quirk) {
+	for (quirk = quirk_comp->data; quirk->ifnum >= 0; ++quirk) {
 		iface = usb_ifnum_to_if(chip->dev, quirk->ifnum);
 		if (!iface)
 			continue;
@@ -60,7 +61,7 @@  static int create_composite_quirk(struct snd_usb_audio *chip,
 			return err;
 	}
 
-	for (quirk = quirk->data; quirk->ifnum >= 0; ++quirk) {
+	for (quirk = quirk_comp->data; quirk->ifnum >= 0; ++quirk) {
 		iface = usb_ifnum_to_if(chip->dev, quirk->ifnum);
 		if (!iface)
 			continue;