[U-Boot] [PATCH] nds32: split common cache access from cpu into lib

[Macpaul Lin]

Hi Marek,

2012/7/15 Marek Vasut <marex at denx.de>:
> Dear Macpaul Lin,
>
>> This commit does the following updates.
>> 1. Split the common cache access from cpu.c into lib folder.
>> 2. Rename the following cache api to adapt common.h
>>  - dcache_flush_rang -> flush_dcache_rang
>>  - icache_inval_range -> invalidate_icache_range
>> 3. Add invalidate_dcache_range
>
> So basically this connects it to standard cache api?

Yes! Basically this is compatible with all nds32 cpu family if they
have cache inside.
The cache is configurable by the customer if they used softcore FPGA.
This API could help them to figure out the hardware configuration in
software level.

> [...]
>> diff --git a/arch/nds32/lib/cache.c b/arch/nds32/lib/cache.c
> [...]
>> +static inline unsigned long CACHE_LINE_SIZE(enum cache_t cache)
>> +{
>> +     if (cache == ICACHE)
>> +             return 8 << (((GET_ICM_CFG() & ICM_CFG_MSK_ISZ) \
>> +                                     >> ICM_CFG_OFF_ISZ) - 1);
>
> These crazy macros can be probably done easier and can share some code maybe?
> Maybe not though, I dunno, just a thought.
>

The macro GET_ICM_CFG() is inside arch/nds32/include/cache.h
#define DEFINE_GET_SYS_REG(reg) \
        static inline unsigned long GET_##reg(void)             \

Since the assembly is really long, rewrite them into macros should be
easier to maintain and read.
I'll prefer not to modify this function to call CACHE_LINE_SIZE.

>> +void flush_dcache_range(unsigned long start, unsigned long end)
>> +     while (end > start) {
>> +             __asm__ volatile ("\n\tcctl %0, L1D_VA_WB" : : "r"(start));
>> +             __asm__ volatile ("\n\tcctl %0, L1D_VA_INVAL" : : "r"(start));
>
> You probably want to do those two calls in one asm volatile () block. And this
> too ... use "asm volatile()" and not "__asm__ volatile" or "asm __volatile__"
> and similar combinations of these, just a nit ;-)

Ok, it will be fixed. :)

>> +void flush_cache(unsigned long addr, unsigned long size)
>> +{
>> +     flush_dcache_range(addr, addr + size);
>> +     invalidate_icache_range(addr, addr + size);
>
> You probably want to flush dcache in here and that's it.
>
>> +}
>> +
>> +void icache_enable(void)
>> +{
>> +     __asm__ __volatile__ (
>
> See my nit above for the rest ;-) But all in all it's good, we should be
> applying this around V2-V3 of this patch, for the -next release. Thanks for your
> patch!

It looks like this fix cannot be checkin before this coming rc1 or rc2.
But I think it is okay for users to wait until the -next release. :)

-- 
Best regards,
Macpaul Lin