[PATCH v2] usb: gadget: f_midi: fixed a bug when buflen was smaller than wMaxPacketSize

Felipe F. Tonello
2016-03-23T17:07:57Z

buflen by default (256) is smaller than wMaxPacketSize (512) in high-speed
devices.

That caused the OUT endpoint to freeze if the host send any data packet of
length greater than 256 bytes.

This is an example dump of what happended on that enpoint:
HOST: [DATA][Length=260][...]
DEVICE: [NAK]
HOST: [PING]
DEVICE: [NAK]
HOST: [PING]
DEVICE: [NAK]
...
HOST: [PING]
DEVICE: [NAK]

Users should not change the buffer size to anything other than wMaxPacketSize
because that can cause bugs (this bug) or performance issues. Thus, this patch
fixes this problem by eliminating buflen entirely and replacing it with
wMaxPacketSize of the appropriate endpoint where needed.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---

v2:
- Removed buflen
- use le16_to_cpu() in order to avoid endianess issues

drivers/usb/gadget/function/f_midi.c | 14 ++++++--------
drivers/usb/gadget/function/u_midi.h | 1 -
drivers/usb/gadget/legacy/gmidi.c | 5 -----
3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 8475e3dc82d4..42545538f565 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -93,7 +93,7 @@ struct f_midi {
unsigned int out_ports;
int index;
char *id;
- unsigned int buflen, qlen;
+ unsigned int qlen;
/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
spinlock_t transmit_lock;
@@ -352,7 +352,8 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
/* pre-allocate write usb requests to use on f_midi_transmit. */
while (kfifo_avail(&midi->in_req_fifo)) {
struct usb_request *req =
- midi_alloc_ep_req(midi->in_ep, midi->buflen);
+ midi_alloc_ep_req(midi->in_ep,
+ le16_to_cpu(bulk_in_desc.wMaxPacketSize));

if (req == NULL)
return -ENOMEM;
@@ -366,7 +367,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, midi->buflen);
+ midi_alloc_ep_req(midi->out_ep,
+ le16_to_cpu(bulk_out_desc.wMaxPacketSize));
if (req == NULL)
return -ENOMEM;

@@ -615,7 +617,7 @@ static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
if (!port->active || !substream)
continue;

- while (req->length + 3 < midi->buflen) {
+ while (req->length + 3 < le16_to_cpu(bulk_in_desc.wMaxPacketSize)) {
uint8_t b;

if (snd_rawmidi_transmit(substream, &b, 1) != 1) {
@@ -1080,7 +1082,6 @@ end: \
CONFIGFS_ATTR(f_midi_opts_, name);

F_MIDI_OPT(index, true, SNDRV_CARDS);
-F_MIDI_OPT(buflen, false, 0);
F_MIDI_OPT(qlen, false, 0);
F_MIDI_OPT(in_ports, true, MAX_PORTS);
F_MIDI_OPT(out_ports, true, MAX_PORTS);
@@ -1135,7 +1136,6 @@ CONFIGFS_ATTR(f_midi_opts_, id);

static struct configfs_attribute *midi_attrs[] = {
&f_midi_opts_attr_index,
- &f_midi_opts_attr_buflen,
&f_midi_opts_attr_qlen,
&f_midi_opts_attr_in_ports,
&f_midi_opts_attr_out_ports,
@@ -1173,7 +1173,6 @@ 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->qlen = 32;
opts->in_ports = 1;
opts->out_ports = 1;
@@ -1254,7 +1253,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
midi->in_ports = opts->in_ports;
midi->out_ports = opts->out_ports;
midi->index = opts->index;
- midi->buflen = opts->buflen;
midi->qlen = opts->qlen;
midi->in_last_port = 0;

diff --git a/drivers/usb/gadget/function/u_midi.h b/drivers/usb/gadget/function/u_midi.h
index 22510189758e..f48b18fcea25 100644
--- a/drivers/usb/gadget/function/u_midi.h
+++ b/drivers/usb/gadget/function/u_midi.h
@@ -25,7 +25,6 @@ struct f_midi_opts {
bool id_allocated;
unsigned int in_ports;
unsigned int out_ports;
- unsigned int buflen;
unsigned int qlen;

/*
diff --git a/drivers/usb/gadget/legacy/gmidi.c b/drivers/usb/gadget/legacy/gmidi.c
index fc2ac150f5ff..436b09155edb 100644
--- a/drivers/usb/gadget/legacy/gmidi.c
+++ b/drivers/usb/gadget/legacy/gmidi.c
@@ -47,10 +47,6 @@ 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;
-module_param(buflen, uint, S_IRUGO);
-MODULE_PARM_DESC(buflen, "MIDI buffer length");
-
static unsigned int qlen = 32;
module_param(qlen, uint, S_IRUGO);
MODULE_PARM_DESC(qlen, "USB read and write request queue length");
@@ -154,7 +150,6 @@ static int midi_bind(struct usb_composite_dev *cdev)
midi_opts->id = id;
midi_opts->in_ports = in_ports;
midi_opts->out_ports = out_ports;
- midi_opts->buflen = buflen;
midi_opts->qlen = qlen;

status = usb_string_ids_tab(cdev, strings_dev);
--
2.7.4