From bbe097f092b0d13e9736bd2794d0ab24547d0e5d Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Thu, 15 Sep 2016 17:07:22 +0200 Subject: [PATCH 01/11] usb: gadget: udc: atmel: fix endpoint name Since commit c32b5bcfa3c4 ("ARM: dts: at91: Fix USB endpoint nodes"), atmel_usba_udc fails with: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 0 at include/linux/usb/gadget.h:405 ecm_do_notify+0x188/0x1a0 Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0+ #15 Hardware name: Atmel SAMA5 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xe4/0xfc) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (ecm_do_notify+0x188/0x1a0) [] (ecm_do_notify) from [] (ecm_set_alt+0x74/0x1ac) [] (ecm_set_alt) from [] (composite_setup+0xfc0/0x19f8) [] (composite_setup) from [] (usba_udc_irq+0x8f4/0xd9c) [] (usba_udc_irq) from [] (handle_irq_event_percpu+0x9c/0x158) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x28/0x3c) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xa0/0x168) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x24/0x34) [] (generic_handle_irq) from [] (__handle_domain_irq+0x54/0xa8) [] (__handle_domain_irq) from [] (__irq_svc+0x54/0x70) [] (__irq_svc) from [] (arch_cpu_idle+0x38/0x3c) [] (arch_cpu_idle) from [] (cpu_startup_entry+0x9c/0xdc) [] (cpu_startup_entry) from [] (start_kernel+0x354/0x360) [] (start_kernel) from [<20008078>] (0x20008078) ---[ end trace e7cf9dcebf4815a6 ]--- Fixes: c32b5bcfa3c4 ("ARM: dts: at91: Fix USB endpoint nodes") Cc: Reported-by: Richard Genoud Acked-by: Nicolas Ferre Signed-off-by: Alexandre Belloni Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/atmel_usba_udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index bb1f6c8f0f01..45bc997d0711 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -1978,7 +1978,7 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev, dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret); goto err; } - ep->ep.name = name; + ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index); ep->ep_regs = udc->regs + USBA_EPT_BASE(i); ep->dma_regs = udc->regs + USBA_DMA_BASE(i); From 6c83f77278f17a7679001027e9231291c20f0d8a Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Tue, 4 Oct 2016 15:14:43 +0300 Subject: [PATCH 02/11] usb: gadget: function: u_ether: don't starve tx request queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we don't guarantee that we will always get an interrupt at least when we're queueing our very last request, we could fall into situation where we queue every request with 'no_interrupt' set. This will cause the link to get stuck. The behavior above has been triggered with g_ether and dwc3. Cc: Reported-by: Ville Syrjälä Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/u_ether.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 9c8c9ed1dc9e..fe1811650dbc 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -590,8 +590,9 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb, /* throttle high/super speed IRQ rate back slightly */ if (gadget_is_dualspeed(dev->gadget)) - req->no_interrupt = (dev->gadget->speed == USB_SPEED_HIGH || - dev->gadget->speed == USB_SPEED_SUPER) + req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH || + dev->gadget->speed == USB_SPEED_SUPER)) && + !list_empty(&dev->tx_reqs)) ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0) : 0; From a9c3ca5fae6bf73770f0576eaf57d5f1305ef4b3 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Wed, 5 Oct 2016 14:24:37 +0300 Subject: [PATCH 03/11] usb: dwc3: gadget: properly account queued requests Some requests could be accounted for multiple times. Let's fix that so each and every requests is accounted for only once. Cc: # v4.8 Fixes: 55a0237f8f47 ("usb: dwc3: gadget: use allocated/queued reqs for LST bit") Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 07cc8929f271..3c3ced128c77 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -783,6 +783,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep, req->trb = trb; req->trb_dma = dwc3_trb_dma_offset(dep, trb); req->first_trb_index = dep->trb_enqueue; + dep->queued_requests++; } dwc3_ep_inc_enq(dep); @@ -833,8 +834,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep, trb->ctrl |= DWC3_TRB_CTRL_HWO; - dep->queued_requests++; - trace_dwc3_prepare_trb(dep, trb); } @@ -1861,8 +1860,11 @@ static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep *dep, unsigned int s_pkt = 0; unsigned int trb_status; - dep->queued_requests--; dwc3_ep_inc_deq(dep); + + if (req->trb == trb) + dep->queued_requests--; + trace_dwc3_complete_trb(dep, trb); /* From d889c23ce4e3159e3d737f55f9d686a030a7ba87 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Thu, 29 Sep 2016 15:44:29 +0300 Subject: [PATCH 04/11] usb: dwc3: gadget: never pre-start Isochronous endpoints We cannot pre-start isochronous endpoints because we rely on the micro-frame number passed via XferNotReady command for proper Isochronous scheduling. Fixes: 08a36b543803 ("usb: dwc3: gadget: simplify __dwc3_gadget_ep_queue()") Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 3c3ced128c77..f15147f79d14 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1073,9 +1073,17 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req) list_add_tail(&req->list, &dep->pending_list); - if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && - dep->flags & DWC3_EP_PENDING_REQUEST) { - if (list_empty(&dep->started_list)) { + /* + * NOTICE: Isochronous endpoints should NEVER be prestarted. We must + * wait for a XferNotReady event so we will know what's the current + * (micro-)frame number. + * + * Without this trick, we are very, very likely gonna get Bus Expiry + * errors which will force us issue EndTransfer command. + */ + if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) { + if ((dep->flags & DWC3_EP_PENDING_REQUEST) && + list_empty(&dep->started_list)) { dwc3_stop_active_transfer(dwc, dep->number, true); dep->flags = DWC3_EP_ENABLED; } From a07ce8d34eb3d9c6cec3aa25f7713e6aafad2260 Mon Sep 17 00:00:00 2001 From: Heiko Stuebner Date: Fri, 14 Oct 2016 10:47:24 -0700 Subject: [PATCH 05/11] usb: dwc2: Add msleep for host-only Although a host-only controller should not have any associated delay, some rockchip SOC platforms will not show the correct host-values of registers until after a delay. So add a 50 ms sleep when in host-only mode. Signed-off-by: John Youn Signed-off-by: Heiko Stuebner Signed-off-by: Felipe Balbi --- drivers/usb/dwc2/core.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index fa9b26b91507..4c0fa0b17353 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg) */ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg) { + bool ret; + switch (hsotg->dr_mode) { case USB_DR_MODE_HOST: - dwc2_force_mode(hsotg, true); + ret = dwc2_force_mode(hsotg, true); + /* + * NOTE: This is required for some rockchip soc based + * platforms on their host-only dwc2. + */ + if (!ret) + msleep(50); + break; case USB_DR_MODE_PERIPHERAL: dwc2_force_mode(hsotg, false); From 454915dde06a51133750c6745f0ba57361ba209d Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 4 Oct 2016 02:07:33 +0200 Subject: [PATCH 06/11] usb: gadget: f_fs: edit epfile->ep under lock epfile->ep is protected by ffs->eps_lock (not epfile->mutex) so clear it while holding the spin lock. Tested-by: John Stultz Tested-by: Chen Yu Signed-off-by: Michal Nazarewicz Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_fs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 54ad100af35b..b31aa9572723 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1725,17 +1725,17 @@ static void ffs_func_eps_disable(struct ffs_function *func) unsigned long flags; do { - if (epfile) - mutex_lock(&epfile->mutex); spin_lock_irqsave(&func->ffs->eps_lock, flags); /* pending requests get nuked */ if (likely(ep->ep)) usb_ep_disable(ep->ep); ++ep; + if (epfile) + epfile->ep = NULL; spin_unlock_irqrestore(&func->ffs->eps_lock, flags); if (epfile) { - epfile->ep = NULL; + mutex_lock(&epfile->mutex); kfree(epfile->read_buffer); epfile->read_buffer = NULL; mutex_unlock(&epfile->mutex); From a9e6f83c2df199187a5248f824f31b6787ae23ae Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 4 Oct 2016 02:07:34 +0200 Subject: [PATCH 07/11] usb: gadget: f_fs: stop sleeping in ffs_func_eps_disable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ffs_func_eps_disable is called from atomic context so it cannot sleep thus cannot grab a mutex. Change the handling of epfile->read_buffer to use non-sleeping synchronisation method. Reported-by: Chen Yu Signed-off-by: Michał Nazarewicz Fixes: 9353afbbfa7b ("buffer data from ‘oversized’ OUT requests") Tested-by: John Stultz Tested-by: Chen Yu Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_fs.c | 109 ++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 16 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index b31aa9572723..e40d47d47d82 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -136,8 +136,60 @@ struct ffs_epfile { /* * Buffer for holding data from partial reads which may happen since * we’re rounding user read requests to a multiple of a max packet size. + * + * The pointer is initialised with NULL value and may be set by + * __ffs_epfile_read_data function to point to a temporary buffer. + * + * In normal operation, calls to __ffs_epfile_read_buffered will consume + * data from said buffer and eventually free it. Importantly, while the + * function is using the buffer, it sets the pointer to NULL. This is + * all right since __ffs_epfile_read_data and __ffs_epfile_read_buffered + * can never run concurrently (they are synchronised by epfile->mutex) + * so the latter will not assign a new value to the pointer. + * + * Meanwhile ffs_func_eps_disable frees the buffer (if the pointer is + * valid) and sets the pointer to READ_BUFFER_DROP value. This special + * value is crux of the synchronisation between ffs_func_eps_disable and + * __ffs_epfile_read_data. + * + * Once __ffs_epfile_read_data is about to finish it will try to set the + * pointer back to its old value (as described above), but seeing as the + * pointer is not-NULL (namely READ_BUFFER_DROP) it will instead free + * the buffer. + * + * == State transitions == + * + * • ptr == NULL: (initial state) + * ◦ __ffs_epfile_read_buffer_free: go to ptr == DROP + * ◦ __ffs_epfile_read_buffered: nop + * ◦ __ffs_epfile_read_data allocates temp buffer: go to ptr == buf + * ◦ reading finishes: n/a, not in ‘and reading’ state + * • ptr == DROP: + * ◦ __ffs_epfile_read_buffer_free: nop + * ◦ __ffs_epfile_read_buffered: go to ptr == NULL + * ◦ __ffs_epfile_read_data allocates temp buffer: free buf, nop + * ◦ reading finishes: n/a, not in ‘and reading’ state + * • ptr == buf: + * ◦ __ffs_epfile_read_buffer_free: free buf, go to ptr == DROP + * ◦ __ffs_epfile_read_buffered: go to ptr == NULL and reading + * ◦ __ffs_epfile_read_data: n/a, __ffs_epfile_read_buffered + * is always called first + * ◦ reading finishes: n/a, not in ‘and reading’ state + * • ptr == NULL and reading: + * ◦ __ffs_epfile_read_buffer_free: go to ptr == DROP and reading + * ◦ __ffs_epfile_read_buffered: n/a, mutex is held + * ◦ __ffs_epfile_read_data: n/a, mutex is held + * ◦ reading finishes and … + * … all data read: free buf, go to ptr == NULL + * … otherwise: go to ptr == buf and reading + * • ptr == DROP and reading: + * ◦ __ffs_epfile_read_buffer_free: nop + * ◦ __ffs_epfile_read_buffered: n/a, mutex is held + * ◦ __ffs_epfile_read_data: n/a, mutex is held + * ◦ reading finishes: free buf, go to ptr == DROP */ - struct ffs_buffer *read_buffer; /* P: epfile->mutex */ + struct ffs_buffer *read_buffer; +#define READ_BUFFER_DROP ((struct ffs_buffer *)ERR_PTR(-ESHUTDOWN)) char name[5]; @@ -736,25 +788,47 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep, schedule_work(&io_data->work); } +static void __ffs_epfile_read_buffer_free(struct ffs_epfile *epfile) +{ + /* + * See comment in struct ffs_epfile for full read_buffer pointer + * synchronisation story. + */ + struct ffs_buffer *buf = xchg(&epfile->read_buffer, READ_BUFFER_DROP); + if (buf && buf != READ_BUFFER_DROP) + kfree(buf); +} + /* Assumes epfile->mutex is held. */ static ssize_t __ffs_epfile_read_buffered(struct ffs_epfile *epfile, struct iov_iter *iter) { - struct ffs_buffer *buf = epfile->read_buffer; + /* + * Null out epfile->read_buffer so ffs_func_eps_disable does not free + * the buffer while we are using it. See comment in struct ffs_epfile + * for full read_buffer pointer synchronisation story. + */ + struct ffs_buffer *buf = xchg(&epfile->read_buffer, NULL); ssize_t ret; - if (!buf) + if (!buf || buf == READ_BUFFER_DROP) return 0; ret = copy_to_iter(buf->data, buf->length, iter); if (buf->length == ret) { kfree(buf); - epfile->read_buffer = NULL; - } else if (unlikely(iov_iter_count(iter))) { + return ret; + } + + if (unlikely(iov_iter_count(iter))) { ret = -EFAULT; } else { buf->length -= ret; buf->data += ret; } + + if (cmpxchg(&epfile->read_buffer, NULL, buf)) + kfree(buf); + return ret; } @@ -783,7 +857,15 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile, buf->length = data_len; buf->data = buf->storage; memcpy(buf->storage, data + ret, data_len); - epfile->read_buffer = buf; + + /* + * At this point read_buffer is NULL or READ_BUFFER_DROP (if + * ffs_func_eps_disable has been called in the meanwhile). See comment + * in struct ffs_epfile for full read_buffer pointer synchronisation + * story. + */ + if (unlikely(cmpxchg(&epfile->read_buffer, NULL, buf))) + kfree(buf); return ret; } @@ -1097,8 +1179,7 @@ ffs_epfile_release(struct inode *inode, struct file *file) ENTER(); - kfree(epfile->read_buffer); - epfile->read_buffer = NULL; + __ffs_epfile_read_buffer_free(epfile); ffs_data_closed(epfile->ffs); return 0; @@ -1724,24 +1805,20 @@ static void ffs_func_eps_disable(struct ffs_function *func) unsigned count = func->ffs->eps_count; unsigned long flags; + spin_lock_irqsave(&func->ffs->eps_lock, flags); do { - spin_lock_irqsave(&func->ffs->eps_lock, flags); /* pending requests get nuked */ if (likely(ep->ep)) usb_ep_disable(ep->ep); ++ep; - if (epfile) - epfile->ep = NULL; - spin_unlock_irqrestore(&func->ffs->eps_lock, flags); if (epfile) { - mutex_lock(&epfile->mutex); - kfree(epfile->read_buffer); - epfile->read_buffer = NULL; - mutex_unlock(&epfile->mutex); + epfile->ep = NULL; + __ffs_epfile_read_buffer_free(epfile); ++epfile; } } while (--count); + spin_unlock_irqrestore(&func->ffs->eps_lock, flags); } static int ffs_func_eps_enable(struct ffs_function *func) From 51fbc7c06c8900370c6da5fc4a4685add8fa4fb0 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Fri, 7 Oct 2016 22:12:39 +0200 Subject: [PATCH 08/11] usb: dwc3: Fix size used in dma_free_coherent() In commit 2abd9d5fa60f9 ("usb: dwc3: ep0: Add chained TRB support"), the size of the memory allocated with 'dma_alloc_coherent()' has been modified but the corresponding calls to 'dma_free_coherent()' have not been updated accordingly. This has been spotted with coccinelle, using the following script: //////////////////// @r@ expression x0, x1, y0, y1, z0, z1, t0, t1, ret; @@ * ret = dma_alloc_coherent(x0, y0, z0, t0); ... * dma_free_coherent(x1, y1, ret, t1); @script:python@ y0 << r.y0; y1 << r.y1; @@ if y1.find(y0) == -1: print "WARNING: sizes look different: '%s' vs '%s'" % (y0, y1) //////////////////// Fixes: 2abd9d5fa60f9 ("usb: dwc3: ep0: Add chained TRB support") Signed-off-by: Christophe JAILLET Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f15147f79d14..1dfa56a5f1c5 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2990,7 +2990,7 @@ err3: kfree(dwc->setup_buf); err2: - dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb), + dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb) * 2, dwc->ep0_trb, dwc->ep0_trb_addr); err1: @@ -3015,7 +3015,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) kfree(dwc->setup_buf); kfree(dwc->zlp_buf); - dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb), + dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb) * 2, dwc->ep0_trb, dwc->ep0_trb_addr); dma_free_coherent(dwc->dev, sizeof(*dwc->ctrl_req), From 2317eacd9cf9dc1beee74ddb453bdd7552a64a27 Mon Sep 17 00:00:00 2001 From: John Youn Date: Mon, 17 Oct 2016 17:36:23 -0700 Subject: [PATCH 09/11] Revert "usb: dwc2: gadget: change variable name to more meaningful" This reverts commit ba48eab8866c ("usb: dwc2: gadget: change variable name to more meaningful"). This is needed to cleanly revert commit aa381a7259c3 ("usb: dwc2: gadget: fix TX FIFO size and address initialization") which may cause regressions on some platforms. Signed-off-by: John Youn Cc: Robert Baldyga Cc: Stefan Wahren Signed-off-by: Felipe Balbi --- drivers/usb/dwc2/gadget.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4cd6403a7566..aac4af3ea68f 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -186,7 +186,7 @@ static void dwc2_hsotg_ctrl_epint(struct dwc2_hsotg *hsotg, */ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) { - unsigned int fifo; + unsigned int ep; unsigned int addr; int timeout; u32 dptxfsizn; @@ -217,8 +217,8 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) * them to endpoints dynamically according to maxpacket size value of * given endpoint. */ - for (fifo = 1; fifo < MAX_EPS_CHANNELS; fifo++) { - dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)); + for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) { + dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(ep)); val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr; addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT; @@ -226,7 +226,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) if (addr > hsotg->fifo_mem) break; - dwc2_writel(val, hsotg->regs + DPTXFSIZN(fifo)); + dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep)); } /* From 3fa9538539ac737096c81f3315a14670b1609092 Mon Sep 17 00:00:00 2001 From: John Youn Date: Mon, 17 Oct 2016 17:36:25 -0700 Subject: [PATCH 10/11] Revert "usb: dwc2: gadget: fix TX FIFO size and address initialization" This reverts commit aa381a7259c3 ("usb: dwc2: gadget: fix TX FIFO size and address initialization"). The original commit removed the FIFO size programming per endpoint. The DPTXFSIZn register is also used for DIEPTXFn and the SIZE field is r/w in dedicated fifo mode. So it isn't appropriate to simply remove this initialization as it might break existing behavior. Also, some cores might not have enough fifo space to handle the programming method used in the reverted patch, resulting in fifo initialization failure. Signed-off-by: John Youn Cc: Robert Baldyga Cc: Stefan Wahren Signed-off-by: Felipe Balbi --- drivers/usb/dwc2/core.h | 7 ++++++ drivers/usb/dwc2/gadget.c | 47 ++++++++++++++++++++++++++++++++------- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index aad4107ef927..2a21a0414b1d 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -259,6 +259,13 @@ enum dwc2_lx_state { DWC2_L3, /* Off state */ }; +/* + * Gadget periodic tx fifo sizes as used by legacy driver + * EP0 is not included + */ +#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \ + 768, 0, 0, 0, 0, 0, 0, 0} + /* Gadget ep0 states */ enum dwc2_ep0_state { DWC2_EP0_SETUP, diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index aac4af3ea68f..24fbebc9b409 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -189,7 +189,6 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) unsigned int ep; unsigned int addr; int timeout; - u32 dptxfsizn; u32 val; /* Reset fifo map if not correctly cleared during previous session */ @@ -218,13 +217,13 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) * given endpoint. */ for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) { - dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(ep)); - - val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr; - addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT; - - if (addr > hsotg->fifo_mem) - break; + if (!hsotg->g_tx_fifo_sz[ep]) + continue; + val = addr; + val |= hsotg->g_tx_fifo_sz[ep] << FIFOSIZE_DEPTH_SHIFT; + WARN_ONCE(addr + hsotg->g_tx_fifo_sz[ep] > hsotg->fifo_mem, + "insufficient fifo memory"); + addr += hsotg->g_tx_fifo_sz[ep]; dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep)); } @@ -3807,10 +3806,36 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg) static void dwc2_hsotg_of_probe(struct dwc2_hsotg *hsotg) { struct device_node *np = hsotg->dev->of_node; + u32 len = 0; + u32 i = 0; /* Enable dma if requested in device tree */ hsotg->g_using_dma = of_property_read_bool(np, "g-use-dma"); + /* + * Register TX periodic fifo size per endpoint. + * EP0 is excluded since it has no fifo configuration. + */ + if (!of_find_property(np, "g-tx-fifo-size", &len)) + goto rx_fifo; + + len /= sizeof(u32); + + /* Read tx fifo sizes other than ep0 */ + if (of_property_read_u32_array(np, "g-tx-fifo-size", + &hsotg->g_tx_fifo_sz[1], len)) + goto rx_fifo; + + /* Add ep0 */ + len++; + + /* Make remaining TX fifos unavailable */ + if (len < MAX_EPS_CHANNELS) { + for (i = len; i < MAX_EPS_CHANNELS; i++) + hsotg->g_tx_fifo_sz[i] = 0; + } + +rx_fifo: /* Register RX fifo size */ of_property_read_u32(np, "g-rx-fifo-size", &hsotg->g_rx_fifo_sz); @@ -3832,10 +3857,13 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq) struct device *dev = hsotg->dev; int epnum; int ret; + int i; + u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE; /* Initialize to legacy fifo configuration values */ hsotg->g_rx_fifo_sz = 2048; hsotg->g_np_g_tx_fifo_sz = 1024; + memcpy(&hsotg->g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo)); /* Device tree specific probe */ dwc2_hsotg_of_probe(hsotg); @@ -3853,6 +3881,9 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq) dev_dbg(dev, "NonPeriodic TXFIFO size: %d\n", hsotg->g_np_g_tx_fifo_sz); dev_dbg(dev, "RXFIFO size: %d\n", hsotg->g_rx_fifo_sz); + for (i = 0; i < MAX_EPS_CHANNELS; i++) + dev_dbg(dev, "Periodic TXFIFO%2d size: %d\n", i, + hsotg->g_tx_fifo_sz[i]); hsotg->gadget.max_speed = USB_SPEED_HIGH; hsotg->gadget.ops = &dwc2_hsotg_gadget_ops; From a1aa8cf6471b17c0fa7132ea5eeef0ae07ca07cd Mon Sep 17 00:00:00 2001 From: John Youn Date: Mon, 17 Oct 2016 17:36:28 -0700 Subject: [PATCH 11/11] Revert "Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size" This binding was deprecated due to commit aa381a7259c3 ("usb: dwc2: gadget: fix TX FIFO size and address initialization"). However that commit is now reverted, so also revert this commit. The binding is valid and shouldn't be deprecated. This reverts commit 65e1ff7f4b5b ("Documentation: devicetree: dwc2: Deprecate g-tx-fifo-size"). Signed-off-by: John Youn Acked-by: Rob Herring Signed-off-by: Felipe Balbi --- Documentation/devicetree/bindings/usb/dwc2.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt index 455f2c310a1b..2c30a5479069 100644 --- a/Documentation/devicetree/bindings/usb/dwc2.txt +++ b/Documentation/devicetree/bindings/usb/dwc2.txt @@ -28,10 +28,7 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties - g-use-dma: enable dma usage in gadget driver. - g-rx-fifo-size: size of rx fifo size in gadget mode. - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode. - -Deprecated properties: -- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) - in gadget mode. +- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode. Example: