Hi Gabriel,

Thank you for your quick response. The patch against the alsa-kernel is also attached at https://bugs.launchpad.net/mixxx/+bug/1096687 and here:

diff --git a/sound/usb/helper.c b/sound/usb/helper.c index c1db28f..e044804 100644 --- a/sound/usb/helper.c +++ b/sound/usb/helper.c @@ -23,6 +23,9 @@ #include "helper.h" #include "quirks.h"

+/* Hercules RMX2 needs 1240 ms for setting the sample rate the first time */ +#define USB_MSG_TIMEOUT 1500 + /* * combine bytes and get an integer value */ @@ -93,7 +96,7 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe, __u8 request, return -ENOMEM; } err = usb_control_msg(dev, pipe, request, requesttype, - value, index, buf, size, 1000); + value, index, buf, size, USB_MSG_TIMEOUT); if (size > 0) { memcpy(data, buf, size); kfree(buf);

Kind regards,

Daniel

2013/4/16 Gabriel M. Beddingfield gabrbedd@gmail.com

Hi Daniel,

On 04/15/2013 03:04 PM, Daniel Schürmann wrote:

...

Hallo Alsa Developers,

I have fixed Support for Hercules RMX2.

You can find the Bug and the patch here: https://bugs.launchpad.net/**mixxx/+bug/1096687https://bugs.launchpad.net/mixxx/+bug/1096687

[snip]

Is it possible to include the fix in trunk?

...

Since the RMX2's /driver/ isn't in the Linux kernel -- the answer is probably no. Notice that the DEB package has "DKMS" in the name -- indicating that this is an out-of-tree driver.

So, you would need to start by submitting a patch to add the RMX driver to the kernel.

-gabriel

On 16.04.2013 17:35, Gabriel M. Beddingfield wrote:

On 04/16/2013 08:12 AM, Daniel Schürmann wrote:

...

Hi Gabriel,

Thank you for your quick response. The patch against the alsa-kernel is also attached at https://bugs.launchpad.net/mixxx/+bug/1096687 and here:

OK, I see now. FYI, most maintainers prefer that you submit patches according to the guidelines in Documentation/SubmittingPatches. (I'm not a maintainer, BTW...)

...

diff --git a/sound/usb/helper.c b/sound/usb/helper.c index c1db28f..e044804 100644 --- a/sound/usb/helper.c +++ b/sound/usb/helper.c @@ -23,6 +23,9 @@ #include "helper.h" #include "quirks.h"

+/* Hercules RMX2 needs 1240 ms for setting the sample rate the first time */ +#define USB_MSG_TIMEOUT 1500

• /*
  • combine bytes and get an integer value
  */

@@ -93,7 +96,7 @@ int snd_usb_ctl_msg(struct usb_device *dev, unsigned int pipe, __u8 request, return -ENOMEM; } err = usb_control_msg(dev, pipe, request, requesttype,

• 	      value, index, buf, size, 1000);
  

• 	      value, index, buf, size, USB_MSG_TIMEOUT);
  

  if (size > 0) { memcpy(data, buf, size); kfree(buf);

This changes the value for every USB audio device... not just the RMX2. Daniel, is there a better way to do this?

Yes, you can store an integer in struct snd_usb_audio (call it usb_msg_timeout or s.th.), initialize it to 1000 from snd_usb_audio_probe() and add a quirk to override that value in snd_usb_apply_boot_quirk().

Thanks, Daniel

Hi Daniel,

On 16.04.2013 21:14, Daniel Schürmann wrote:

Thank you for the suggestion. If there is no other solution than increasing the timeout, IMHO the code is simpler if we just increase the timeout for all devices. This way we will automatically fix all other devices based on the same hardware.

How many are there? Can you come up with a list of VID/PID? I never heard of problems of that kind really.

OTOH, I can't think of any obvious reason how a longer timeout should cause any problem for other devices, but as a general concern, we like to keep quirks as specific to the device as possible.

The long timeout is only quired by the first write command and 1 s is already quite long. Do you have another idea how to fix this?

Fix the firmware :) I can't think of anything that take a full second inside such a device.

Do you have a Idea what the remaining error messages are about? I cannot see a misbehavior anyway.

Also firmware problems. You can ignore them for now.

Just read: https://www.kernel.org/doc/Documentation/SubmittingPatches Is the patch fine to send out?

Which one? Also, before sending it, run scripts/checkpatch.pl on it, which will point out obvious style issues.

Against which Kernel should I patch?

https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/log/?h=for-ne...

Where should I send it?

To the alsa-devel ML, and take Takashi and Clemens in Cc: (both copied in this mail).

Thanks, Daniel

2013/4/16 Daniel Mack <zonque@gmail.com mailto:zonque@gmail.com>

On 16.04.2013 17:35, Gabriel M. Beddingfield wrote:
> On 04/16/2013 08:12 AM, Daniel Schürmann wrote:
>> Hi Gabriel,
>>
>> Thank you for your quick response.
>> The patch against the alsa-kernel is also attached at
>> https://bugs.launchpad.net/mixxx/+bug/1096687 and here:
>
> OK, I see now.  FYI, most maintainers prefer that you submit patches
> according to the guidelines in Documentation/SubmittingPatches.  (I'm
> not a maintainer, BTW...)
>
>>
>> diff --git a/sound/usb/helper.c b/sound/usb/helper.c
>> index c1db28f..e044804 100644
>> --- a/sound/usb/helper.c
>> +++ b/sound/usb/helper.c
>> @@ -23,6 +23,9 @@
>>   #include "helper.h"
>>   #include "quirks.h"
>>
>> +/* Hercules RMX2 needs 1240 ms for setting the sample rate the
first time */
>> +#define USB_MSG_TIMEOUT 1500
>> +
>>   /*
>>    * combine bytes and get an integer value
>>    */
>> @@ -93,7 +96,7 @@ int snd_usb_ctl_msg(struct usb_device *dev,
unsigned int pipe, __u8 request,
>>                      return -ENOMEM;
>>      }
>>      err = usb_control_msg(dev, pipe, request, requesttype,
>> -                          value, index, buf, size, 1000);
>> +                          value, index, buf, size, USB_MSG_TIMEOUT);
>>      if (size > 0) {
>>              memcpy(data, buf, size);
>>              kfree(buf);
>
> This changes the value for every USB audio device... not just the
RMX2.
>   Daniel, is there a better way to do this?

Yes, you can store an integer in struct snd_usb_audio (call it
usb_msg_timeout or s.th.), initialize it to 1000 from
snd_usb_audio_probe() and add a quirk to override that value in
snd_usb_apply_boot_quirk().


Thanks,
Daniel

This is a 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..b4f3876 100644 --- a/sound/usb/helper.c +++ b/sound/usb/helper.c @@ -21,7 +21,11 @@

#include "usbaudio.h" #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 +90,24 @@ 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) { + /* Get Request */ + timeout = USB_MSG_GET_TIMEOUT; + } else { + /* Set Request */ + 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);