Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755713AbcCWRH5 (ORCPT ); Wed, 23 Mar 2016 13:07:57 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34554 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751088AbcCWRHz (ORCPT ); Wed, 23 Mar 2016 13:07:55 -0400 From: "Felipe F. Tonello" To: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Michal Nazarewicz , Felipe Balbi , Clemens Ladisch Subject: [PATCH v2] usb: gadget: f_midi: fixed a bug when buflen was smaller than wMaxPacketSize Date: Wed, 23 Mar 2016 17:10:21 +0000 Message-Id: <1458753021-27400-1-git-send-email-eu@felipetonello.com> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5077 Lines: 145 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 --- 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