usb: Endpoints sharing the same interface

usb: Endpoints sharing the same interface

[Mark Hills]

Novation Twitch [1] is a USB device with audio capture and playback 
endpoints on the same bInterfaceNumber.

This has thrown up some tricky cases in the USB audio driver, and I'd 
appreciate some advice.

I'll follow up with my patches so far as an RFC.

This device has endpoints for 2 channels capture (0x01), 4 channels 
playback (0x82), both on interface #0. Interface #1 is raw MIDI.

I seem to be finding code that assumes pcm to always use the first 
endpoint on an interface. eg. problems with maxpacksize (see patch in 
thread)

Now I'm in a position where playback _or_ capture works reliably. But to 
start capture stops playback, and vice-versa (or sometimes causes hangs)

It seems like snd_pcm_prepare() concludes that need_setup_ep is required, 
but I think this may be based on something going on in 
set_sample_rate_v1() also looking to the first endpoint -- 
usb_sndctrlpipe(dev,0).

Before I do any more work on this, are there any hints where to go with 
this?

If we can overcome thse limitations then the same would likely apply to 
Saffire 6 USB and other Focusrite devices [2], and maybe some of the other 
devices with quirks that only support playback.

Many thanks

[1] http://uk.novationmusic.com/digital-dj/twitch/
[2] http://focusritedevelopmentteam.wordpress.com/category/drivers/

-- 
Mark


---
Bus 005 Device 010: ID 1235:0018 Novation EMS 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x1235 Novation EMS
  idProduct          0x0018 
  bcdDevice            1.00
  iManufacturer           1 Focusrite A.E. Ltd
  iProduct                2 Twitch
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           64
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              498mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           0
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       1
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x024c  1x 588 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0126  1x 294 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
Device Status:     0x0000
  (Bus Powered)

[PATCH RFC 1/4] snd-usb-audio: Playback and MIDI support for Novation Twitch DJ controller

[Mark Hills]

The hardware also has a PCM capture device which is not implemented
in this patch.

It may be possible to generalise this to Saffire 6 USB support
and some of the other Focusrite interfaces, but as I don't have
access to these devices we should wait until capture support is
working first.
---
 sound/usb/quirks-table.h | 40 ++++++++++++++++++++++++++++++++++++++++
 sound/usb/quirks.c       | 15 +++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 820580a..1c8db85 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2677,6 +2677,46 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 	}
 },
 {
+	USB_DEVICE(0x1235, 0x0018),
+	.driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) {
+		.vendor_name = "Novation",
+		.product_name = "Twitch",
+		.ifnum = QUIRK_ANY_INTERFACE,
+		.type = QUIRK_COMPOSITE,
+		.data = (const struct snd_usb_audio_quirk[]) {
+			{
+				.ifnum = 0,
+				.type = QUIRK_AUDIO_FIXED_ENDPOINT,
+				.data = & (const struct audioformat) {
+					.formats = SNDRV_PCM_FMTBIT_S24_3LE,
+					.channels = 4,
+					.iface = 0,
+					.altsetting = 1,
+					.altset_idx = 1,
+					.attributes = UAC_EP_CS_ATTR_SAMPLE_RATE,
+					.endpoint = 0x01,
+					.ep_attr = USB_ENDPOINT_XFER_ISOC,
+					.rates = SNDRV_PCM_RATE_44100 |
+						 SNDRV_PCM_RATE_48000,
+					.rate_min = 44100,
+					.rate_max = 48000,
+					.nr_rates = 2,
+					.rate_table = (unsigned int[]) {
+						44100, 48000
+					}
+				}
+			},
+			{
+				.ifnum = 1,
+				.type = QUIRK_MIDI_RAW_BYTES
+			},
+			{
+				.ifnum = -1
+			}
+		}
+	}
+},
+{
 	USB_DEVICE_VENDOR_SPEC(0x1235, 0x4661),
 	.driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) {
 		.vendor_name = "Novation",
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 0115289..b0f7140 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -446,6 +446,17 @@ static int snd_usb_cm6206_boot_quirk(struct usb_device *dev)
 }
 
 /*
+ * Novation Twitch DJ controller
+ */
+static int snd_usb_twitch_boot_quirk(struct usb_device *dev)
+{
+	/* preemptively set up the device because otherwise the
+	 * raw MIDI endpoints are not active */
+	usb_set_interface(dev, 0, 1);
+	return 0;
+}
+
+/*
  * This call will put the synth in "USB send" mode, i.e it will send MIDI
  * messages through USB (this is disabled at startup). The synth will
  * acknowledge by sending a sysex on endpoint 0x85 and by displaying a USB
@@ -746,6 +757,10 @@ int snd_usb_apply_boot_quirk(struct usb_device *dev,
 		/* Digidesign Mbox 2 */
 		return snd_usb_mbox2_boot_quirk(dev);
 
+	case USB_ID(0x1235, 0x0018):
+		/* Focusrite Novation Twitch */
+		return snd_usb_twitch_boot_quirk(dev);
+
 	case USB_ID(0x133e, 0x0815):
 		/* Access Music VirusTI Desktop */
 		return snd_usb_accessmusic_boot_quirk(dev);
-- 
1.7.12.1

[PATCH RFC 2/4] usb: Allow us to distinguish between different returns of EMSGSIZE

[Mark Hills]

---
 drivers/usb/core/urb.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index e0d9d94..7f00213 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -408,16 +408,24 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 			return -EINVAL;
 		for (n = 0; n < urb->number_of_packets; n++) {
 			len = urb->iso_frame_desc[n].length;
-			if (len < 0 || len > max)
+			if (len < 0 || len > max) {
+				dev_dbg(&dev->dev,
+					"bad packet length %d (max %d)",
+					len,  max);
 				return -EMSGSIZE;
+			}
 			urb->iso_frame_desc[n].status = -EXDEV;
 			urb->iso_frame_desc[n].actual_length = 0;
 		}
 	}
 
 	/* the I/O buffer must be mapped/unmapped, except when length=0 */
-	if (urb->transfer_buffer_length > INT_MAX)
+	if (urb->transfer_buffer_length > INT_MAX) {
+		dev_dbg(&dev->dev,
+			"transfer buffer length (%d) too large",
+			urb->transfer_buffer_length);
 		return -EMSGSIZE;
+	}
 
 #ifdef DEBUG
 	/* stuff that drivers shouldn't do, but which shouldn't
-- 
1.7.12.1

[PATCH RFC 3/4] snd-usb-audio: Trust fields given in the quirk

[Mark Hills]

The maxpacksize field is given in some quirks, but it gets ignored
(in favour of wMaxPacketSize from the first endpoint.) This patch
favours the one in the quirk.

Digidesign Mbox and Mbox 2 are the only affected quirks and the
devices are assumed to be working without this patch. So for safety
against the values in the quirk being incorrect, remove them.

The datainterval is also ignored but there are not currently any
quirks which choose to override this.

Cc: Damien Zammit <damien@zamaudio.com>
Cc: Chris J Arges <christopherarges@gmail.com>
---
 sound/usb/quirks-table.h | 3 ---
 sound/usb/quirks.c       | 6 ++++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 1c8db85..3614e9b 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2965,7 +2965,6 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 					.attributes = UAC_EP_CS_ATTR_SAMPLE_RATE,
 					.endpoint = 0x02,
 					.ep_attr = 0x01,
-					.maxpacksize = 0x130,
 					.rates = SNDRV_PCM_RATE_44100 |
 						 SNDRV_PCM_RATE_48000,
 					.rate_min = 44100,
@@ -3013,7 +3012,6 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 					.attributes = 0x00,
 					.endpoint = 0x03,
 					.ep_attr = USB_ENDPOINT_SYNC_ASYNC,
-					.maxpacksize = 0x128,
 					.rates = SNDRV_PCM_RATE_48000,
 					.rate_min = 48000,
 					.rate_max = 48000,
@@ -3039,7 +3037,6 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 					.attributes = UAC_EP_CS_ATTR_SAMPLE_RATE,
 					.endpoint = 0x85,
 					.ep_attr = USB_ENDPOINT_SYNC_SYNC,
-					.maxpacksize = 0x128,
 					.rates = SNDRV_PCM_RATE_48000,
 					.rate_min = 48000,
 					.rate_max = 48000,
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index b0f7140..0745eea 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -165,8 +165,10 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
 		return -EINVAL;
 	}
 	alts = &iface->altsetting[fp->altset_idx];
-	fp->datainterval = snd_usb_parse_datainterval(chip, alts);
-	fp->maxpacksize = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize);
+	if (fp->datainterval == 0)
+		fp->datainterval = snd_usb_parse_datainterval(chip, alts);
+	if (fp->maxpacksize == 0)
+		fp->maxpacksize = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize);
 	usb_set_interface(chip->dev, fp->iface, 0);
 	snd_usb_init_pitch(chip, fp->iface, alts, fp);
 	snd_usb_init_sample_rate(chip, fp->iface, alts, fp, fp->rate_max);
-- 
1.7.12.1

[PATCH RFC 4/4] snd-usb-audio: Prototype capture support for Novation Twitch

[Mark Hills]

*** This patch is incomplete, as the device re-initialises
*** if playback and capture is attempted at the same time.

The capture endpoint shares the same interface as the playback
endpoint, and it seems that this is the first quirk to do this.

We must override the maxpacksize (which is not done in the
playback interface) because create_fixed_stream_quirk() takes the
wMaxPacketSize from the first endpoint by default.
---
 sound/usb/quirks-table.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 3614e9b..c62f2a3 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2707,6 +2707,29 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 				}
 			},
 			{
+				.ifnum = 0,
+				.type = QUIRK_AUDIO_FIXED_ENDPOINT,
+				.data = & (const struct audioformat) {
+					.formats = SNDRV_PCM_FMTBIT_S24_3LE,
+					.channels = 2,
+					.iface = 0,
+					.altsetting = 1,
+ 					.altset_idx = 1,
+					.attributes = UAC_EP_CS_ATTR_SAMPLE_RATE,
+					.endpoint = 0x82,
+					.ep_attr = USB_ENDPOINT_XFER_ISOC,
+					.maxpacksize = 0x126,
+					.rates = SNDRV_PCM_RATE_44100 |
+						 SNDRV_PCM_RATE_48000,
+					.rate_min = 44100,
+					.rate_max = 48000,
+					.nr_rates = 2,
+					.rate_table = (unsigned int[]) {
+						44100, 48000
+					}
+				}
+			},
+			{
 				.ifnum = 1,
 				.type = QUIRK_MIDI_RAW_BYTES
 			},
-- 
1.7.12.1