From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Wed, 23 Nov 2011 15:40:47 +0100 Subject: [U-Boot] [PATCH] microblaze: usable uart16550 for big endian systems In-Reply-To: <1321899537.3870.132.camel@keto> References: <1321704237-10626-1-git-send-email-linz@li-pro.net> <4EC9FBDF.8070800@monstr.eu> <1321899537.3870.132.camel@keto> Message-ID: <4ECD05EF.3020005@monstr.eu> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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