All of lore.kernel.org
 help / color / mirror / Atom feed
* DWC3 linux driver query
@ 2015-05-28 11:23 sundeep subbaraya
  2015-05-28 14:11 ` Felipe Balbi
  0 siblings, 1 reply; 5+ messages in thread
From: sundeep subbaraya @ 2015-05-28 11:23 UTC (permalink / raw)
  To: balbi, Paul Zimmerman; +Cc: linux-kernel, linux-usb

Hi Felipe and Paul,

I am seeing an issue while testing iperf for USB ethernet gadget with
dwc3 controller in 2.0 mode. After debugging I figured out that:

1. Network gadget queues say 3 requests. (for IN endpoint)
2. It turns out with req.no_interrupt flag,
    DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
3. As per driver state machine, we get XFERNOTREADY then prepare these
TRBs and issue start transfer. Make Endpoint state as Busy.
4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
5. The issue I see here is there are NAKs going to host (seen in
analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
events in between XFERINPROGRESS and          XFERCOMPLETE  events.
6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
start transfer command is issued in XFERNOTREADY handler.The command
fails since controller did not release the xfer resource yet.

I feel controller behaviour is fine since it sends NAK and writes that
event. Driver may have to be modified to make EP as free only in
XFERCOMPLETE event handler (ofcourse not for Isoc).
Note I am testing on a platform which is very slow (the interface
between DDR and core runs at 4Mhz).
Where as on Zynq (DWC3 in PL), a faster system compared to above one I
do not see any NAKs in between Start transfer command and XFERCOMPLETE
event.

What do you guys say? Do you agree linux driver has to be modified or
Core should never issue NAKs in between Start transfer and
XFERCOMPLETE?

A patch correcting DEPCMD status macros has been applied. Thank you
Felipe for trace points in driver otherwise it would have taken very
long time to figure out the root cause :) .
Below is the trace log:(enabled only for IN bulk endpoint)

     irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Not Active
     irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
req ffffffc039a68580 dma 011c60a2 length 1558 IOC

     irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
req ffffffc039a687c0 dma 011c10a2 length 1558

     irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
req ffffffc039a68700 dma 011c18a2 length 1558 last IOC

     irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
     irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
     irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
     irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
XFERINPROGRESS
     irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0

     irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
     irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
     irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
XFERCOMPLETE
     irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0

     irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0

     irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Not Active
     irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
req ffffffc039a68ac0 dma 398b18a2 length 1558

     irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
req ffffffc039a68c40 dma 3a1ce8a2 length 1558

     irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
req ffffffc039a68580 dma 3cc258a2 length 1558 last

     irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
     irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
     irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
     irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
XFERNOTREADY.Transfer Active
     irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3



Thanks,
Sundeep

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: DWC3 linux driver query
  2015-05-28 11:23 DWC3 linux driver query sundeep subbaraya
@ 2015-05-28 14:11 ` Felipe Balbi
  2015-05-29 13:31   ` sundeep subbaraya
  0 siblings, 1 reply; 5+ messages in thread
From: Felipe Balbi @ 2015-05-28 14:11 UTC (permalink / raw)
  To: sundeep subbaraya; +Cc: balbi, Paul Zimmerman, linux-kernel, linux-usb

[-- Attachment #1: Type: text/plain, Size: 6295 bytes --]

Hi,

On Thu, May 28, 2015 at 04:53:09PM +0530, sundeep subbaraya wrote:
> Hi Felipe and Paul,

btw, Paul has left Synopys :-)

> I am seeing an issue while testing iperf for USB ethernet gadget with
> dwc3 controller in 2.0 mode. After debugging I figured out that:
> 
> 1. Network gadget queues say 3 requests. (for IN endpoint)
> 2. It turns out with req.no_interrupt flag,
>     DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
> 3. As per driver state machine, we get XFERNOTREADY then prepare these
> TRBs and issue start transfer. Make Endpoint state as Busy.
> 4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
> 5. The issue I see here is there are NAKs going to host (seen in
> analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
> events in between XFERINPROGRESS and          XFERCOMPLETE  events.
> 6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
> start transfer command is issued in XFERNOTREADY handler.The command
> fails since controller did not release the xfer resource yet.
> 
> I feel controller behaviour is fine since it sends NAK and writes that
> event. Driver may have to be modified to make EP as free only in
> XFERCOMPLETE event handler (ofcourse not for Isoc).

this sounds like the correct solution.

> Note I am testing on a platform which is very slow (the interface
> between DDR and core runs at 4Mhz).

sweet :-)

> Where as on Zynq (DWC3 in PL), a faster system compared to above one I

hey, when will we see a glue layer for Zynq ? :-)

> do not see any NAKs in between Start transfer command and XFERCOMPLETE
> event.
> 
> What do you guys say? Do you agree linux driver has to be modified or
> Core should never issue NAKs in between Start transfer and
> XFERCOMPLETE?

well, if we queued enough transfers, it shouldn't NAK. OTOH, we
shouldn't allow for a new StartTransfer command until all pending
requests have been transferred.

> A patch correcting DEPCMD status macros has been applied. Thank you
> Felipe for trace points in driver otherwise it would have taken very
> long time to figure out the root cause :) .

yeah, those are really helpful :-)

> Below is the trace log:(enabled only for IN bulk endpoint)
> 
>      irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Not Active
>      irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
> req ffffffc039a68580 dma 011c60a2 length 1558 IOC
> 
>      irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
> req ffffffc039a687c0 dma 011c10a2 length 1558
> 
>      irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
> req ffffffc039a68700 dma 011c18a2 length 1558 last IOC
> 
>      irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
>      irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
>      irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
>      irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
> XFERINPROGRESS
>      irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
> ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0
> 
>      irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
>      irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
>      irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
> XFERCOMPLETE
>      irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
> ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0
> 
>      irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
> ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0
> 
>      irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Not Active
>      irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
> req ffffffc039a68ac0 dma 398b18a2 length 1558
> 
>      irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
> req ffffffc039a68c40 dma 3a1ce8a2 length 1558
> 
>      irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
> req ffffffc039a68580 dma 3cc258a2 length 1558 last
> 
>      irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
>      irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
>      irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
>      irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
> XFERNOTREADY.Transfer Active
>      irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3

Can you try patch below ?

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a03a485205c7..cad747865ce0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1907,12 +1907,16 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
 {
 	unsigned		status = 0;
 	int			clean_busy;
+	u32			is_xfer_complete;
+
+	is_xfer_complete = (event->endpoint_event == DWC3_DEPEVT_XFERCOMPLETE);
 
 	if (event->status & DEPEVT_STATUS_BUSERR)
 		status = -ECONNRESET;
 
 	clean_busy = dwc3_cleanup_done_reqs(dwc, dep, event, status);
-	if (clean_busy)
+	if (clean_busy & (is_xfer_complete ||
+				usb_endpoint_xfer_isoc(dep->endpoint.desc)))
 		dep->flags &= ~DWC3_EP_BUSY;
 
 	/*

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: DWC3 linux driver query
  2015-05-28 14:11 ` Felipe Balbi
@ 2015-05-29 13:31   ` sundeep subbaraya
  2015-05-29 15:05     ` Felipe Balbi
  0 siblings, 1 reply; 5+ messages in thread
From: sundeep subbaraya @ 2015-05-29 13:31 UTC (permalink / raw)
  To: balbi; +Cc: Paul Zimmerman, linux-kernel, linux-usb

Hi Felipe,

On Thu, May 28, 2015 at 7:41 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, May 28, 2015 at 04:53:09PM +0530, sundeep subbaraya wrote:
>> Hi Felipe and Paul,
>
> btw, Paul has left Synopys :-)

ahh I see..
>
>> I am seeing an issue while testing iperf for USB ethernet gadget with
>> dwc3 controller in 2.0 mode. After debugging I figured out that:
>>
>> 1. Network gadget queues say 3 requests. (for IN endpoint)
>> 2. It turns out with req.no_interrupt flag,
>>     DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
>> 3. As per driver state machine, we get XFERNOTREADY then prepare these
>> TRBs and issue start transfer. Make Endpoint state as Busy.
>> 4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
>> 5. The issue I see here is there are NAKs going to host (seen in
>> analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
>> events in between XFERINPROGRESS and          XFERCOMPLETE  events.
>> 6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
>> start transfer command is issued in XFERNOTREADY handler.The command
>> fails since controller did not release the xfer resource yet.
>>
>> I feel controller behaviour is fine since it sends NAK and writes that
>> event. Driver may have to be modified to make EP as free only in
>> XFERCOMPLETE event handler (ofcourse not for Isoc).
>
> this sounds like the correct solution.
>
>> Note I am testing on a platform which is very slow (the interface
>> between DDR and core runs at 4Mhz).
>
> sweet :-)

No it is not :) :). I struggled a lot while debugging :(

>
>> Where as on Zynq (DWC3 in PL), a faster system compared to above one I
>
> hey, when will we see a glue layer for Zynq ? :-)

we don't have hardware with 2.0 and 3.0 PHY together currently. I will
write and test
once the hardware is ready :)

>
>> do not see any NAKs in between Start transfer command and XFERCOMPLETE
>> event.
>>
>> What do you guys say? Do you agree linux driver has to be modified or
>> Core should never issue NAKs in between Start transfer and
>> XFERCOMPLETE?
>
> well, if we queued enough transfers, it shouldn't NAK. OTOH, we
> shouldn't allow for a new StartTransfer command until all pending
> requests have been transferred.

Yes correct but the hardware design is very slow here so hitting this case
>
>> A patch correcting DEPCMD status macros has been applied. Thank you
>> Felipe for trace points in driver otherwise it would have taken very
>> long time to figure out the root cause :) .
>
> yeah, those are really helpful :-)
>
>> Below is the trace log:(enabled only for IN bulk endpoint)
>>
>>      irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Not Active
>>      irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
>> req ffffffc039a68580 dma 011c60a2 length 1558 IOC
>>
>>      irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
>> req ffffffc039a687c0 dma 011c10a2 length 1558
>>
>>      irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
>> req ffffffc039a68700 dma 011c18a2 length 1558 last IOC
>>
>>      irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
>>      irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
>>      irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
>>      irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
>> XFERINPROGRESS
>>      irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
>> ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0
>>
>>      irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
>>      irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
>>      irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
>> XFERCOMPLETE
>>      irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
>> ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0
>>
>>      irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
>> ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0
>>
>>      irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Not Active
>>      irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
>> req ffffffc039a68ac0 dma 398b18a2 length 1558
>>
>>      irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
>> req ffffffc039a68c40 dma 3a1ce8a2 length 1558
>>
>>      irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
>> req ffffffc039a68580 dma 3cc258a2 length 1558 last
>>
>>      irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
>>      irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
>>      irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
>>      irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
>> XFERNOTREADY.Transfer Active
>>      irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3
>
> Can you try patch below ?
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a03a485205c7..cad747865ce0 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1907,12 +1907,16 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
>  {
>         unsigned                status = 0;
>         int                     clean_busy;
> +       u32                     is_xfer_complete;
> +
> +       is_xfer_complete = (event->endpoint_event == DWC3_DEPEVT_XFERCOMPLETE);
>
>         if (event->status & DEPEVT_STATUS_BUSERR)
>                 status = -ECONNRESET;

>
>         clean_busy = dwc3_cleanup_done_reqs(dwc, dep, event, status);
> -       if (clean_busy)
> +       if (clean_busy & (is_xfer_complete ||
> +                               usb_endpoint_xfer_isoc(dep->endpoint.desc)))

works perfect now and it should be &&.
Thanks for the immediate patch. What's next? Shall I send patch for
this or you applied this one already?

Sundeep.

>                 dep->flags &= ~DWC3_EP_BUSY;
>
>         /*
>
> --
> balbi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: DWC3 linux driver query
  2015-05-29 13:31   ` sundeep subbaraya
@ 2015-05-29 15:05     ` Felipe Balbi
  2015-05-30  1:32       ` sundeep subbaraya
  0 siblings, 1 reply; 5+ messages in thread
From: Felipe Balbi @ 2015-05-29 15:05 UTC (permalink / raw)
  To: sundeep subbaraya; +Cc: balbi, Paul Zimmerman, linux-kernel, linux-usb

[-- Attachment #1: Type: text/plain, Size: 7557 bytes --]

On Fri, May 29, 2015 at 07:01:16PM +0530, sundeep subbaraya wrote:
> Hi Felipe,
> 
> On Thu, May 28, 2015 at 7:41 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Thu, May 28, 2015 at 04:53:09PM +0530, sundeep subbaraya wrote:
> >> Hi Felipe and Paul,
> >
> > btw, Paul has left Synopys :-)
> 
> ahh I see..
> >
> >> I am seeing an issue while testing iperf for USB ethernet gadget with
> >> dwc3 controller in 2.0 mode. After debugging I figured out that:
> >>
> >> 1. Network gadget queues say 3 requests. (for IN endpoint)
> >> 2. It turns out with req.no_interrupt flag,
> >>     DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
> >> 3. As per driver state machine, we get XFERNOTREADY then prepare these
> >> TRBs and issue start transfer. Make Endpoint state as Busy.
> >> 4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
> >> 5. The issue I see here is there are NAKs going to host (seen in
> >> analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
> >> events in between XFERINPROGRESS and          XFERCOMPLETE  events.
> >> 6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
> >> start transfer command is issued in XFERNOTREADY handler.The command
> >> fails since controller did not release the xfer resource yet.
> >>
> >> I feel controller behaviour is fine since it sends NAK and writes that
> >> event. Driver may have to be modified to make EP as free only in
> >> XFERCOMPLETE event handler (ofcourse not for Isoc).
> >
> > this sounds like the correct solution.
> >
> >> Note I am testing on a platform which is very slow (the interface
> >> between DDR and core runs at 4Mhz).
> >
> > sweet :-)
> 
> No it is not :) :). I struggled a lot while debugging :(
> 
> >
> >> Where as on Zynq (DWC3 in PL), a faster system compared to above one I
> >
> > hey, when will we see a glue layer for Zynq ? :-)
> 
> we don't have hardware with 2.0 and 3.0 PHY together currently. I will
> write and test
> once the hardware is ready :)
> 
> >
> >> do not see any NAKs in between Start transfer command and XFERCOMPLETE
> >> event.
> >>
> >> What do you guys say? Do you agree linux driver has to be modified or
> >> Core should never issue NAKs in between Start transfer and
> >> XFERCOMPLETE?
> >
> > well, if we queued enough transfers, it shouldn't NAK. OTOH, we
> > shouldn't allow for a new StartTransfer command until all pending
> > requests have been transferred.
> 
> Yes correct but the hardware design is very slow here so hitting this case
> >
> >> A patch correcting DEPCMD status macros has been applied. Thank you
> >> Felipe for trace points in driver otherwise it would have taken very
> >> long time to figure out the root cause :) .
> >
> > yeah, those are really helpful :-)
> >
> >> Below is the trace log:(enabled only for IN bulk endpoint)
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Not Active
> >>      irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a68580 dma 011c60a2 length 1558 IOC
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a687c0 dma 011c10a2 length 1558
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a68700 dma 011c18a2 length 1558 last IOC
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
> >>      irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
> >>      irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
> >>      irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
> >> XFERINPROGRESS
> >>      irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
> >> ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
> >>      irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
> >>      irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
> >> XFERCOMPLETE
> >>      irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
> >> ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
> >> ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Not Active
> >>      irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a68ac0 dma 398b18a2 length 1558
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a68c40 dma 3a1ce8a2 length 1558
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
> >> req ffffffc039a68580 dma 3cc258a2 length 1558 last
> >>
> >>      irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
> >>      irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
> >>      irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
> >>      irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
> >> XFERNOTREADY.Transfer Active
> >>      irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3
> >
> > Can you try patch below ?
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index a03a485205c7..cad747865ce0 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1907,12 +1907,16 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
> >  {
> >         unsigned                status = 0;
> >         int                     clean_busy;
> > +       u32                     is_xfer_complete;
> > +
> > +       is_xfer_complete = (event->endpoint_event == DWC3_DEPEVT_XFERCOMPLETE);
> >
> >         if (event->status & DEPEVT_STATUS_BUSERR)
> >                 status = -ECONNRESET;
> 
> >
> >         clean_busy = dwc3_cleanup_done_reqs(dwc, dep, event, status);
> > -       if (clean_busy)
> > +       if (clean_busy & (is_xfer_complete ||
> > +                               usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> 
> works perfect now and it should be &&.
> Thanks for the immediate patch. What's next? Shall I send patch for
> this or you applied this one already?

I'll send this formally and apply for v4.2 merge window with a Cc stable
tag. Can I add your Tested-by ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: DWC3 linux driver query
  2015-05-29 15:05     ` Felipe Balbi
@ 2015-05-30  1:32       ` sundeep subbaraya
  0 siblings, 0 replies; 5+ messages in thread
From: sundeep subbaraya @ 2015-05-30  1:32 UTC (permalink / raw)
  To: balbi; +Cc: Paul Zimmerman, linux-kernel, linux-usb

Hi,

On Fri, May 29, 2015 at 8:35 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Fri, May 29, 2015 at 07:01:16PM +0530, sundeep subbaraya wrote:
>> Hi Felipe,
>>
>> On Thu, May 28, 2015 at 7:41 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > Hi,
>> >
>> > On Thu, May 28, 2015 at 04:53:09PM +0530, sundeep subbaraya wrote:
>> >> Hi Felipe and Paul,
>> >
>> > btw, Paul has left Synopys :-)
>>
>> ahh I see..
>> >
>> >> I am seeing an issue while testing iperf for USB ethernet gadget with
>> >> dwc3 controller in 2.0 mode. After debugging I figured out that:
>> >>
>> >> 1. Network gadget queues say 3 requests. (for IN endpoint)
>> >> 2. It turns out with req.no_interrupt flag,
>> >>     DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
>> >> 3. As per driver state machine, we get XFERNOTREADY then prepare these
>> >> TRBs and issue start transfer. Make Endpoint state as Busy.
>> >> 4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
>> >> 5. The issue I see here is there are NAKs going to host (seen in
>> >> analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
>> >> events in between XFERINPROGRESS and          XFERCOMPLETE  events.
>> >> 6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
>> >> start transfer command is issued in XFERNOTREADY handler.The command
>> >> fails since controller did not release the xfer resource yet.
>> >>
>> >> I feel controller behaviour is fine since it sends NAK and writes that
>> >> event. Driver may have to be modified to make EP as free only in
>> >> XFERCOMPLETE event handler (ofcourse not for Isoc).
>> >
>> > this sounds like the correct solution.
>> >
>> >> Note I am testing on a platform which is very slow (the interface
>> >> between DDR and core runs at 4Mhz).
>> >
>> > sweet :-)
>>
>> No it is not :) :). I struggled a lot while debugging :(
>>
>> >
>> >> Where as on Zynq (DWC3 in PL), a faster system compared to above one I
>> >
>> > hey, when will we see a glue layer for Zynq ? :-)
>>
>> we don't have hardware with 2.0 and 3.0 PHY together currently. I will
>> write and test
>> once the hardware is ready :)
>>
>> >
>> >> do not see any NAKs in between Start transfer command and XFERCOMPLETE
>> >> event.
>> >>
>> >> What do you guys say? Do you agree linux driver has to be modified or
>> >> Core should never issue NAKs in between Start transfer and
>> >> XFERCOMPLETE?
>> >
>> > well, if we queued enough transfers, it shouldn't NAK. OTOH, we
>> > shouldn't allow for a new StartTransfer command until all pending
>> > requests have been transferred.
>>
>> Yes correct but the hardware design is very slow here so hitting this case
>> >
>> >> A patch correcting DEPCMD status macros has been applied. Thank you
>> >> Felipe for trace points in driver otherwise it would have taken very
>> >> long time to figure out the root cause :) .
>> >
>> > yeah, those are really helpful :-)
>> >
>> >> Below is the trace log:(enabled only for IN bulk endpoint)
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Not Active
>> >>      irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68580 dma 011c60a2 length 1558 IOC
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a687c0 dma 011c10a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68700 dma 011c18a2 length 1558 last IOC
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
>> >> XFERINPROGRESS
>> >>      irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
>> >> ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
>> >> XFERCOMPLETE
>> >>      irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
>> >> ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
>> >> ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Not Active
>> >>      irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68ac0 dma 398b18a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68c40 dma 3a1ce8a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68580 dma 3cc258a2 length 1558 last
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3
>> >
>> > Can you try patch below ?
>> >
>> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> > index a03a485205c7..cad747865ce0 100644
>> > --- a/drivers/usb/dwc3/gadget.c
>> > +++ b/drivers/usb/dwc3/gadget.c
>> > @@ -1907,12 +1907,16 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
>> >  {
>> >         unsigned                status = 0;
>> >         int                     clean_busy;
>> > +       u32                     is_xfer_complete;
>> > +
>> > +       is_xfer_complete = (event->endpoint_event == DWC3_DEPEVT_XFERCOMPLETE);
>> >
>> >         if (event->status & DEPEVT_STATUS_BUSERR)
>> >                 status = -ECONNRESET;
>>
>> >
>> >         clean_busy = dwc3_cleanup_done_reqs(dwc, dep, event, status);
>> > -       if (clean_busy)
>> > +       if (clean_busy & (is_xfer_complete ||
>> > +                               usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>>
>> works perfect now and it should be &&.
>> Thanks for the immediate patch. What's next? Shall I send patch for
>> this or you applied this one already?
>
> I'll send this formally and apply for v4.2 merge window with a Cc stable
> tag. Can I add your Tested-by ?

Sure. Thanks.

Sundeep.
>
> --
> balbi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-05-30  1:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 11:23 DWC3 linux driver query sundeep subbaraya
2015-05-28 14:11 ` Felipe Balbi
2015-05-29 13:31   ` sundeep subbaraya
2015-05-29 15:05     ` Felipe Balbi
2015-05-30  1:32       ` sundeep subbaraya

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.