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

Detlev Zundel dzu at denx.de
Mon Apr 27 15:41:30 CEST 2009


Dear Shinya,

> Detlev Zundel wrote:
>> Instead of special casing the different access patterns, use common
>> code with light macros sprinkled in to accomodate for the different
>> layouts of the register structure.
>> 
>> Note that this also changes the types of the registers for the
>> "positively packed (>1)" cases.  As the registers truly are unsigned
>> chars, this is surely the Right Thing, but it is a semantic change.
>> Note that for this case depending on the endianness on the bus, we may
>> see a change of behaviour.
>> 
>> Signed-off-by: Detlev Zundel <dzu at denx.de>
>> ---
>>  include/ns16550.h |  130 +++++++++++++++--------------------------------------
>>  1 files changed, 37 insertions(+), 93 deletions(-)
>> 
>> 
>> Note, that I checked that the offsets are ok in the used cases
>> switching from the old to the new code.  They *do* shift however in
>> the positive packed cases, because the old code uses data types
>> different than unsigned char.  Note that doing this, I also noticed
>> that using "unsigned long" for 4 byte registers is also no longer true
>> on 64-bit architectures.  One more reason to change the code.
>> 
>> Apart from that the code was also compile tested on several
>> configurations using different REG_SIZES and compiles without
>> warnings.  The special interesting case of +4 was successfully tested
>> on CU824.
>
> My hardware required 32-bit word access to NS16550 registers due to
> byte-enable-lane reason (note that it's different from endian-ness).
>
> I mean,
>
> struct NS16550 {
>     unsigned char rbr;
>     unsigned char postpad_rbr[15];
>         :
>         :
> };
>
> if different from
>
> struct NS16550 {
>     unsigned long rbr;
>     unsigned long postpad_rbr[3];
>         :
>         :
> };
>
> , at least for my hardware.

To be honest, I did not expect such problems, as I saw no hints from
comments on why this code was needed.  Thinking afresh, it now makes at
least some sense why the code was.  It nevertheless was inconsistent, as
the word access was only done in an asymmetric way regarding the
REG_SIZES parameter.

> How do I supposed to configure UART in my board config file?

I'm not sure at all.  I believe you tested with 4 and -4 and it doesn't
work, right?

Now we have the problem that we have byte registers (after all, there
are only 8 bits significant even for your platform) which need to be
accessed as 32-bit entities (or 16 bit for other platforms maybe).

I don't see any way out here than to probably re-introduce different
data-types again - which I certainly do not like too much as the
registers stay 8 bit wide.

Does anyone else have a good idea here?

Thanks
  Detlev

-- 
"Oh, didn't you know, the Lord did the original programming of the universe in
COBOL." - "That's why the world is the evil work of Satan. A true divine being
would have used Scheme."  -  "And, if so, Jesus would have been crucified on a
big lambda symbol."  -- K. Chafin, K. Schilling & D. Hanley, on comp.lang.lisp
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de


More information about the U-Boot mailing list