[PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[Daniel Schürmann]

Thank you Daniel for review.
Here is an updated version of the patch against
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/
581cbef46ae60073252208fc1f26dd1044f6e215

Set the timeout for USB control set messages according to the USB 2 spec
§9.2.6.4 to 5000 ms.
To avoid new issues, the get timeout is unchanged at 1000 ms, though it
is 500 ms in the spec.
This patch is required to run the Hercules RMX2 which needs a timeout >
1240 ms

Signed-off-by: Daniel Schürmann <daschuer@mixxx.org>
---
diff --git a/sound/usb/helper.c b/sound/usb/helper.c
index c1db28f..c1caae4 100644
--- a/sound/usb/helper.c
+++ b/sound/usb/helper.c
@@ -23,6 +23,11 @@
 #include "helper.h"
 #include "quirks.h"

+/* Value from 9.2.6.4 USB 2 spec */
+#define USB_MSG_SET_TIMEOUT 5000
+/* Value from spec is 500 but we pick 1000 for legacy reasons */
+#define USB_MSG_GET_TIMEOUT 1000
+
 /*
  * combine bytes and get an integer value
  */
@@ -86,14 +91,19 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned
int pipe, __u8 request,
 {
     int err;
     void *buf = NULL;
+    int timeout;

     if (size > 0) {
         buf = kmemdup(data, size, GFP_KERNEL);
         if (!buf)
             return -ENOMEM;
     }
+    if (requesttype & USB_DIR_IN)
+        timeout = USB_MSG_GET_TIMEOUT;
+    else
+        timeout = USB_MSG_SET_TIMEOUT;
     err = usb_control_msg(dev, pipe, request, requesttype,
-                  value, index, buf, size, 1000);
+                  value, index, buf, size, timeout);
     if (size > 0) {
         memcpy(data, buf, size);
         kfree(buf);

Re: [PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[Daniel Mack]

On 20.04.2013 20:58, Daniel Schürmann wrote:
> Thank you Daniel for review.
> Here is an updated version of the patch against
> https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/
> 581cbef46ae60073252208fc1f26dd1044f6e215
> 
> Set the timeout for USB control set messages according to the USB 2 spec
> §9.2.6.4 to 5000 ms.
> To avoid new issues, the get timeout is unchanged at 1000 ms, though it
> is 500 ms in the spec.
> This patch is required to run the Hercules RMX2 which needs a timeout >
> 1240 ms
> 
> Signed-off-by: Daniel Schürmann <daschuer@mixxx.org
> <mailto:daschuer@mixxx.org>>
> ---
> diff --git a/sound/usb/helper.c b/sound/usb/helper.c
> index c1db28f..c1caae4 100644
> --- a/sound/usb/helper.c
> +++ b/sound/usb/helper.c
> @@ -23,6 +23,11 @@
>  #include "helper.h"
>  #include "quirks.h"
>  
> +/* Value from 9.2.6.4 USB 2 spec */
> +#define USB_MSG_SET_TIMEOUT 5000
> +/* Value from spec is 500 but we pick 1000 for legacy reasons */
> +#define USB_MSG_GET_TIMEOUT 1000
> +
>  /*
>   * combine bytes and get an integer value
>   */
> @@ -86,14 +91,19 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned
> int pipe, __u8 request,
>  {
>      int err;
>      void *buf = NULL;
> +    int timeout;
>  
>      if (size > 0) {
>          buf = kmemdup(data, size, GFP_KERNEL);
>          if (!buf)
>              return -ENOMEM;
>      }

Blank line please.

> +    if (requesttype & USB_DIR_IN)
> +        timeout = USB_MSG_GET_TIMEOUT;
> +    else
> +        timeout = USB_MSG_SET_TIMEOUT;

Dito.


Also, the patch does not apply to Takashi's for-next branch, but I
believe that's just because your mailer mangled the patch. Could you
resend using 'git send-email'?


Thanks,
Daniel

Re: [PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[Torstein Hegge]

On Sat, Apr 20, 2013 at 20:58:56 +0200, Daniel Schürmann wrote:
> Thank you Daniel for review.
> Here is an updated version of the patch against
> https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/
> 581cbef46ae60073252208fc1f26dd1044f6e215
> 
> Set the timeout for USB control set messages according to the USB 2 spec
> §9.2.6.4 to 5000 ms.
> To avoid new issues, the get timeout is unchanged at 1000 ms, though it
> is 500 ms in the spec.
> This patch is required to run the Hercules RMX2 which needs a timeout >
> 1240 ms
> 
> Signed-off-by: Daniel Schürmann <daschuer@mixxx.org>
> ---
> diff --git a/sound/usb/helper.c b/sound/usb/helper.c
> index c1db28f..c1caae4 100644
> --- a/sound/usb/helper.c
> +++ b/sound/usb/helper.c
> @@ -23,6 +23,11 @@
>  #include "helper.h"
>  #include "quirks.h"
> 
> +/* Value from 9.2.6.4 USB 2 spec */
> +#define USB_MSG_SET_TIMEOUT 5000

There is an USB_CTRL_SET_TIMEOUT of 5000 ms defined in
include/linux/usb.h, which is used by other callers of
usb_control_msg(). And linux/usb.h is already included by helper.c.

> +/* Value from spec is 500 but we pick 1000 for legacy reasons */
> +#define USB_MSG_GET_TIMEOUT 1000
> +
>  /*
>   * combine bytes and get an integer value
>   */
> @@ -86,14 +91,19 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned
> int pipe, __u8 request,
>  {
>      int err;
>      void *buf = NULL;
> +    int timeout;
> 
>      if (size > 0) {
>          buf = kmemdup(data, size, GFP_KERNEL);
>          if (!buf)
>              return -ENOMEM;
>      }
> +    if (requesttype & USB_DIR_IN)
> +        timeout = USB_MSG_GET_TIMEOUT;
> +    else
> +        timeout = USB_MSG_SET_TIMEOUT;
>      err = usb_control_msg(dev, pipe, request, requesttype,
> -                  value, index, buf, size, 1000);
> +                  value, index, buf, size, timeout);
>      if (size > 0) {
>          memcpy(data, buf, size);
>          kfree(buf);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[Daniel Schürmann]

Hi Torstein, Hi Daniel

Thank you for your hint.

Just noticed that the get timeout is also set to 5000 ms even if the USB 2
spec only allows 500 ms.

Do you think it is also a good idea to adopt the 5000 ms get timout for
snd-usb-audio?

Kind regards,

Daniel

/*
 * timeouts, in milliseconds, used for sending/receiving control messages
 * they typically complete within a few frames (msec) after they're issued
 * USB identifies 5 second timeouts, maybe more in a few cases, and a few
 * slow devices (like some MGE Ellipse UPSes) actually push that limit.
 */
#define USB_CTRL_GET_TIMEOUT    5000
#define USB_CTRL_SET_TIMEOUT    5000






2013/4/20 Torstein Hegge <hegge@resisty.net>

> On Sat, Apr 20, 2013 at 20:58:56 +0200, Daniel Schürmann wrote:
> > Thank you Daniel for review.
> > Here is an updated version of the patch against
> > https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/
> > 581cbef46ae60073252208fc1f26dd1044f6e215
> >
> > Set the timeout for USB control set messages according to the USB 2 spec
> > §9.2.6.4 to 5000 ms.
> > To avoid new issues, the get timeout is unchanged at 1000 ms, though it
> > is 500 ms in the spec.
> > This patch is required to run the Hercules RMX2 which needs a timeout >
> > 1240 ms
> >
> > Signed-off-by: Daniel Schürmann <daschuer@mixxx.org>
> > ---
> > diff --git a/sound/usb/helper.c b/sound/usb/helper.c
> > index c1db28f..c1caae4 100644
> > --- a/sound/usb/helper.c
> > +++ b/sound/usb/helper.c
> > @@ -23,6 +23,11 @@
> >  #include "helper.h"
> >  #include "quirks.h"
> >
> > +/* Value from 9.2.6.4 USB 2 spec */
> > +#define USB_MSG_SET_TIMEOUT 5000
>
> There is an USB_CTRL_SET_TIMEOUT of 5000 ms defined in
> include/linux/usb.h, which is used by other callers of
> usb_control_msg(). And linux/usb.h is already included by helper.c.
>
> > +/* Value from spec is 500 but we pick 1000 for legacy reasons */
> > +#define USB_MSG_GET_TIMEOUT 1000
> > +
> >  /*
> >   * combine bytes and get an integer value
> >   */
> > @@ -86,14 +91,19 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned
> > int pipe, __u8 request,
> >  {
> >      int err;
> >      void *buf = NULL;
> > +    int timeout;
> >
> >      if (size > 0) {
> >          buf = kmemdup(data, size, GFP_KERNEL);
> >          if (!buf)
> >              return -ENOMEM;
> >      }
> > +    if (requesttype & USB_DIR_IN)
> > +        timeout = USB_MSG_GET_TIMEOUT;
> > +    else
> > +        timeout = USB_MSG_SET_TIMEOUT;
> >      err = usb_control_msg(dev, pipe, request, requesttype,
> > -                  value, index, buf, size, 1000);
> > +                  value, index, buf, size, timeout);
> >      if (size > 0) {
> >          memcpy(data, buf, size);
> >          kfree(buf);
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2] ALSA: snd-usb-audio: set the timeout for usb control set messages to 5000 ms

[Daniel Mack]

On 20.04.2013 22:17, Daniel Schürmann wrote:
> Hi Torstein, Hi Daniel 
> 
> Thank you for your hint.
> 
> Just noticed that the get timeout is also set to 5000 ms even if the USB
> 2 spec only allows 500 ms.
> 
> Do you think it is also a good idea to adopt the 5000 ms get timout for
> snd-usb-audio?

Well, in that case, we can indeed just set it to 5000 unconditionally,
which makes your patch a one-liner.


Thanks,
Daniel

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel