[U-Boot] [PATCH] include/ns16550.h: Unify structure declaration for registers

Shinya Kuribayashi skuribay at pobox.com
Wed Apr 29 21:12:18 CEST 2009


Shinya Kuribayashi wrote:
> Detlev Zundel wrote:
>> As I said, I understand now why there were different data-types involved
>> although this was kind of non-obvious.  So I take it, you had a working
>> configuration with REG_SIZE = 4, correct?
> 
> I might be unclear. I used to use REG_SIZE = -16, as 16550 registers
> are located at 0, +0x10, +0x20, ..., .
> 
> In this case, I don't think REG_SIZE = 4/-4 works.  Let's see:
> 
> REG_SIZE = 4
> ------------
> struct NS16550 {
>         unsigned char prepad_rbr[3];
>         unsigned char rbr;
>         unsigned char prepad_ier[3];
>         unsigned char ier;
>         :
>         :
> };
> 
> REG_SIZE = -4
> -------------
> struct NS16550 {
>         unsigned char rbr;
>         unsigned char postpad_rbr[3];
>         unsigned char ier;
>         unsigned char postpad_ier[3];
>         :
>         :
> };
> 
> because 16550 registers can be aligned in 16-bytes-interval.

s/can be aligned/can not be aligned/

>> diff --git a/include/ns16550.h b/include/ns16550.h
>> index ce606b5..7924396 100644
>> --- a/include/ns16550.h
>> +++ b/include/ns16550.h
>> @@ -21,16 +21,20 @@
>>   * will not allocate storage for arrays of size 0
>>   */
>>  
>> +#if !defined(CONFIG_SYS_NS16550_REG_TYPE)
>> +#define UART_REG_TYPE unsigned char
>> +#endif
>> +
>>  #if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
>>  #error "Please define NS16550 registers size."
>>  #elif (CONFIG_SYS_NS16550_REG_SIZE > 0)
>> -#define UART_REG(x)						   \
>> -	unsigned char prepad_##x[CONFIG_SYS_NS16550_REG_SIZE - 1]; \
>> -	unsigned char x;
>> +#define UART_REG(x)							\
>> +	UART_REG_TYPE prepad_##x[CONFIG_SYS_NS16550_REG_SIZE - sizeof(UART_REG_TYPE)]; \
>> +	UART_REG_TYPE x;
>>  #elif (CONFIG_SYS_NS16550_REG_SIZE < 0)
>>  #define UART_REG(x)							\
>> -	unsigned char x;						\
>> -	unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
>> +	UART_REG_TYPE x;						\
>> +	UART_REG_TYPE postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - sizeof(UART_REG_TYPE)];
>>  #endif
>>  
>>  struct NS16550 {
>>
>>
>> Then you could do a
>>
>> #define CONFIG_SYS_NS16550_REG_SIZE 4
>> #define CONFIG_SYS_NS16550_REG_TYPE unsigned long
>>
>> This of course needs to be documented once it works ;)
> 
> Looks to me like playing with macros...

Looks to me like playing with macros... but,
this is better than before, and would work for my machine.

I'm alo not interested in maintaining UART driver, so such work
reducing our maintenance cost is highly appreciated.  Thanks,

  Shinya



More information about the U-Boot mailing list