[U-Boot] [PATCH 3/3 v3] ARM: ARM926EJS - Add cache operations

[Hong Xu <hong.xu@atmel.com>]

Hi Marek,

On 08/11/2011 12:47 PM, Marek Vasut wrote:
> On Thursday, August 11, 2011 05:27:37 AM Hong Xu wrote:
>> Add a new file arch/arm/cpu/arm926ejs/cache.c and put cache operations
>> into this file.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> ---
>> V2:
>> 	Fixed a typo when CONFIG_SYS_DCACHE_OFF is defined
>> V3:
>> 	Undo changes in include/configs/at91sam9260ek.h
>> 	It's for testing purpose
>>
>>   arch/arm/cpu/arm926ejs/Makefile |    2 +-
>>   arch/arm/cpu/arm926ejs/cache.c  |  142
>> +++++++++++++++++++++++++++++++++++++++ 2 files changed, 143
>> insertions(+), 1 deletions(-)
>>   create mode 100644 arch/arm/cpu/arm926ejs/cache.c
>>
>> diff --git a/arch/arm/cpu/arm926ejs/Makefile
>> b/arch/arm/cpu/arm926ejs/Makefile index 930e0d1..5b5f330 100644
>> --- a/arch/arm/cpu/arm926ejs/Makefile
>> +++ b/arch/arm/cpu/arm926ejs/Makefile
>> @@ -26,7 +26,7 @@ include $(TOPDIR)/config.mk
>>   LIB	= $(obj)lib$(CPU).o
>>
>>   START	= start.o
>> -COBJS	= cpu.o
>> +COBJS	= cpu.o cache.o
>>
>>   SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
>>   OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
>> diff --git a/arch/arm/cpu/arm926ejs/cache.c
>> b/arch/arm/cpu/arm926ejs/cache.c new file mode 100644
>> index 0000000..fc321f6
>> --- /dev/null
>> +++ b/arch/arm/cpu/arm926ejs/cache.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * (C) Copyright 2002
>> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>
> Again ... really ?
>
not sure...
add a new copyright line on top of it?

>> + *
>> + * 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
>> + */
>> +
>> +#include<common.h>
>> +
>> +#define FLUSH_CACHE_OP		0
>> +#define INVALIDATE_CACHE_OP	1
>> +
>> +#ifndef CONFIG_SYS_DCACHE_OFF
>> +/*
>> + * Flush or Invalidate DCache respectively
>> + */
>> +static void cache_range_op(unsigned long start, unsigned long stop, int
>> op) +{
>> +	int cache_line_len;
>> +	unsigned long mva;
>> +	char *func_name;
>> +
>> +	if (op == FLUSH_CACHE_OP)
>> +		func_name = "flush_dcache_range";
>> +	else if (op == INVALIDATE_CACHE_OP)
>> +		func_name = "invalidate_dcache_range";
>> +	else {
>> +		printf("WARNING: %s - Invalid cache operation!\n", __func__);
>> +		return;
>> +	}

well, looks removing it and printing the `op' code is enough.

>
> hm...
> if (op>  2)
>   printf(warning);
>
> otherwise, just declare an array of strings ...
> char *name[] = { "flush", "invalidate" };
>
> and access it where needed ... printf("%s", name[OP]); ... maybe ?
>> +
>> +#ifdef CONFIG_SYS_CACHELINE_SIZE
>> +	cache_line_len = CONFIG_SYS_CACHELINE_SIZE;
>> +#else
>> +	/*
>> +	 * ARM926EJ-S Technical Reference Manual, Chap 2.3.1 Table 2-9
>> +	 * only b'10, aka. 32 bytes cache line len is valid
>> +	 */
>> +	cache_line_len = 32;
>> +#endif
>
> Move this at the begining of the file
>
> #ifndef CONFIG_SYS...
> #define
> #endif
>
> include the comment, just modify it a bit accordingly, explaining what it does
> here. Then use CONFIG_SYS... as it'll always be defined.
>

OK, looks better.

BR,
Eric

> Otherwise looks good.
>
> I need sleep badly :-) Good night !
>
>> +	mva = start;
>> +	if ((mva&  (cache_line_len - 1)) != 0) {
>> +		printf("WARNING: %s - start address 0x%08x not aligned to"
>> +			"cache line size(%d bytes)\n", func_name, start,

[...]