From 4a655f152e7932e43356b683958200094b368d5a Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Mon, 8 Aug 2016 21:30:07 +0100 Subject: usb: gadget: f_midi: remove alignment code for OUT endpoint The new version of alloc_ep_req() already aligns the buffer size to wMaxPacketSize on OUT endpoints. Signed-off-by: Felipe F. Tonello Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/usb/gadget/function/f_midi.c') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 58fc199a18ec..39018dea7035 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -360,9 +360,8 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt) /* allocate a bunch of read buffers and queue them all at once. */ for (i = 0; i < midi->qlen && err == 0; i++) { struct usb_request *req = - midi_alloc_ep_req(midi->out_ep, - max_t(unsigned, midi->buflen, - bulk_out_desc.wMaxPacketSize)); + midi_alloc_ep_req(midi->out_ep, midi->buflen); + if (req == NULL) return -ENOMEM; -- cgit v1.2.3 From 7ea9fde7605c3d224ba28cd17ffd8ac408453a96 Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Mon, 8 Aug 2016 21:30:08 +0100 Subject: usb: gadget: f_midi: defaults buflen sizes to 512 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 Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 2 +- drivers/usb/gadget/legacy/gmidi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/gadget/function/f_midi.c') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 39018dea7035..a7b50ac947f8 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -1122,7 +1122,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"); -- cgit v1.2.3 From f42ab18cc8aed6b5a3532c468f99285de9eabab5 Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Mon, 8 Aug 2016 21:30:09 +0100 Subject: usb: gadget: f_midi: refactor state machine 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 Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 204 ++++++++++++++++++++++------------- 1 file changed, 129 insertions(+), 75 deletions(-) (limited to 'drivers/usb/gadget/function/f_midi.c') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index a7b50ac947f8..09d769e18b50 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]; }; @@ -403,118 +409,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) @@ -641,7 +695,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; } -- cgit v1.2.3 From f8ca46aeb7b422b8c22e286e25269e4fefc560bd Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Mon, 8 Aug 2016 21:30:10 +0100 Subject: usb: gadget: f_midi: drop substreams when disabling endpoint 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 Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'drivers/usb/gadget/function/f_midi.c') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 09d769e18b50..3a47596afcab 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) @@ -402,6 +415,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) @@ -571,18 +586,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; -- cgit v1.2.3 From aadbe812463f8af1751debb0eaaeec8a778d7ff1 Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Tue, 23 Aug 2016 18:24:49 +0100 Subject: usb: gadget: remove useless parameter in alloc_ep_req() The default_length parameter of alloc_ep_req was not really necessary and gadget drivers would almost always create an inline function to pass the same value to len and default_len. This patch removes that parameter and updates all calls to alloc_ep_req() to use the new API. Signed-off-by: Felipe F. Tonello Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_hid.c | 2 +- drivers/usb/gadget/function/f_loopback.c | 6 ++---- drivers/usb/gadget/function/f_midi.c | 2 +- drivers/usb/gadget/function/f_sourcesink.c | 6 ++---- drivers/usb/gadget/u_f.c | 7 +++---- drivers/usb/gadget/u_f.h | 3 +-- 6 files changed, 10 insertions(+), 16 deletions(-) (limited to 'drivers/usb/gadget/function/f_midi.c') diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 4cd486600a9b..fa807ce7fc0e 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -365,7 +365,7 @@ static int f_hidg_open(struct inode *inode, struct file *fd) static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep, unsigned length) { - return alloc_ep_req(ep, length, length); + return alloc_ep_req(ep, length); } static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req) diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c index 3a9f8f9c77bd..ddb8c896eaa3 100644 --- a/drivers/usb/gadget/function/f_loopback.c +++ b/drivers/usb/gadget/function/f_loopback.c @@ -308,9 +308,7 @@ static void disable_loopback(struct f_loopback *loop) static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len) { - struct f_loopback *loop = ep->driver_data; - - return alloc_ep_req(ep, len, loop->buflen); + return alloc_ep_req(ep, len); } static int alloc_requests(struct usb_composite_dev *cdev, @@ -333,7 +331,7 @@ static int alloc_requests(struct usb_composite_dev *cdev, if (!in_req) goto fail; - out_req = lb_alloc_ep_req(loop->out_ep, 0); + out_req = lb_alloc_ep_req(loop->out_ep, loop->buflen); if (!out_req) goto fail_in; diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 3a47596afcab..a5719f271bf0 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -211,7 +211,7 @@ static struct usb_gadget_strings *midi_strings[] = { static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep, unsigned length) { - return alloc_ep_req(ep, length, length); + return alloc_ep_req(ep, length); } static const uint8_t f_midi_cin_length[] = { diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c index df0189ddfdd5..8784fa12ea2c 100644 --- a/drivers/usb/gadget/function/f_sourcesink.c +++ b/drivers/usb/gadget/function/f_sourcesink.c @@ -293,9 +293,7 @@ static struct usb_gadget_strings *sourcesink_strings[] = { static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len) { - struct f_sourcesink *ss = ep->driver_data; - - return alloc_ep_req(ep, len, ss->buflen); + return alloc_ep_req(ep, len); } static void disable_ep(struct usb_composite_dev *cdev, struct usb_ep *ep) @@ -606,7 +604,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in, } else { ep = is_in ? ss->in_ep : ss->out_ep; qlen = ss->bulk_qlen; - size = 0; + size = ss->buflen; } for (i = 0; i < qlen; i++) { diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c index 907f8144813c..18839732c840 100644 --- a/drivers/usb/gadget/u_f.c +++ b/drivers/usb/gadget/u_f.c @@ -14,15 +14,14 @@ #include "u_f.h" #include -struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int default_len) +struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len) { struct usb_request *req; req = usb_ep_alloc_request(ep, GFP_ATOMIC); if (req) { - req->length = len ?: default_len; - if (usb_endpoint_dir_out(ep->desc)) - req->length = usb_ep_align(ep, req->length); + req->length = usb_endpoint_dir_out(ep->desc) ? + usb_ep_align(ep, len) : len; req->buf = kmalloc(req->length, GFP_ATOMIC); if (!req->buf) { usb_ep_free_request(ep, req); diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h index 69a1d10df04f..7d53a4773d1a 100644 --- a/drivers/usb/gadget/u_f.h +++ b/drivers/usb/gadget/u_f.h @@ -53,14 +53,13 @@ struct usb_request; * * @ep: the endpoint to allocate a usb_request * @len: usb_requests's buffer suggested size - * @default_len: used if @len is not provided, ie, is 0 * * In case @ep direction is OUT, the @len will be aligned to ep's * wMaxPacketSize. In order to avoid memory leaks or drops, *always* use * usb_requests's length (req->length) to refer to the allocated buffer size. * Requests allocated via alloc_ep_req() *must* be freed by free_ep_req(). */ -struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int default_len); +struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len); /* Frees a usb_request previously allocated by alloc_ep_req() */ static inline void free_ep_req(struct usb_ep *ep, struct usb_request *req) -- cgit v1.2.3