All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] teac ud-501/ud-503 usb delay
@ 2016-01-24 16:36 Guillaume Fougnies
  2016-01-25 10:59 ` Takashi Iwai
  2016-03-01  0:38 ` Rob O'Donnell
  0 siblings, 2 replies; 6+ messages in thread
From: Guillaume Fougnies @ 2016-01-24 16:36 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai


Hello,

This is a small patch against sound/usb/quirks.c to correct "Teac
Corp." products delay.
Tested on Teac UD-501 and UD-503 products, could not harm other
Teac products.
It corrects the "clock source 41 is not valid, cannot use" error
for both devices.
And more critically on the new UD-503, a complete freeze of the
teac usb interface when switching between different rate/format.
(Freeze only solved by a power switch of the device...)

The patch matches the problem of the "Playback Design" products. I just
refactored a bit the code to include Teac products properly.

Regards,
Guillaume


---
 sound/usb/quirks.c | 58 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 23ea6d8..f3faaef 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -1202,40 +1202,48 @@ void snd_usb_endpoint_start_quirk(struct snd_usb_endpoint *ep)
 void snd_usb_set_interface_quirk(struct usb_device *dev)
 {
 	/*
-	 * "Playback Design" products need a 50ms delay after setting the
+	 * Products needing a 50ms delay after setting the
 	 * USB interface.
 	 */
-	if (le16_to_cpu(dev->descriptor.idVendor) == 0x23ba)
-		mdelay(50);
+	switch (le16_to_cpu(dev->descriptor.idVendor)) {
+		case 0x23ba: /* Playback Design */
+		case 0x0644: /* TEAC Corp. */
+			mdelay(50);
+	}
 }
 
 void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
 			   __u8 request, __u8 requesttype, __u16 value,
 			   __u16 index, void *data, __u16 size)
 {
-	/*
-	 * "Playback Design" products need a 20ms delay after each
-	 * class compliant request
-	 */
-	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x23ba) &&
-	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
-		mdelay(20);
-
-	/* Marantz/Denon devices with USB DAC functionality need a delay
-	 * after each class compliant request
-	 */
-	if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
-					le16_to_cpu(dev->descriptor.idProduct)))
-	    && (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
-		mdelay(20);
+	if ((requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS) {
+		switch (le16_to_cpu(dev->descriptor.idVendor)) {
+			/*
+			 * "Playback Design"/"TEAC Corp." products need a 20ms delay after each
+			 * class compliant request
+			 */
+			case 0x23ba: /* "Playback Design" */
+			case 0x0644: /* TEAC Corp. */
+				mdelay(20);
+				return;
+
+				/* Zoom R16/24 needs a tiny delay here, otherwise requests like
+				 * get/set frequency return as failed despite actually succeeding.
+				 */
+			case 0x1686:
+				/* restrict to 0x00dd */
+				if (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd)
+					mdelay(1);
+				return;
+		}
 
-	/* Zoom R16/24 needs a tiny delay here, otherwise requests like
-	 * get/set frequency return as failed despite actually succeeding.
-	 */
-	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x1686) &&
-	    (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd) &&
-	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
-		mdelay(1);
+		/* Marantz/Denon devices with USB DAC functionality need a delay
+		 * after each class compliant request
+		 */
+		if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
+						le16_to_cpu(dev->descriptor.idProduct))))
+			mdelay(20);
+	}
 }
 
 /*
-- 
2.6.4

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

* Re: [PATCH] teac ud-501/ud-503 usb delay
  2016-01-24 16:36 [PATCH] teac ud-501/ud-503 usb delay Guillaume Fougnies
@ 2016-01-25 10:59 ` Takashi Iwai
  2016-01-25 23:28   ` Guillaume Fougnies
  2016-03-01  0:38 ` Rob O'Donnell
  1 sibling, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2016-01-25 10:59 UTC (permalink / raw)
  To: Guillaume Fougnies; +Cc: alsa-devel

On Sun, 24 Jan 2016 17:36:17 +0100,
Guillaume Fougnies wrote:
> 
> 
> Hello,
> 
> This is a small patch against sound/usb/quirks.c to correct "Teac
> Corp." products delay.
> Tested on Teac UD-501 and UD-503 products, could not harm other
> Teac products.
> It corrects the "clock source 41 is not valid, cannot use" error
> for both devices.
> And more critically on the new UD-503, a complete freeze of the
> teac usb interface when switching between different rate/format.
> (Freeze only solved by a power switch of the device...)
> 
> The patch matches the problem of the "Playback Design" products. I just
> refactored a bit the code to include Teac products properly.

The changes look almost good to me.  Could you fix small nitpick below
and resubmit with a proper changelog, and most importantly, with your
sign-off, in order to merge to upstream?

> 
> Regards,
> Guillaume
> 
> 
> ---
>  sound/usb/quirks.c | 58 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 25 deletions(-)
> 
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index 23ea6d8..f3faaef 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -1202,40 +1202,48 @@ void snd_usb_endpoint_start_quirk(struct snd_usb_endpoint *ep)
>  void snd_usb_set_interface_quirk(struct usb_device *dev)
>  {
>  	/*
> -	 * "Playback Design" products need a 50ms delay after setting the
> +	 * Products needing a 50ms delay after setting the
>  	 * USB interface.
>  	 */
> -	if (le16_to_cpu(dev->descriptor.idVendor) == 0x23ba)
> -		mdelay(50);
> +	switch (le16_to_cpu(dev->descriptor.idVendor)) {
> +		case 0x23ba: /* Playback Design */
> +		case 0x0644: /* TEAC Corp. */

In the kernel code, case is has the same indent level as switch.

> +			mdelay(50);
> +	}

Put break after mdelay() call as the common practice.


>  }
>  
>  void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
>  			   __u8 request, __u8 requesttype, __u16 value,
>  			   __u16 index, void *data, __u16 size)
>  {
> -	/*
> -	 * "Playback Design" products need a 20ms delay after each
> -	 * class compliant request
> -	 */
> -	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x23ba) &&
> -	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> -		mdelay(20);
> -
> -	/* Marantz/Denon devices with USB DAC functionality need a delay
> -	 * after each class compliant request
> -	 */
> -	if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
> -					le16_to_cpu(dev->descriptor.idProduct)))
> -	    && (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> -		mdelay(20);
> +	if ((requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS) {
> +		switch (le16_to_cpu(dev->descriptor.idVendor)) {
> +			/*
> +			 * "Playback Design"/"TEAC Corp." products need a 20ms delay after each
> +			 * class compliant request
> +			 */
> +			case 0x23ba: /* "Playback Design" */
> +			case 0x0644: /* TEAC Corp. */
> +				mdelay(20);
> +				return;
> +
> +				/* Zoom R16/24 needs a tiny delay here, otherwise requests like
> +				 * get/set frequency return as failed despite actually succeeding.
> +				 */
> +			case 0x1686:
> +				/* restrict to 0x00dd */
> +				if (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd)
> +					mdelay(1);
> +				return;
> +		}
>  
> -	/* Zoom R16/24 needs a tiny delay here, otherwise requests like
> -	 * get/set frequency return as failed despite actually succeeding.
> -	 */
> -	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x1686) &&
> -	    (le16_to_cpu(dev->descriptor.idProduct) == 0x00dd) &&
> -	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> -		mdelay(1);
> +		/* Marantz/Denon devices with USB DAC functionality need a delay
> +		 * after each class compliant request
> +		 */
> +		if (is_marantz_denon_dac(USB_ID(le16_to_cpu(dev->descriptor.idVendor),
> +						le16_to_cpu(dev->descriptor.idProduct))))
> +			mdelay(20);
> +	}

In this case, I prefer keeping the old code as is.
Just add the check for your device there in a new if block.


thanks,

Takashi

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

* Re: [PATCH] teac ud-501/ud-503 usb delay
  2016-01-25 10:59 ` Takashi Iwai
@ 2016-01-25 23:28   ` Guillaume Fougnies
  2016-01-26  6:03     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Fougnies @ 2016-01-25 23:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

ALSA: usb: Fix TEAC UD-501/UD-503/NT-503 usb delay

TEAC UD-501/UD-503/NT-503 fail to switch properly between different rate/format.
Similar to 'Playback Design', this patch corrects the invalid clock source error
for TEAC products and avoids complete freeze of the usb interface of 503 series.

Signed-off-by: Guillaume Fougnies <guillaume@eulerian.com>
---
 sound/usb/quirks.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 23ea6d8..a75d9ce 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -1205,8 +1205,12 @@ void snd_usb_set_interface_quirk(struct usb_device *dev)
 	 * "Playback Design" products need a 50ms delay after setting the
 	 * USB interface.
 	 */
-	if (le16_to_cpu(dev->descriptor.idVendor) == 0x23ba)
+	switch (le16_to_cpu(dev->descriptor.idVendor)) {
+	case 0x23ba: /* Playback Design */
+	case 0x0644: /* TEAC Corp. */
 		mdelay(50);
+		break;
+	}
 }
 
 void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
@@ -1221,6 +1225,14 @@ void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
 	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
 		mdelay(20);
 
+	/*
+	 * "TEAC Corp." products need a 20ms delay after each
+	 * class compliant request
+	 */
+	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x0644) &&
+	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
+		mdelay(20);
+
 	/* Marantz/Denon devices with USB DAC functionality need a delay
 	 * after each class compliant request
 	 */
-- 
2.7.0.rc3

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

* Re: [PATCH] teac ud-501/ud-503 usb delay
  2016-01-25 23:28   ` Guillaume Fougnies
@ 2016-01-26  6:03     ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2016-01-26  6:03 UTC (permalink / raw)
  To: Guillaume Fougnies; +Cc: alsa-devel

On Tue, 26 Jan 2016 00:28:27 +0100,
Guillaume Fougnies wrote:
> 
> ALSA: usb: Fix TEAC UD-501/UD-503/NT-503 usb delay
> 
> TEAC UD-501/UD-503/NT-503 fail to switch properly between different rate/format.
> Similar to 'Playback Design', this patch corrects the invalid clock source error
> for TEAC products and avoids complete freeze of the usb interface of 503 series.
> 
> Signed-off-by: Guillaume Fougnies <guillaume@eulerian.com>

Applied, thanks.


Takashi

> ---
>  sound/usb/quirks.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index 23ea6d8..a75d9ce 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -1205,8 +1205,12 @@ void snd_usb_set_interface_quirk(struct usb_device *dev)
>  	 * "Playback Design" products need a 50ms delay after setting the
>  	 * USB interface.
>  	 */
> -	if (le16_to_cpu(dev->descriptor.idVendor) == 0x23ba)
> +	switch (le16_to_cpu(dev->descriptor.idVendor)) {
> +	case 0x23ba: /* Playback Design */
> +	case 0x0644: /* TEAC Corp. */
>  		mdelay(50);
> +		break;
> +	}
>  }
>  
>  void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
> @@ -1221,6 +1225,14 @@ void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe,
>  	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
>  		mdelay(20);
>  
> +	/*
> +	 * "TEAC Corp." products need a 20ms delay after each
> +	 * class compliant request
> +	 */
> +	if ((le16_to_cpu(dev->descriptor.idVendor) == 0x0644) &&
> +	    (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS)
> +		mdelay(20);
> +
>  	/* Marantz/Denon devices with USB DAC functionality need a delay
>  	 * after each class compliant request
>  	 */
> -- 
> 2.7.0.rc3
> 

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

* Re: [PATCH] teac ud-501/ud-503 usb delay
  2016-01-24 16:36 [PATCH] teac ud-501/ud-503 usb delay Guillaume Fougnies
  2016-01-25 10:59 ` Takashi Iwai
@ 2016-03-01  0:38 ` Rob O'Donnell
  2016-03-01  7:37   ` Clemens Ladisch
  1 sibling, 1 reply; 6+ messages in thread
From: Rob O'Donnell @ 2016-03-01  0:38 UTC (permalink / raw)
  To: alsa-devel

Hi,  

I have a friend with a new UD 503 running on Fedora 23 (vortexbox) and is 
getting USB dropout issues.  Can your fix be run in Fedora?  Any tips to 
implement?  

Thanks.

Rob.

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

* Re: [PATCH] teac ud-501/ud-503 usb delay
  2016-03-01  0:38 ` Rob O'Donnell
@ 2016-03-01  7:37   ` Clemens Ladisch
  0 siblings, 0 replies; 6+ messages in thread
From: Clemens Ladisch @ 2016-03-01  7:37 UTC (permalink / raw)
  To: Rob O'Donnell, alsa-devel

Rob O'Donnell wrote:
> Can your fix be run in Fedora?  Any tips to implement?

https://fedoraproject.org/wiki/Building_a_custom_kernel


Regards,
Clemens

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

end of thread, other threads:[~2016-03-01  7:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-24 16:36 [PATCH] teac ud-501/ud-503 usb delay Guillaume Fougnies
2016-01-25 10:59 ` Takashi Iwai
2016-01-25 23:28   ` Guillaume Fougnies
2016-01-26  6:03     ` Takashi Iwai
2016-03-01  0:38 ` Rob O'Donnell
2016-03-01  7:37   ` Clemens Ladisch

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.