[U-Boot] [PATCH 1/2] usb: ci_udc: allow multiple buffer allocs per ep

[Stephen Warren]

On 06/12/2014 02:36 AM, Jörg Krause wrote:
> 
> On 06/10/2014 09:34 PM, Stephen Warren wrote:
>> On 06/02/2014 05:02 PM, Jörg Krause wrote:
>>> Sorry, my previous post was not shown correctly. The raw text was missing. I
>>> removed the annotation.
>>>
>>> Since this commit my Ethernet Gadget on a custom Freescale i.MX28 board is
>>> broken. Using tftp to download files I get in almost all cases a timeout:
...
>> The only other thing I can think of is that there's some issue with the
>> bounce buffer alignment or size being wrong somehow. Perhaps try
>> increasing those?
>
> I checked this and I found that ARCH_DMA_MINALIGN is set to 64, which is
> not true for Freescale i.MX28. This processor has an ARM926EJ-S, which
> has an cache line size of 32.

This should be fine. The only effect of setting ARCH_DMA_MINALIGN to a
value that's larger than it needs to be is: a tiny amount of RAM will be
wasted, since structures are allocated aligned to a larger boundary/size
than is strictly necessary in HW, and since SW should always round the
size of an area to be flushed up to ARCH_DMA_MINALIGN too, then perhaps
a few more cache lines are flushed than is strictly necessary.

> In arch/arm/include/asm/cache.h the macro ARCH_DMA_MINALIGN is defined
> as followed:
> 
> #ifdef CONFIG_SYS_CACHELINE_SIZE
> #define ARCH_DMA_MINALIGN    CONFIG_SYS_CACHELINE_SIZE
> #else
> #define ARCH_DMA_MINALIGN    64
> #endif
> 
> And in /arch/arm/cpu/arm926ejs/cache.c as followed:
> 
> #ifndef CONFIG_SYS_CACHELINE_SIZE
> #define CONFIG_SYS_CACHELINE_SIZE    32
> #endif
> 
> arch/arm/include/asm/cache.h does not see the definition of
> CONFIG_SYS_CACHELINE_SIZE in /arch/arm/cpu/arm926ejs/cache.c, so it's a
> bad place to put it in there.

Ah yes. That ifdef should certainly be in some global header that is
always included before/from arch/arm/include/asm/cache.h.

Without the correct CACHELINE_SIZE definition, the cache flushing
routines are probably flushing entirely the address or amount of memory.

> I defined CONFIG_SYS_CACHELINE_SIZE to 32 in my config header file and

Yes, that seems like the best fix.

FWIW, for Tegra we have a per-SoC header that defines SoC-specific
constants, so that each board file doesn't need to duplicate them.

> it works under the following circumstances: I have to disable the macro
> CONFIG_USB_GADGET_DUALSPEED. But, and this is strange, it works only the
> first time of a tftp download after a reset of the board. If I try to
> use tftp a second time, I get the same timeout error as before.

That's very odd. I can't explain why this wouldn't fix the issue
completely, unless there's *also* some other bug.

...
> Lastly, I changed CONFIG_SYS_CACHELINE_SIZE to 16 and this works for me
> in normal mode an in dual speed mode.

That's quite odd, and doesn't make much sense at all. You're sure the
cache line size really isn't 16?

...
>> I'm slightly confused by this log. Do you have 2 boards running U-Boot,
>> one running the USB controller in device mode, and this is the log from
>> some other board that's talking to that first board?
>
> I have one board connect to a PC. The log shows two different errors
> happening on the board. The first log shows a tftp command on the board
> stopping with a timeout after receiving some packets. The second log
> shows a tftp command on the same board throwing an error before
> receiving any packet.

So U-Boot is running on the board, and the logs are from the board, i.e.
you're running the "tftp" command in U-Boot on the board?

If so, I'm confused how ci_udc comes into play at all; doesn't "tftp"
use the USB controller in host mode, whereas ci_udc is only used for
device mode?