A number of devices have named substreams which are hard to remember /
decypher from <device> MIDI n names. Eg. Korg puts a pass through on
one substream and iConnectivity devices name the connections.
This makes it easier to connect to the correct device. Devices which
handle naming through quirks are unaffected by this change.
Addresses TODO comment in sound/usb/midi.c
Signed-off-by: George Harker <[email protected]>
---
sound/usb/midi.c | 122 ++++++++++++++++++++++++++++++++++++++---------
sound/usb/midi.h | 2 +
2 files changed, 101 insertions(+), 23 deletions(-)
diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 0c23fa6d8..c6651a566 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -47,6 +47,7 @@
#include <linux/usb.h>
#include <linux/wait.h>
#include <linux/usb/audio.h>
+#include <linux/usb/midi.h>
#include <linux/module.h>
#include <sound/core.h>
@@ -77,23 +78,6 @@ MODULE_AUTHOR("Clemens Ladisch <[email protected]>");
MODULE_DESCRIPTION("USB Audio/MIDI helper module");
MODULE_LICENSE("Dual BSD/GPL");
-
-struct usb_ms_header_descriptor {
- __u8 bLength;
- __u8 bDescriptorType;
- __u8 bDescriptorSubtype;
- __u8 bcdMSC[2];
- __le16 wTotalLength;
-} __attribute__ ((packed));
-
-struct usb_ms_endpoint_descriptor {
- __u8 bLength;
- __u8 bDescriptorType;
- __u8 bDescriptorSubtype;
- __u8 bNumEmbMIDIJack;
- __u8 baAssocJackID[];
-} __attribute__ ((packed));
-
struct snd_usb_midi_in_endpoint;
struct snd_usb_midi_out_endpoint;
struct snd_usb_midi_endpoint;
@@ -1756,12 +1740,68 @@ static void snd_usbmidi_get_port_info(struct snd_rawmidi *rmidi, int number,
}
}
+static struct usb_midi_in_jack_descriptor *find_usb_in_jack_descriptor(
+ struct usb_host_interface *hostif, uint8_t jack_id)
+{
+ unsigned char *extra = hostif->extra;
+ int extralen = hostif->extralen;
+
+ while (extralen > 4) {
+ struct usb_midi_in_jack_descriptor *injd =
+ (struct usb_midi_in_jack_descriptor *)extra;
+
+ if (injd->bLength > 4 &&
+ injd->bDescriptorType == USB_DT_CS_INTERFACE &&
+ injd->bDescriptorSubtype == UAC_MIDI_IN_JACK &&
+ injd->bJackID == jack_id)
+ return injd;
+ if (!extra[0])
+ break;
+ extralen -= extra[0];
+ extra += extra[0];
+ }
+ return NULL;
+}
+
+static struct usb_midi_out_jack_descriptor *find_usb_out_jack_descriptor(
+ struct usb_host_interface *hostif, uint8_t jack_id)
+{
+ unsigned char *extra = hostif->extra;
+ int extralen = hostif->extralen;
+
+ while (extralen > 4) {
+ struct usb_midi_out_jack_descriptor *outjd =
+ (struct usb_midi_out_jack_descriptor *)extra;
+
+ if (outjd->bLength > 4 &&
+ outjd->bDescriptorType == USB_DT_CS_INTERFACE &&
+ outjd->bDescriptorSubtype == UAC_MIDI_OUT_JACK &&
+ outjd->bJackID == jack_id)
+ return outjd;
+ if (!extra[0])
+ break;
+ extralen -= extra[0];
+ extra += extra[0];
+ }
+ return NULL;
+}
+
static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
- int stream, int number,
+ int stream, int number, int jack_id,
struct snd_rawmidi_substream **rsubstream)
{
struct port_info *port_info;
const char *name_format;
+ struct usb_interface *intf;
+ struct usb_host_interface *hostif;
+ struct usb_midi_in_jack_descriptor *injd;
+ struct usb_midi_out_jack_descriptor *outjd;
+ uint8_t jack_name_buf[32];
+ uint8_t *default_jack_name = "MIDI";
+ uint8_t *jack_name = default_jack_name;
+ uint8_t iJack;
+ size_t sz;
+ int res;
struct snd_rawmidi_substream *substream =
snd_usbmidi_find_substream(umidi, stream, number);
@@ -1771,11 +1811,35 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
return;
}
- /* TODO: read port name from jack descriptor */
+ intf = umidi->iface;
+ if (intf && jack_id >= 0) {
+ hostif = intf->cur_altsetting;
+ iJack = 0;
+ if (stream != SNDRV_RAWMIDI_STREAM_OUTPUT) {
+ /* in jacks connect to outs */
+ outjd = find_usb_out_jack_descriptor(hostif, jack_id);
+ if (outjd) {
+ sz = USB_DT_MIDI_OUT_SIZE(outjd->bNrInputPins);
+ iJack = *(((uint8_t *) outjd) + sz - 1);
+ }
+ } else {
+ /* and out jacks connect to ins */
+ injd = find_usb_in_jack_descriptor(hostif, jack_id);
+ if (injd)
+ iJack = injd->iJack;
+ }
+ if (iJack != 0) {
+ res = usb_string(umidi->dev, iJack, jack_name_buf, 32);
+ if (res)
+ jack_name = jack_name_buf;
+ }
+ }
+
port_info = find_port_info(umidi, number);
- name_format = port_info ? port_info->name : "%s MIDI %d";
+ name_format = port_info ? port_info->name :
+ (jack_name != default_jack_name ? "%s %s" : "%s %s %d");
snprintf(substream->name, sizeof(substream->name),
- name_format, umidi->card->shortname, number + 1);
+ name_format, umidi->card->shortname, jack_name, number + 1);
*rsubstream = substream;
}
@@ -1810,6 +1874,7 @@ static int snd_usbmidi_create_endpoints(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_OUTPUT,
out_ports,
+ endpoints[i].assoc_out_jacks[j],
&umidi->endpoints[i].out->ports[j].substream);
++out_ports;
}
@@ -1817,6 +1882,7 @@ static int snd_usbmidi_create_endpoints(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_INPUT,
in_ports,
+ endpoints[i].assoc_in_jacks[j],
&umidi->endpoints[i].in->ports[j].substream);
++in_ports;
}
@@ -1862,7 +1928,7 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
struct usb_host_endpoint *hostep;
struct usb_endpoint_descriptor *ep;
struct usb_ms_endpoint_descriptor *ms_ep;
- int i, epidx;
+ int i, j, epidx;
intf = umidi->iface;
if (!intf)
@@ -1875,7 +1941,7 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
ms_header->bDescriptorType == USB_DT_CS_INTERFACE &&
ms_header->bDescriptorSubtype == UAC_HEADER)
dev_dbg(&umidi->dev->dev, "MIDIStreaming version %02x.%02x\n",
- ms_header->bcdMSC[1], ms_header->bcdMSC[0]);
+ ((uint8_t *)&ms_header->bcdMSC)[1], ((uint8_t *)&ms_header->bcdMSC)[0]);
else
dev_warn(&umidi->dev->dev,
"MIDIStreaming interface descriptor not found\n");
@@ -1911,6 +1977,10 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
endpoints[epidx].out_interval = 1;
endpoints[epidx].out_cables =
(1 << ms_ep->bNumEmbMIDIJack) - 1;
+ for (j = 0; j < ms_ep->bNumEmbMIDIJack; ++j)
+ endpoints[epidx].assoc_out_jacks[j] = ms_ep->baAssocJackID[j];
+ for (; j < 0x10; ++j)
+ endpoints[epidx].assoc_out_jacks[j] = -1;
dev_dbg(&umidi->dev->dev, "EP %02X: %d jack(s)\n",
ep->bEndpointAddress, ms_ep->bNumEmbMIDIJack);
} else {
@@ -1928,6 +1998,10 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
endpoints[epidx].in_interval = 1;
endpoints[epidx].in_cables =
(1 << ms_ep->bNumEmbMIDIJack) - 1;
+ for (j = 0; j < ms_ep->bNumEmbMIDIJack; ++j)
+ endpoints[epidx].assoc_in_jacks[j] = ms_ep->baAssocJackID[j];
+ for (; j < 0x10; ++j)
+ endpoints[epidx].assoc_in_jacks[j] = -1;
dev_dbg(&umidi->dev->dev, "EP %02X: %d jack(s)\n",
ep->bEndpointAddress, ms_ep->bNumEmbMIDIJack);
}
@@ -2244,11 +2318,13 @@ static int snd_usbmidi_create_endpoints_midiman(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_OUTPUT,
cable,
+ -1 /* prevent trying to find jack */,
&umidi->endpoints[cable & 1].out->ports[cable].substream);
if (endpoint->in_cables & (1 << cable))
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_INPUT,
cable,
+ -1 /* prevent trying to find jack */,
&umidi->endpoints[0].in->ports[cable].substream);
}
return 0;
diff --git a/sound/usb/midi.h b/sound/usb/midi.h
index 8c38aec22..3f153195c 100644
--- a/sound/usb/midi.h
+++ b/sound/usb/midi.h
@@ -13,6 +13,8 @@ struct snd_usb_midi_endpoint_info {
uint8_t in_interval;
uint16_t out_cables; /* bitmask */
uint16_t in_cables; /* bitmask */
+ int16_t assoc_in_jacks[16];
+ int16_t assoc_out_jacks[16];
};
/* for QUIRK_MIDI_YAMAHA, data is NULL */
--
2.20.1
On Sun, 21 Feb 2021 10:43:32 +0100,
George Harker wrote:
>
> A number of devices have named substreams which are hard to remember /
> decypher from <device> MIDI n names. Eg. Korg puts a pass through on
> one substream and iConnectivity devices name the connections.
>
> This makes it easier to connect to the correct device. Devices which
> handle naming through quirks are unaffected by this change.
>
> Addresses TODO comment in sound/usb/midi.c
>
> Signed-off-by: George Harker <[email protected]>
The code changes look almost OK, but could you try the following?
- Split the patch: one for rewriting with the structs in
linux/usb/midi.h, another for adding the MIDI device name support
- Try to avoid magic numbers: a few places should be replaced with
sizeof() or ARRAY_SIZE().
thanks,
Takashi
This makes it easier to connect to the correct device. Devices which
handle naming through quirks are unaffected by this change.
Addresses TODO comment in sound/usb/midi.c
Signed-off-by: George Harker <[email protected]>
---
sound/usb/midi.c | 103 ++++++++++++++++++++++++++++++++++++++++++++---
sound/usb/midi.h | 2 +
2 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 610cf54ee..9efda4b06 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1740,12 +1740,68 @@ static void snd_usbmidi_get_port_info(struct snd_rawmidi *rmidi, int number,
}
}
+static struct usb_midi_in_jack_descriptor *find_usb_in_jack_descriptor(
+ struct usb_host_interface *hostif, uint8_t jack_id)
+{
+ unsigned char *extra = hostif->extra;
+ int extralen = hostif->extralen;
+
+ while (extralen > 4) {
+ struct usb_midi_in_jack_descriptor *injd =
+ (struct usb_midi_in_jack_descriptor *)extra;
+
+ if (injd->bLength > 4 &&
+ injd->bDescriptorType == USB_DT_CS_INTERFACE &&
+ injd->bDescriptorSubtype == UAC_MIDI_IN_JACK &&
+ injd->bJackID == jack_id)
+ return injd;
+ if (!extra[0])
+ break;
+ extralen -= extra[0];
+ extra += extra[0];
+ }
+ return NULL;
+}
+
+static struct usb_midi_out_jack_descriptor *find_usb_out_jack_descriptor(
+ struct usb_host_interface *hostif, uint8_t jack_id)
+{
+ unsigned char *extra = hostif->extra;
+ int extralen = hostif->extralen;
+
+ while (extralen > 4) {
+ struct usb_midi_out_jack_descriptor *outjd =
+ (struct usb_midi_out_jack_descriptor *)extra;
+
+ if (outjd->bLength > 4 &&
+ outjd->bDescriptorType == USB_DT_CS_INTERFACE &&
+ outjd->bDescriptorSubtype == UAC_MIDI_OUT_JACK &&
+ outjd->bJackID == jack_id)
+ return outjd;
+ if (!extra[0])
+ break;
+ extralen -= extra[0];
+ extra += extra[0];
+ }
+ return NULL;
+}
+
static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
- int stream, int number,
+ int stream, int number, int jack_id,
struct snd_rawmidi_substream **rsubstream)
{
struct port_info *port_info;
const char *name_format;
+ struct usb_interface *intf;
+ struct usb_host_interface *hostif;
+ struct usb_midi_in_jack_descriptor *injd;
+ struct usb_midi_out_jack_descriptor *outjd;
+ uint8_t jack_name_buf[32];
+ uint8_t *default_jack_name = "MIDI";
+ uint8_t *jack_name = default_jack_name;
+ uint8_t iJack;
+ size_t sz;
+ int res;
struct snd_rawmidi_substream *substream =
snd_usbmidi_find_substream(umidi, stream, number);
@@ -1755,11 +1811,36 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
return;
}
- /* TODO: read port name from jack descriptor */
+ intf = umidi->iface;
+ if (intf && jack_id >= 0) {
+ hostif = intf->cur_altsetting;
+ iJack = 0;
+ if (stream != SNDRV_RAWMIDI_STREAM_OUTPUT) {
+ /* in jacks connect to outs */
+ outjd = find_usb_out_jack_descriptor(hostif, jack_id);
+ if (outjd) {
+ sz = USB_DT_MIDI_OUT_SIZE(outjd->bNrInputPins);
+ iJack = *(((uint8_t *) outjd) + sz - sizeof(uint8_t));
+ }
+ } else {
+ /* and out jacks connect to ins */
+ injd = find_usb_in_jack_descriptor(hostif, jack_id);
+ if (injd)
+ iJack = injd->iJack;
+ }
+ if (iJack != 0) {
+ res = usb_string(umidi->dev, iJack, jack_name_buf,
+ ARRAY_SIZE(jack_name_buf));
+ if (res)
+ jack_name = jack_name_buf;
+ }
+ }
+
port_info = find_port_info(umidi, number);
- name_format = port_info ? port_info->name : "%s MIDI %d";
+ name_format = port_info ? port_info->name :
+ (jack_name != default_jack_name ? "%s %s" : "%s %s %d");
snprintf(substream->name, sizeof(substream->name),
- name_format, umidi->card->shortname, number + 1);
+ name_format, umidi->card->shortname, jack_name, number + 1);
*rsubstream = substream;
}
@@ -1794,6 +1875,7 @@ static int snd_usbmidi_create_endpoints(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_OUTPUT,
out_ports,
+ endpoints[i].assoc_out_jacks[j],
&umidi->endpoints[i].out->ports[j].substream);
++out_ports;
}
@@ -1801,6 +1883,7 @@ static int snd_usbmidi_create_endpoints(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_INPUT,
in_ports,
+ endpoints[i].assoc_in_jacks[j],
&umidi->endpoints[i].in->ports[j].substream);
++in_ports;
}
@@ -1846,7 +1929,7 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
struct usb_host_endpoint *hostep;
struct usb_endpoint_descriptor *ep;
struct usb_ms_endpoint_descriptor *ms_ep;
- int i, epidx;
+ int i, j, epidx;
intf = umidi->iface;
if (!intf)
@@ -1895,6 +1978,10 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
endpoints[epidx].out_interval = 1;
endpoints[epidx].out_cables =
(1 << ms_ep->bNumEmbMIDIJack) - 1;
+ for (j = 0; j < ms_ep->bNumEmbMIDIJack; ++j)
+ endpoints[epidx].assoc_out_jacks[j] = ms_ep->baAssocJackID[j];
+ for (; j < ARRAY_SIZE(endpoints[epidx].assoc_out_jacks); ++j)
+ endpoints[epidx].assoc_out_jacks[j] = -1;
dev_dbg(&umidi->dev->dev, "EP %02X: %d jack(s)\n",
ep->bEndpointAddress, ms_ep->bNumEmbMIDIJack);
} else {
@@ -1912,6 +1999,10 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
endpoints[epidx].in_interval = 1;
endpoints[epidx].in_cables =
(1 << ms_ep->bNumEmbMIDIJack) - 1;
+ for (j = 0; j < ms_ep->bNumEmbMIDIJack; ++j)
+ endpoints[epidx].assoc_in_jacks[j] = ms_ep->baAssocJackID[j];
+ for (; j < ARRAY_SIZE(endpoints[epidx].assoc_in_jacks); ++j)
+ endpoints[epidx].assoc_in_jacks[j] = -1;
dev_dbg(&umidi->dev->dev, "EP %02X: %d jack(s)\n",
ep->bEndpointAddress, ms_ep->bNumEmbMIDIJack);
}
@@ -2228,11 +2319,13 @@ static int snd_usbmidi_create_endpoints_midiman(struct snd_usb_midi *umidi,
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_OUTPUT,
cable,
+ -1 /* prevent trying to find jack */,
&umidi->endpoints[cable & 1].out->ports[cable].substream);
if (endpoint->in_cables & (1 << cable))
snd_usbmidi_init_substream(umidi,
SNDRV_RAWMIDI_STREAM_INPUT,
cable,
+ -1 /* prevent trying to find jack */,
&umidi->endpoints[0].in->ports[cable].substream);
}
return 0;
diff --git a/sound/usb/midi.h b/sound/usb/midi.h
index 8c38aec22..3f153195c 100644
--- a/sound/usb/midi.h
+++ b/sound/usb/midi.h
@@ -13,6 +13,8 @@ struct snd_usb_midi_endpoint_info {
uint8_t in_interval;
uint16_t out_cables; /* bitmask */
uint16_t in_cables; /* bitmask */
+ int16_t assoc_in_jacks[16];
+ int16_t assoc_out_jacks[16];
};
/* for QUIRK_MIDI_YAMAHA, data is NULL */
--
2.20.1
Use struct definitions from linux/usb/midi.h rather than locally define
the structs in sound/usb/midi.c .
Signed-off-by: George Harker <[email protected]>
---
sound/usb/midi.c | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 0c23fa6d8..610cf54ee 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -47,6 +47,7 @@
#include <linux/usb.h>
#include <linux/wait.h>
#include <linux/usb/audio.h>
+#include <linux/usb/midi.h>
#include <linux/module.h>
#include <sound/core.h>
@@ -77,23 +78,6 @@ MODULE_AUTHOR("Clemens Ladisch <[email protected]>");
MODULE_DESCRIPTION("USB Audio/MIDI helper module");
MODULE_LICENSE("Dual BSD/GPL");
-
-struct usb_ms_header_descriptor {
- __u8 bLength;
- __u8 bDescriptorType;
- __u8 bDescriptorSubtype;
- __u8 bcdMSC[2];
- __le16 wTotalLength;
-} __attribute__ ((packed));
-
-struct usb_ms_endpoint_descriptor {
- __u8 bLength;
- __u8 bDescriptorType;
- __u8 bDescriptorSubtype;
- __u8 bNumEmbMIDIJack;
- __u8 baAssocJackID[];
-} __attribute__ ((packed));
-
struct snd_usb_midi_in_endpoint;
struct snd_usb_midi_out_endpoint;
struct snd_usb_midi_endpoint;
@@ -1875,7 +1859,7 @@ static int snd_usbmidi_get_ms_info(struct snd_usb_midi *umidi,
ms_header->bDescriptorType == USB_DT_CS_INTERFACE &&
ms_header->bDescriptorSubtype == UAC_HEADER)
dev_dbg(&umidi->dev->dev, "MIDIStreaming version %02x.%02x\n",
- ms_header->bcdMSC[1], ms_header->bcdMSC[0]);
+ ((uint8_t *)&ms_header->bcdMSC)[1], ((uint8_t *)&ms_header->bcdMSC)[0]);
else
dev_warn(&umidi->dev->dev,
"MIDIStreaming interface descriptor not found\n");
--
2.20.1
Thanks for the feedback, addressed in the two patches. I’m not sure why it pulled the body up into the title.I’m sorry about that. I can resubmit if that needs fixing.
George
On Fri, 26 Feb 2021 22:36:42 +0100,
George Harker wrote:
>
> Thanks for the feedback, addressed in the two patches. I’m not sure why it pulled the body up into the title.I’m sorry about that. I can resubmit if that needs fixing.
I fixed locally and applied those two patches now in for-next branch
(i.e. for 5.13 kernel).
thanks,
Takashi