From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Fri, 17 Dec 2010 14:05:22 +0100 Subject: [U-Boot] [PATCH 5/5] MX5:MX53: add initial support for MX53EVK board In-Reply-To: References: <1292494665-25674-1-git-send-email-r64343@freescale.com> <1292494665-25674-6-git-send-email-r64343@freescale.com> <4D0A11F2.3070106@denx.de> Message-ID: <4D0B6012.4090407@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 12/17/2010 04:05 AM, Jason Liu wrote: > There is pretty much different with I.MX51 ROM, we will not use DCD > data file to config the > DDR script since ROM has the DCD size limitation and use the advance > feature of what we > called plug-in, the plug-in code must be in the first 2K of MMC card > from 0x400 offset, that's > why we need put this code section before start.S. The plug-in code > will be called by boot ROM > to do DDR init first and copy u-boot to DDR and jump to _start to run it. As I am not understanding what you mean as "plugin", it seems to me you need some code able to set-up RAM and copy the u-boot code. This method looks like very similar to other SoCs, where a first stage boot loader is needed or when we boot from NAND. I do not find any reasonable argument to follow the methods you are proposing and to link statically this code to u-boot. In U-Boot we have already two mechanisms for do this initial setup, one using special images (with .kwb and .imx images when the RBL of the SoC is able to do the whole job), the second one with a first stage bootloader as nand_spl does and as Albert has already suggested. > Does anyone of expert like you and Albert and Wolfgang tell me how to > do it? It's very important for FSL > coming many SOCs support, since we will use plug-in feature of boot ROM. I am really not understanding if this "plugin" feature is really different as what we have with other SoCs or when we boot from NAND on other SOCs. As I can up now understand, it is similar to other approaches we have support already and I do not see a reason to implement a new one. Is there some released Freescale's documentation to allow to me to better understand what you are meaning ? > Yes, we need write it in assembly. Please see the comments above. > We can't put it into board_init or board_early_init_f, because it does > DDR init. > It *must* run in IRAM which load by ROM first. Running in IRAM is not a sufficient reason to do in this way. If I am not wrong, we could reach the same behavior on a i.MX51, too, if we set the app_code_jump in the header to point to an IRAM address. Then even the i.MX51 RBL should copy the code from storage to IRAM and then jump to this address, isnt it ? > I can't catch your mean. The function just do i2c iomux setup, anything wrong? My concern is related to the fact that the controllers are normally enumerated as "port numbers" and you make the selection based on the base address, that could be confusing. I see an approach of the type: static void setup_i2c(unsigned int port_number) { switch (port_number) { 0: ..... break; 1: ......... } more intuitive to understand. >>> + /* Set core voltage VDDGP to 1.05V for 800MHZ */ >>> + buf[0] = 0x45; >>> + buf[1] = 0x4a; >>> + buf[2] = 0x52; >> >> Please remove all fixed constants in the file and replaced them with >> named constants, defined in a header file. Check vision2.c as reference. >> This board uses the MC13892 PMIC controller, and ./include/mc13892.h >> contains all required defines. > > OK, As further comment: there is already a pmic driver in u-boot (fsl_pmic.c), that hides the underlying communication between processor and pmic itself. At the moment, only spi is implemented. It is better to expand the fsl_pmic code to support i2c, too, and to use the accessors pmic_reg_read() and pmic_reg_write() to access to the pmic internal registers in the board related code. > No, this is not the case like MX53. The power of FEC is default on of > this board. > We need change power before we can run CPU at full speed. Ok, understood. >>> +#define CONFIG_SYS_I2C_PORT I2C2_BASE_ADDR >> >> As stated before: port means an enumeration value (0,1,..N), and it is >> set to a physical address. > > Yes, I will fix it. This comment is related to the previous one for the setup_i2c() function. Sorry if I was not clear enough. As you describe the define CONFIG_SYS_I2C_PORT, this should be a port number (0,1,..N) and the setup_i2() should accept as parameter this enumeration value, not a base address. Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================