From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Mon, 13 Aug 2012 11:46:35 +0200
Subject: [U-Boot] [PATCH v3] Consolidate bootcount code into
	drivers/bootcount
In-Reply-To: <201208111220.40656.vapier@gentoo.org>
References: <1338878275-1918-1-git-send-email-sr@denx.de>
	<201208111220.40656.vapier@gentoo.org>
Message-ID: <5028CCFB.8010700@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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