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

Hong Xu
2011-08-15T07:08:51Z

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, …