Dear Marek,
30.06.2012 23:27, Marek Vasut wrote:
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.
I guess that's because they have to be both address and size aligned.
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?
Hm.. Fixing alignment for qh, qd and stuff is easy, I can surely submit a patch for this. But I don't really know what to do this the last one: if we are at this point there is no correct way out... We can increase the range to cover the buffer or decrease it to be inside buffer -- but both options are incorrect. Actually we should return error when unaligned buffer is submitted in the first place... But this will likely break a lot of systems... Probably put this check under if (dcache_enabled())?
Regards, Ilya.