All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Felipe Balbi <felipe.balbi@linux.intel.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: Tue, 22 Jan 2019 02:18:12 +0000	[thread overview]
Message-ID: <30102591E157244384E984126FC3CB4F639B0A26@us01wembx1.internal.synopsys.com> (raw)

Hi Felipe,

On 1/21/2019 4:15 AM, Felipe Balbi wrote:
> 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:

Right. That was what I try to point in my previous reply.

>>>
>>> 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?

I think you need to fix this:

        /*


Other than that, these changes seem to fix the issue.

Thanks,
Thinh

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1b7c44271b11..73e3a402f63d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2621,7 +2621,7 @@ static void dwc3_stop_active_transfer(struct
dwc3_ep *dep, bool force)
        u32 cmd;
        int ret;
 
-       if (dep->flags & DWC3_EP_TRANSFER_STARTED)
+       if (!(dep->flags & DWC3_EP_TRANSFER_STARTED))
                return;
 

             reply	other threads:[~2019-01-22  2:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22  2:18 Thinh Nguyen [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-21 12:15 Felipe Balbi
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=30102591E157244384E984126FC3CB4F639B0A26@us01wembx1.internal.synopsys.com \
    --to=thinh.nguyen@synopsys.com \
    --cc=felipe.balbi@linux.intel.com \
    --cc=linux-usb@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.