[U-Boot] [PATCH v3 02/10] armv7: add miscellaneous utility macros

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTik3cXeMzU0qpxDLSQMtK-AbEoDCPQ at 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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Steal five dollars and you were a petty  thief.  Steal  thousands  of
dollars and you are either a government or a hero.
                                   - Terry Pratchett, _Going_Postal_