linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Balbi <felipe.balbi@linux.intel.com>
To: Thinh Nguyen <thinh.nguyen@synopsys.com>Thinh Nguyen
	<thinh.nguyen@synopsys.com>,
	Linux USB <linux-usb@vger.kernel.org>
Subject: [10/14] usb: dwc3: gadget: remove wait_end_transfer
Date: Mon, 21 Jan 2019 14:15:29 +0200	[thread overview]
Message-ID: <87d0oqjjfi.fsf@linux.intel.com> (raw)

Hi

Felipe Balbi <felipe.balbi@linux.intel.com> writes:
> Hi,
>
> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>> Hi again,
>>
>> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>>
>> <snip>
>>
>>> Try to dequeue a request that was already completed. Odd. Why are we
>>> missing a call to giveback??
>>
>> Got a little more information:
>>
>>     file-storage-3982  [006] d...   131.010663: dwc3_ep_queue: ep1in: req 00000000eccaa10f length 0/16384 zsI ==> -115
>>     file-storage-3982  [006] d...   131.010667: dwc3_prepare_trb: ep1in: trb 000000002ab8a1f9 buf 00000000bde24000 size 16384 ctrl 00000811 (Hlcs:sC:normal)
>>     file-storage-3982  [006] d...   131.010674: dwc3_gadget_ep_cmd: ep1in: cmd 'Update Transfer' [30007] params 00000000 00000000 00000000 --> status: Successful
>>      irq/16-dwc3-3983  [004] d...   131.010942: dwc3_event: event (00004086): ep1in: Transfer In Progress [0] (sIm)
>>      irq/16-dwc3-3983  [004] d...   131.010942: dwc3_complete_trb: ep1in: trb 00000000426cd8cf buf 00000000bde20000 size 0 ctrl 00000810 (hlcs:sC:normal)
>>      irq/16-dwc3-3983  [004] d...   131.010944: dwc3_gadget_giveback: ep1in: req 00000000f7765e56 length 16384/16384 zsI ==> 0
>>     file-storage-3982  [006] d...   131.010994: dwc3_ep_queue: ep1in: req 00000000f7765e56 length 0/16384 zsI ==> -115
>>     file-storage-3982  [006] d...   131.010998: dwc3_prepare_trb: ep1in: trb 0000000065d9143d buf 00000000bde28000 size 16384 ctrl 00000811 (Hlcs:sC:normal)
>>     file-storage-3982  [006] d...   131.011005: dwc3_gadget_ep_cmd: ep1in: cmd 'Update Transfer' [30007] params 00000000 00000000 00000000 --> status: Successful
>>      irq/16-dwc3-3983  [004] d...   131.065517: dwc3_event: event (00000001): Disconnect: [U0]
>>     file-storage-3982  [006] ....   131.065558: dwc3_ep_dequeue: ep1in: req 00000000f7765e56 length 0/16384 zsI ==> -115
>>     file-storage-3982  [006] d...   131.065687: dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: Successful
>>      irq/16-dwc3-3983  [004] d...   131.065729: dwc3_event: event (080301c6): ep1in: Endpoint Command Complete
>>      irq/16-dwc3-3983  [004] d...   131.065731: dwc3_gadget_giveback: ep1in: req 00000000f7765e56 length 0/16384 zsI ==> -104
>>     file-storage-3982  [006] ....   131.065766: dwc3_ep_dequeue: ep1in: req 00000000eccaa10f length 0/16384 zsI ==> -115
>>      irq/16-dwc3-3983  [004] d...   135.071714: dwc3_event: event (00000101): Reset [U0]
>>      irq/16-dwc3-3983  [004] d...   135.126440: dwc3_event: event (00000201): Connection Done [U0]
>>
>> From this snippet above it seems like we got End Transfer completed
>> *before* we tried to dequeue the request. This is a race in the driver,
>> since it will wait for End Transfer complete forever :-p We can see that
>> ep_dequeue won't send a new End Transfer unless it's necessary:
>>
>> static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force)
>> {
>> 	struct dwc3 *dwc = dep->dwc;
>> 	struct dwc3_gadget_ep_cmd_params params;
>> 	u32 cmd;
>> 	int ret;
>>
>> 	if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) ||
>> 	    !dep->resource_index)
>> 		return;
>> [...]
>> }
>>
>> So this will wait forever. Here's a patch that takes into consideration
>> this possibility:
>
> a version that compiles now (#facepalm):

I've prepared a branch with a few patches on top of my testing/fixes to
test this out. Seems to work fine on my end. Can you try with your test
setup?

The following changes since commit 87b6d2c56825c3119a0e64cc208ae6d795810a2e:

  usb: dwc2: gadget: Fix Remote Wakeup interrupt bit clearing (2019-01-17 15:56:53 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git dwc3/fix-cancellation

for you to fetch changes up to 4cae1d5786f8ffdc4c7d840fd7e01580cee3c4ad:

  usb: dwc3: gadget: remove DWC3_EP_END_TRANSFER_PENDING (2019-01-21 13:08:44 +0200)

----------------------------------------------------------------
Felipe Balbi (4):
      usb: dwc3: gadget: clear DWC3_EP_TRANSFER_STARTED on cmd complete
      usb: dwc3: gadget: don't use resource_index as a flag
      usb: dwc3: gadget: early giveback if End Transfer already completed
      usb: dwc3: gadget: remove DWC3_EP_END_TRANSFER_PENDING

 drivers/usb/dwc3/core.h   |  1 -
 drivers/usb/dwc3/gadget.c | 36 +++++++++++++-----------------------
 drivers/usb/dwc3/trace.h  |  3 +--
 3 files changed, 14 insertions(+), 26 deletions(-)


Thanks

             reply	other threads:[~2019-01-21 12:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 12:15 Felipe Balbi [this message]
  -- strict thread matches above, loose matches on Subject: below --
2019-01-22  7:27 [10/14] usb: dwc3: gadget: remove wait_end_transfer Felipe Balbi
2019-01-22  2:18 Thinh Nguyen
2019-01-21  9:31 Felipe Balbi
2019-01-21  9:29 Felipe Balbi
2019-01-21  8:40 Felipe Balbi
2019-01-19  2:51 Thinh Nguyen
2019-01-18  6:57 Felipe Balbi
2018-11-14  9:18 Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d0oqjjfi.fsf@linux.intel.com \
    --to=felipe.balbi@linux.intel.com \
    --cc=thinh.nguyen@synopsys.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).