Dear Ilya Yanok,
Dear Marek,
28.06.2012 19:41, Marek Vasut wrote:
Surely. (but that probably was an AM3517 with 64 byte cache line)
m28 is imx28 with 32byte cacheline
You are lucky then. But some systems have bigger cacheline, right?
Yes, and I just got a perfect system for the job.
patch) and loading from ext2 and vfat (worked).
This is just a coincedence ;)
Not really, did you check mainline uboot and the fixes in those?
Surely I did. Let's take a look:
do { /* Invalidate dcache */ invalidate_dcache_range((uint32_t)&qh_list, (uint32_t)&qh_list + sizeof(struct QH)); invalidate_dcache_range((uint32_t)&qh, (uint32_t)&qh + sizeof(struct QH)); invalidate_dcache_range((uint32_t)qtd, (uint32_t)qtd + sizeof(qtd));These guys are 32-byte aligned, right? So the assumption is that cacheline is not greater than 32. Sounds like a bug to me (though could be fixed rather easily with USB_MINALIGN introduced by Tom).
Oooh, good catch indeed. Actually, look at the structures, even they have some weird alignment crap in them. Maybe that can also be dropped and the alignment shall be fixed properly. I'll have to research this more.
token = hc32_to_cpu(vtd->qt_token); if (!(token & 0x80)) break; WATCHDOG_RESET(); } while (get_timer(ts) < timeout); /* Invalidate the memory area occupied by buffer */ invalidate_dcache_range(((uint32_t)buffer & ~31), ((uint32_t)buffer & ~31) + roundup(length, 32));That's worse. First of all, rounding is wrong: consider buffer=31, length=32. But that's easy to fix too. The bad part is that with writeback cache you can't just enlarge the range to cover the buffer and fit alignment requirements -- this can potentially destroy some changes that are in the same cache line but not yet stored to RAM.
Correct, so we have multiple bugs in here that need to be squashed ASAP.
Ilya, can you possibly submit a patch for this?
Regards, Ilya.
Best regards, Marek Vasut