On 16/06/11 18:15, Wolfgang Denk wrote:
> Dear Graeme Russ,
>
> In message <banlktik3cxemzu0qpxdlsqmtk-abeod...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
