From mboxrd@z Thu Jan  1 00:00:00 1970
From: Graeme Russ <graeme.russ@gmail.com>
Date: Thu, 16 Jun 2011 21:10:53 +1000
Subject: [U-Boot] [PATCH v3 02/10] armv7: add miscellaneous utility
	macros
In-Reply-To: <20110616081543.EAC1D1E9FC3@gemini.denx.de>
References: <1299589658-30896-1-git-send-email-aneesh@ti.com>
	<1305202276-27784-3-git-send-email-aneesh@ti.com>
	<20110515184421.D7AB91491B06@gemini.denx.de>
	<4DD13DA3.3030505@ti.com> <4DECF8DA.9030806@ti.com>
	<20110606185046.BB8991736815@gemini.denx.de>
	<4DEDE8D9.7030306@ti.com>
	<20110607103923.7E1CC1B993A8@gemini.denx.de>
	<4DEE161B.2050402@ti.com>
	<20110607154028.D6540195494F@gemini.denx.de>
	<4DEF62A6.7060706@ti.com>
	<20110608214124.849821B993A8@gemini.denx.de>
	<4DF71FBF.6030408@ti.com>
	<20110614105115.01D6611EEDAF@gemini.denx.de>
	<4DF7488A.6000909@ti.com>
	<20110614135335.A750811EEDAF@gemini.denx.de>
	<4DF871E3.8080307@ti.com>
	<20110615092014.F080219E5AC1@gemini.denx.de>
	<4DF89102.9040508@ti.com>
	<20110615120447.9EEC4655C6@gemini.denx.de>
	<4DF8A8CF.5000308@gmail.com> <4DF9972B.30500@ti.com>
	<BANLkTik3cXeMzU0qpxDLSQMtK-AbEoDCPQ@mail.gmail.com>
	<20110616081543.EAC1D1E9FC3@gemini.denx.de>
Message-ID: <4DF9E4BD.8030608@gmail.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

On 16/06/11 18:15, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <BANLkTik3cXeMzU0qpxDLSQMtK-AbEoDCPQ@mail.gmail.com> you wrote:
>>
>> Now, that being said, I see no reason not to do the following if I had,
>> for example, multiple serial port configuration registers which are all
>> identical:
>>
>> /* num data bits is stored in bits 2-4 of the serial config register */
>> #define DATA_BITS_MASK		0x001c
>> #define DATA_BITS_OFFSET	2
>>
>> u32 set_serial_data_bits(u32 ser_cfg, u8 data_bits)
>> {
>> 	ser_cfg &= ~DATA_BITS_MASK;
>> 	ser_cfg |= ((u32)ser_cfg << DATA_BITS_OFFSET) &  DATA_BITS_MASK;
>>
>> 	return ser_cfg;
>> }
>>
>> void serial_init(void)
>> {
>> 	u32 ser_cfg;
>>
>> 	for (i=0; i<NUM_SERIAL_PORTS; i++) {
>> 		ser_cfg = read_serial_cfg(i);
>> 		ser_cfg = set_serial_data_bits(ser_cfg, 7);
>> 		write_serial_cfg(i, ser_cfg);
>> 	}
>> }
> 
> One reason for not doing this is that we should not reinvent the wheel
> again and again, and instead use standard APIs.
> 
> I cannot find any such code in U-Boot, so I cannot check, but to me it
> smells a lot as if this code should rather use clrsetbits_*() and
> other proper I/O accessors.

Except nobody outside ARM and PPC knows about clrsetbits and friends, so I
would not call them a standard API

I will, however, keep them in mind and implement them for x86 when I have a
need for bit-field operations

Regards,

Graeme