[U-Boot] [PATCH 2/8] armv7: cache maintenance operations for armv7

[Aneesh V]

On Saturday 08 January 2011 12:06 PM, Albert ARIBAUD wrote:
> Hi Aneesh,
Pressed the Send button too fast last time. Missed answering the last
few questions.

<snip..>
>> +
>> +void invalidate_dcache_all(void)
>> +{
>> +	v7_maint_dcache_all(ARMV7_DCACHE_INVAL_ALL);
>> +	if (v7_outer_cache.inval_all)
>> +		v7_outer_cache.inval_all();
>
> Why use pointers here rather than weak functions?

In fact, I hadn't thought about it. Maybe I was biased by the Linux
implementation.The only reason I can think of is that pointer gives the
flexibility of doing this assignment at run-time. Let's say we had a
multi-platform u-boot that detects the SOC at run-time?

>
>> +}
>> +
>> +/*
>> + * Performs a clean&   invalidation of the entire data cache
>> + * at all levels
>> + */
>> +void flush_dcache_all(void)
>> +{
>> +	v7_maint_dcache_all(ARMV7_DCACHE_CLEAN_INVAL_ALL);
>> +	if (v7_outer_cache.flush_all)
>> +		v7_outer_cache.flush_all();
>> +}
>> +
>> +/*
>> + * Invalidates range in all levels of D-cache/unified cache used:
>> + * Affects the range [start, stop - 1]
>> + */
>> +void invalidate_dcache_range(unsigned long start, unsigned long stop)
>> +{
>> +
>> +	v7_dcache_maint_range(start, stop, ARMV7_DCACHE_INVAL_RANGE);
>> +	if (v7_outer_cache.inval_range)
>> +		/* physical address is same as virtual address */
>> +		v7_outer_cache.inval_range(start, stop);
>> +}
>> +
>> +/*
>> + * Flush range(clean&   invalidate) from all levels of D-cache/unified
>> + * cache used:
>> + * Affects the range [start, stop - 1]
>> + */
>> +void flush_dcache_range(unsigned long start, unsigned long stop)
>> +{
>> +	v7_dcache_maint_range(start, stop, ARMV7_DCACHE_CLEAN_INVAL_RANGE);
>> +	if (v7_outer_cache.flush_range)
>> +		/* physical address is same as virtual address */
>> +		v7_outer_cache.flush_range(start, stop);
>> +}
>> +static void __v7_setup_outer_cache_ops(void)
>> +{
>> +	puts("v7_setup_outer_cache_ops: dummy implementation! "
>> +	     "real implementation not available!!\n");
>> +}
>> +
>> +void v7_setup_outer_cache_ops(void)
>> +	__attribute__((weak, alias("__v7_setup_outer_cache_ops")));
>> +
>> +void arm_init_before_mmu(void)
>> +{
>> +	v7_setup_outer_cache_ops();
>> +	if (v7_outer_cache.enable)
>> +		v7_outer_cache.enable();
>> +	invalidate_dcache_all();
>> +	v7_inval_tlb();
>> +}
>> +#else
>> +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)
>> +{
>> +}
>> +#endif
>> +
>> +#ifndef CONFIG_SYS_NO_ICACHE
>> +/* Invalidate entire I-cache and branch predictor array */
>> +void invalidate_icache_all(void)
>> +{
>> +	/*
>> +	 * Invalidate all instruction caches to PoU.
>> +	 * Also flushes branch target cache.
>> +	 */
>> +	asm volatile ("mcr p15, 0, %0, c7, c5, 0" : : "r" (0));
>> +
>> +	/* Invalidate entire branch predictor array */
>> +	asm volatile ("mcr p15, 0, %0, c7, c5, 6" : : "r" (0));
>> +
>> +	/* Full system DSB - make sure that the invalidation is complete */
>> +	asm volatile ("DSB");
>> +	/* Full system ISB - make sure the instruction stream sees it */
>> +	asm volatile ("ISB");
>> +}
>> +#else
>> +void invalidate_icache_all(void)
>> +{
>> +}
>> +#endif
>> +
>> +/*
>> + * Flush range from all levels of d-cache/unified-cache used:
>> + * Affects the range [start, start + size - 1]
>> + */
>> +void  flush_cache(unsigned long start, unsigned long size)
>> +{
>> +	flush_dcache_range(start, start + size);
>> +}
>
> This function is the only one which is defined to something non-empty
> when CONFIG_SYS_NO_DCACHE is defined. Why is it not in the big #ifndef
> for dcache above ?

Although this function is non-empty, flush_dcache_range() is in turn
empty. Effect will be the same, right?

>
>> diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
>> index 49ac9c7..7f9b171 100644
>> --- a/arch/arm/cpu/armv7/config.mk
>> +++ b/arch/arm/cpu/armv7/config.mk
>> @@ -23,7 +23,7 @@
>>    PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
>>
>>    # Make ARMv5 to allow more compilers to work, even though its v7a.
>> -PLATFORM_CPPFLAGS += -march=armv5
>> +PLATFORM_CPPFLAGS += -march=armv7-a
>
> Did you check that this does not break any board using armv7?

I checked only Codesourcery tool chain.
Linux kernel build for a v7 architecture processor uses armv7-a. Is it
not fair to assume that the toolchain used for bootloader also supports
it? Do we have to support those out-dated compilers?

>>    # =========================================================================
>>    #
>>    # Supply options according to compiler version
>> diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
>> new file mode 100644
>> index 0000000..57409b6
>> --- /dev/null
>> +++ b/arch/arm/include/asm/armv7.h
>> @@ -0,0 +1,63 @@
>> +/*
>> + * (C) Copyright 2010
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + *
>> + * Aneesh V<aneesh at ti.com>
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +#ifndef ARMV7_H
>> +#define ARMV7_H
>> +
>> +#include<linux/types.h>
>> +
>> +/*
>> + * Values for InD field in CSSELR
>> + * Selects the type of cache
>> + */
>> +#define ARMV7_CSSELR_IND_DATA_UNIFIED	0
>> +#define ARMV7_CSSELR_IND_INSTRUCTION	1
>> +
>> +/* Values for Ctype fields in CLIDR */
>> +#define ARMV7_CLIDR_CTYPE_NO_CACHE		0
>> +#define ARMV7_CLIDR_CTYPE_INSTRUCTION_ONLY	1
>> +#define ARMV7_CLIDR_CTYPE_DATA_ONLY		2
>> +#define ARMV7_CLIDR_CTYPE_INSTRUCTION_DATA	3
>> +#define ARMV7_CLIDR_CTYPE_UNIFIED		4
>> +
>> +/* some utility macros */
>> +#define mask(start, end) \
>> +	(((1<<   ((end) - (start) + 1)) - 1)<<   (start))
>> +
>> +#define mask_n_get(reg, start, end) \
>> +	(((reg)&   mask(start, end))>>   (start))
>
> Seeing as these functions are only used in the ARMv7 cache C file, they
> should be moved there.

I plan to use a modified version of mask_n_get() and its set couterpart
mask_n_set() in my subsequent works in more files.

Can I keep it here itself or should I move it to an OMAP specific
header file or can I move it to a more generic header file? Please
suggest.

Best regards,
Aneesh