[U-Boot] [PATCH v3] Consolidate bootcount code into drivers/bootcount

Stefan Roese sr at denx.de
Mon Aug 13 11:46:35 CEST 2012


On 08/11/2012 06:20 PM, Mike Frysinger wrote:
> On Tuesday 05 June 2012 02:37:55 Stefan Roese wrote:
>> --- /dev/null
>> +++ b/drivers/bootcount/Makefile
>>
>> +COBJS-$(CONFIG_BFIN_CPU)	+= bootcount_blackfin.o
> 
> needs to be CONFIG_BLACKFIN

Okay.

>> +all:	$(LIB)
> 
> unused rule -> delete

Okay.

>> --- /dev/null
>> +++ b/include/bootcount.h
>>
>> +#ifdef CONFIG_SYS_BOOTCOUNT_LE
>> +static inline void bc_out32(volatile u32 *addr, u32 data)
> 
> the bc_xxx names are a little confusing since they overlap so much with the 
> existing io.h api.  how about "raw_bootcount_store" ?

Okay.

>> +{
>> +	out_le32(addr, data);
>> +}
>> +
>> +static inline u32 bc_in32(volatile u32 *addr)
>> +{
>> +	return in_le32(addr);
>> +}
>> +#else
>> +static inline void bc_out32(volatile u32 *addr, u32 data)
>> +{
>> +	out_be32(addr, bdata);
>> +}
>> +
>> +static inline u32 bc_in32(volatile u32 *addr)
>> +{
>> +	return in_be32(addr);
>> +}
>> +#endif
> 
> i'm not a big fan of defaulting to an endian regardless of the host.  in this 
> case, it appears to benefit ppc only.
> 
> what about:
> #include <asm/byteorder.h>
> #if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)
> # if __BYTE_ORDER == __LITTLE_ENDIAN
> #  define CONFIG_SYS_BOOTCOUNT_LE
> # else
> #  define CONFIG_SYS_BOOTCOUNT_BE
> # endif
> #endif
> 
> or if you're not a fan of that, then:
> #if defined(CONFIG_SYS_BOOTCOUNT_LE)
> ... current in_le logic ...
> #elif defined(CONFIG_SYS_BOOTCOUNT_BE)
> ... current in_be logic ...
> #else
> # error "please select one of CONFIG_SYS_BOOTCOUNT_{L,B}E"
> #endif
> 
> and then add a default to arch/powerpc/include/asm/config.h

Good idea. I'll send a new version with the 2nd approach later today.

Thanks,
Stefan



More information about the U-Boot mailing list