From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Wed, 04 Jun 2008 12:43:38 +0200
Subject: [U-Boot-Users] [PATCH] PPC: add accessor macros to clear and
 set bits in one shot
In-Reply-To: <m2hcc9bj1u.fsf@ohwell.denx.de>
References: <4843C68B.8090006@grandegger.com> <m2hcc9bj1u.fsf@ohwell.denx.de>
Message-ID: <484671DA.7070704@grandegger.com>
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

Hi Detlev,

Detlev Zundel wrote:
> Hi Wolfgang,
> 
>> PPC: add accessor macros to clear and set bits in one shot
> 
> Although I think this is a good idea, I do not particularly like some
> details.
> 
>> This patch adds macros from linux/include/asm-powerpc/io.h to clear and
>> set bits in one shot using the in_be32, out_be32, etc. accessor functions.
>> They are very handy to manipulate bits it I/O registers.
>>
>> This patch is required for my forthcoming FSL NAND UPM driver re-write and
>> the support for the TQM8548 module.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>  include/asm-ppc/io.h |   28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> Index: u-boot/include/asm-ppc/io.h
>> ===================================================================
>> --- u-boot.orig/include/asm-ppc/io.h
>> +++ u-boot/include/asm-ppc/io.h
>> @@ -238,6 +238,34 @@ extern inline void out_be32(volatile uns
>>  	__asm__ __volatile__("sync; stw%U0%X0 %1,%0" : "=m" (*addr) : "r" (val));
>>  }
>>  
>> +/* access ports */
>> +#define setbits32(_addr, _v) out_be32((_addr), in_be32(_addr) |  (_v))
>> +#define clrbits32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
>> +
>> +#define setbits16(_addr, _v) out_be16((_addr), in_be16(_addr) |  (_v))
>> +#define clrbits16(_addr, _v) out_be16((_addr), in_be16(_addr) & ~(_v))
> 
> Below are versions for _{be,le} which makes total sense to me, but the
> variants here miss the endianness, _implicitely_ doing big-endian
> transfers.  That is neither consistent, nor usable, i.e. how do I set a
> bit in a le-register?
> 
>> +
>> +#define setbits8(_addr, _v) out_8((_addr), in_8(_addr) |  (_v))
>> +#define clrbits8(_addr, _v) out_8((_addr), in_8(_addr) & ~(_v))
>> +
>> +/* Clear and set bits in one shot.  These macros can be used to clear and
>> + * set multiple bits in a register using a single read-modify-write.  These
>> + * macros can also be used to set a multiple-bit bit pattern using a mask,
>> + * by specifying the mask in the 'clear' parameter and the new bit pattern
>> + * in the 'set' parameter.
>> + */
>> +
>> +#define clrsetbits(type, addr, clear, set) \
>> +	out_##type((addr), (in_##type(addr) & ~(clear)) | (set))
>> +
>> +#define clrsetbits_be32(addr, clear, set) clrsetbits(be32, addr, clear, set)
>> +#define clrsetbits_le32(addr, clear, set) clrsetbits(le32, addr, clear, set)
>> +
>> +#define clrsetbits_be16(addr, clear, set) clrsetbits(be16, addr, clear, set)
>> +#define clrsetbits_le16(addr, clear, set) clrsetbits(le32, addr, clear, set)
> 
> Error --------------------------------------------------^^^^

Ah, oh, I blindly copied that code from Linux include/asm-powerpc/io.h.
A revised patch addressing the above issues will follow.

Wolfgang.