* [PATCH 2/4] usb: gadget: f_midi: defaults buflen sizes to 512
2016-07-25 23:15 [PATCH 0/4] MIDI Function improvements Felipe F. Tonello
2016-07-25 23:15 ` [PATCH 1/4] usb: gadget: f_midi: fixed endianness when using wMaxPacketSize Felipe F. Tonello
@ 2016-07-25 23:15 ` Felipe F. Tonello
2016-08-10 11:25 ` Felipe Balbi
2016-07-25 23:15 ` [PATCH 3/4] usb: gadget: f_midi: refactor state machine Felipe F. Tonello
2016-07-25 23:15 ` [PATCH 4/4] usb: gadget: f_midi: drop substreams when disabling endpoint Felipe F. Tonello
3 siblings, 1 reply; 10+ messages in thread
From: Felipe F. Tonello @ 2016-07-25 23:15 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, linux-kernel
512 is the value used by wMaxPacketSize, as specified by the USB Spec. This
makes sure this driver uses, by default, the most optimal value for IN and OUT
endpoint requests.
Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
drivers/usb/gadget/function/f_midi.c | 2 +-
drivers/usb/gadget/legacy/gmidi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index a83d852b1da5..dd5cca6eae7c 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1123,7 +1123,7 @@ static struct usb_function_instance *f_midi_alloc_inst(void)
opts->func_inst.free_func_inst = f_midi_free_inst;
opts->index = SNDRV_DEFAULT_IDX1;
opts->id = SNDRV_DEFAULT_STR1;
- opts->buflen = 256;
+ opts->buflen = 512;
opts->qlen = 32;
opts->in_ports = 1;
opts->out_ports = 1;
diff --git a/drivers/usb/gadget/legacy/gmidi.c b/drivers/usb/gadget/legacy/gmidi.c
index fc2ac150f5ff..0bf39c3ccdb1 100644
--- a/drivers/usb/gadget/legacy/gmidi.c
+++ b/drivers/usb/gadget/legacy/gmidi.c
@@ -47,7 +47,7 @@ static char *id = SNDRV_DEFAULT_STR1;
module_param(id, charp, S_IRUGO);
MODULE_PARM_DESC(id, "ID string for the USB MIDI Gadget adapter.");
-static unsigned int buflen = 256;
+static unsigned int buflen = 512;
module_param(buflen, uint, S_IRUGO);
MODULE_PARM_DESC(buflen, "MIDI buffer length");
--
2.9.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] usb: gadget: f_midi: refactor state machine
2016-07-25 23:15 [PATCH 0/4] MIDI Function improvements Felipe F. Tonello
2016-07-25 23:15 ` [PATCH 1/4] usb: gadget: f_midi: fixed endianness when using wMaxPacketSize Felipe F. Tonello
2016-07-25 23:15 ` [PATCH 2/4] usb: gadget: f_midi: defaults buflen sizes to 512 Felipe F. Tonello
@ 2016-07-25 23:15 ` Felipe F. Tonello
2016-07-25 23:15 ` [PATCH 4/4] usb: gadget: f_midi: drop substreams when disabling endpoint Felipe F. Tonello
3 siblings, 0 replies; 10+ messages in thread
From: Felipe F. Tonello @ 2016-07-25 23:15 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, linux-kernel
This refactor results in a cleaner state machine code and promotes
consistency, readability, and maintanability of this driver.
This refactor state machine was well tested and it is currently running in
production code and devices.
Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
drivers/usb/gadget/function/f_midi.c | 204 ++++++++++++++++++++++-------------
1 file changed, 129 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index dd5cca6eae7c..49328f208a63 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -51,6 +51,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
*/
#define MAX_PORTS 16
+/* MIDI message states */
+enum {
+ STATE_INITIAL = 0, /* pseudo state */
+ STATE_1PARAM,
+ STATE_2PARAM_1,
+ STATE_2PARAM_2,
+ STATE_SYSEX_0,
+ STATE_SYSEX_1,
+ STATE_SYSEX_2,
+ STATE_REAL_TIME,
+ STATE_FINISHED, /* pseudo state */
+};
+
/*
* This is a gadget, and the IN/OUT naming is from the host's perspective.
* USB -> OUT endpoint -> rawmidi
@@ -61,13 +74,6 @@ struct gmidi_in_port {
int active;
uint8_t cable;
uint8_t state;
-#define STATE_UNKNOWN 0
-#define STATE_1PARAM 1
-#define STATE_2PARAM_1 2
-#define STATE_2PARAM_2 3
-#define STATE_SYSEX_0 4
-#define STATE_SYSEX_1 5
-#define STATE_SYSEX_2 6
uint8_t data[2];
};
@@ -404,118 +410,166 @@ static int f_midi_snd_free(struct snd_device *device)
return 0;
}
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
- uint8_t p1, uint8_t p2, uint8_t p3)
-{
- unsigned length = req->length;
- u8 *buf = (u8 *)req->buf + length;
-
- buf[0] = p0;
- buf[1] = p1;
- buf[2] = p2;
- buf[3] = p3;
- req->length = length + 4;
-}
-
/*
* Converts MIDI commands to USB MIDI packets.
*/
static void f_midi_transmit_byte(struct usb_request *req,
struct gmidi_in_port *port, uint8_t b)
{
- uint8_t p0 = port->cable << 4;
+ uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+ uint8_t next_state = STATE_INITIAL;
+
+ switch (b) {
+ case 0xf8 ... 0xff:
+ /* System Real-Time Messages */
+ p[0] |= 0x0f;
+ p[1] = b;
+ next_state = port->state;
+ port->state = STATE_REAL_TIME;
+ break;
+
+ case 0xf7:
+ /* End of SysEx */
+ switch (port->state) {
+ case STATE_SYSEX_0:
+ p[0] |= 0x05;
+ p[1] = 0xf7;
+ next_state = STATE_FINISHED;
+ break;
+ case STATE_SYSEX_1:
+ p[0] |= 0x06;
+ p[1] = port->data[0];
+ p[2] = 0xf7;
+ next_state = STATE_FINISHED;
+ break;
+ case STATE_SYSEX_2:
+ p[0] |= 0x07;
+ p[1] = port->data[0];
+ p[2] = port->data[1];
+ p[3] = 0xf7;
+ next_state = STATE_FINISHED;
+ break;
+ default:
+ /* Ignore byte */
+ next_state = port->state;
+ port->state = STATE_INITIAL;
+ }
+ break;
- if (b >= 0xf8) {
- f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
- } else if (b >= 0xf0) {
+ case 0xf0 ... 0xf6:
+ /* System Common Messages */
+ port->data[0] = port->data[1] = 0;
+ port->state = STATE_INITIAL;
switch (b) {
case 0xf0:
port->data[0] = b;
- port->state = STATE_SYSEX_1;
+ port->data[1] = 0;
+ next_state = STATE_SYSEX_1;
break;
case 0xf1:
case 0xf3:
port->data[0] = b;
- port->state = STATE_1PARAM;
+ next_state = STATE_1PARAM;
break;
case 0xf2:
port->data[0] = b;
- port->state = STATE_2PARAM_1;
+ next_state = STATE_2PARAM_1;
break;
case 0xf4:
case 0xf5:
- port->state = STATE_UNKNOWN;
+ next_state = STATE_INITIAL;
break;
case 0xf6:
- f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
- port->state = STATE_UNKNOWN;
- break;
- case 0xf7:
- switch (port->state) {
- case STATE_SYSEX_0:
- f_midi_transmit_packet(req,
- p0 | 0x05, 0xf7, 0, 0);
- break;
- case STATE_SYSEX_1:
- f_midi_transmit_packet(req,
- p0 | 0x06, port->data[0], 0xf7, 0);
- break;
- case STATE_SYSEX_2:
- f_midi_transmit_packet(req,
- p0 | 0x07, port->data[0],
- port->data[1], 0xf7);
- break;
- }
- port->state = STATE_UNKNOWN;
+ p[0] |= 0x05;
+ p[1] = 0xf6;
+ next_state = STATE_FINISHED;
break;
}
- } else if (b >= 0x80) {
+ break;
+
+ case 0x80 ... 0xef:
+ /*
+ * Channel Voice Messages, Channel Mode Messages
+ * and Control Change Messages.
+ */
port->data[0] = b;
+ port->data[1] = 0;
+ port->state = STATE_INITIAL;
if (b >= 0xc0 && b <= 0xdf)
- port->state = STATE_1PARAM;
+ next_state = STATE_1PARAM;
else
- port->state = STATE_2PARAM_1;
- } else { /* b < 0x80 */
+ next_state = STATE_2PARAM_1;
+ break;
+
+ case 0x00 ... 0x7f:
+ /* Message parameters */
switch (port->state) {
case STATE_1PARAM:
- if (port->data[0] < 0xf0) {
- p0 |= port->data[0] >> 4;
- } else {
- p0 |= 0x02;
- port->state = STATE_UNKNOWN;
- }
- f_midi_transmit_packet(req, p0, port->data[0], b, 0);
+ if (port->data[0] < 0xf0)
+ p[0] |= port->data[0] >> 4;
+ else
+ p[0] |= 0x02;
+
+ p[1] = port->data[0];
+ p[2] = b;
+ /* This is to allow Running State Messages */
+ next_state = STATE_1PARAM;
break;
case STATE_2PARAM_1:
port->data[1] = b;
- port->state = STATE_2PARAM_2;
+ next_state = STATE_2PARAM_2;
break;
case STATE_2PARAM_2:
- if (port->data[0] < 0xf0) {
- p0 |= port->data[0] >> 4;
- port->state = STATE_2PARAM_1;
- } else {
- p0 |= 0x03;
- port->state = STATE_UNKNOWN;
- }
- f_midi_transmit_packet(req,
- p0, port->data[0], port->data[1], b);
+ if (port->data[0] < 0xf0)
+ p[0] |= port->data[0] >> 4;
+ else
+ p[0] |= 0x03;
+
+ p[1] = port->data[0];
+ p[2] = port->data[1];
+ p[3] = b;
+ /* This is to allow Running State Messages */
+ next_state = STATE_2PARAM_1;
break;
case STATE_SYSEX_0:
port->data[0] = b;
- port->state = STATE_SYSEX_1;
+ next_state = STATE_SYSEX_1;
break;
case STATE_SYSEX_1:
port->data[1] = b;
- port->state = STATE_SYSEX_2;
+ next_state = STATE_SYSEX_2;
break;
case STATE_SYSEX_2:
- f_midi_transmit_packet(req,
- p0 | 0x04, port->data[0], port->data[1], b);
- port->state = STATE_SYSEX_0;
+ p[0] |= 0x04;
+ p[1] = port->data[0];
+ p[2] = port->data[1];
+ p[3] = b;
+ next_state = STATE_SYSEX_0;
break;
}
+ break;
+ }
+
+ /* States where we have to write into the USB request */
+ if (next_state == STATE_FINISHED ||
+ port->state == STATE_SYSEX_2 ||
+ port->state == STATE_1PARAM ||
+ port->state == STATE_2PARAM_2 ||
+ port->state == STATE_REAL_TIME) {
+
+ unsigned int length = req->length;
+ u8 *buf = (u8 *)req->buf + length;
+
+ memcpy(buf, p, sizeof(p));
+ req->length = length + sizeof(p);
+
+ if (next_state == STATE_FINISHED) {
+ next_state = STATE_INITIAL;
+ port->data[0] = port->data[1] = 0;
+ }
}
+
+ port->state = next_state;
}
static void f_midi_drop_out_substreams(struct f_midi *midi)
@@ -642,7 +696,7 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
VDBG(midi, "%s()\n", __func__);
port = midi->in_ports_array + substream->number;
port->substream = substream;
- port->state = STATE_UNKNOWN;
+ port->state = STATE_INITIAL;
return 0;
}
--
2.9.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] usb: gadget: f_midi: drop substreams when disabling endpoint
2016-07-25 23:15 [PATCH 0/4] MIDI Function improvements Felipe F. Tonello
` (2 preceding siblings ...)
2016-07-25 23:15 ` [PATCH 3/4] usb: gadget: f_midi: refactor state machine Felipe F. Tonello
@ 2016-07-25 23:15 ` Felipe F. Tonello
3 siblings, 0 replies; 10+ messages in thread
From: Felipe F. Tonello @ 2016-07-25 23:15 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, linux-kernel
This change makes sure that the ALSA buffers are cleaned if an endpoint
becomes disabled.
Before this change, if the internal ALSA buffer did overflow, the MIDI
function would stop sending MIDI to the host.
Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
drivers/usb/gadget/function/f_midi.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 49328f208a63..fdfdd357e15c 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -305,6 +305,19 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
}
}
+static void f_midi_drop_out_substreams(struct f_midi *midi)
+{
+ unsigned int i;
+
+ for (i = 0; i < midi->in_ports; i++) {
+ struct gmidi_in_port *port = midi->in_ports_array + i;
+ struct snd_rawmidi_substream *substream = port->substream;
+
+ if (port->active && substream)
+ snd_rawmidi_drop_output(substream);
+ }
+}
+
static int f_midi_start_ep(struct f_midi *midi,
struct usb_function *f,
struct usb_ep *ep)
@@ -403,6 +416,8 @@ static void f_midi_disable(struct usb_function *f)
/* release IN requests */
while (kfifo_get(&midi->in_req_fifo, &req))
free_ep_req(midi->in_ep, req);
+
+ f_midi_drop_out_substreams(midi);
}
static int f_midi_snd_free(struct snd_device *device)
@@ -572,18 +587,6 @@ static void f_midi_transmit_byte(struct usb_request *req,
port->state = next_state;
}
-static void f_midi_drop_out_substreams(struct f_midi *midi)
-{
- unsigned int i;
-
- for (i = 0; i < midi->in_ports; i++) {
- struct gmidi_in_port *port = midi->in_ports_array + i;
- struct snd_rawmidi_substream *substream = port->substream;
- if (port->active && substream)
- snd_rawmidi_drop_output(substream);
- }
-}
-
static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
{
struct usb_request *req = NULL;
--
2.9.2
^ permalink raw reply related [flat|nested] 10+ messages in thread