RE: [BUG REPORT] usb: dwc3: Timeouts with USB 2.0 LPM active

[Felipe Balbi <balbi@kernel.org>]

[-- 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 --]