From patchwork Wed Nov 23 14:40:47 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Simek X-Patchwork-Id: 127315 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id A3ECA1007D4 for ; Thu, 24 Nov 2011 01:40:58 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id A5F542820D; Wed, 23 Nov 2011 15:40:56 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TkiAAT2YR7GK; Wed, 23 Nov 2011 15:40:56 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id ABD5128202; Wed, 23 Nov 2011 15:40:54 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id DA48828202 for ; Wed, 23 Nov 2011 15:40:52 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NgeQ7yauqOin for ; Wed, 23 Nov 2011 15:40:52 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-bw0-f44.google.com (mail-bw0-f44.google.com [209.85.214.44]) by theia.denx.de (Postfix) with ESMTPS id 0EBA7281F5 for ; Wed, 23 Nov 2011 15:40:50 +0100 (CET) Received: by bkbzv15 with SMTP id zv15so1268241bkb.3 for ; Wed, 23 Nov 2011 06:40:50 -0800 (PST) Received: by 10.204.149.216 with SMTP id u24mr23954260bkv.73.1322059249121; Wed, 23 Nov 2011 06:40:49 -0800 (PST) Received: from monstr.eu ([178.23.216.97]) by mx.google.com with ESMTPS id x19sm22231271fag.5.2011.11.23.06.40.48 (version=SSLv3 cipher=OTHER); Wed, 23 Nov 2011 06:40:48 -0800 (PST) Message-ID: <4ECD05EF.3020005@monstr.eu> Date: Wed, 23 Nov 2011 15:40:47 +0100 From: Michal Simek User-Agent: Thunderbird 2.0.0.22 (X11/20090625) MIME-Version: 1.0 To: linz@li-pro.net References: <1321704237-10626-1-git-send-email-linz@li-pro.net> <4EC9FBDF.8070800@monstr.eu> <1321899537.3870.132.camel@keto> In-Reply-To: <1321899537.3870.132.camel@keto> Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] microblaze: usable uart16550 for big endian systems X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list Reply-To: monstr@monstr.eu List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@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