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

Stephen Warren
2014-06-30T19:55:33Z

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.

You mentioned that your board support is not upstream. In order to test,
you should probably do: For each commit I mention above, check it out,
apply the minimal set of patches needed to support your board, and test.

Please don't apply any other patches while testing this series. Please
only test TFTP download, and not ubifs writes, etc.

I've also pushed out branch ci-udc-debug-tegra-dfu-working which has a
whole load of other commits I needed to test the split up patches. These
were needed to add board support for the board I'm currently using, and
to fix/enhance the core DFU code. Most of these commits are already
applied in u-boot.git/master, it's just that I cherry-picked them on top
of the old base commit so I would have something to test with.

BTW, I'm going on vacation from Jul 3rd-July 20th. I might answer some
limited email during this time, but I certainly won't have access to any
test HW.