From 9acdf4df2fc4680b08fa242b09717892cd687d4a Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Tue, 8 Mar 2016 20:21:47 +0000 Subject: usb: gadget: f_midi: added spinlock on transmit function Since f_midi_transmit is called by both ALSA and USB sub-systems, it can potentially cause a race condition between both calls because f_midi_transmit is not reentrant nor thread-safe. This is due to an implementation detail that the transmit function looks for the next available usb request from the fifo and only enqueues it if there is data to send, otherwise just re-uses it. So, if both ALSA and USB frameworks calls this function at the same time, kfifo_seek() will return the same usb_request, which will cause a race condition. To solve this problem a syncronization mechanism is necessary. In this case it is used a spinlock since f_midi_transmit is also called by usb_request->complete callback in interrupt context. Cc: # v4.5+ Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests") Signed-off-by: Felipe F. Tonello Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 84c0ee5ebd1e..56e2dde99b03 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -89,6 +90,7 @@ struct f_midi { unsigned int buflen, 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; unsigned int in_last_port; struct gmidi_in_port in_ports_array[/* in_ports */]; @@ -597,17 +599,22 @@ static void f_midi_transmit(struct f_midi *midi) { struct usb_ep *ep = midi->in_ep; int ret; + unsigned long flags; /* We only care about USB requests if IN endpoint is enabled */ if (!ep || !ep->enabled) goto drop_out; + spin_lock_irqsave(&midi->transmit_lock, flags); + do { ret = f_midi_do_transmit(midi, ep); if (ret < 0) goto drop_out; } while (ret); + spin_unlock_irqrestore(&midi->transmit_lock, flags); + return; drop_out: @@ -1201,6 +1208,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi) if (status) goto setup_fail; + spin_lock_init(&midi->transmit_lock); + ++opts->refcnt; mutex_unlock(&opts->lock); -- cgit v1.2.1 From 38e58986e6fc14617f6361ec5178e5191e7ab5c1 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Mon, 21 Mar 2016 09:04:23 +0200 Subject: usb: gadget: udc: atmel: don't disable enpdoints we don't own UDC driver should NEVER do anything behind udc-core's back, so let's stop disabling endpoints we don't exactly own - rather we provide as resources for gadget drivers. This fixes the regression reported by Gil. Reported-by: Gil Weber Tested-by: Gil Weber Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/atmel_usba_udc.c | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index 81d42cce885a..18569de06b04 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -1045,20 +1045,6 @@ static void reset_all_endpoints(struct usba_udc *udc) list_del_init(&req->queue); request_complete(ep, req, -ECONNRESET); } - - /* NOTE: normally, the next call to the gadget driver is in - * charge of disabling endpoints... usually disconnect(). - * The exception would be entering a high speed test mode. - * - * FIXME remove this code ... and retest thoroughly. - */ - list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) { - if (ep->ep.desc) { - spin_unlock(&udc->lock); - usba_ep_disable(&ep->ep); - spin_lock(&udc->lock); - } - } } static struct usba_ep *get_ep_by_addr(struct usba_udc *udc, u16 wIndex) -- cgit v1.2.1 From 08f8cabf715654634a0bae8bee7afea964c6c9cb Mon Sep 17 00:00:00 2001 From: John Youn Date: Mon, 28 Mar 2016 16:12:24 -0700 Subject: usb: gadget: composite: Access SSP Dev Cap fields properly Access multi-byte fields of the SSP Dev Cap descriptor using the correct endianness. Fixes: f228a8de242a ("usb: gadget: composite: Return SSP Dev Cap descriptor") Signed-off-by: John Youn Signed-off-by: Felipe Balbi --- drivers/usb/gadget/composite.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index a5c62093c26c..de9ffd60fcfa 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -656,7 +656,8 @@ static int bos_desc(struct usb_composite_dev *cdev) ssp_cap->bmAttributes = cpu_to_le32(1); /* Min RX/TX Lane Count = 1 */ - ssp_cap->wFunctionalitySupport = (1 << 8) | (1 << 12); + ssp_cap->wFunctionalitySupport = + cpu_to_le16((1 << 8) | (1 << 12)); /* * bmSublinkSpeedAttr[0]: @@ -666,7 +667,7 @@ static int bos_desc(struct usb_composite_dev *cdev) * LSM = 10 (10 Gbps) */ ssp_cap->bmSublinkSpeedAttr[0] = - (3 << 4) | (1 << 14) | (0xa << 16); + cpu_to_le32((3 << 4) | (1 << 14) | (0xa << 16)); /* * bmSublinkSpeedAttr[1] = * ST = Symmetric, TX @@ -675,7 +676,8 @@ static int bos_desc(struct usb_composite_dev *cdev) * LSM = 10 (10 Gbps) */ ssp_cap->bmSublinkSpeedAttr[1] = - (3 << 4) | (1 << 14) | (0xa << 16) | (1 << 7); + cpu_to_le32((3 << 4) | (1 << 14) | + (0xa << 16) | (1 << 7)); } return le16_to_cpu(bos->wTotalLength); -- cgit v1.2.1 From 03d27ade4941076b34c823d63d91dc895731a595 Mon Sep 17 00:00:00 2001 From: "Felipe F. Tonello" Date: Wed, 9 Mar 2016 19:39:30 +0000 Subject: usb: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize 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] This patch fixes this problem by setting the minimum usb_request's buffer size for the OUT endpoint as its wMaxPacketSize. Acked-by: Michal Nazarewicz Signed-off-by: Felipe F. Tonello Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 56e2dde99b03..9ad51dcab982 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -360,7 +360,9 @@ 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, + max_t(unsigned, midi->buflen, + bulk_out_desc.wMaxPacketSize)); if (req == NULL) return -ENOMEM; -- cgit v1.2.1 From 4fc50ba5965ac2b360499d4a23eb10d04414dd36 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 2 Apr 2016 07:51:08 +0300 Subject: usb: gadget: f_midi: unlock on error We added some new locking here, but missed an error path where we need to unlock. Fixes: 9acdf4df2fc4 ('usb: gadget: f_midi: added spinlock on transmit function') Acked-by: Michal Nazarewicz Acked-by: Felipe F. Tonello Signed-off-by: Dan Carpenter Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_midi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index 9ad51dcab982..58fc199a18ec 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -611,8 +611,10 @@ static void f_midi_transmit(struct f_midi *midi) do { ret = f_midi_do_transmit(midi, ep); - if (ret < 0) + if (ret < 0) { + spin_unlock_irqrestore(&midi->transmit_lock, flags); goto drop_out; + } } while (ret); spin_unlock_irqrestore(&midi->transmit_lock, flags); -- cgit v1.2.1 From 79171e9b90b133686a96912cd7b528655cc9a07a Mon Sep 17 00:00:00 2001 From: Grygorii Strashko Date: Mon, 4 Apr 2016 14:31:54 +0300 Subject: usb: gadget: udc-core: remove manual dma configuration Since commit 7ace8fc8219e ("usb: gadget: udc: core: Fix argument of dma_map_single for IOMMU") it is not necessary to configure DMA for usb_gadget device manually, because all DMA operation are performed using parent/controller device. Cc: Yoshihiro Shimoda Signed-off-by: Grygorii Strashko Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/udc-core.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/usb/gadget') diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 4151597e9d28..e4e70e11d0f6 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -371,12 +371,6 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, INIT_WORK(&gadget->work, usb_gadget_state_work); gadget->dev.parent = parent; -#ifdef CONFIG_HAS_DMA - dma_set_coherent_mask(&gadget->dev, parent->coherent_dma_mask); - gadget->dev.dma_parms = parent->dma_parms; - gadget->dev.dma_mask = parent->dma_mask; -#endif - if (release) gadget->dev.release = release; else -- cgit v1.2.1