Patch for Digidesign Mbox 2 (set sample rate)

Patch for Digidesign Mbox 2 (set sample rate)

[Viktor Radnai]

Hi there,

I have recently bought a Digidesign Mbox 2 Mini and found the lack of
support for 44100Hz sample rate in the driver limiting (the sample rate was
hard-set to 48000Hz by the original author even though the device supports
it, because he couldn't get the frame rate switching to work reliably).

I had a go at hacking the driver and came up with the following patch which
appears to work on the MBox 2 Mini:
https://pastebin.com/kzzTDj9T

I have built and tested this with Linux 4.8.15 without any realtime
patches. I am planning to pick up a "full size" Mbox 2 as well, in the
meantime if anyone on the list owns one I would appreciate if you could
test this and let me know the result. There is a possibility that it will
not work without the "magic" init sequence that I have removed because it
does not look like it's needed on the Mini. Although I would be a bit
surprised because the two devices are apparently identical to the point
that the Mini allows the use of it's physically nonexistent features.

Once someone could confirm that this works, I would like to get it merged.
This is my first attempt at submitting a patch for any Linux device driver
so please let me know if I need to do anything else :)

Cheers,
Vik
-- 
My other sig is hilarious

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Takashi Iwai]

On Fri, 28 Apr 2017 22:36:09 +0200,
Viktor Radnai wrote:
> 
> Hi there,
> 
> I have recently bought a Digidesign Mbox 2 Mini and found the lack of
> support for 44100Hz sample rate in the driver limiting (the sample rate was
> hard-set to 48000Hz by the original author even though the device supports
> it, because he couldn't get the frame rate switching to work reliably).
> 
> I had a go at hacking the driver and came up with the following patch which
> appears to work on the MBox 2 Mini:
> https://pastebin.com/kzzTDj9T
> 
> I have built and tested this with Linux 4.8.15 without any realtime
> patches. I am planning to pick up a "full size" Mbox 2 as well, in the
> meantime if anyone on the list owns one I would appreciate if you could
> test this and let me know the result. There is a possibility that it will
> not work without the "magic" init sequence that I have removed because it
> does not look like it's needed on the Mini. Although I would be a bit
> surprised because the two devices are apparently identical to the point
> that the Mini allows the use of it's physically nonexistent features.
> 
> Once someone could confirm that this works, I would like to get it merged.
> This is my first attempt at submitting a patch for any Linux device driver
> so please let me know if I need to do anything else :)

At least it'd be better to the original author in the loop.
Damien, could you take a look if you still have the device?


thanks,

Takashi

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Viktor Radnai]

Hi Takashi,

Thanks, I have also reached out to Damien and he just got back to me.
Unfortunately he no longer has the device and is quite busy with other
things.

I am going to buy an Mbox 2 and test this myself. You can expect results
within two weeks, hopefully sooner.

In the meantime if anyone on the list has this device I would really
appreciate if you could test it.

Cheers,
Vik

On 3 May 2017 at 11:16, Takashi Iwai <tiwai@suse.de> wrote:

> On Fri, 28 Apr 2017 22:36:09 +0200,
> Viktor Radnai wrote:
> >
> > Hi there,
> >
> > I have recently bought a Digidesign Mbox 2 Mini and found the lack of
> > support for 44100Hz sample rate in the driver limiting (the sample rate
> was
> > hard-set to 48000Hz by the original author even though the device
> supports
> > it, because he couldn't get the frame rate switching to work reliably).
> >
> > I had a go at hacking the driver and came up with the following patch
> which
> > appears to work on the MBox 2 Mini:
> > https://pastebin.com/kzzTDj9T
> >
> > I have built and tested this with Linux 4.8.15 without any realtime
> > patches. I am planning to pick up a "full size" Mbox 2 as well, in the
> > meantime if anyone on the list owns one I would appreciate if you could
> > test this and let me know the result. There is a possibility that it will
> > not work without the "magic" init sequence that I have removed because it
> > does not look like it's needed on the Mini. Although I would be a bit
> > surprised because the two devices are apparently identical to the point
> > that the Mini allows the use of it's physically nonexistent features.
> >
> > Once someone could confirm that this works, I would like to get it
> merged.
> > This is my first attempt at submitting a patch for any Linux device
> driver
> > so please let me know if I need to do anything else :)
>
> At least it'd be better to the original author in the loop.
> Damien, could you take a look if you still have the device?
>
>
> thanks,
>
> Takashi
>



-- 
My other sig is hilarious

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Viktor Radnai]

Hi again,

I have bought an Mbox 2 and tested playback at both 44.1 and 48kHz using
Jackd, and 44.1kHz using PulseAudio. It appeared to work without problems
(it played without any dropouts and at the correct speed / pitch). I
haven't tested recording or done more rigorous testing on the Mbox yet as
it only arrived today (I did test recording with the Mini though).

Have you got a proper list of things to test before merging the patch or
should I try to make it up myself?

Cheers,
Vik

On 6 May 2017 at 09:03, Viktor Radnai <viktor.radnai@gmail.com> wrote:

> Hi Takashi,
>
> Thanks, I have also reached out to Damien and he just got back to me.
> Unfortunately he no longer has the device and is quite busy with other
> things.
>
> I am going to buy an Mbox 2 and test this myself. You can expect results
> within two weeks, hopefully sooner.
>
> In the meantime if anyone on the list has this device I would really
> appreciate if you could test it.
>
> Cheers,
> Vik
>
> On 3 May 2017 at 11:16, Takashi Iwai <tiwai@suse.de> wrote:
>
>> On Fri, 28 Apr 2017 22:36:09 +0200,
>> Viktor Radnai wrote:
>> >
>> > Hi there,
>> >
>> > I have recently bought a Digidesign Mbox 2 Mini and found the lack of
>> > support for 44100Hz sample rate in the driver limiting (the sample rate
>> was
>> > hard-set to 48000Hz by the original author even though the device
>> supports
>> > it, because he couldn't get the frame rate switching to work reliably).
>> >
>> > I had a go at hacking the driver and came up with the following patch
>> which
>> > appears to work on the MBox 2 Mini:
>> > https://pastebin.com/kzzTDj9T
>> >
>> > I have built and tested this with Linux 4.8.15 without any realtime
>> > patches. I am planning to pick up a "full size" Mbox 2 as well, in the
>> > meantime if anyone on the list owns one I would appreciate if you could
>> > test this and let me know the result. There is a possibility that it
>> will
>> > not work without the "magic" init sequence that I have removed because
>> it
>> > does not look like it's needed on the Mini. Although I would be a bit
>> > surprised because the two devices are apparently identical to the point
>> > that the Mini allows the use of it's physically nonexistent features.
>> >
>> > Once someone could confirm that this works, I would like to get it
>> merged.
>> > This is my first attempt at submitting a patch for any Linux device
>> driver
>> > so please let me know if I need to do anything else :)
>>
>> At least it'd be better to the original author in the loop.
>> Damien, could you take a look if you still have the device?
>>
>>
>> thanks,
>>
>> Takashi
>>
>
>
>
> --
> My other sig is hilarious
>



-- 
My other sig is hilarious

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Damien Zammit]

Hi all,

I have tested Viktor's patch on "Mbox 2" and it works very well.
Looks like my 'magic' boot sequence was not needed.
Rate switching works between 44.1 and 48k.
Good job.

Damien

On 03/05/17 20:16, Takashi Iwai wrote:
> On Fri, 28 Apr 2017 22:36:09 +0200,
> Viktor Radnai wrote:
>>
>> Hi there,
>>
>> I have recently bought a Digidesign Mbox 2 Mini and found the lack of
>> support for 44100Hz sample rate in the driver limiting (the sample rate was
>> hard-set to 48000Hz by the original author even though the device supports
>> it, because he couldn't get the frame rate switching to work reliably).
>>
>> I had a go at hacking the driver and came up with the following patch which
>> appears to work on the MBox 2 Mini:
>> https://pastebin.com/kzzTDj9T
>>
>> I have built and tested this with Linux 4.8.15 without any realtime
>> patches. I am planning to pick up a "full size" Mbox 2 as well, in the
>> meantime if anyone on the list owns one I would appreciate if you could
>> test this and let me know the result. There is a possibility that it will
>> not work without the "magic" init sequence that I have removed because it
>> does not look like it's needed on the Mini. Although I would be a bit
>> surprised because the two devices are apparently identical to the point
>> that the Mini allows the use of it's physically nonexistent features.
>>
>> Once someone could confirm that this works, I would like to get it merged.
>> This is my first attempt at submitting a patch for any Linux device driver
>> so please let me know if I need to do anything else :)
> 
> At least it'd be better to the original author in the loop.
> Damien, could you take a look if you still have the device?
> 
> 
> thanks,
> 
> Takashi
> 

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Viktor Radnai]

Hi Damien,

Thanks for the test report.

I have now successfully recorded with both the Mbox 2 and Mbox 2 Mini,
however I had one occurrence where the Mbox 2 would not initialize even
when I repeatedly unplugged and plugged it back. I have added your "magic"
boot sequence back and since then I haven't seen this problem. I have been
using that version ever since, so I cannot tell if simply unloading and
reloading the driver would have fixed it. In any case I will re-submit the
patch with the magic boot sequence for inclusion.

Cheers,
Vik

On 9 July 2017 at 03:14, Damien Zammit <damien@zamaudio.com> wrote:

> Hi all,
>
> I have tested Viktor's patch on "Mbox 2" and it works very well.
> Looks like my 'magic' boot sequence was not needed.
> Rate switching works between 44.1 and 48k.
> Good job.
>
> Damien
>
> On 03/05/17 20:16, Takashi Iwai wrote:
> > On Fri, 28 Apr 2017 22:36:09 +0200,
> > Viktor Radnai wrote:
> >>
> >> Hi there,
> >>
> >> I have recently bought a Digidesign Mbox 2 Mini and found the lack of
> >> support for 44100Hz sample rate in the driver limiting (the sample rate
> was
> >> hard-set to 48000Hz by the original author even though the device
> supports
> >> it, because he couldn't get the frame rate switching to work reliably).
> >>
> >> I had a go at hacking the driver and came up with the following patch
> which
> >> appears to work on the MBox 2 Mini:
> >> https://pastebin.com/kzzTDj9T
> >>
> >> I have built and tested this with Linux 4.8.15 without any realtime
> >> patches. I am planning to pick up a "full size" Mbox 2 as well, in the
> >> meantime if anyone on the list owns one I would appreciate if you could
> >> test this and let me know the result. There is a possibility that it
> will
> >> not work without the "magic" init sequence that I have removed because
> it
> >> does not look like it's needed on the Mini. Although I would be a bit
> >> surprised because the two devices are apparently identical to the point
> >> that the Mini allows the use of it's physically nonexistent features.
> >>
> >> Once someone could confirm that this works, I would like to get it
> merged.
> >> This is my first attempt at submitting a patch for any Linux device
> driver
> >> so please let me know if I need to do anything else :)
> >
> > At least it'd be better to the original author in the loop.
> > Damien, could you take a look if you still have the device?
> >
> >
> > thanks,
> >
> > Takashi
> >
>



-- 
My other sig is hilarious

Re: Patch for Digidesign Mbox 2 (set sample rate)

[Viktor Radnai]

Hi again,

I think this patch is ready to be applied. I have kept the "magic" init
sequence but I have substituted the hexadecimal numbers with preprocessor
macros to make the code easier to read.

I will try to run without the init magic for a while and if I don't see any
issues I will submit a follow-up patch to remove the entire
mbox2_setup_48_24_magic function.

Cheers,
Vik

diff -ur linux-source-4.11-orig/sound/usb/quirks.c
linux-source-4.11/sound/usb/quirks.c
--- linux-source-4.11-orig/sound/usb/quirks.c    2017-06-17
05:47:27.000000000 +0100
+++ linux-source-4.11/sound/usb/quirks.c    2017-07-18 22:37:58.522858262
+0100
@@ -768,23 +768,32 @@

 static void mbox2_setup_48_24_magic(struct usb_device *dev)
 {
-    u8 srate[3];
-    u8 temp[12];
-
     /* Choose 48000Hz permanently */
-    srate[0] = 0x80;
-    srate[1] = 0xbb;
-    srate[2] = 0x00;
+    u8 srate[3] = { 0x80, 0xbb, 0x00 };
+    u8 temp[3];
+    int err;

     /* Send the magic! */
-    snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
-        0x01, 0x22, 0x0100, 0x0085, &temp, 0x0003);
-    snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0),
-        0x81, 0xa2, 0x0100, 0x0085, &srate, 0x0003);
-    snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0),
-        0x81, 0xa2, 0x0100, 0x0086, &srate, 0x0003);
-    snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0),
-        0x81, 0xa2, 0x0100, 0x0003, &srate, 0x0003);
+    if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC_SET_CUR,
+        USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT,
+        UAC_EP_CS_ATTR_SAMPLE_RATE << 8, 0x0085, &temp, sizeof(temp))) < 0)
+        dev_err(&dev->dev, "mbox2: magic setup step #1 failed\n");
+
+    if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_GET_CUR,
+        USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_IN,
+        UAC_EP_CS_ATTR_SAMPLE_RATE << 8, 0x0085, &srate, sizeof(srate))) <
0)
+        dev_err(&dev->dev, "mbox2: magic setup step #2 failed\n");
+
+    if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_GET_CUR,
+        USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_IN,
+        UAC_EP_CS_ATTR_SAMPLE_RATE << 8, 0x0086, &srate, sizeof(srate))) <
0)
+        dev_err(&dev->dev, "mbox2: magic setup step #3 failed\n");
+
+    if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_GET_CUR,
+        USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_IN,
+        UAC_EP_CS_ATTR_SAMPLE_RATE << 8, 0x0003, &srate, sizeof(srate))) <
0)
+        dev_err(&dev->dev, "mbox2: magic setup step #4 failed\n");
+
     return;
 }

@@ -815,16 +824,16 @@

     count = 0;
     bootresponse[0] = MBOX2_BOOT_LOADING;
-    while ((bootresponse[0] == MBOX2_BOOT_LOADING) && (count < 10)) {
-        msleep(500); /* 0.5 second delay */
+    do {
         snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
             /* Control magic - load onboard firmware */
-            0x85, 0xc0, 0x0001, 0x0000, &bootresponse, 0x0012);
+            UAC_GET_MEM, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
0x0001, 0x0000, &bootresponse, sizeof(bootresponse));
         if (bootresponse[0] == MBOX2_BOOT_READY)
             break;
+        msleep(500); /* 0.5 second delay */
         dev_dbg(&dev->dev, "device not ready, resending boot
sequence...\n");
         count++;
-    }
+    } while ((bootresponse[0] == MBOX2_BOOT_LOADING) && (count < 10));

     if (bootresponse[0] != MBOX2_BOOT_READY) {
         dev_err(&dev->dev, "Unknown bootresponse=%d, or timed out,
ignoring device.\n", bootresponse[0]);
@@ -847,7 +856,7 @@

     mbox2_setup_48_24_magic(dev);

-    dev_info(&dev->dev, "Digidesign Mbox 2: 24bit 48kHz");
+    dev_info(&dev->dev, "Digidesign Mbox 2 configured");

     return 0; /* Successful boot */
 }
@@ -1111,6 +1120,26 @@
     subs->pkt_offset_adj = (emu_samplerate_id >= EMU_QUIRK_SR_176400HZ) ?
4 : 0;
 }

+static void set_format_mbox2_quirk(struct snd_usb_substream *subs,
+                 struct audioformat *fmt)
+{
+    struct usb_device *dev = subs->dev;
+    u8 srate[3];
+    int err;
+
+    srate[0] = subs->cur_rate;
+    srate[1] = subs->cur_rate >> 8;
+    srate[2] = subs->cur_rate >> 16;
+
+    /* Set the sample rate for endpoint IN 5 (0x0085) until we succeed */
+    while ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0),
UAC_SET_CUR,
+        USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT,
+        UAC_EP_CS_ATTR_SAMPLE_RATE << 8, 0x0085, srate, sizeof(srate))) <
0)
+        dev_err(&dev->dev, "mbox2: set sample rate failed, retrying\n");
+
+    dev_info(&subs->dev->dev, "mbox2: set sample rate to %dHz\n",
subs->cur_rate);
+}
+
 void snd_usb_set_format_quirk(struct snd_usb_substream *subs,
                   struct audioformat *fmt)
 {
@@ -1121,6 +1150,9 @@
     case USB_ID(0x041e, 0x3f19): /* E-Mu 0204 USB */
         set_format_emu_quirk(subs, fmt);
         break;
+    case USB_ID(0x0dba, 0x3000): /* Digidesign Mbox 2 */
+        set_format_mbox2_quirk(subs, fmt);
+        break;
     }
 }

diff -ur linux-source-4.11-orig/sound/usb/quirks-table.h
linux-source-4.11/sound/usb/quirks-table.h
--- linux-source-4.11-orig/sound/usb/quirks-table.h    2017-06-17
05:47:27.000000000 +0100
+++ linux-source-4.11/sound/usb/quirks-table.h    2017-07-18
17:33:57.620424012 +0100
@@ -3020,12 +3020,13 @@
                     .attributes = 0x00,
                     .endpoint = 0x03,
                     .ep_attr = USB_ENDPOINT_SYNC_ASYNC,
-                    .rates = SNDRV_PCM_RATE_48000,
-                    .rate_min = 48000,
+                    .rates = SNDRV_PCM_RATE_44100|
+                        SNDRV_PCM_RATE_48000,
+                    .rate_min = 44100,
                     .rate_max = 48000,
-                    .nr_rates = 1,
+                    .nr_rates = 2,
                     .rate_table = (unsigned int[]) {
-                        48000
+                        44100, 48000
                     }
                 }
             },
@@ -3045,12 +3046,13 @@
                     .attributes = UAC_EP_CS_ATTR_SAMPLE_RATE,
                     .endpoint = 0x85,
                     .ep_attr = USB_ENDPOINT_SYNC_SYNC,
-                    .rates = SNDRV_PCM_RATE_48000,
-                    .rate_min = 48000,
+                    .rates = SNDRV_PCM_RATE_44100|
+                        SNDRV_PCM_RATE_48000,
+                    .rate_min = 44100,
                     .rate_max = 48000,
-                    .nr_rates = 1,
+                    .nr_rates = 2,
                     .rate_table = (unsigned int[]) {
-                        48000
+                        44100, 48000
                     }
                 }
             },