[U-Boot] [PATCH 2/4] cache_v7: Check for dcache enablement in dcache flush functions

Aneesh V
2012-06-27T23:40:22Z

Sricharan,

On 06/21/2012 08:23 AM, R, Sricharan wrote:
> Hi Aneesh,
>
> On Thu, Jun 21, 2012 at 2:55 PM, Sricharan R<r.sricharan@ti.com> wrote:
>> Hi,
>> [snip..]
>>> On 06/15/2012 07:48 AM, R, Sricharan wrote:
>>>> Hi,
>>>>
>>>>>> On Fri, Jun 15, 2012 at 12:31 AM, Tom Rini<trini@ti.com> wrote:
>>>>>>> If we are built with D-CACHE enabled but have run 'dcache off' and
>>> then
>>>>>>> attempt to flush unaligned regions we spam the console with
>>> problems
>>>>>>> that aren't true (as the cache was off).
>>>>>>>
>>>>>> Today we do cache maintenance operations after the dcache is
>>> turned off.
>>>>>> One example is before jumping to kernel, we try to invalidate
>>> the caches,
>>>>>> in cache turned off state. So with this patch those maintenance
>>> calls will
>>>>>> do nothing, which is not correct.
>>>>>
>>>>> Ah yes, But, shouldn't we be doing these same operations as part of
>>>>> turning the cache off?
>>>>>
>>>> The problem is that while turning of dcaches, we flush it, and
>>> turn
>>>> cache and MMU off. But these operations are not happening
>>>> automatically in a single call. So there is a chance of valid
>>>> entries present in cache even after it is OFF.
>>>
>>> I think this is what we need to fix. Otherwise, Tom's change looks good
>>> to me. How about an invalidate in dcache_disable() or something like
>>> that?
>>>
>> Correct. So if the cleaning up sequence is fine, then ok.
>> So there should be no need to check if caches are OFF inside
>> the maintenance functions. Kernel does not looks like doing such checks.
>> The real issue looks like we should have had assembly level functions
>> to do the cleanup routines for flush-invalidate-turn OFF caches/MM
>> atomically.
> Actually, with something like speculation in the behind, only way of
> ensuring that
> we do not have any valid entries is to do a invalidate all, after
> turn of caches/mmu.
> So this means that we will have a maintenance after turning off.

Good point. But we need that invalidate just one last time after the
disable, right? How about making the cache_status a variable rather
than reading the C bit. And then disable_cache() shall be like this:

1. Flush all
2. Disable C bit
3. Invalidate all
4. cache_status = disabled

Maybe, not the best solution. But I can't think of anything better.
Please note that after this point there won't be any valid entries in
the cache per ARMv7 Architecture reference manual(section B2.2.2):

"If the cache is disabled, it is guaranteed that no new allocation of
memory locations into the cache will occur."

br,
Aneeesh