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

[Jörg Krause]

On 06/30/2014 06:02 PM, 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.
>>
>>> 2) You applied "allow multiple buffer allocs per ep"
>> Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped
>> here. But still timeouts. First run almost always runs fine, only
>> sometimes timeouts while receiving a packet, but always running to the
>> end. Running tftp after this a second time and more fails with a ERROR:
>> The remote end did not respond in time. at
>> drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
>>
>> Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I
>> previously wrote it).
Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32).
>>
>> Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am
>> getting also errors after the second or third run.
> Sigh. There's been a lot of flip-flopping re: whether the cacheline size
> affects the issue or not, and whether the first TFTP download always
> works fine, or whether only the 3rd fails. This makes it very hard for
> me to understand the issue that you're seeing. For instance, if even the
> first TFTP download can fail (even if intermittently), then there's
> clearly a problem with basic driver operation. However, if only the 2nd
> or 3rd TFTP ever fails, then the problem is likely isolated to some part
> of the cleanup/shutdown code. Given that your reports of the problem
> keep flip-flopping about, it makes it hard to decide which part of the
> code to look at.
I am very sorry if I confused you with my attempt to explain you what I 
am seeing. The "flip-flopping" comes from the different results and 
behaviour after applying a patch or a series of patches. And I also 
tried this and that and looked if it changes the behaviour. I must admit 
that this is not good testing practise.
> For now, I'm going to simply assume that any TFTP download (1st, 2nd, or
> 100th) can fail intermittently, and that cache line size is irrelevant.
>
> I'll look at the problematic commit you mentioned (2813006fecda "usb:
> ci_udc: allow multiple buffer allocs per ep") and see if I can create a
> series of patches that do the same thing, and have you bisect that. If I
> can do that, I will ask you to test 100 times each: the commit before
> the bad commit, then each of the commits in my series, always resetting
> the board between each test and doing a single TFTP download. Then I'd
> like to see the raw results from those tests.
I will do this and I hope it brings some clarifications.