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

Ilya Yanok yanok at emcraft.com
Tue Jul 3 22:10:24 CEST 2012


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.



More information about the U-Boot mailing list