[U-Boot] [PATCH] microblaze: usable uart16550 for big endian systems

[Stephan Linz]

Am Montag, den 21.11.2011, 08:21 +0100 schrieb Michal Simek: 
> Hi Stephan,
> 
> Stephan Linz wrote:
> > As a result of the commit 6833260 the uart16550 driver
> > is broken for Microblaze big endian systems, because of
> > the missing 3 byte offset. Other than as described, the
> > U-Boot BSP does not treat properly the 3 byte offset.
> > 
> > However, with the new 32 bit access to ns16550 registers
> > we can enable correct register access for Microblaze big
> > and little endian systems in the same manner.
> 
> The reason why I have applied that patch is that baseaddress generation
> was moved to u-boot BSP out of u-boot configs.
> 
> Here is example how addresses are generated.
> BE system:
> #define XILINX_UART16550
> #define XILINX_UART16550_BASEADDR	0x83e00003

Hi Michal,

Who is generating this entry (especially incl. this offset)? Was it the
MDL environment from PetaLinux? If so, which version?


I use my own MDL environment from TPOS, and the generator for
xparameters.h does not add this offset, see proc put_uart16550_cfg():

https://gitorious.org/mbref/mbref/blobs/master/edk-repository/ThirdParty/lib/tpos_misclib.tcl#line1384

And seriously we never need this offset. With a sane endianess handling
in software we will access the right bytes in uart16550. The Xilinx FPGA
synthesis produce results that are good enough for us. All NS16550 8 bit
registers alligned on 32 bit memory access: 0x000000rr on BE and
0xrr000000 in LE.


The BSP generator (Xilinx MDL part) may never knows specifics about
software or unclean code. Moreover we have to change the code ;-)


Till now we have set CONFIG_SYS_NS16550_REG_SIZE to -4 and a offset of 3
to the NS16550 base address for Microblaze BE systems. As I can see in
ns16550.h that was completely wrong, or not? See:

#if !defined(CONFIG_SYS_NS16550_REG_SIZE)
#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;
#elif (CONFIG_SYS_NS16550_REG_SIZE < 0)
#define UART_REG(x)     \
        unsigned char x;\
        unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
#endif

struct NS16550 {
        UART_REG(rbr);          /* 0 */
        UART_REG(ier);          /* 1 */

... and so on. For BE systems we should use CONFIG_SYS_NS16550_REG_SIZE
set to 4 --> have a 3 byte gap on NS16550 base address and then point to
the right byte on offset 3, or not? On LE systems we need to set -4 for
*_REG_SIZE --> have a 3 byte gap after and betweeen each 8 bit
registers.

> Anyway you solution looks interesting and I will test it.

However since commit 79df120 we can use direct 32 bit access to 8 bit
NS16550 registers without gap generation in ns16550.h ... we need sane
in_*/out_* implementation.



-- 
Best regards,
Stephan Linz
______________________________________________________________________________
MB-Ref: http://www.li-pro.de/xilinx_mb:mbref:start
OpenDCC: http://www.li-pro.net/opendcc.phtml
PC/M: http://www.li-pro.net/pcm.phtml
Sourceforge: http://sourceforge.net/users/slz
Gitorious: https://gitorious.org/~slz