Skip to contentSkip to conversation

usb-audio: some UAC2 fixes

PATCH Applied Daniel Mack 4 participants 19 messages Jun 11, 2010 – Jun 14, 2010 1 message not in the archive Sound

lore
Share & tools
lore

Daniel Mack

author
#0 lore raw

Hi,

here are four small patches that fix some minor bugs in the UAC2 implementation. Thanks to Alex Lee for reporting them.

Don't know if it's possible to get them into 2.6.35 as they're no regressions in respect to 2.6.34 (I followed the recent discussions on LKML). However, they fix real bugs and I would also label them [email protected] if 2.6.35 would have already been released. So .. I don't know :)

I have a number of more patches that do cleanups and implement more functions, but they can certainly go into 2.6.36 and they're not finished yet, so I'm cooking them for some more time.

Thanks,
Daniel

[PATCH 1/4] ALSA: usb-audio: add check for faulty clock in parse_audio_format_rates_v2() [PATCH 2/4] ALSA: usb-audio: fix control messages for USB_RECIP_INTERFACE [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly [PATCH 4/4] ALSA: usb-audio: fix UAC2 control value queries

author
#1 lore raw

PATCH 1/4 ALSA: usb-audio: add check for faulty clock in parse_audio_format_rates_v2()

Signed-off-by Daniel Mack <[email protected]>
---
 sound/usb/format.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
sound/usb/format.c +6 -0
index 5367cd1..df5b29f 100644
@@ -218,6 +218,12 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	int i, nr_rates, data_size, ret = 0;
 	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
 
+	if (clock < 0) {
+		snd_printk(KERN_ERR "%s(): unable to find clock source (clock %d)\n",
+				__func__, clock);
+		goto err;
+	}
+
 	/* get the number of sample rates first by only fetching 2 bytes */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
Signature · 1 line
1.7.1
author
#2 lore raw

PATCH 2/4 ALSA: usb-audio: fix control messages for USB_RECIP_INTERFACE

In reply to Daniel Mack

Control messages directed to an interface must have the interface number set in the lower 8 bits of wIndex. This wasn't done correctly for some clock and mixer messages.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/clock.c  |   12 ++++++++----
 sound/usb/format.c |    6 ++++--
 sound/usb/helper.h |    4 ++++
 3 files changed, 16 insertions(+), 6 deletions(-)
sound/usb/clock.c +8 -4
index b7aadd6..b585511 100644
@@ -103,7 +103,8 @@ static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_i
 	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
 			      UAC2_CS_CUR,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-			      UAC2_CX_CLOCK_SELECTOR << 8, selector_id << 8,
+			      UAC2_CX_CLOCK_SELECTOR << 8,
+			      snd_usb_ctrl_intf(chip) | (selector_id << 8),
 			      &buf, sizeof(buf), 1000);
 
 	if (ret < 0)

@@ -120,7 +121,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
 
 	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_CLOCK_VALID << 8, source_id << 8,
+			      UAC2_CS_CONTROL_CLOCK_VALID << 8,
+			      snd_usb_ctrl_intf(chip) | (source_id << 8),
 			      &data, sizeof(data), 1000);
 
 	if (err < 0) {

@@ -269,7 +271,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 	data[3] = rate >> 24;
 	if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR,
 				   USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
-				   UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+				   UAC2_CS_CONTROL_SAM_FREQ << 8,
+				   snd_usb_ctrl_intf(chip) | (clock << 8),
 				   data, sizeof(data), 1000)) < 0) {
 		snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2)\n",
 			   dev->devnum, iface, fmt->altsetting, rate);

@@ -278,7 +281,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 
 	if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
 				   USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-				   UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+				   UAC2_CS_CONTROL_SAM_FREQ << 8,
+				   snd_usb_ctrl_intf(chip) | (clock << 8),
 				   data, sizeof(data), 1000)) < 0) {
 		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
 			   dev->devnum, iface, fmt->altsetting);
sound/usb/format.c +4 -2
index df5b29f..8eccf17 100644
@@ -227,7 +227,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	/* get the number of sample rates first by only fetching 2 bytes */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+			      UAC2_CS_CONTROL_SAM_FREQ << 8,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
 			      tmp, sizeof(tmp), 1000);
 
 	if (ret < 0) {

@@ -247,7 +248,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	/* now get the full information */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+			      UAC2_CS_CONTROL_SAM_FREQ << 8,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
 			      data, data_size, 1000);
 
 	if (ret < 0) {
sound/usb/helper.h +4 -0
index a6b0e51..09bd943 100644
@@ -28,5 +28,9 @@ unsigned char snd_usb_parse_datainterval(struct snd_usb_audio *chip,
 #define snd_usb_get_speed(dev) ((dev)->speed)
 #endif
 
+static inline int snd_usb_ctrl_intf(struct snd_usb_audio *chip)
+{
+	return get_iface_desc(chip->ctrl_intf)->bInterfaceNumber;
+}
 
 #endif /* __USBAUDIO_HELPER_H */
Signature · 1 line
1.7.1
author
#3 lore raw

PATCH 3/4 ALSA: usb-audio: parse UAC2 sample rate ranges correctly

In reply to Daniel Mack

A device may report its supported sample rates in ranges rather than in discrete triplets. The code used to only parse the MIN field instead of properly paying attention to the MAX and RES values.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/format.c |   80 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 62 insertions(+), 18 deletions(-)
sound/usb/format.c +62 -18
index 8eccf17..e946db7 100644
@@ -206,6 +206,47 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
 }
 
 /*
+ * Helper function to walk the array of sample rate triplets reported by
+ * the device. The problem is that we need to parse whole array first to
+ * get to know how many sample rates we have to expect.
+ * Then fp->rate_table can be allocated and filled.
+ */
+static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
+					const unsigned char *data)
+{
+	int i, nr_rates = 0;
+
+	fp->rates = fp->rate_min = fp->rate_max = 0;
+
+	for (i = 0; i < nr_triplets; i++) {
+		int min = combine_quad(&data[2 + 12 * i]);
+		int max = combine_quad(&data[6 + 12 * i]);
+		int res = combine_quad(&data[10 + 12 * i]);
+		int rate;
+
+		if ((max < 0) || (min < 0) || (res < 0) || (max < min))
+			continue;
+
+		for (rate = min; rate <= max; rate += res) {
+			if (fp->rate_table)
+				fp->rate_table[nr_rates] = rate;
+			if (!fp->rate_min || rate < fp->rate_min)
+				fp->rate_min = rate;
+			if (!fp->rate_max || rate > fp->rate_max)
+				fp->rate_max = rate;
+			fp->rates |= snd_pcm_rate_to_rate_bit(rate);
+			nr_rates++;
+
+			/* avoid endless loop */
+			if (res == 0)
+				break;
+		}
+	}
+
+	return nr_rates;
+}
+
+/*
  * parse the format descriptor and stores the possible sample rates
  * on the audioformat table (audio class v2).
  */

@@ -215,7 +256,7 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char tmp[2], *data;
-	int i, nr_rates, data_size, ret = 0;
+	int nr_triplets, data_size, ret = 0;
 	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
 
 	if (clock < 0) {

@@ -237,8 +278,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err;
 	}
 
-	nr_rates = (tmp[1] << 8) | tmp[0];
-	data_size = 2 + 12 * nr_rates;
+	nr_triplets = (tmp[1] << 8) | tmp[0];
+	data_size = 2 + 12 * nr_triplets;
 	data = kzalloc(data_size, GFP_KERNEL);
 	if (!data) {
 		ret = -ENOMEM;

@@ -259,26 +300,29 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err_free;
 	}
 
-	fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
+	/* Call the triplet parser, and make sure fp->rate_table is NULL.
+	 * We just use the return value to know how many sample rates we
+	 * will have to deal with. */
+	kfree(fp->rate_table);
+	fp->rate_table = NULL;
+	fp->nr_rates = parse_uac2_sample_rate_range(fp, nr_triplets, data);
+
+	if (fp->nr_rates == 0) {
+		snd_printk(KERN_ERR "%s(): unable to parse sample rate triplets\n",
+				__func__);
+		ret = -EINVAL;
+		goto err_free;
+	}
+
+	fp->rate_table = kmalloc(sizeof(int) * fp->nr_rates, GFP_KERNEL);
 	if (!fp->rate_table) {
 		ret = -ENOMEM;
 		goto err_free;
 	}
 
-	fp->nr_rates = 0;
-	fp->rate_min = fp->rate_max = 0;
-
-	for (i = 0; i < nr_rates; i++) {
-		int rate = combine_quad(&data[2 + 12 * i]);
-
-		fp->rate_table[fp->nr_rates] = rate;
-		if (!fp->rate_min || rate < fp->rate_min)
-			fp->rate_min = rate;
-		if (!fp->rate_max || rate > fp->rate_max)
-			fp->rate_max = rate;
-		fp->rates |= snd_pcm_rate_to_rate_bit(rate);
-		fp->nr_rates++;
-	}
+	/* Call the triplet parser again, but this time, fp->rate_table is
+	 * allocated, so the rates will be stored */
+	parse_uac2_sample_rate_range(fp, nr_triplets, data);
 
 err_free:
 	kfree(data);
Signature · 1 line
1.7.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#5 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

Quoted 3 lines · Alex Lee

If the device has a variable clock, it may report a RES of only 1. So you may end up with thousands (and even millions) of sample rates. It might be better to have separate code to cater to discrete clocks vs variable clocks.

AFAIK, ALSA cannot cope with that yet. Am I right?

Daniel

Quoted 141 lines · Daniel Mack
Quoted 140 lines

A device may report its supported sample rates in ranges rather than in discrete triplets. The code used to only parse the MIN field instead of properly paying attention to the MAX and RES values.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/format.c |   80
++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 62 insertions(+), 18 deletions(-)
sound/usb/format.c +0 -0
index 8eccf17..e946db7 100644
@@ -206,6 +206,47 @@ static int parse_audio_format_rates_v1(struct

snd_usb_audio *chip, struct audiof
}

/*
+ * Helper function to walk the array of sample rate triplets reported by
+ * the device. The problem is that we need to parse whole array first to
+ * get to know how many sample rates we have to expect.
+ * Then fp->rate_table can be allocated and filled.
+ */
+static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,

sound/usb/format.c +36 -1
+                                       const unsigned char *data)
+{
+       int i, nr_rates = 0;
+
+       fp->rates = fp->rate_min = fp->rate_max = 0;
+
+       for (i = 0; i < nr_triplets; i++) {
+               int min = combine_quad(&data[2 + 12 * i]);
+               int max = combine_quad(&data[6 + 12 * i]);
+               int res = combine_quad(&data[10 + 12 * i]);
+               int rate;
+
+               if ((max < 0) || (min < 0) || (res < 0) || (max < min))
+                       continue;
+
+               for (rate = min; rate <= max; rate += res) {
+                       if (fp->rate_table)
+                               fp->rate_table[nr_rates] = rate;
+                       if (!fp->rate_min || rate < fp->rate_min)
+                               fp->rate_min = rate;
+                       if (!fp->rate_max || rate > fp->rate_max)
+                               fp->rate_max = rate;
+                       fp->rates |= snd_pcm_rate_to_rate_bit(rate);
+                       nr_rates++;
+
+                       /* avoid endless loop */
+                       if (res == 0)
+                               break;
+               }
+       }
+
+       return nr_rates;
+}
+
+/*
 * parse the format descriptor and stores the possible sample rates
 * on the audioformat table (audio class v2).
 */

@@ -215,7 +256,7 @@ static int parse_audio_format_rates_v2(struct
 snd_usb_audio *chip,
 {
       struct usb_device *dev = chip->dev;
       unsigned char tmp[2], *data;
-       int i, nr_rates, data_size, ret = 0;
+       int nr_triplets, data_size, ret = 0;
       int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf,
 fp->clock);

       if (clock < 0) {
sound/usb/format.c +2 -2
@@ -237,8 +278,8 @@ static int parse_audio_format_rates_v2(struct
 snd_usb_audio *chip,
               goto err;
       }
 
-       nr_rates = (tmp[1] << 8) | tmp[0];
-       data_size = 2 + 12 * nr_rates;
+       nr_triplets = (tmp[1] << 8) | tmp[0];
+       data_size = 2 + 12 * nr_triplets;
       data = kzalloc(data_size, GFP_KERNEL);
       if (!data) {

               ret = -ENOMEM;
sound/usb/format.c +18 -15
@@ -259,26 +300,29 @@ static int parse_audio_format_rates_v2(struct
 snd_usb_audio *chip,
               goto err_free;
       }
 
-       fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
+       /* Call the triplet parser, and make sure fp->rate_table is NULL.
+        * We just use the return value to know how many sample rates we
+        * will have to deal with. */
+       kfree(fp->rate_table);
+       fp->rate_table = NULL;
+       fp->nr_rates = parse_uac2_sample_rate_range(fp, nr_triplets, data);
+
+       if (fp->nr_rates == 0) {
+               snd_printk(KERN_ERR "%s(): unable to parse sample rate
 triplets\n",
+                               __func__);
+               ret = -EINVAL;
+               goto err_free;
+       }
+
+       fp->rate_table = kmalloc(sizeof(int) * fp->nr_rates, GFP_KERNEL);
       if (!fp->rate_table) {
               ret = -ENOMEM;
               goto err_free;
       }
 
-       fp->nr_rates = 0;
-       fp->rate_min = fp->rate_max = 0;
-
-       for (i = 0; i < nr_rates; i++) {
-               int rate = combine_quad(&data[2 + 12 * i]);
-
-               fp->rate_table[fp->nr_rates] = rate;
-               if (!fp->rate_min || rate < fp->rate_min)
-                       fp->rate_min = rate;
-               if (!fp->rate_max || rate > fp->rate_max)
-                       fp->rate_max = rate;
-               fp->rates |= snd_pcm_rate_to_rate_bit(rate);
-               fp->nr_rates++;
-       }
+       /* Call the triplet parser again, but this time, fp->rate_table is
+        * allocated, so the rates will be stored */
+       parse_uac2_sample_rate_range(fp, nr_triplets, data);

err_free:

       kfree(data);

--
1.7.1

#6 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

Quoted 6 lines · Daniel Mack
Quoted 3 lines · Alex Lee

If the device has a variable clock, it may report a RES of only 1. So you may end up with thousands (and even millions) of sample rates. It might be better to have separate code to cater to discrete clocks vs variable clocks.

AFAIK, ALSA can[...] cope with that [...]. Am I right?

Yes; set fp->rates to SNDRV_PCM_RATE_CONTINUOUS to allow all rates between fp->rate_min and fp->rate_max. See, for example, parse_audio_format_rates_v1.

Regards,
Clemens

author
#7 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

Quoted 10 lines · Clemens Ladisch
Quoted 6 lines · Daniel Mack
Quoted 3 lines · Alex Lee

If the device has a variable clock, it may report a RES of only 1. So you may end up with thousands (and even millions) of sample rates. It might be better to have separate code to cater to discrete clocks vs variable clocks.

AFAIK, ALSA can[...] cope with that [...]. Am I right?

Yes; set fp->rates to SNDRV_PCM_RATE_CONTINUOUS to allow all rates between fp->rate_min and fp->rate_max. See, for example, parse_audio_format_rates_v1.

Ah, ok. So we should we do this for all USB devices that report a RES value of 1. I'll respin this series then. Thanks!

Daniel

author
#8 lore raw

PATCH 1/4 ALSA: usb-audio: add check for faulty clock in parse_audio_format_rates_v2()

Signed-off-by Daniel Mack <[email protected]>
---
 sound/usb/format.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
sound/usb/format.c +6 -0
index 5367cd1..df5b29f 100644
@@ -218,6 +218,12 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	int i, nr_rates, data_size, ret = 0;
 	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
 
+	if (clock < 0) {
+		snd_printk(KERN_ERR "%s(): unable to find clock source (clock %d)\n",
+				__func__, clock);
+		goto err;
+	}
+
 	/* get the number of sample rates first by only fetching 2 bytes */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
Signature · 1 line
1.7.1
Applied
#9 lore raw

Re: [PATCH 1/4] ALSA: usb-audio: add check for faulty clock in parse_audio_format_rates_v2()

At Fri, 11 Jun 2010 17:34:19 +0200,

Quoted 2 lines · Daniel Mack
Signed-off-by Daniel Mack <[email protected]>

Applied all (fixed) patches now. Thanks!

Takashi

author
#10 lore raw

PATCH 2/4 ALSA: usb-audio: fix control messages for USB_RECIP_INTERFACE

In reply to Daniel Mack

Control messages directed to an interface must have the interface number set in the lower 8 bits of wIndex. This wasn't done correctly for some clock and mixer messages.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/clock.c  |   12 ++++++++----
 sound/usb/format.c |    6 ++++--
 sound/usb/helper.h |    4 ++++
 3 files changed, 16 insertions(+), 6 deletions(-)
sound/usb/clock.c +8 -4
index b7aadd6..b585511 100644
@@ -103,7 +103,8 @@ static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_i
 	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0),
 			      UAC2_CS_CUR,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-			      UAC2_CX_CLOCK_SELECTOR << 8, selector_id << 8,
+			      UAC2_CX_CLOCK_SELECTOR << 8,
+			      snd_usb_ctrl_intf(chip) | (selector_id << 8),
 			      &buf, sizeof(buf), 1000);
 
 	if (ret < 0)

@@ -120,7 +121,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
 
 	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_CLOCK_VALID << 8, source_id << 8,
+			      UAC2_CS_CONTROL_CLOCK_VALID << 8,
+			      snd_usb_ctrl_intf(chip) | (source_id << 8),
 			      &data, sizeof(data), 1000);
 
 	if (err < 0) {

@@ -269,7 +271,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 	data[3] = rate >> 24;
 	if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR,
 				   USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
-				   UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+				   UAC2_CS_CONTROL_SAM_FREQ << 8,
+				   snd_usb_ctrl_intf(chip) | (clock << 8),
 				   data, sizeof(data), 1000)) < 0) {
 		snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2)\n",
 			   dev->devnum, iface, fmt->altsetting, rate);

@@ -278,7 +281,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 
 	if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
 				   USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-				   UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+				   UAC2_CS_CONTROL_SAM_FREQ << 8,
+				   snd_usb_ctrl_intf(chip) | (clock << 8),
 				   data, sizeof(data), 1000)) < 0) {
 		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
 			   dev->devnum, iface, fmt->altsetting);
sound/usb/format.c +4 -2
index df5b29f..8eccf17 100644
@@ -227,7 +227,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	/* get the number of sample rates first by only fetching 2 bytes */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+			      UAC2_CS_CONTROL_SAM_FREQ << 8,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
 			      tmp, sizeof(tmp), 1000);
 
 	if (ret < 0) {

@@ -247,7 +248,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 	/* now get the full information */
 	ret = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_RANGE,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
-			      UAC2_CS_CONTROL_SAM_FREQ << 8, clock << 8,
+			      UAC2_CS_CONTROL_SAM_FREQ << 8,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
 			      data, data_size, 1000);
 
 	if (ret < 0) {
sound/usb/helper.h +4 -0
index a6b0e51..09bd943 100644
@@ -28,5 +28,9 @@ unsigned char snd_usb_parse_datainterval(struct snd_usb_audio *chip,
 #define snd_usb_get_speed(dev) ((dev)->speed)
 #endif
 
+static inline int snd_usb_ctrl_intf(struct snd_usb_audio *chip)
+{
+	return get_iface_desc(chip->ctrl_intf)->bInterfaceNumber;
+}
 
 #endif /* __USBAUDIO_HELPER_H */
Signature · 1 line
1.7.1
author
#11 lore raw

PATCH 3/4 ALSA: usb-audio: parse UAC2 sample rate ranges correctly

In reply to Daniel Mack

A device may report its supported sample rates in ranges rather than in discrete triplets. The code used to only parse the MIN field instead of properly paying attention to the MAX and RES values.

Also, handle RES values of 1 corrently and announce a continous sample rate range in this case.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/format.c |   89 +++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 71 insertions(+), 18 deletions(-)
sound/usb/format.c +71 -18
index 8eccf17..7a54a0a 100644
@@ -206,6 +206,57 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
 }
 
 /*
+ * Helper function to walk the array of sample rate triplets reported by
+ * the device. The problem is that we need to parse whole array first to
+ * get to know how many sample rates we have to expect.
+ * Then fp->rate_table can be allocated and filled.
+ */
+static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
+					const unsigned char *data)
+{
+	int i, nr_rates = 0;
+
+	fp->rates = fp->rate_min = fp->rate_max = 0;
+
+	for (i = 0; i < nr_triplets; i++) {
+		int min = combine_quad(&data[2 + 12 * i]);
+		int max = combine_quad(&data[6 + 12 * i]);
+		int res = combine_quad(&data[10 + 12 * i]);
+		int rate;
+
+		if ((max < 0) || (min < 0) || (res < 0) || (max < min))
+			continue;
+
+		/*
+		 * for ranges with res == 1, we want a continuous sample
+		 * rate range, so this function should return 0.
+		 */
+		if (res == 1) {
+			fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
+			return 0;
+		}
+
+		for (rate = min; rate <= max; rate += res) {
+			if (fp->rate_table)
+				fp->rate_table[nr_rates] = rate;
+			if (!fp->rate_min || rate < fp->rate_min)
+				fp->rate_min = rate;
+			if (!fp->rate_max || rate > fp->rate_max)
+				fp->rate_max = rate;
+			fp->rates |= snd_pcm_rate_to_rate_bit(rate);
+
+			nr_rates++;
+
+			/* avoid endless loop */
+			if (res == 0)
+				break;
+		}
+	}
+
+	return nr_rates;
+}
+
+/*
  * parse the format descriptor and stores the possible sample rates
  * on the audioformat table (audio class v2).
  */

@@ -215,7 +266,7 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char tmp[2], *data;
-	int i, nr_rates, data_size, ret = 0;
+	int nr_triplets, data_size, ret = 0;
 	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
 
 	if (clock < 0) {

@@ -237,8 +288,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err;
 	}
 
-	nr_rates = (tmp[1] << 8) | tmp[0];
-	data_size = 2 + 12 * nr_rates;
+	nr_triplets = (tmp[1] << 8) | tmp[0];
+	data_size = 2 + 12 * nr_triplets;
 	data = kzalloc(data_size, GFP_KERNEL);
 	if (!data) {
 		ret = -ENOMEM;

@@ -259,26 +310,28 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err_free;
 	}
 
-	fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
+	/* Call the triplet parser, and make sure fp->rate_table is NULL.
+	 * We just use the return value to know how many sample rates we
+	 * will have to deal with. */
+	kfree(fp->rate_table);
+	fp->rate_table = NULL;
+	fp->nr_rates = parse_uac2_sample_rate_range(fp, nr_triplets, data);
+
+	if (fp->nr_rates == 0) {
+		/* SNDRV_PCM_RATE_CONTINUOUS */
+		err = 0;
+		goto err_free;
+	}
+
+	fp->rate_table = kmalloc(sizeof(int) * fp->nr_rates, GFP_KERNEL);
 	if (!fp->rate_table) {
 		ret = -ENOMEM;
 		goto err_free;
 	}
 
-	fp->nr_rates = 0;
-	fp->rate_min = fp->rate_max = 0;
-
-	for (i = 0; i < nr_rates; i++) {
-		int rate = combine_quad(&data[2 + 12 * i]);
-
-		fp->rate_table[fp->nr_rates] = rate;
-		if (!fp->rate_min || rate < fp->rate_min)
-			fp->rate_min = rate;
-		if (!fp->rate_max || rate > fp->rate_max)
-			fp->rate_max = rate;
-		fp->rates |= snd_pcm_rate_to_rate_bit(rate);
-		fp->nr_rates++;
-	}
+	/* Call the triplet parser again, but this time, fp->rate_table is
+	 * allocated, so the rates will be stored */
+	parse_uac2_sample_rate_range(fp, nr_triplets, data);
 
 err_free:
 	kfree(data);
Signature · 1 line
1.7.1
#12 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

At Fri, 11 Jun 2010 17:34:21 +0200,

sound/usb/format.c static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof · line 236

Quoted 8 lines · Daniel Mack
sound/usb/format.c +7 -0
+		/*
+		 * for ranges with res == 1, we want a continuous sample
+		 * rate range, so this function should return 0.
+		 */
+		if (res == 1) {
+			fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
+			return 0;

Missing setup of fp->rate_min and fp->rate_max here?

Takashi

author
#13 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

Quoted 12 lines · Takashi Iwai

At Fri, 11 Jun 2010 17:34:21 +0200,

Review

Quoted 8 lines · Daniel Mack
(quoted diff) +7 -0
+		/*
+		 * for ranges with res == 1, we want a continuous sample
+		 * rate range, so this function should return 0.
+		 */
+		if (res == 1) {
+			fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
+			return 0;

Missing setup of fp->rate_min and fp->rate_max here?

Gna, not my day. I did that, and of course tested it with a modified firmware and it worked well.

But appearantly I exported the wrong patch. Sorry.

Daniel

Quoted 1 line

From 6cefee7ab5168b0875af0aaaa78a0e7b6cce1890 Mon Sep 17 00:00:00 2001

From: Daniel Mack <[email protected]>
Date: Fri, 11 Jun 2010 11:32:29 +0200
Subject: [PATCH] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

A device may report its supported sample rates in ranges rather than in discrete triplets. The code used to only parse the MIN field instead of properly paying attention to the MAX and RES values.

Also, handle RES values of 1 correctly and announce a continous sample rate range in this case.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/format.c |   92 +++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 74 insertions(+), 18 deletions(-)
sound/usb/format.c +74 -18
index 8eccf17..30364ab 100644
@@ -206,6 +206,60 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
 }
 
 /*
+ * Helper function to walk the array of sample rate triplets reported by
+ * the device. The problem is that we need to parse whole array first to
+ * get to know how many sample rates we have to expect.
+ * Then fp->rate_table can be allocated and filled.
+ */
+static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
+					const unsigned char *data)
+{
+	int i, nr_rates = 0;
+
+	fp->rates = fp->rate_min = fp->rate_max = 0;
+
+	for (i = 0; i < nr_triplets; i++) {
+		int min = combine_quad(&data[2 + 12 * i]);
+		int max = combine_quad(&data[6 + 12 * i]);
+		int res = combine_quad(&data[10 + 12 * i]);
+		int rate;
+
+		if ((max < 0) || (min < 0) || (res < 0) || (max < min))
+			continue;
+
+		/*
+		 * for ranges with res == 1, we announce a continuous sample
+		 * rate range, and this function should return 0 for no further
+		 * parsing.
+		 */
+		if (res == 1) {
+			fp->rate_min = min;
+			fp->rate_max = max;
+			fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
+			return 0;
+		}
+
+		for (rate = min; rate <= max; rate += res) {
+			if (fp->rate_table)
+				fp->rate_table[nr_rates] = rate;
+			if (!fp->rate_min || rate < fp->rate_min)
+				fp->rate_min = rate;
+			if (!fp->rate_max || rate > fp->rate_max)
+				fp->rate_max = rate;
+			fp->rates |= snd_pcm_rate_to_rate_bit(rate);
+
+			nr_rates++;
+
+			/* avoid endless loop */
+			if (res == 0)
+				break;
+		}
+	}
+
+	return nr_rates;
+}
+
+/*
  * parse the format descriptor and stores the possible sample rates
  * on the audioformat table (audio class v2).
  */

@@ -215,7 +269,7 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char tmp[2], *data;
-	int i, nr_rates, data_size, ret = 0;
+	int nr_triplets, data_size, ret = 0;
 	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
 
 	if (clock < 0) {

@@ -237,8 +291,8 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err;
 	}
 
-	nr_rates = (tmp[1] << 8) | tmp[0];
-	data_size = 2 + 12 * nr_rates;
+	nr_triplets = (tmp[1] << 8) | tmp[0];
+	data_size = 2 + 12 * nr_triplets;
 	data = kzalloc(data_size, GFP_KERNEL);
 	if (!data) {
 		ret = -ENOMEM;

@@ -259,26 +313,28 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
 		goto err_free;
 	}
 
-	fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
+	/* Call the triplet parser, and make sure fp->rate_table is NULL.
+	 * We just use the return value to know how many sample rates we
+	 * will have to deal with. */
+	kfree(fp->rate_table);
+	fp->rate_table = NULL;
+	fp->nr_rates = parse_uac2_sample_rate_range(fp, nr_triplets, data);
+
+	if (fp->nr_rates == 0) {
+		/* SNDRV_PCM_RATE_CONTINUOUS */
+		ret = 0;
+		goto err_free;
+	}
+
+	fp->rate_table = kmalloc(sizeof(int) * fp->nr_rates, GFP_KERNEL);
 	if (!fp->rate_table) {
 		ret = -ENOMEM;
 		goto err_free;
 	}
 
-	fp->nr_rates = 0;
-	fp->rate_min = fp->rate_max = 0;
-
-	for (i = 0; i < nr_rates; i++) {
-		int rate = combine_quad(&data[2 + 12 * i]);
-
-		fp->rate_table[fp->nr_rates] = rate;
-		if (!fp->rate_min || rate < fp->rate_min)
-			fp->rate_min = rate;
-		if (!fp->rate_max || rate > fp->rate_max)
-			fp->rate_max = rate;
-		fp->rates |= snd_pcm_rate_to_rate_bit(rate);
-		fp->nr_rates++;
-	}
+	/* Call the triplet parser again, but this time, fp->rate_table is
+	 * allocated, so the rates will be stored */
+	parse_uac2_sample_rate_range(fp, nr_triplets, data);
 
 err_free:
 	kfree(data);
Signature · 1 line
1.7.1
#14 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

At Fri, 11 Jun 2010 17:46:33 +0200,

Quoted 15 lines · Daniel Mack
Quoted 12 lines · Takashi Iwai

At Fri, 11 Jun 2010 17:34:21 +0200,

sound/usb/format.c static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof · line 239

Quoted 8 lines · Daniel Mack
sound/usb/format.c +7 -0
+		/*
+		 * for ranges with res == 1, we want a continuous sample
+		 * rate range, so this function should return 0.
+		 */
+		if (res == 1) {
+			fp->rates = SNDRV_PCM_RATE_CONTINUOUS;
+			return 0;

Missing setup of fp->rate_min and fp->rate_max here?

Quoted 4 lines

Gna, not my day. I did that, and of course tested it with a modified firmware and it worked well.

But appearantly I exported the wrong patch. Sorry.

Well, people can understand it if they see the current score of the opening football game ;)

Takashi

author
#15 lore raw

PATCH 4/4 ALSA: usb-audio: fix UAC2 control value queries

In reply to Daniel Mack

For RANGE requests, we should only query as much bytes as we're in fact interested in.

For CUR requests, we shouldn't confuse the firmware with an overlong request but just ask for 2 bytes.

This might need fixing in the future as it's not entirely clear when to dispatch 1-byte, 2-byte and 4-byte request blocks. For now, we assume everything is coded in 16bit - this works for all firmware implementations I've seen.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/mixer.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)
sound/usb/mixer.c +14 -5
index a060d00..6939d0f 100644
@@ -297,20 +297,27 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret)
 {
-	unsigned char buf[14]; /* enough space for one range of 4 bytes */
+	unsigned char buf[2 + 3*sizeof(__u16)]; /* enough space for one range */
 	unsigned char *val;
-	int ret;
+	int ret, size;
 	__u8 bRequest;
 
-	bRequest = (request == UAC_GET_CUR) ?
-		UAC2_CS_CUR : UAC2_CS_RANGE;
+	if (request == UAC_GET_CUR) {
+		bRequest = UAC2_CS_CUR;
+		size = sizeof(__u16);
+	} else {
+		bRequest = UAC2_CS_RANGE;
+		size = sizeof(buf);
+	}
+
+	memset(buf, 0, sizeof(buf));
 
 	ret = snd_usb_ctl_msg(cval->mixer->chip->dev,
 			      usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
 			      bRequest,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 			      validx, cval->mixer->ctrlif | (cval->id << 8),
-			      buf, sizeof(buf), 1000);
+			      buf, size, 1000);
 
 	if (ret < 0) {
 		snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",

@@ -318,6 +325,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
 		return ret;
 	}
 
+	/* FIXME: how should we handle multiple triplets here? */
+
 	switch (request) {
 	case UAC_GET_CUR:
 		val = buf;
Signature · 1 line
1.7.1
#16 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

In reply to Daniel Mack
Quoted 6 lines · Daniel Mack
Quoted 3 lines · Alex Lee

If the device has a variable clock, it may report a RES of only 1. So you may end up with thousands (and even millions) of sample rates. It might be better to have separate code to cater to discrete clocks vs variable clocks.

AFAIK, ALSA cannot cope with that yet. Am I right?

There's _CONTINUOUS and _KNOT.

author
#17 lore raw

Re: [PATCH 3/4] ALSA: usb-audio: parse UAC2 sample rate ranges correctly

In reply to a message not in the archive
Quoted 3 lines · Alex Lee

If the device has a variable clock, it may report a RES of only 1. So you may end up with thousands (and even millions) of sample rates. It might be better to have separate code to cater to discrete clocks vs variable clocks.

Just for the records: this is exactly what Mac OS X does, and it will comsume 100% CPU when a device with such descriptors is plugged in. So even though this is now sanely handled by ALSA, it really shouldn't be done that way by implementors.

Daniel

author
#18 lore raw

PATCH 4/4 ALSA: usb-audio: fix UAC2 control value queries

In reply to Daniel Mack

UAC2 controls can be queried in 1-byte, 2-byte or 4-byte request blocks. The size is actually determined by the number of bytes requested, and as everything is little-endian, we can always go for the 4-byte variant.

Also, the MIN, MAX and RES value computations were using a wrong offset.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/mixer.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)
sound/usb/mixer.c +14 -5
index a060d00..6939d0f 100644
@@ -297,20 +297,27 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret)
 {
-	unsigned char buf[14]; /* enough space for one range of 4 bytes */
+	unsigned char buf[2 + 3*sizeof(__u16)]; /* enough space for one range */
 	unsigned char *val;
-	int ret;
+	int ret, size;
 	__u8 bRequest;
 
-	bRequest = (request == UAC_GET_CUR) ?
-		UAC2_CS_CUR : UAC2_CS_RANGE;
+	if (request == UAC_GET_CUR) {
+		bRequest = UAC2_CS_CUR;
+		size = sizeof(__u16);
+	} else {
+		bRequest = UAC2_CS_RANGE;
+		size = sizeof(buf);
+	}
+
+	memset(buf, 0, sizeof(buf));
 
 	ret = snd_usb_ctl_msg(cval->mixer->chip->dev,
 			      usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
 			      bRequest,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 			      validx, cval->mixer->ctrlif | (cval->id << 8),
-			      buf, sizeof(buf), 1000);
+			      buf, size, 1000);
 
 	if (ret < 0) {
 		snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",

@@ -318,6 +325,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
 		return ret;
 	}
 
+	/* FIXME: how should we handle multiple triplets here? */
+
 	switch (request) {
 	case UAC_GET_CUR:
 		val = buf;
Signature · 1 line
1.7.1
author
#19 lore raw

Re: [PATCH 4/4] ALSA: usb-audio: fix UAC2 control value queries

Quoted 8 lines · Daniel Mack

UAC2 controls can be queried in 1-byte, 2-byte or 4-byte request blocks. The size is actually determined by the number of bytes requested, and as everything is little-endian, we can always go for the 4-byte variant.

Also, the MIN, MAX and RES value computations were using a wrong offset.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>

Damn, I forgot to pick the right commit message for this patch. Below is a new version. Sorry.

Daniel

Quoted 1 line

From 9a8be646c65df2960bda3588cbd2cea4834c7750 Mon Sep 17 00:00:00 2001

From: Daniel Mack <[email protected]>
Date: Fri, 11 Jun 2010 13:49:56 +0200
Subject: [PATCH] ALSA: usb-audio: fix UAC2 control value queries

For RANGE requests, we should only query as much bytes as we're in face interested in.

For CUR requests, we shouldn't confuse the firmware with an overlong request but just ask for 2 bytes.

This might need fixing in the future as it's not entirely clear when to dispatch 1-byte, 2-byte and 4-byte request blocks. For now, we assume everything is coded in 16bit - this works for all firmware implementations I've seen.

Signed-off-by Daniel Mack <[email protected]>
Reported-by Alex Lee <[email protected]>
---
 sound/usb/mixer.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)
sound/usb/mixer.c +14 -5
index a060d00..6939d0f 100644
@@ -297,20 +297,27 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret)
 {
-	unsigned char buf[14]; /* enough space for one range of 4 bytes */
+	unsigned char buf[2 + 3*sizeof(__u16)]; /* enough space for one range */
 	unsigned char *val;
-	int ret;
+	int ret, size;
 	__u8 bRequest;
 
-	bRequest = (request == UAC_GET_CUR) ?
-		UAC2_CS_CUR : UAC2_CS_RANGE;
+	if (request == UAC_GET_CUR) {
+		bRequest = UAC2_CS_CUR;
+		size = sizeof(__u16);
+	} else {
+		bRequest = UAC2_CS_RANGE;
+		size = sizeof(buf);
+	}
+
+	memset(buf, 0, sizeof(buf));
 
 	ret = snd_usb_ctl_msg(cval->mixer->chip->dev,
 			      usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
 			      bRequest,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 			      validx, cval->mixer->ctrlif | (cval->id << 8),
-			      buf, sizeof(buf), 1000);
+			      buf, size, 1000);
 
 	if (ret < 0) {
 		snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",

@@ -318,6 +325,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
 		return ret;
 	}
 
+	/* FIXME: how should we handle multiple triplets here? */
+
 	switch (request) {
 	case UAC_GET_CUR:
 		val = buf;
Signature · 1 line
1.7.1