From 7369090a9fb57c3fc705ce355d2e4523a5a24716 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Tue, 31 Jan 2017 13:24:54 +0200 Subject: usb: dwc3: gadget: make Set Endpoint Configuration macros safe Some gadget drivers are bad, bad boys. We notice that ADB was passing bad Burst Size which caused top bits of param0 to be overwritten which confused DWC3 when running this command. In order to avoid future issues, we're going to make sure values passed by macros are always safe for the controller. Note that ADB still needs a fix to *not* pass bad values. Cc: # v3.2+ Reported-by: Mohamed Abbas Sugested-by: Adam Andruszak Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h index 3129bcf74d7d..265e223ab645 100644 --- a/drivers/usb/dwc3/gadget.h +++ b/drivers/usb/dwc3/gadget.h @@ -28,23 +28,23 @@ struct dwc3; #define gadget_to_dwc(g) (container_of(g, struct dwc3, gadget)) /* DEPCFG parameter 1 */ -#define DWC3_DEPCFG_INT_NUM(n) ((n) << 0) +#define DWC3_DEPCFG_INT_NUM(n) (((n) & 0x1f) << 0) #define DWC3_DEPCFG_XFER_COMPLETE_EN (1 << 8) #define DWC3_DEPCFG_XFER_IN_PROGRESS_EN (1 << 9) #define DWC3_DEPCFG_XFER_NOT_READY_EN (1 << 10) #define DWC3_DEPCFG_FIFO_ERROR_EN (1 << 11) #define DWC3_DEPCFG_STREAM_EVENT_EN (1 << 13) -#define DWC3_DEPCFG_BINTERVAL_M1(n) ((n) << 16) +#define DWC3_DEPCFG_BINTERVAL_M1(n) (((n) & 0xff) << 16) #define DWC3_DEPCFG_STREAM_CAPABLE (1 << 24) -#define DWC3_DEPCFG_EP_NUMBER(n) ((n) << 25) +#define DWC3_DEPCFG_EP_NUMBER(n) (((n) & 0x1f) << 25) #define DWC3_DEPCFG_BULK_BASED (1 << 30) #define DWC3_DEPCFG_FIFO_BASED (1 << 31) /* DEPCFG parameter 0 */ -#define DWC3_DEPCFG_EP_TYPE(n) ((n) << 1) -#define DWC3_DEPCFG_MAX_PACKET_SIZE(n) ((n) << 3) -#define DWC3_DEPCFG_FIFO_NUMBER(n) ((n) << 17) -#define DWC3_DEPCFG_BURST_SIZE(n) ((n) << 22) +#define DWC3_DEPCFG_EP_TYPE(n) (((n) & 0x3) << 1) +#define DWC3_DEPCFG_MAX_PACKET_SIZE(n) (((n) & 0x7ff) << 3) +#define DWC3_DEPCFG_FIFO_NUMBER(n) (((n) & 0x1f) << 17) +#define DWC3_DEPCFG_BURST_SIZE(n) (((n) & 0xf) << 22) #define DWC3_DEPCFG_DATA_SEQ_NUM(n) ((n) << 26) /* This applies for core versions earlier than 1.94a */ #define DWC3_DEPCFG_IGN_SEQ_NUM (1 << 31) -- cgit v1.2.3 From 2bfa0719ac2a9b2f3c91345873d3cdebd0296ba9 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Tue, 31 Jan 2017 14:54:45 +0200 Subject: usb: gadget: function: f_fs: pass companion descriptor along If we're dealing with SuperSpeed endpoints, we need to make sure to pass along the companion descriptor and initialize fields needed by the Gadget API. Eventually, f_fs.c should be converted to use config_ep_by_speed() like all other functions, though. Cc: Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_fs.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index a5b7cd615698..7e976f00cb02 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1834,11 +1834,14 @@ static int ffs_func_eps_enable(struct ffs_function *func) spin_lock_irqsave(&func->ffs->eps_lock, flags); while(count--) { struct usb_endpoint_descriptor *ds; + struct usb_ss_ep_comp_descriptor *comp_desc = NULL; + int needs_comp_desc = false; int desc_idx; - if (ffs->gadget->speed == USB_SPEED_SUPER) + if (ffs->gadget->speed == USB_SPEED_SUPER) { desc_idx = 2; - else if (ffs->gadget->speed == USB_SPEED_HIGH) + needs_comp_desc = true; + } else if (ffs->gadget->speed == USB_SPEED_HIGH) desc_idx = 1; else desc_idx = 0; @@ -1855,6 +1858,14 @@ static int ffs_func_eps_enable(struct ffs_function *func) ep->ep->driver_data = ep; ep->ep->desc = ds; + + comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds + + USB_DT_ENDPOINT_SIZE); + ep->ep->maxburst = comp_desc->bMaxBurst + 1; + + if (needs_comp_desc) + ep->ep->comp_desc = comp_desc; + ret = usb_ep_enable(ep->ep); if (likely(!ret)) { epfile->ep = ep; -- cgit v1.2.3 From cf3113d893d4427b166ec8695460efa7aa660923 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 17 Feb 2017 11:12:44 +0200 Subject: usb: dwc3: gadget: properly increment dequeue pointer on ep_dequeue If request was already started, this means we had to stop the transfer. With that we also need to ignore all TRBs used by the request, however TRBs can only be modified after completion of END_TRANSFER command. So what we have to do here is wait for END_TRANSFER completion and only after that jump over TRBs by clearing HWO and incrementing dequeue pointer. Note that we have 2 possible types of transfers here: i) Linear buffer request ii) SG-list based request SG-list based requests will have r->num_pending_sgs set to a valid number (> 0). Linear requests, normally use a single TRB. For each of these two cases, if r->unaligned flag is set, one extra TRB has been used to align transfer size to wMaxPacketSize. All of these cases need to be taken into consideration so we don't mess up our TRB ring pointers. Tested-by: Janusz Dziedzic Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 4db97ecae885..f875b3f4b0ec 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1342,6 +1342,68 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (r == req) { /* wait until it is processed */ dwc3_stop_active_transfer(dwc, dep->number, true); + + /* + * If request was already started, this means we had to + * stop the transfer. With that we also need to ignore + * all TRBs used by the request, however TRBs can only + * be modified after completion of END_TRANSFER + * command. So what we do here is that we wait for + * END_TRANSFER completion and only after that, we jump + * over TRBs by clearing HWO and incrementing dequeue + * pointer. + * + * Note that we have 2 possible types of transfers here: + * + * i) Linear buffer request + * ii) SG-list based request + * + * SG-list based requests will have r->num_pending_sgs + * set to a valid number (> 0). Linear requests, + * normally use a single TRB. + * + * For each of these two cases, if r->unaligned flag is + * set, one extra TRB has been used to align transfer + * size to wMaxPacketSize. + * + * All of these cases need to be taken into + * consideration so we don't mess up our TRB ring + * pointers. + */ + wait_event_lock_irq(dep->wait_end_transfer, + !(dep->flags & DWC3_EP_END_TRANSFER_PENDING), + dwc->lock); + + if (!r->trb) + goto out1; + + if (r->num_pending_sgs) { + struct dwc3_trb *trb; + int i = 0; + + for (i = 0; i < r->num_pending_sgs; i++) { + trb = r->trb + i; + trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_deq(dep); + } + + if (r->unaligned) { + trb = r->trb + r->num_pending_sgs + 1; + trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_deq(dep); + } + } else { + struct dwc3_trb *trb = r->trb; + + trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_deq(dep); + + if (r->unaligned) { + trb = r->trb + 1; + trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_deq(dep); + } + } goto out1; } dev_err(dwc->dev, "request %p was not queued to %s\n", @@ -1352,6 +1414,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, out1: /* giveback the request */ + dep->queued_requests--; dwc3_gadget_giveback(dep, req, -ECONNRESET); out0: -- cgit v1.2.3 From eb38d913c27f32f4df173791051fecf6aca34173 Mon Sep 17 00:00:00 2001 From: Roger Quadros Date: Thu, 2 Mar 2017 10:44:58 +0200 Subject: Revert "usb: gadget: uvc: Add missing call for additional setup data" This reverts commit 4fbac5206afd01b717d4bdc58793d471f3391b4b. This commit breaks g_webcam when used with uvc-gadget [1]. The user space application (e.g. uvc-gadget) is responsible for sending response to UVC class specific requests on control endpoint in uvc_send_response() in uvc_v4l2.c. The bad commit was causing a duplicate response to be sent with incorrect response data thus causing UVC probe to fail at the host and broken control transfer endpoint at the gadget. [1] - git://git.ideasonboard.org/uvc-gadget.git Cc: # v4.9+ Acked-by: Laurent Pinchart Signed-off-by: Roger Quadros Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_uvc.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 27ed51b5082f..29b41b5dee04 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -258,13 +258,6 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req)); v4l2_event_queue(&uvc->vdev, &v4l2_event); - /* Pass additional setup data to userspace */ - if (uvc->event_setup_out && uvc->event_length) { - uvc->control_req->length = uvc->event_length; - return usb_ep_queue(uvc->func.config->cdev->gadget->ep0, - uvc->control_req, GFP_ATOMIC); - } - return 0; } -- cgit v1.2.3 From 5bbc852676ae08e818241cf66a3ffe4be44225c4 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 28 Feb 2017 14:25:45 +0800 Subject: usb: gadget: dummy_hcd: clear usb_gadget region before registration When the user does device unbind and rebind test, the kernel will show below dump due to usb_gadget memory region is dirty after unbind. Clear usb_gadget region for every new probe. root@imx6qdlsolo:/sys/bus/platform/drivers/dummy_udc# echo dummy_udc.0 > bind [ 102.523312] kobject (eddd78b0): tried to init an initialized object, something is seriously wrong. [ 102.532447] CPU: 0 PID: 734 Comm: sh Not tainted 4.10.0-rc7-00872-g1b2b8e9 #1298 [ 102.539866] Hardware name: Freescale i.MX6 SoloX (Device Tree) [ 102.545717] Backtrace: [ 102.548225] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 102.555822] r7:ede34000 r6:60010013 r5:00000000 r4:c0f29418 [ 102.561512] [] (show_stack) from [] (dump_stack+0xb4/0xe8) [ 102.568764] [] (dump_stack) from [] (kobject_init+0x80/0x9c) [ 102.576187] r10:0000001f r9:eddd7000 r8:eeaf8c10 r7:eddd78a8 r6:c177891c r5:c0f3b060 [ 102.584036] r4:eddd78b0 r3:00000000 [ 102.587641] [] (kobject_init) from [] (device_initialize+0x28/0xf8) [ 102.595665] r5:eebc4800 r4:eddd78a8 [ 102.599268] [] (device_initialize) from [] (device_register+0x14/0x20) [ 102.607556] r7:eddd78a8 r6:00000000 r5:eebc4800 r4:eddd78a8 [ 102.613256] [] (device_register) from [] (usb_add_gadget_udc_release+0x8c/0x1ec) [ 102.622410] r5:eebc4800 r4:eddd7860 [ 102.626015] [] (usb_add_gadget_udc_release) from [] (usb_add_gadget_udc+0x14/0x18) [ 102.635351] r10:0000001f r9:eddd7000 r8:eddd788c r7:bf003770 r6:eddd77f8 r5:eddd7818 [ 102.643198] r4:eddd785c r3:eddd7b24 [ 102.646834] [] (usb_add_gadget_udc) from [] (dummy_udc_probe+0x170/0x1c4 [dummy_hcd]) [ 102.656458] [] (dummy_udc_probe [dummy_hcd]) from [] (platform_drv_probe+0x54/0xb8) [ 102.665881] r10:00000008 r9:c1778960 r8:bf004128 r7:fffffdfb r6:bf004128 r5:eeaf8c10 [ 102.673727] r4:eeaf8c10 [ 102.676293] [] (platform_drv_probe) from [] (driver_probe_device+0x264/0x474) [ 102.685186] r7:00000000 r6:00000000 r5:c1778960 r4:eeaf8c10 [ 102.690876] [] (driver_probe_device) from [] (bind_store+0xb8/0x14c) [ 102.698994] r10:eeb3bb4c r9:ede34000 r8:0000000c r7:eeaf8c44 r6:bf004128 r5:c0f3b668 [ 102.706840] r4:eeaf8c10 [ 102.709402] [] (bind_store) from [] (drv_attr_store+0x28/0x34) [ 102.716998] r9:ede34000 r8:00000000 r7:ee3863c0 r6:ee3863c0 r5:c0538c80 r4:c053970c [ 102.724776] [] (drv_attr_store) from [] (sysfs_kf_write+0x50/0x54) [ 102.732711] r5:c0538c80 r4:0000000c [ 102.736313] [] (sysfs_kf_write) from [] (kernfs_fop_write+0x100/0x214) [ 102.744599] r7:ee3863c0 r6:eeb3bb40 r5:00000000 r4:00000000 [ 102.750287] [] (kernfs_fop_write) from [] (__vfs_write+0x34/0x120) [ 102.758231] r10:00000000 r9:ede34000 r8:c0108bc4 r7:0000000c r6:ede35f80 r5:c029bd84 [ 102.766077] r4:ee223780 [ 102.768638] [] (__vfs_write) from [] (vfs_write+0xa8/0x170) [ 102.775974] r9:ede34000 r8:c0108bc4 r7:ede35f80 r6:01861cb0 r5:ee223780 r4:0000000c [ 102.783743] [] (vfs_write) from [] (SyS_write+0x4c/0xa8) [ 102.790818] r9:ede34000 r8:c0108bc4 r7:0000000c r6:01861cb0 r5:ee223780 r4:ee223780 [ 102.798595] [] (SyS_write) from [] (ret_fast_syscall+0x0/0x1c) [ 102.806188] r7:00000004 r6:b6e83d58 r5:01861cb0 r4:0000000c Fixes: 90fccb529d24 ("usb: gadget: Gadget directory cleanup - group UDC drivers") Cc: stable Acked-by: Alan Stern Signed-off-by: Peter Chen Tested-by: Xiaolong Ye Reported-by: Fengguang Wu Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/dummy_hcd.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c index c60abe3a68f9..8cabc5944d5f 100644 --- a/drivers/usb/gadget/udc/dummy_hcd.c +++ b/drivers/usb/gadget/udc/dummy_hcd.c @@ -1031,6 +1031,8 @@ static int dummy_udc_probe(struct platform_device *pdev) int rc; dum = *((void **)dev_get_platdata(&pdev->dev)); + /* Clear usb_gadget region for new registration to udc-core */ + memzero_explicit(&dum->gadget, sizeof(struct usb_gadget)); dum->gadget.name = gadget_name; dum->gadget.ops = &dummy_ops; dum->gadget.max_speed = USB_SPEED_SUPER; -- cgit v1.2.3 From 38355b2a44776c25b0f2ad466e8c51bb805b3032 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Tue, 28 Feb 2017 10:55:30 +0000 Subject: usb: gadget: configs: plug memory leak When binding a gadget to a device, "name" is stored in gi->udc_name, but this does not happen when unregistering and the string is leaked. Signed-off-by: John Keeping Signed-off-by: Felipe Balbi --- drivers/usb/gadget/configfs.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 78c44979dde3..cbff3b02840d 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -269,6 +269,7 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, ret = unregister_gadget(gi); if (ret) goto err; + kfree(name); } else { if (gi->composite.gadget_driver.udc_name) { ret = -EBUSY; -- cgit v1.2.3 From 73561128eb72ca04c3be6e240f162a861bf68a86 Mon Sep 17 00:00:00 2001 From: Franck Demathieu Date: Mon, 27 Feb 2017 11:52:46 +0100 Subject: usb: dwc3: Fix incorrect type for utmi mode The utmi mode is unsigned according the dt-bindings. Fix sparse issue (-Wtypesign): drivers/usb/dwc3/dwc3-omap.c:391:50: warning: incorrect type in argument 3 (different signedness) drivers/usb/dwc3/dwc3-omap.c:391:50: expected unsigned int [usertype] *out_value drivers/usb/dwc3/dwc3-omap.c:391:50: got int * Signed-off-by: Franck Demathieu Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/dwc3-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 2092e46b1380..4a595777969e 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -392,7 +392,7 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap) { u32 reg; struct device_node *node = omap->dev->of_node; - int utmi_mode = 0; + u32 utmi_mode = 0; reg = dwc3_omap_read_utmi_ctrl(omap); -- cgit v1.2.3 From 4242820277b5378c9e4064e79306f326d731472f Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 22 Feb 2017 11:33:27 +0100 Subject: usb: gadget: udc: atmel: fix debug output The debug output now contains the wrong variable, as seen from the compiler warning: drivers/usb/gadget/udc/atmel_usba_udc.c: In function 'usba_ep_enable': drivers/usb/gadget/udc/atmel_usba_udc.c:632:550: error: 'ept_cfg' may be used uninitialized in this function [-Werror=maybe-uninitialized] DBG(DBG_ERR, "%s: EPT_CFG = 0x%lx (maxpacket = %lu)\n", This changes the debug output the same way as the other code. Fixes: 741d2558bf0a ("usb: gadget: udc: atmel: Update endpoint allocation scheme") Signed-off-by: Arnd Bergmann Acked-by: Alexandre Belloni Acked-by: Nicolas Ferre Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index 11bbce28bc23..2035906b8ced 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -610,7 +610,7 @@ usba_ep_enable(struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc) { struct usba_ep *ep = to_usba_ep(_ep); struct usba_udc *udc = ep->udc; - unsigned long flags, ept_cfg, maxpacket; + unsigned long flags, maxpacket; unsigned int nr_trans; DBG(DBG_GADGET, "%s: ep_enable: desc=%p\n", ep->ep.name, desc); @@ -630,7 +630,7 @@ usba_ep_enable(struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc) ep->is_in = 0; DBG(DBG_ERR, "%s: EPT_CFG = 0x%lx (maxpacket = %lu)\n", - ep->ep.name, ept_cfg, maxpacket); + ep->ep.name, ep->ept_cfg, maxpacket); if (usb_endpoint_dir_in(desc)) { ep->is_in = 1; -- cgit v1.2.3 From b6e7aeeaf235901c42ec35de4633c7c69501d303 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 21 Feb 2017 22:33:11 +0100 Subject: USB: gadgetfs: Fix a potential memory leak in 'dev_config()' 'kbuf' is allocated just a few lines above using 'memdup_user()'. If the 'if (dev->buf)' test fails, this memory is never released. Signed-off-by: Christophe JAILLET Signed-off-by: Felipe Balbi --- drivers/usb/gadget/legacy/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index a2615d64d07c..0513dfa008e6 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -1782,8 +1782,10 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) spin_lock_irq (&dev->lock); value = -EINVAL; - if (dev->buf) + if (dev->buf) { + kfree(kbuf); goto fail; + } dev->buf = kbuf; /* full or low speed config */ -- cgit v1.2.3 From 3ba534df815f233535b5a3dd3de41055666bbd21 Mon Sep 17 00:00:00 2001 From: Janusz Dziedzic Date: Thu, 16 Feb 2017 14:54:12 +0100 Subject: Revert "usb: gadget: f_fs: Fix ExtCompat descriptor validation" This reverts commit ac670a3a650b899fc020b81f63e810d06015b865. This introduce bug we already fixed in commit 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 wof OS_DESC_EXT_COMPAT") Next FFS (adb) SS enumeration fail with Windows OS. Signed-off-by: Janusz Dziedzic Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 7e976f00cb02..a0085571824d 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -2264,7 +2264,7 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type type, if (len < sizeof(*d) || d->bFirstInterfaceNumber >= ffs->interfaces_count || - d->Reserved1) + !d->Reserved1) return -EINVAL; for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i) if (d->Reserved2[i]) -- cgit v1.2.3 From 1551e35ea4189c1f7199fe278395fc94196715f2 Mon Sep 17 00:00:00 2001 From: Roger Quadros Date: Wed, 15 Feb 2017 14:16:26 +0200 Subject: usb: dwc3: gadget: Fix system suspend/resume on TI platforms On TI platforms (dra7, am437x), the DWC3_DSTS_DEVCTRLHLT bit is not set after the device controller is stopped via DWC3_DCTL_RUN_STOP. If we don't disconnect and stop the gadget, it stops working after a system resume with the trace below. There is no point in preventing gadget disconnect and gadget stop during system suspend/resume as we're going to suspend in any case, whether DEVCTRLHLT timed out or not. [ 141.727480] ------------[ cut here ]------------ [ 141.732349] WARNING: CPU: 1 PID: 2135 at drivers/usb/dwc3/gadget.c:2384 dwc3_stop_active_transfer.constprop.4+0xc4/0xe4 [dwc3] [ 141.744299] Modules linked in: usb_f_ss_lb g_zero libcomposite xhci_plat_hcd xhci_hcd usbcore dwc3 evdev udc_core m25p80 usb_common spi_nor snd_soc_davinci_mcasp snd_soc_simple_card snd_soc_edma snd_soc_tlv3e [ 141.792163] CPU: 1 PID: 2135 Comm: irq/456-dwc3 Not tainted 4.10.0-rc8 #1138 [ 141.799547] Hardware name: Generic DRA74X (Flattened Device Tree) [ 141.805940] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 141.814066] [] (show_stack) from [] (dump_stack+0xac/0xe0) [ 141.821648] [] (dump_stack) from [] (__warn+0xd8/0x104) [ 141.828955] [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [ 141.836902] [] (warn_slowpath_null) from [] (dwc3_stop_active_transfer.constprop.4+0xc4/0xe4 [dwc3]) [ 141.848329] [] (dwc3_stop_active_transfer.constprop.4 [dwc3]) from [] (__dwc3_gadget_ep_disable+0x64/0x528 [dwc3]) [ 141.861034] [] (__dwc3_gadget_ep_disable [dwc3]) from [] (dwc3_gadget_ep_disable+0x3c/0xc8 [dwc3]) [ 141.872280] [] (dwc3_gadget_ep_disable [dwc3]) from [] (usb_ep_disable+0x11c/0x18c [udc_core]) [ 141.883160] [] (usb_ep_disable [udc_core]) from [] (disable_ep+0x18/0x54 [usb_f_ss_lb]) [ 141.893408] [] (disable_ep [usb_f_ss_lb]) from [] (disable_endpoints+0x18/0x50 [usb_f_ss_lb]) [ 141.904168] [] (disable_endpoints [usb_f_ss_lb]) from [] (disable_source_sink+0x2c/0x34 [usb_f_ss_lb]) [ 141.915771] [] (disable_source_sink [usb_f_ss_lb]) from [] (reset_config+0x48/0x7c [libcomposite]) [ 141.927012] [] (reset_config [libcomposite]) from [] (composite_disconnect+0x2c/0x54 [libcomposite]) [ 141.938444] [] (composite_disconnect [libcomposite]) from [] (usb_gadget_udc_reset+0x10/0x34 [udc_core]) [ 141.950237] [] (usb_gadget_udc_reset [udc_core]) from [] (dwc3_gadget_reset_interrupt+0x64/0x698 [dwc3]) [ 141.962022] [] (dwc3_gadget_reset_interrupt [dwc3]) from [] (dwc3_thread_interrupt+0x618/0x1a3c [dwc3]) [ 141.973723] [] (dwc3_thread_interrupt [dwc3]) from [] (irq_thread_fn+0x1c/0x54) [ 141.983215] [] (irq_thread_fn) from [] (irq_thread+0x120/0x1f0) [ 141.991247] [] (irq_thread) from [] (kthread+0xf8/0x138) [ 141.998641] [] (kthread) from [] (ret_from_fork+0x14/0x24) [ 142.006213] ---[ end trace b4ecfe9f175b9a9c ]--- Signed-off-by: Roger Quadros Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f875b3f4b0ec..3db5eeae2bfb 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3291,15 +3291,10 @@ void dwc3_gadget_exit(struct dwc3 *dwc) int dwc3_gadget_suspend(struct dwc3 *dwc) { - int ret; - if (!dwc->gadget_driver) return 0; - ret = dwc3_gadget_run_stop(dwc, false, false); - if (ret < 0) - return ret; - + dwc3_gadget_run_stop(dwc, false, false); dwc3_disconnect_gadget(dwc); __dwc3_gadget_stop(dwc); -- cgit v1.2.3 From 0913750f9fb6f26bcd00c8f9dd9a8d1b8d031246 Mon Sep 17 00:00:00 2001 From: Roger Quadros Date: Wed, 15 Feb 2017 13:38:22 +0200 Subject: usb: dwc3-omap: Fix missing break in dwc3_omap_set_mailbox() We need to break from all cases if we want to treat each one of them separately. Reported-by: Gustavo A. R. Silva Fixes: d2728fb3e01f ("usb: dwc3: omap: Pass VBUS and ID events transparently") Cc: #v4.8+ Signed-off-by: Roger Quadros Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/dwc3-omap.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 4a595777969e..f8d0747810e7 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -250,6 +250,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, val = dwc3_omap_read_utmi_ctrl(omap); val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG; dwc3_omap_write_utmi_ctrl(omap, val); + break; case OMAP_DWC3_VBUS_OFF: val = dwc3_omap_read_utmi_ctrl(omap); -- cgit v1.2.3 From df7545719a14fa7b481896fb8689e23d0a00f682 Mon Sep 17 00:00:00 2001 From: Petr Cvek Date: Fri, 24 Feb 2017 02:54:56 +0100 Subject: usb: gadget: pxa27x: Test for a valid argument pointer A call usb_put_phy(udc->transceiver) must be tested for a valid pointer. Use an already existing test for usb_unregister_notifier call. Acked-by: Robert Jarzmik Reported-by: Robert Jarzmik Signed-off-by: Petr Cvek Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/pxa27x_udc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c index e1335ad5bce9..832c4fdbe985 100644 --- a/drivers/usb/gadget/udc/pxa27x_udc.c +++ b/drivers/usb/gadget/udc/pxa27x_udc.c @@ -2534,9 +2534,10 @@ static int pxa_udc_remove(struct platform_device *_dev) usb_del_gadget_udc(&udc->gadget); pxa_cleanup_debugfs(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) + if (!IS_ERR_OR_NULL(udc->transceiver)) { usb_unregister_notifier(udc->transceiver, &pxa27x_udc_phy); - usb_put_phy(udc->transceiver); + usb_put_phy(udc->transceiver); + } udc->transceiver = NULL; the_controller = NULL; -- cgit v1.2.3 From ef5e2fa9f65befa12f1113c734602d2c1964d2a5 Mon Sep 17 00:00:00 2001 From: Raz Manor Date: Thu, 9 Feb 2017 09:41:08 +0200 Subject: usb: gadget: udc: net2280: Fix tmp reusage in net2280 driver In the function scan_dma_completions() there is a reusage of tmp variable. That coused a wrong value being used in some case when reading a short packet terminated transaction from an endpoint, in 2 concecutive reads. This was my logic for the patch: The req->td->dmadesc equals to 0 iff: -- There was a transaction ending with a short packet, and -- The read() to read it was shorter than the transaction length, and -- The read() to complete it is longer than the residue. I believe this is true from the printouts of various cases, but I can't be positive it is correct. Entering this if, there should be no more data in the endpoint (a short packet terminated the transaction). If there is, the transaction wasn't really done and we should exit and wait for it to finish entirely. That is the inner if. That inner if should never happen, but it is there to be on the safe side. That is why it is marked with the comment /* paranoia */. The size of the data available in the endpoint is ep->dma->dmacount and it is read to tmp. This entire clause is based on my own educated guesses. If we passed that inner if without breaking in the original code, than tmp & DMA_BYTE_MASK_COUNT== 0. That means we will always pass dma bytes count of 0 to dma_done(), meaning all the requested bytes were read. dma_done() reports back to the upper layer that the request (read()) was done and how many bytes were read. In the original code that would always be the request size, regardless of the actual size of the data. That did not make sense to me at all. However, the original value of tmp is req->td->dmacount, which is the dmacount value when the request's dma transaction was finished. And that is a much more reasonable value to report back to the caller. To recreate the problem: Read from a bulk out endpoint in a loop, 1024 * n bytes in each iteration. Connect the PLX to a host you can control. Send to that endpoint 1024 * n + x bytes, such that 0 < x < 1024 * n and (x % 1024) != 0 You would expect the first read() to return 1024 * n and the second read() to return x. But you will get the first read to return 1024 * n and the second one to return 1024 * n. That is true for every positive integer n. Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Signed-off-by: Raz Manor Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/net2280.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c index 85504419ab31..3828c2ec8623 100644 --- a/drivers/usb/gadget/udc/net2280.c +++ b/drivers/usb/gadget/udc/net2280.c @@ -1146,15 +1146,15 @@ static int scan_dma_completions(struct net2280_ep *ep) */ while (!list_empty(&ep->queue)) { struct net2280_request *req; - u32 tmp; + u32 req_dma_count; req = list_entry(ep->queue.next, struct net2280_request, queue); if (!req->valid) break; rmb(); - tmp = le32_to_cpup(&req->td->dmacount); - if ((tmp & BIT(VALID_BIT)) != 0) + req_dma_count = le32_to_cpup(&req->td->dmacount); + if ((req_dma_count & BIT(VALID_BIT)) != 0) break; /* SHORT_PACKET_TRANSFERRED_INTERRUPT handles "usb-short" @@ -1163,40 +1163,41 @@ static int scan_dma_completions(struct net2280_ep *ep) */ if (unlikely(req->td->dmadesc == 0)) { /* paranoia */ - tmp = readl(&ep->dma->dmacount); - if (tmp & DMA_BYTE_COUNT_MASK) + u32 const ep_dmacount = readl(&ep->dma->dmacount); + + if (ep_dmacount & DMA_BYTE_COUNT_MASK) break; /* single transfer mode */ - dma_done(ep, req, tmp, 0); + dma_done(ep, req, req_dma_count, 0); num_completed++; break; } else if (!ep->is_in && (req->req.length % ep->ep.maxpacket) && !(ep->dev->quirks & PLX_PCIE)) { - tmp = readl(&ep->regs->ep_stat); + u32 const ep_stat = readl(&ep->regs->ep_stat); /* AVOID TROUBLE HERE by not issuing short reads from * your gadget driver. That helps avoids errata 0121, * 0122, and 0124; not all cases trigger the warning. */ - if ((tmp & BIT(NAK_OUT_PACKETS)) == 0) { + if ((ep_stat & BIT(NAK_OUT_PACKETS)) == 0) { ep_warn(ep->dev, "%s lost packet sync!\n", ep->ep.name); req->req.status = -EOVERFLOW; } else { - tmp = readl(&ep->regs->ep_avail); - if (tmp) { + u32 const ep_avail = readl(&ep->regs->ep_avail); + if (ep_avail) { /* fifo gets flushed later */ ep->out_overflow = 1; ep_dbg(ep->dev, "%s dma, discard %d len %d\n", - ep->ep.name, tmp, + ep->ep.name, ep_avail, req->req.length); req->req.status = -EOVERFLOW; } } } - dma_done(ep, req, tmp, 0); + dma_done(ep, req, req_dma_count, 0); num_completed++; } -- cgit v1.2.3 From 35b2719e72d375f3e32c819858165668d948d5f2 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Wed, 8 Mar 2017 13:56:37 +0200 Subject: usb: dwc3: gadget: make to increment req->remaining in all cases Sometimes, we might get a completion for a TRB which is left with HWO bit. Even in these cases, we should increment req->remaining to properly report total transferred size. I noticed this while debuggin a separate problem seen with MSC tests from USBCV. Sometimes we would erroneously report a completion for a 512-byte transfer when, in reality, we transferred 0 bytes. Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 3db5eeae2bfb..0d75158e43fe 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2189,12 +2189,12 @@ static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep *dep, return 1; } - if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN) - return 1; - count = trb->size & DWC3_TRB_SIZE_MASK; req->remaining += count; + if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN) + return 1; + if (dep->direction) { if (count) { trb_status = DWC3_TRB_SIZE_TRBSTS(trb->size); -- cgit v1.2.3