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

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.

Signed-off-by: Stephan Linz <linz@li-pro.net>
---
 include/configs/microblaze-generic.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

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

LE system:
#define XILINX_UART16550
#define XILINX_UART16550_BASEADDR	0x83e00000

Then you can use origin config file and change is only in xparameters.h in board folder.

Anyway you solution looks interesting and I will test it.

Thanks,
Michal

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.