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/dwc3') 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 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/dwc3') 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 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/dwc3') 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 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/dwc3') 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 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/dwc3') 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 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/dwc3') 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 From de288e36fe33f7e06fa272bc8e2f85aa386d99aa Mon Sep 17 00:00:00 2001 From: Janusz Dziedzic Date: Mon, 13 Mar 2017 14:11:32 +0200 Subject: usb: dwc3: gadget: delay unmap of bounced requests In the case of bounced ep0 requests, we must delay DMA operation until after ->complete() otherwise we might overwrite contents of req->buf. This caused problems with RNDIS gadget. Signed-off-by: Janusz Dziedzic Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'drivers/usb/dwc3') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 0d75158e43fe..79e7a3480d51 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -171,6 +171,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req, int status) { struct dwc3 *dwc = dep->dwc; + unsigned int unmap_after_complete = false; req->started = false; list_del(&req->list); @@ -180,11 +181,19 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req, if (req->request.status == -EINPROGRESS) req->request.status = status; - if (dwc->ep0_bounced && dep->number <= 1) + /* + * NOTICE we don't want to unmap before calling ->complete() if we're + * dealing with a bounced ep0 request. If we unmap it here, we would end + * up overwritting the contents of req->buf and this could confuse the + * gadget driver. + */ + if (dwc->ep0_bounced && dep->number <= 1) { dwc->ep0_bounced = false; - - usb_gadget_unmap_request_by_dev(dwc->sysdev, - &req->request, req->direction); + unmap_after_complete = true; + } else { + usb_gadget_unmap_request_by_dev(dwc->sysdev, + &req->request, req->direction); + } trace_dwc3_gadget_giveback(req); @@ -192,6 +201,10 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req, usb_gadget_giveback_request(&dep->endpoint, &req->request); spin_lock(&dwc->lock); + if (unmap_after_complete) + usb_gadget_unmap_request_by_dev(dwc->sysdev, + &req->request, req->direction); + if (dep->number > 1) pm_runtime_put(dwc->dev); } -- cgit