From: Felipe Balbi <balbi@kernel.org>
To: Sebastian von Ohr <vonohr@smaract.com>,
Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Thinh Nguyen <thinhn@synopsys.com>
Subject: RE: [BUG REPORT] usb: dwc3: Timeouts with USB 2.0 LPM active
Date: Wed, 05 May 2021 15:37:48 +0300 [thread overview]
Message-ID: <878s4ticf7.fsf@kernel.org> (raw)
In-Reply-To: <da6ebfb4f58a4249a095d250d9abe3ed@smaract.com>
[-- Attachment #1: Type: text/plain, Size: 3895 bytes --]
Hi,
Sebastian von Ohr <vonohr@smaract.com> writes:
>> From: Felipe Balbi [mailto:balbi@kernel.org]
>
>> For U1/U2 it's mostly handled by the HW itself. The only thing we do is
>> set the appropriate bits for the relevant SetFeature requests, see ep0.c.
>
> Is this also the case for USB 2.0 LPM? USB 3.0 U1/U2 transitions seem to be
> completely different from USB 2.0. The SetFeature functions in ep0.c only
> handle SuperSpeed and SuperSpeed-plus. My connection is only USB 2.0 high-speed.
should behave the same for USB2
>> > bU1DevExitLat 10 micro seconds
>>
>> Hmm, this is the maximum allowed value
>>
>> > bU2DevExitLat 511 micro seconds
>>
>> This is not. Can you try setting this to 0x7ff and see if the problem
>> goes away? It could be that your platform needs more time to
>> wakeup. Then you're going to have to characterize it to figure out how
>> much this value should be.
>
> I've changed it to 0x7ff but no difference. Also isn't his field for USB 3.0 only?
It should be part of one of the BOS descriptors that describes LPM, I
don't recall which one in particular.
> Meanwhile I've spent some more time looking at the driver code and enabled the
> link change interrupt. I've attached a new trace where we can actually see what
> transitions happen:
>
> irq/13-dwc3-236 [000] d..1 174.435986: dwc3_event: event (00006084): ep1out: Transfer In Progress [0] (SIm)
> irq/13-dwc3-236 [000] d..1 174.435988: dwc3_complete_trb: ep1out: trb 000000005384b162 (E2:D2) buf 000000007348a000 size 4080 ctrl 00000818 (hlcS:sC:normal)
> irq/13-dwc3-236 [000] d..1 174.435992: dwc3_gadget_giveback: ep1out: req 00000000f8e0932d length 16/4096 zsI ==> 0
> irq/13-dwc3-236 [000] d..1 174.436497: dwc3_event: event (00050301): Link Change [RX.Detect]
> irq/13-dwc3-236 [000] d..1 174.436544: dwc3_event: event (00020301): Link Change [U2]
> usb_recv-812 [000] d..2 174.636131: dwc3_ep_queue: ep1out: req 00000000f8e0932d length 0/4096 zsI ==> -115
> usb_recv-812 [000] d..2 174.636139: dwc3_prepare_trb: ep1out: trb 0000000085f38bb7 (E3:D2) buf 000000007348b000 size 4096 ctrl 00000819 (HlcS:sC:normal)
> usb_recv-812 [000] d..2 174.636147: dwc3_gadget_ep_cmd: ep1out: cmd 'Update Transfer' [20007] params 00000000 00000000 00000000 --> status: Successful
> irq/13-dwc3-236 [000] d..1 175.438282: dwc3_event: event (00000401): WakeUp [U0]
> irq/13-dwc3-236 [000] d..1 175.438353: dwc3_event: event (00000401): WakeUp [U0]
> irq/13-dwc3-236 [000] d..1 175.438357: dwc3_event: event (00000301): Link Change [U0]
>
> We see that 500us after the last transaction the link state is changed to
> RX.Detect (in HS, means Early Suspend) and then shortly after to U2 (in HS,
> means SLEEP). I'm not sure what early suspend is supposed to be as I can't find
> in the USB spec (dwc3 specific?). Then a new receive request is queued, but the
yes, that's dwc3-specific
> link state doesn't change even though the host has data to send. Data is only
it could be we're missing a wakeup somewhere.
> transferred way later after the host write times out and tries again.
>
> For a test I've changed some conditions in the driver so that
> __dwc3_gadget_wakeup is also called on transfer updates and the link state
> change also happens when in U2. This change actually fixed my timeout issue.
> However, I'm not sure if this is actually the correct thing to do. I'm by far
> no USB expert and I don't have access to the dwc3 databook.
Right, AFAIR the databook was a bit unclear about this. It stated that
it was required only for Start Transfer, but I always had the same
doubt. No idea if the databook has been clarified since then.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]
next prev parent reply other threads:[~2021-05-05 12:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-03 13:12 [BUG REPORT] usb: dwc3: Timeouts with USB 2.0 LPM active Sebastian von Ohr
2021-05-03 13:52 ` Felipe Balbi
2021-05-03 14:15 ` Sebastian von Ohr
2021-05-04 6:28 ` Felipe Balbi
2021-05-04 9:47 ` Sebastian von Ohr
2021-05-05 12:37 ` Felipe Balbi [this message]
2021-05-12 9:28 ` Sebastian von Ohr
2021-05-05 8:02 ` Mathias Nyman
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=878s4ticf7.fsf@kernel.org \
--to=balbi@kernel.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
--cc=thinhn@synopsys.com \
--cc=vonohr@smaract.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).