Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933216AbbLVQIX (ORCPT ); Tue, 22 Dec 2015 11:08:23 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:35494 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933194AbbLVQIK (ORCPT ); Tue, 22 Dec 2015 11:08:10 -0500 From: "Felipe F. Tonello" To: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Felipe Balbi , Greg Kroah-Hartman , Robert Baldyga , Clemens Ladisch Subject: [PATCH 1/2] usb: gadget: f_midi: refactor state machine Date: Tue, 22 Dec 2015 16:08:05 +0000 Message-Id: <1450800486-18150-1-git-send-email-eu@felipetonello.com> X-Mailer: git-send-email 2.5.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7519 Lines: 313 This refactor includes the following: * Cleaner state machine code; * Reset state if MIDI message parsed is non-conformant; * Fixed bug when a conformant MIDI message was followed by a non-conformant causing the MIDI-USB message to use old temporary data (port->data[0..1]), thus packing a wrong MIDI-USB request. Signed-off-by: Felipe F. Tonello --- drivers/usb/gadget/function/f_midi.c | 243 ++++++++++++++++++++--------------- 1 file changed, 141 insertions(+), 102 deletions(-) diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index fb1fe96d..b70a830 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -50,6 +50,18 @@ static const char f_midi_longname[] = "MIDI Gadget"; */ #define MAX_PORTS 16 +/* MIDI message states */ +enum { + STATE_INITIAL = 0, + STATE_1PARAM, + STATE_2PARAM_1, + STATE_2PARAM_2, + STATE_SYSEX_0, + STATE_SYSEX_1, + STATE_SYSEX_2, + STATE_FINISHED, +}; + /* * This is a gadget, and the IN/OUT naming is from the host's perspective. * USB -> OUT endpoint -> rawmidi @@ -60,13 +72,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]; }; @@ -400,118 +405,152 @@ 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; - - if (b >= 0xf8) { - f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0); - } else if (b >= 0xf0) { - switch (b) { - case 0xf0: - port->data[0] = b; - port->state = STATE_SYSEX_1; - break; - case 0xf1: - case 0xf3: - port->data[0] = b; - port->state = STATE_1PARAM; - break; - case 0xf2: + uint8_t p[4] = { port->cable << 4, 0, 0, 0 }; + uint8_t next_state = STATE_INITIAL; + + switch (port->state) { + case STATE_INITIAL: { + if (b >= 0xf8) { + p[0] |= 0x0f; + p[1] = b; + next_state = STATE_FINISHED; + } else if (b >= 0xf0) { + switch (b) { + case 0xf0: + port->data[0] = b; + next_state = STATE_SYSEX_1; + break; + case 0xf1: + case 0xf3: + port->data[0] = b; + next_state = STATE_1PARAM; + break; + case 0xf2: + port->data[0] = b; + next_state = STATE_2PARAM_1; + break; + case 0xf4: + case 0xf5: + next_state = STATE_INITIAL; + break; + case 0xf6: + p[0] |= 0x05; + p[1] = 0xf6; + next_state = STATE_FINISHED; + break; + } + } else if (b >= 0x80) { port->data[0] = b; - port->state = STATE_2PARAM_1; - break; - case 0xf4: - case 0xf5: - port->state = STATE_UNKNOWN; - break; - case 0xf6: - f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0); - port->state = STATE_UNKNOWN; - break; - case 0xf7: + if (b >= 0xc0 && b <= 0xdf) + next_state = STATE_1PARAM; + else + next_state = STATE_2PARAM_1; + } + break; + } + case STATE_1PARAM: + case STATE_2PARAM_1: + case STATE_2PARAM_2: + case STATE_SYSEX_0: + case STATE_SYSEX_1: + case STATE_SYSEX_2: { + if (b == 0xf7) { switch (port->state) { case STATE_SYSEX_0: - f_midi_transmit_packet(req, - p0 | 0x05, 0xf7, 0, 0); + p[0] |= 0x05; + p[1] = 0xf7; break; case STATE_SYSEX_1: - f_midi_transmit_packet(req, - p0 | 0x06, port->data[0], 0xf7, 0); + p[0] |= 0x06; + p[1] = port->data[0]; + p[2] = 0xf7; break; case STATE_SYSEX_2: - f_midi_transmit_packet(req, - p0 | 0x07, port->data[0], - port->data[1], 0xf7); + p[0] |= 0x07; + p[1] = port->data[0]; + p[2] = port->data[1]; + p[3] = 0xf7; break; } - port->state = STATE_UNKNOWN; - break; - } - } else if (b >= 0x80) { - port->data[0] = b; - if (b >= 0xc0 && b <= 0xdf) - port->state = STATE_1PARAM; - else - port->state = STATE_2PARAM_1; - } else { /* b < 0x80 */ - 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); - break; - case STATE_2PARAM_1: - port->data[1] = b; - port->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; + next_state = STATE_FINISHED; + } else if (b < 0x80) { + switch (port->state) { + case STATE_1PARAM: + if (port->data[0] < 0xf0) + p[0] |= port->data[0] >> 4; + else + p[0] |= 0x02; + + p[1] = port->data[0]; + p[2] = b; + next_state = STATE_FINISHED; + break; + case STATE_2PARAM_1: + port->data[1] = b; + next_state = STATE_2PARAM_2; + break; + case STATE_2PARAM_2: + 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; + next_state = STATE_FINISHED; + break; + case STATE_SYSEX_0: + port->data[0] = b; + next_state = STATE_SYSEX_1; + break; + case STATE_SYSEX_1: + port->data[1] = b; + next_state = STATE_SYSEX_2; + break; + case STATE_SYSEX_2: + p[0] |= 0x04; + p[1] = port->data[0]; + p[2] = port->data[1]; + p[3] = b; + next_state = STATE_SYSEX_0; + break; } - f_midi_transmit_packet(req, - p0, port->data[0], port->data[1], b); - break; - case STATE_SYSEX_0: - port->data[0] = b; - port->state = STATE_SYSEX_1; - break; - case STATE_SYSEX_1: - port->data[1] = b; - port->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; - break; + } else { + /* + * Wrong MIDI message + * Reset state + */ + next_state = STATE_INITIAL; + port->data[0] = 0; + port->data[1] = 0; } + break; + } } + + /* States where we have to write into the USB request */ + if (next_state == STATE_FINISHED || + next_state == STATE_SYSEX_0) { + unsigned 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) @@ -632,7 +671,7 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream) VDBG(midi, "%s()\n", __func__); midi->in_substream[substream->number] = substream; - midi->in_port[substream->number]->state = STATE_UNKNOWN; + midi->in_port[substream->number]->state = STATE_INITIAL; return 0; } -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/