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?
u-boot BSP.
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.
I have look at it and tested on BE/LE. For 32bit accesses we need to implement
in/out_le32 functions which we don't have right now that's why please remove
this macro
from your patch.
Our BSP generates/ed +3 offset that's why I prefer to mask it in the same patch
to be sure that baseaddr is correct and compatible with old versions.
Here is patch I have used. Please add that changes to v2 patch.
Thanks,
Michal
diff --git a/include/configs/microblaze-generic.h
b/include/configs/microblaze-generic.h
index b740a28..8085130 100644
--- a/include/configs/microblaze-generic.h
+++ b/include/configs/microblaze-generic.h
@@ -55,10 +55,16 @@
#elif XILINX_UART16550_BASEADDR
# define CONFIG_SYS_NS16550 1
# define CONFIG_SYS_NS16550_SERIAL
+
+#if defined(__MICROBLAZEEL__)
# define CONFIG_SYS_NS16550_REG_SIZE -4
+#else
+# define CONFIG_SYS_NS16550_REG_SIZE 4
+#endif
+
# define CONFIG_CONS_INDEX 1
# define CONFIG_SYS_NS16550_COM1 \
- (XILINX_UART16550_BASEADDR + 0x1000)
+ ((XILINX_UART16550_BASEADDR & ~0xF) + 0x1000)
# define CONFIG_SYS_NS16550_CLK XILINX_UART16550_CLOCK_HZ
# define CONFIG_BAUDRATE 115200
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
