[U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC

[Stephen Warren]

On 06/30/2014 04:44 PM, Jörg Krause wrote:
> 
> On 06/30/2014 09:55 PM, Stephen Warren wrote:
>> On 06/30/2014 10:02 AM, Stephen Warren wrote:
>>> On 06/27/2014 07:34 PM, Jörg Krause wrote:
>>>> On 06/28/2014 01:37 AM, Stephen Warren wrote:
>>>>> On 06/27/2014 05:16 PM, Jörg Krause wrote:
>>>>>> On 06/27/2014 11:55 PM, Stephen Warren wrote:
>>>>>>> On 06/27/2014 03:37 PM, Jörg Krause wrote:
>>>>>>>> I added the last series of patches beginning from 2014-06-10 for
>>>>>>>> testing
>>>>>>>> purposes. The patches from 2014-05-29 were already applied.
>>>>>>>>
>>>>>>>> First series of patches:
>>>>>>>>
>>>>>>>>      Applying: usb: ci_udc: call udc_disconnect() from ci_pullup()
>>>>>>>>      Applying: usb: ci_udc: fix freeing of ep0 req
>>>>>>>>      Applying: usb: ci_udc: fix probe error cleanup
>>>>>>>>      Applying: usb: ci_udc: clean up all allocations in unregister
>>>>>>>>
>>>>>>>> Calling tftp the first time after a reset runs fine,
>>>>>>> I thought the issue you reported was that the *first* time you run the
>>>>>>> "tftp" command, it has issues such as timeouts? Did I misunderstand, or
>>>>>>> did that issue somehow go away?
>>>>>> That's right! This was the state before applying a series of patches
>>>>>> after allow multiple buffer allocs per ep. Now, the first run of tftp
>>>>>> runs without any errors.
>>>>> Just to make sure I understand, here's what you saw:
>>>>>
>>>>> 1) tftp works fine to start with. No timeouts even on repeated
>>>>> invocation.
>>>>
>>>> I went back in time and now I can be more precise. Everything worked
>>>> fine until commit usb: ci_udc: allow multiple buffer allocs per ep which
>>>> introduces timeouts in almost all tftp file downloads. Even the first
>>>> run of tfpt can end in a timeout.
>>
>> I've pushed out some branches for you to test at:
>> git://github.com/swarren/u-boot.git
>>
>> I looked back to see where the problematic commit 2813006fecda "usb:
>> ci_udc: allow multiple buffer allocs per ep" was first applied. I then
>> started with that same commit, and split up the problematic commit into
>> tiny pieces, and applied each piece in turn. The result is in branch
>> ci-udc-debug-base. If you could please test every commit in that branch:
>>
>>> fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs"
>>> 373098f12a1d usb: ci_udc: dynamically alloc USB request objects
>>> 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer
>>> f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts
>>> 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len
>>> 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more
>>> 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce
>>> b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req
>>> 82f35a839e31 usb: ci_udc: introduce struct ci_req
>>> c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce
>>> 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
>>
>> ... and tell me which commit causes the problem, that would be useful.
>> Since the problem is intermittent, please make sure you test each commit
>> many times (at least 10-20, preferably nearer 100). You can stop early
>> if you see the problem, but if you don't, please test many times to make
>> sure there is no problem.

Thanks very much for testing.

(when you remove the blank lines between the message you're replying to
and your response, it's much harder to read. Perhaps this is because
you're writing HTML email. Most people won't read that; please use plain
text).

> Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects
> cannot be build.

Oh dear. Differentiating between that commit and the next one is perhaps
the most important thing. Can you please apply the following patch to
that commit and retest it:

diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index b00fc2613779..c8c64d755195 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -423,7 +423,7 @@ static void handle_ep_complete(struct ci_ep *ep)
 		       num, in ? "in" : "out", item->info, item->page0);

 	len = (item->info >> 16) & 0x7fff;
-	ci_ep->current_req = 0;
+	ep->current_req = 0;
 	ci_req->req.actual = ci_req->req.length - len;
 	ci_debounce(ci_req, in);

> The error is introduced with the latest commit fabf0df9523a usb: ci_udc:
> remainder of "allow multiple buffer allocs". I attached the output
> message for this commit.

OK, at least we can rule out all the bounce buffer and cache handling
changes.