On Monday 16 May 2011 12:21 AM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<1305202276-27784-4-git-send-email-ane...@ti.com>  you wrote:
>> - Add a framework for layered cache maintenance
>>      - separate out SOC specific outer cache maintenance from
>>        maintenance of caches known to CPU
>>
>> - Add generic ARMv7 cache maintenance operations that affect all
>>    caches known to ARMv7 CPUs. For instance in Cortex-A8 these
>>    opertions will affect both L1 and L2 caches. In Cortex-A9
>>    these will affect only L1 cache
>>
>> - D-cache operations supported:
>>      - Invalidate entire D-cache
Needed before enabling the caches.
>>      - Invalidate D-cache range
Needed if you are doing DMA.
>>      - Flush(clean&  invalidate) entire D-cache
Needed before Linux before disabling the caches.
>>      - Flush D-cache range
Needed in case of DMA out.

>
> How much of this is actually needed in the context of U-Boot?

Please see above. As far as I know OMAP doesn't do DMA in U-Boot.

>
> ...
>> +    for (way = num_ways - 1; way>= 0 ; way--)
>> +            for (set = num_sets - 1; set>= 0; set--) {
>> +                    setway = (level<<  1) | (set<<  log2_line_len) |
>> +                             (way<<  way_shift);
>> +                    /* Invalidate data/unified cache line by set/way */
>> +                    asm volatile (" mcr p15, 0, %0, c7, c6, 2"
>> +                                    : : "r" (setway));
>> +            }
>
> Braces needed for multiline for().  Please fix globally.

Ok.

>
> ...

>> +    if (operation == ARMV7_DCACHE_INVAL_ALL)
>> +            v7_inval_dcache_level_setway(level, num_sets, num_ways,
>> +                                  way_shift, log2_line_len);
>> +    else if (operation == ARMV7_DCACHE_CLEAN_INVAL_ALL)
>> +            v7_clean_inval_dcache_level_setway(level, num_sets, num_ways,
>> +                                               way_shift, log2_line_len);
>
> Braces needed for multiline statements.  Please fix globally.
>
>> +    for (mva = start; mva<  stop; mva = mva + line_len)
>> +            /* DCCIMVAC - Clean&  Invalidate data cache by MVA to PoC */
>> +            asm volatile ("mcr p15, 0, %0, c7, c14, 1" : : "r" (mva));
>
> And again etc. etc.
>
> ...
>> +void invalidate_dcache_all(void)
>> +{
>> +}
>> +
>> +void flush_dcache_all(void)
>> +{
>> +}
>> +
>> +void invalidate_dcache_range(unsigned long start, unsigned long stop)
>> +{
>> +}
>> +
>> +void flush_dcache_range(unsigned long start, unsigned long stop)
>> +{
>> +}
>> +
>> +void arm_init_before_mmu(void)
>> +{
>> +}
>> +
>> +void  flush_cache(unsigned long start, unsigned long size)
>> +{
>> +}
>
> Please do not add dead code.  I consider it misleading to have
> functions which promise to perform something, and actually do nothing.

These are empty only in the else part of an #ifdef. When cache support
is compiled out these need to be empty.

best regards,
Aneesh
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to