All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
@ 2013-03-26 21:10 Torstein Hegge
  2013-03-27 10:09 ` Clemens Ladisch
  2013-04-03  9:47 ` Takashi Iwai
  0 siblings, 2 replies; 9+ messages in thread
From: Torstein Hegge @ 2013-03-26 21:10 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, clemens, zonque

The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
while the interface is active. The same behavior is observed in other UAC2
hardware like the VIA VT1731.

Reset the interface after setting the sampling frequency on sample rate
changes, to ensure that the sample rate set by snd_usb_init_sample_rate() is
used. Otherwise, the device will try to use the sample rate of the previous
stream, causing distorted sound on sample rate changes.

The reset is performed for all UAC2 devices, as it should not affect a
standards compliant device, but it is only necessary for C-Media CM6631,
VIA VT1731 and possibly others.

Failure to read sample rate from the device is not handled as an error in
set_sample_rate_v2(), as (permanent or intermittent) failure to read sample
rate isn't essential for a successful sample rate set.

Signed-off-by: Torstein Hegge <hegge@resisty.net>
---
Changes since v4:
- Sample rate read failure is no longer fatal.

Failure to read sample rate from the device is degraded from a fatal error to
a warning message. The Schiit Bifrost has shown intermittent sample rate read
failures, and this shouldn't be a reason to abort the initialization.

- The reset is performed for all UAC2 devices.

The number of devices affected by this issue seems to be larger than just the
C-Media based devices, issues with the VIA VT1731 has been reported by Davor
Herga to alsa-user [1].

The additional reset should not affect standards compliant devices and will
hopefully improve the compatibility with future devices tested with the OS X
UAC2 driver.

This can cause problems for a theoretical device where sample rate read always
fails, causing the reset to be performed every time the sample rate is set,
*and* where repeated resets cause resume after pause to not resume properly,
like the issues seen with CM6631 in the first iteration of this patch.

[1] http://thread.gmane.org/gmane.linux.alsa.user/37312 


Changes since v3:
- Use given target rate instead of the new rate read from device when checking
  if a reset is needed. Some devices (at least Schiit Bifrost) doesn't report a
  reliable sample rate back right after setting a new rate. This appears to
  fix the issues with v3 of this patch with the Schiit, as reported by Chris
  Hermansen in http://thread.gmane.org/gmane.linux.alsa.user/36935/focus=37284


 sound/usb/clock.c |   45 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 5e634a2..9e2703a 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -253,7 +253,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char data[4];
-	int err, crate;
+	int err, cur_rate, prev_rate;
 	int clock = snd_usb_clock_find_source(chip, fmt->clock);
 
 	if (clock < 0)
@@ -266,6 +266,19 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 		return -ENXIO;
 	}
 
+	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,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
+			      data, sizeof(data));
+	if (err < 0) {
+		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
+			   dev->devnum, iface, fmt->altsetting);
+		prev_rate = 0;
+	} else {
+		prev_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
+	}
+
 	data[0] = rate;
 	data[1] = rate >> 8;
 	data[2] = rate >> 16;
@@ -280,19 +293,31 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 		return err;
 	}
 
-	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,
-				   snd_usb_ctrl_intf(chip) | (clock << 8),
-				   data, sizeof(data))) < 0) {
+	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,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
+			      data, sizeof(data));
+	if (err < 0) {
 		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
 			   dev->devnum, iface, fmt->altsetting);
-		return err;
+		cur_rate = 0;
+	} else {
+		cur_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
 	}
 
-	crate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
-	if (crate != rate)
-		snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", crate, rate);
+	if (cur_rate != rate) {
+		snd_printd(KERN_WARNING
+			   "current rate %d is different from the runtime rate %d\n",
+			   cur_rate, rate);
+	}
+
+	/* Some devices doesn't respond to sample rate changes while the
+	 * interface is active. */
+	if (rate != prev_rate) {
+		usb_set_interface(dev, iface, 0);
+		usb_set_interface(dev, iface, fmt->altsetting);
+	}
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-03-26 21:10 [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug Torstein Hegge
@ 2013-03-27 10:09 ` Clemens Ladisch
  2013-03-27 11:47   ` Torstein Hegge
  2013-04-03  9:47 ` Takashi Iwai
  1 sibling, 1 reply; 9+ messages in thread
From: Clemens Ladisch @ 2013-03-27 10:09 UTC (permalink / raw)
  To: Torstein Hegge; +Cc: Takashi Iwai, alsa-devel, Daniel Mack

Torstein Hegge wrote:
> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
> while the interface is active. The same behavior is observed in other UAC2
> hardware like the VIA VT1731.

Might it be possible that these devices have reset the interface (see
UAC2 section 3.16.3), and that the driver hasn't noticed this because it
doesn't handle Active Alternate Setting Control interrupts?


Regards,
Clemens

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-03-27 10:09 ` Clemens Ladisch
@ 2013-03-27 11:47   ` Torstein Hegge
  2013-03-27 11:52     ` Daniel Mack
  0 siblings, 1 reply; 9+ messages in thread
From: Torstein Hegge @ 2013-03-27 11:47 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: Takashi Iwai, alsa-devel, Daniel Mack

On Wed, Mar 27, 2013 at 11:09:01 +0100, Clemens Ladisch wrote:
> Torstein Hegge wrote:
> > The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
> > while the interface is active. The same behavior is observed in other UAC2
> > hardware like the VIA VT1731.
> 
> Might it be possible that these devices have reset the interface (see
> UAC2 section 3.16.3), and that the driver hasn't noticed this because it
> doesn't handle Active Alternate Setting Control interrupts?

If that was the case, wouldn't the device be in altsetting zero after
the sample rate set and not be able to output sound at all? And as long
as the current altsetting supports the target sample rate as reported in
/proc/asound/card1/stream0, Active Alternate Setting Control shouldn't
have to be invoked.


Torstein

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-03-27 11:47   ` Torstein Hegge
@ 2013-03-27 11:52     ` Daniel Mack
  2013-03-27 12:46       ` Torstein Hegge
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Mack @ 2013-03-27 11:52 UTC (permalink / raw)
  To: Torstein Hegge; +Cc: Takashi Iwai, alsa-devel, Clemens Ladisch

On 27.03.2013 12:47, Torstein Hegge wrote:
> On Wed, Mar 27, 2013 at 11:09:01 +0100, Clemens Ladisch wrote:
>> Torstein Hegge wrote:
>>> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
>>> while the interface is active. The same behavior is observed in other UAC2
>>> hardware like the VIA VT1731.
>>
>> Might it be possible that these devices have reset the interface (see
>> UAC2 section 3.16.3), and that the driver hasn't noticed this because it
>> doesn't handle Active Alternate Setting Control interrupts?
> 
> If that was the case, wouldn't the device be in altsetting zero after
> the sample rate set and not be able to output sound at all? And as long
> as the current altsetting supports the target sample rate as reported in
> /proc/asound/card1/stream0, Active Alternate Setting Control shouldn't
> have to be invoked.

You could just try and catch that currently unhandled interrupt. We
should support that anyway at some point ...


Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-03-27 11:52     ` Daniel Mack
@ 2013-03-27 12:46       ` Torstein Hegge
  0 siblings, 0 replies; 9+ messages in thread
From: Torstein Hegge @ 2013-03-27 12:46 UTC (permalink / raw)
  To: Daniel Mack; +Cc: Takashi Iwai, alsa-devel, Clemens Ladisch

On Wed, Mar 27, 2013 at 12:52:14 +0100, Daniel Mack wrote:
> On 27.03.2013 12:47, Torstein Hegge wrote:
> > On Wed, Mar 27, 2013 at 11:09:01 +0100, Clemens Ladisch wrote:
> >> Torstein Hegge wrote:
> >>> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
> >>> while the interface is active. The same behavior is observed in other UAC2
> >>> hardware like the VIA VT1731.
> >>
> >> Might it be possible that these devices have reset the interface (see
> >> UAC2 section 3.16.3), and that the driver hasn't noticed this because it
> >> doesn't handle Active Alternate Setting Control interrupts?
> > 
> > If that was the case, wouldn't the device be in altsetting zero after
> > the sample rate set and not be able to output sound at all? And as long
> > as the current altsetting supports the target sample rate as reported in
> > /proc/asound/card1/stream0, Active Alternate Setting Control shouldn't
> > have to be invoked.
> 
> You could just try and catch that currently unhandled interrupt. We
> should support that anyway at some point ...

It doesn't look like snd_usb_mixer_interrupt_v2() is invoked on sample
rate changes. As far as I can tell, that should be invoked on any
interrupts, but it was never triggered, so I might be wrong.


Torstein

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-03-26 21:10 [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug Torstein Hegge
  2013-03-27 10:09 ` Clemens Ladisch
@ 2013-04-03  9:47 ` Takashi Iwai
  2013-04-03 10:15   ` Clemens Ladisch
  1 sibling, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2013-04-03  9:47 UTC (permalink / raw)
  To: Torstein Hegge; +Cc: alsa-devel, clemens, zonque

At Tue, 26 Mar 2013 22:10:05 +0100,
Torstein Hegge wrote:
> 
> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
> while the interface is active. The same behavior is observed in other UAC2
> hardware like the VIA VT1731.
> 
> Reset the interface after setting the sampling frequency on sample rate
> changes, to ensure that the sample rate set by snd_usb_init_sample_rate() is
> used. Otherwise, the device will try to use the sample rate of the previous
> stream, causing distorted sound on sample rate changes.
> 
> The reset is performed for all UAC2 devices, as it should not affect a
> standards compliant device, but it is only necessary for C-Media CM6631,
> VIA VT1731 and possibly others.
> 
> Failure to read sample rate from the device is not handled as an error in
> set_sample_rate_v2(), as (permanent or intermittent) failure to read sample
> rate isn't essential for a successful sample rate set.
> 
> Signed-off-by: Torstein Hegge <hegge@resisty.net>
> ---
> Changes since v4:
> - Sample rate read failure is no longer fatal.
> 
> Failure to read sample rate from the device is degraded from a fatal error to
> a warning message. The Schiit Bifrost has shown intermittent sample rate read
> failures, and this shouldn't be a reason to abort the initialization.
> 
> - The reset is performed for all UAC2 devices.
> 
> The number of devices affected by this issue seems to be larger than just the
> C-Media based devices, issues with the VIA VT1731 has been reported by Davor
> Herga to alsa-user [1].
> 
> The additional reset should not affect standards compliant devices and will
> hopefully improve the compatibility with future devices tested with the OS X
> UAC2 driver.
> 
> This can cause problems for a theoretical device where sample rate read always
> fails, causing the reset to be performed every time the sample rate is set,
> *and* where repeated resets cause resume after pause to not resume properly,
> like the issues seen with CM6631 in the first iteration of this patch.
> 
> [1] http://thread.gmane.org/gmane.linux.alsa.user/37312 
> 
> 
> Changes since v3:
> - Use given target rate instead of the new rate read from device when checking
>   if a reset is needed. Some devices (at least Schiit Bifrost) doesn't report a
>   reliable sample rate back right after setting a new rate. This appears to
>   fix the issues with v3 of this patch with the Schiit, as reported by Chris
>   Hermansen in http://thread.gmane.org/gmane.linux.alsa.user/36935/focus=37284

What about the latest status of the patch?

If both Clemens and Daniel are happy with it, I can apply it for the
next 3.9-rc.


Though, it would be nicer if two identical calls of snd_usb_ctl_msg
can be put into a single function, such as,

static int get_cur_sample_rate_v2(struct snd_usb_audio *chip, int iface,
				  int altsetting, int clock)
{
	struct usb_device *dev = chip->dev;
	unsigned char data[4];
	int err;

	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,
			      snd_usb_ctrl_intf(chip) | (clock << 8),
			      data, sizeof(data));
	if (err < 0) {
		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
			   dev->devnum, iface, altsetting);
		return 0;
	}

	return data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
}


thanks,

Takashi

> 
> 
>  sound/usb/clock.c |   45 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index 5e634a2..9e2703a 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -253,7 +253,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
>  {
>  	struct usb_device *dev = chip->dev;
>  	unsigned char data[4];
> -	int err, crate;
> +	int err, cur_rate, prev_rate;
>  	int clock = snd_usb_clock_find_source(chip, fmt->clock);
>  
>  	if (clock < 0)
> @@ -266,6 +266,19 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
>  		return -ENXIO;
>  	}
>  
> +	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,
> +			      snd_usb_ctrl_intf(chip) | (clock << 8),
> +			      data, sizeof(data));
> +	if (err < 0) {
> +		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
> +			   dev->devnum, iface, fmt->altsetting);
> +		prev_rate = 0;
> +	} else {
> +		prev_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
> +	}
> +
>  	data[0] = rate;
>  	data[1] = rate >> 8;
>  	data[2] = rate >> 16;
> @@ -280,19 +293,31 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
>  		return err;
>  	}
>  
> -	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,
> -				   snd_usb_ctrl_intf(chip) | (clock << 8),
> -				   data, sizeof(data))) < 0) {
> +	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,
> +			      snd_usb_ctrl_intf(chip) | (clock << 8),
> +			      data, sizeof(data));
> +	if (err < 0) {
>  		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
>  			   dev->devnum, iface, fmt->altsetting);
> -		return err;
> +		cur_rate = 0;
> +	} else {
> +		cur_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
>  	}
>  
> -	crate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
> -	if (crate != rate)
> -		snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", crate, rate);
> +	if (cur_rate != rate) {
> +		snd_printd(KERN_WARNING
> +			   "current rate %d is different from the runtime rate %d\n",
> +			   cur_rate, rate);
> +	}
> +
> +	/* Some devices doesn't respond to sample rate changes while the
> +	 * interface is active. */
> +	if (rate != prev_rate) {
> +		usb_set_interface(dev, iface, 0);
> +		usb_set_interface(dev, iface, fmt->altsetting);
> +	}
>  
>  	return 0;
>  }
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-04-03  9:47 ` Takashi Iwai
@ 2013-04-03 10:15   ` Clemens Ladisch
  2013-04-03 17:22     ` Takashi Iwai
  0 siblings, 1 reply; 9+ messages in thread
From: Clemens Ladisch @ 2013-04-03 10:15 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Torstein Hegge, alsa-devel, zonque

Takashi Iwai wrote:
> Torstein Hegge wrote:
>> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
>> while the interface is active. The same behavior is observed in other UAC2
>> hardware like the VIA VT1731.
>>
>> Reset the interface after setting the sampling frequency on sample rate
>> changes, to ensure that the sample rate set by snd_usb_init_sample_rate() is
>> used. Otherwise, the device will try to use the sample rate of the previous
>> stream, causing distorted sound on sample rate changes.
>>
>> The reset is performed for all UAC2 devices, as it should not affect a
>> standards compliant device, but it is only necessary for C-Media CM6631,
>> VIA VT1731 and possibly others.
>>
>> Failure to read sample rate from the device is not handled as an error in
>> set_sample_rate_v2(), as (permanent or intermittent) failure to read sample
>> rate isn't essential for a successful sample rate set.
>>
>> Signed-off-by: Torstein Hegge <hegge@resisty.net>
>
> What about the latest status of the patch?
>
> If both Clemens and Daniel are happy with it, I can apply it for the
> next 3.9-rc.

Acked-by: Clemens Ladisch <clemens@ladisch.de>
with or without this change:

> Though, it would be nicer if two identical calls of snd_usb_ctl_msg
> can be put into a single function,


Regards,
Clemens

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-04-03 10:15   ` Clemens Ladisch
@ 2013-04-03 17:22     ` Takashi Iwai
  2013-04-03 21:22       ` Eldad Zack
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2013-04-03 17:22 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: Torstein Hegge, alsa-devel, Eldad Zack, zonque

At Wed, 03 Apr 2013 12:15:49 +0200,
Clemens Ladisch wrote:
> 
> Takashi Iwai wrote:
> > Torstein Hegge wrote:
> >> The C-Media CM6631 USB receiver doesn't respond to changes in sample rate
> >> while the interface is active. The same behavior is observed in other UAC2
> >> hardware like the VIA VT1731.
> >>
> >> Reset the interface after setting the sampling frequency on sample rate
> >> changes, to ensure that the sample rate set by snd_usb_init_sample_rate() is
> >> used. Otherwise, the device will try to use the sample rate of the previous
> >> stream, causing distorted sound on sample rate changes.
> >>
> >> The reset is performed for all UAC2 devices, as it should not affect a
> >> standards compliant device, but it is only necessary for C-Media CM6631,
> >> VIA VT1731 and possibly others.
> >>
> >> Failure to read sample rate from the device is not handled as an error in
> >> set_sample_rate_v2(), as (permanent or intermittent) failure to read sample
> >> rate isn't essential for a successful sample rate set.
> >>
> >> Signed-off-by: Torstein Hegge <hegge@resisty.net>
> >
> > What about the latest status of the patch?
> >
> > If both Clemens and Daniel are happy with it, I can apply it for the
> > next 3.9-rc.
> 
> Acked-by: Clemens Ladisch <clemens@ladisch.de>
> with or without this change:

OK, I applied the patch now without modification for for-linus
branch.  Then back-merged to for-next branch with the additional
clean-up patch below.

Eldad, please rebase your patches to for-next branch of sound git
tree.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: usb-audio: Clean up the code in set_sample_rate_v2()

Just for cleaning up, introduce a new function get_sample_rate_v2()
for replacing two identical calls in set_sample_rate_v2().

No functional change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/clock.c | 47 +++++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 9e2703a..e3ccf3d 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -247,6 +247,27 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface,
 	return 0;
 }
 
+static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface,
+			      int altsetting, int clock)
+{
+	struct usb_device *dev = chip->dev;
+	unsigned char data[4];
+	int err;
+
+	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,
+			      snd_usb_ctrl_intf(chip) | (clock << 8),
+			      data, sizeof(data));
+	if (err < 0) {
+		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
+			   dev->devnum, iface, altsetting);
+		return 0;
+	}
+
+	return data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
+}
+
 static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 			      struct usb_host_interface *alts,
 			      struct audioformat *fmt, int rate)
@@ -266,18 +287,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 		return -ENXIO;
 	}
 
-	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,
-			      snd_usb_ctrl_intf(chip) | (clock << 8),
-			      data, sizeof(data));
-	if (err < 0) {
-		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
-			   dev->devnum, iface, fmt->altsetting);
-		prev_rate = 0;
-	} else {
-		prev_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
-	}
+	prev_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
 
 	data[0] = rate;
 	data[1] = rate >> 8;
@@ -293,18 +303,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 		return err;
 	}
 
-	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,
-			      snd_usb_ctrl_intf(chip) | (clock << 8),
-			      data, sizeof(data));
-	if (err < 0) {
-		snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
-			   dev->devnum, iface, fmt->altsetting);
-		cur_rate = 0;
-	} else {
-		cur_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
-	}
+	cur_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
 
 	if (cur_rate != rate) {
 		snd_printd(KERN_WARNING
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug
  2013-04-03 17:22     ` Takashi Iwai
@ 2013-04-03 21:22       ` Eldad Zack
  0 siblings, 0 replies; 9+ messages in thread
From: Eldad Zack @ 2013-04-03 21:22 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Torstein Hegge, alsa-devel, Clemens Ladisch, zonque


On Wed, 3 Apr 2013, Takashi Iwai wrote:

> At Wed, 03 Apr 2013 12:15:49 +0200,
> Clemens Ladisch wrote:
> > 
> > Takashi Iwai wrote:
> > > Torstein Hegge wrote:
> > >>
> > > What about the latest status of the patch?
> > >
> > > If both Clemens and Daniel are happy with it, I can apply it for the
> > > next 3.9-rc.
> > 
> > Acked-by: Clemens Ladisch <clemens@ladisch.de>
> > with or without this change:
> 
> OK, I applied the patch now without modification for for-linus
> branch.  Then back-merged to for-next branch with the additional
> clean-up patch below.
> 
> Eldad, please rebase your patches to for-next branch of sound git
> tree.

Thanks for the heads up, Takashi!
I've sent it out with all the changes moments ago.

Cheers,
Eldad

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-04-03 21:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-26 21:10 [PATCH v5] ALSA: usb: Work around CM6631 sample rate change bug Torstein Hegge
2013-03-27 10:09 ` Clemens Ladisch
2013-03-27 11:47   ` Torstein Hegge
2013-03-27 11:52     ` Daniel Mack
2013-03-27 12:46       ` Torstein Hegge
2013-04-03  9:47 ` Takashi Iwai
2013-04-03 10:15   ` Clemens Ladisch
2013-04-03 17:22     ` Takashi Iwai
2013-04-03 21:22       ` Eldad Zack

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.