From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ilya Yanok <yanok@emcraft.com>
Date: Wed, 04 Jul 2012 00:10:24 +0400
Subject: [U-Boot] [PATCH v4 3/6] mcx: Disable DCACHE since USB EHCI is
	enabled
In-Reply-To: <201206302127.10984.marex@denx.de>
References: <1340209283-3404-1-git-send-email-trini@ti.com>
	<201206281741.59534.marex@denx.de> <4FEF2099.7000902@emcraft.com>
	<201206302127.10984.marex@denx.de>
Message-ID: <4FF351B0.30109@emcraft.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.