[U-Boot] [PATCH v4 3/6] mcx: Disable DCACHE since USB EHCI is enabled

Ilya Yanok
2012-07-03T20:10:24Z

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.