[U-Boot] [PATCH] MIPS: fix mips_cache fallback without __builtin_mips_cache

[Matthias Schiffer]

On 03/06/2016 08:38 PM, Daniel Schwierzeck wrote:
> 
> 
> Am 05.03.2016 um 04:15 schrieb Matthias Schiffer:
>> The "R" constraint supplies the address of an variable in a register. Use
>> "r" instead and adjust asm to supply the content of addr in a register
>> instead.
>>
>> Fixes: 2b8bcc5a ("MIPS: avoid .set ISA for cache operations")
>> Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>
>> Cc: Paul Burton <paul.burton at imgtec.com>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>> ---
>>
>> Hi,
>> I've noticed this when reading the code to understand how the cache
>> instruction is used. I'm not sure if this bug had any practical
>> consequences, or if nowadays all relevant compilers have
>> __builtin_mips_cache anyways.
>>
>> Please keep me in Cc in follow-up mails, I'm not subscribed to the u-boot
>> ML.
>>
>> Matthias
> 
> I've disabled the builtin code and compared dissaemblies with and without your patch. Without your patch, gcc adds an additional store instruction before each cache instruction. 
> 
> E.g. for flush_dcache_range():
> 
>   18:	afa20008 	sw	v0,8(sp)
>   1c:	bfb50008 	cache	0x15,8(sp)
> 
> vs.
> 
>   14:	bc550000 	cache	0x15,0(v0)
> 
> The cache operation works anyway, but with your patch better code is generated.

If I understand this correctly, the code without my patch would rather
invalidate the cache for the address 8(sp), which is part of the stack,
and not the memory pointed at by v0.

> 
>>
>>
>>  arch/mips/include/asm/cacheops.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h
>> index a3b07c6..002b839 100644
>> --- a/arch/mips/include/asm/cacheops.h
>> +++ b/arch/mips/include/asm/cacheops.h
>> @@ -16,7 +16,7 @@ static inline void mips_cache(int op, const volatile void *addr)
>>  #ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE
>>  	__builtin_mips_cache(op, addr);
>>  #else
>> -	__asm__ __volatile__("cache %0, %1" : : "i"(op), "R"(addr));
>> +	__asm__ __volatile__("cache %0, 0(%1)" : : "i"(op), "r"(addr));
>>  #endif
>>  }
>>  
>>
> 
> applied to u-boot-mips/next, thanks!
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160306/b10e10a7/attachment.sig>