[U-Boot] [PATCH 5/5] MX5:MX53: add initial support for MX53EVK board
Jason Liu
liu.h.jason at gmail.com
Fri Dec 17 04:05:10 CET 2010
Hi, Stefano,
2010/12/16 Stefano Babic <sbabic at denx.de>:
> On 12/16/2010 11:17 AM, Jason Liu wrote:
>> Add initial support for MX53EVK board support.
>> FEC, SD/MMC, UART, I2C, have been support.
>>
>> diff --git a/arch/arm/cpu/armv7/u-boot.lds b/arch/arm/cpu/armv7/u-boot.lds
>> index 5725c30..7b6ab66 100644
>> --- a/arch/arm/cpu/armv7/u-boot.lds
>> +++ b/arch/arm/cpu/armv7/u-boot.lds
>> @@ -34,6 +34,7 @@ SECTIONS
>> . = ALIGN(4);
>> .text :
>> {
>> + *(.ivt)
>> arch/arm/cpu/armv7/start.o (.text)
>> *(.text)
>> }
>
> We have already discussed this point, see my previous answer here:
>
> http://marc.info/?l=u-boot&m=127793013695282&w=2
>
> The solution in u-boot is *not* to link statically the IMX header to the
> u-boot.bin, but to generate a u-boot.imx image with a configuration
> file. This solution is already provided for the i.MX51 processor (same
> family), and you should go on this way, modifying tools/imximage.c for
> your needs, if required. This solution was previously discussed and
> accepted on the ML and it is compatible with other processors from
> different manufactures (kirchwood, see also u-boot.kwb).
>
> As already answered by Albert and Wolfgang, the header must not be part
> of u-boot.bin.
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.
I know that you add the .imx format support, that's is simple to add
one flash header which can be
calculated staticly and glue it to u-boot.bin but it's impossible for
plug-in feature support of ROM by
using the same way.
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.
>> +SOBJS := ivt.o
>
> This should be removed, and a imximage.cfg file should be written.
>
>> +plugin_start:
>> + /* Save the return address and the function arguments */
>> + push {r0-r3, lr}
>> +
>> + ldr r0, =ROM_SI_REV
>> + ldr r1, [r0]
>> +
>> + cmp r1, #0x20
>> +
>> + /* IOMUX Setup */
>> + ldr r0, =0x53fa8500
>> + moveq r1, #0x00180000
>> + movne r1, #0x00380000
>> + mov r2, #0x00380000
>> + add r2, r2, #0x40
>> + add r3, r1, #0x40
>> + mov r4, #0x00200000
>> +
>> + str r1, [r0, #0x54]
>> + str r2, [r0, #0x58]
>> + str r1, [r0, #0x60]
>> + str r3, [r0, #0x64]
>> + str r2, [r0, #0x68]
>> +
>> + streq r1, [r0, #0x70]
>> + strne r4, [r0, #0x70]
>> + str r1, [r0, #0x74]
>> + streq r1, [r0, #0x78]
>> + strne r4, [r0, #0x78]
>> + str r2, [r0, #0x7c]
>> + str r3, [r0, #0x80]
>> + str r1, [r0, #0x84]
>> + str r1, [r0, #0x88]
>> + str r2, [r0, #0x90]
>> + str r1, [r0, #0x94]
>> +
>> + ldr r0, =0x53fa86f0
>> + str r1, [r0, #0x0]
>> + mov r2, #0x00000200
>> + str r2, [r0, #0x4]
>> + mov r2, #0x00000000
>> + str r2, [r0, #0xc]
>> +
>> + ldr r0, =0x53fa8700
>> + str r2, [r0, #0x14]
>> + str r1, [r0, #0x18]
>> + str r1, [r0, #0x1c]
>> + str r1, [r0, #0x20]
>> +
>> + moveq r2, #0x02000000
>> + movne r2, #0x06000000
>> +
>> + str r2, [r0, #0x24]
>> + str r1, [r0, #0x28]
>> + str r1, [r0, #0x2c]
>> +
>> + /* Initialize DDR2 memory - Hynix H5PS2G83AFR */
>> + ldr r0, =ESDCTL_BASE_ADDR
>> +
>> + ldreq r1, =0x31333530
>> + ldrne r1, =0x2b2f3031
>> + str r1, [r0, #0x088]
>> +
>> + ldreq r1, =0x4a474a44
>> + ldrne r1, =0x40363333
>> + str r1, [r0, #0x090]
>> +
>> + /* add 3 logic unit of delay to sdclk */
>> + ldr r1, =0x00000f00
>> + str r1, [r0, #0x098]
>> +
>> + ldr r1, =0x00000800
>> + str r1, [r0, #0x0F8]
>> +
>> + ldreq r1, =0x02490241
>> + ldrne r1, =0x01310132
>> + str r1, [r0, #0x07c]
>> +
>> + ldreq r1, =0x01710171
>> + ldrne r1, =0x0133014b
>> + str r1, [r0, #0x080]
>> +
>> + /* Enable bank interleaving, RALAT = 0x4, DDR2_EN = 1 */
>> + ldr r1, =0x00001710
>> + str r1, [r0, #0x018]
>> +
>> + ldr r1, =0xc4110000
>> + str r1, [r0, #0x00]
>> +
>> + ldr r1, =0x4d5122d2
>> + str r1, [r0, #0x0C]
>> +
>> + ldr r1, =0x92d18a22
>> + str r1, [r0, #0x10]
>> +
>> + ldr r1, =0x00c70092
>> + str r1, [r0, #0x14]
>> +
>> + ldr r1, =0x000026d2
>> + str r1, [r0, #0x2C]
>> +
>> + ldr r1, =0x009f000e
>> + str r1, [r0, #0x30]
>> +
>> + ldr r1, =0x12272000
>> + str r1, [r0, #0x08]
>> +
>> + ldr r1, =0x00030012
>> + str r1, [r0, #0x04]
>> +
>> + ldr r1, =0x04008010
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008032
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008033
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008031
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0b5280b0
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x04008010
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008020
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008020
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0a528030
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x03c68031
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00468031
>> + str r1, [r0, #0x1C]
>> +
>> + /* Even though Rev B does not have DDR on CSD1, keep these
>> + * mode register initialization sequences for future uses since
>> + * it does not hurt to keep them
>> + */
>> + ldr r1, =0x04008018
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0000803a
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0000803b
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008039
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0b528138
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x04008018
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008028
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00008028
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x0a528038
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x03c68039
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00468039
>> + str r1, [r0, #0x1C]
>> +
>> + ldr r1, =0x00005800
>> + str r1, [r0, #0x20]
>> +
>> + ldr r1, =0x00033337
>> + str r1, [r0, #0x58]
>> +
>> + ldr r1, =0x00000000
>> + str r1, [r0, #0x1C]
>> +
>
> Why do we need to write these registers in assembly ? Why cannot we move
> them into board_init or board_early_init_f ? And again, should these
> values described better in imximage.cfg ?
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.
>
>> diff --git a/board/freescale/mx53evk/mx53evk.c b/board/freescale/mx53evk/mx53evk.c
>> new file mode 100755
>> index 0000000..ff6bfb2
>> --- /dev/null
>> +++ b/board/freescale/mx53evk/mx53evk.c
>> @@ -0,0 +1,412 @@
>> +/*
>> + * Copyright (C) 2007, Guennadi Liakhovetski <lg at denx.de>
>> + *
>> + * (C) Copyright 2009-2010 Freescale Semiconductor, Inc.
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>
> It seems to me that you derived this file from mx51evk.c, but you copied
> the header with copyrights from another (MX31, maybe) file.
The file from FSL uboot.
>
>> +#ifdef CONFIG_I2C_MXC
>> +static void setup_i2c(unsigned int module_base)
>
> I think it is better to enumerate the i2c controller as done in manual
> as with the physical address. Anyway, I do not see yet any released
> manual for this processor on Freescale's site, so I cannot be more precise.
I can't catch your mean. The function just do i2c iomux setup, anything wrong?
>
>> +void power_init(void)
>> +{
>> + unsigned char buf[4] = { 0 };
>> + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
>> +
>> + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +
>> + /* 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,
>
>> + if (i2c_write(0x8, 24, 1, buf, 3))
>> + return;
>
> Ditto. The same globally.
>
>> + if (is_soc_rev(CHIP_REV_2_0) == 0) {
>
> Please add some comments describing why depending on the processor
> revision we should to manage the pmic in a different way.
OK,
>
>> +int board_mmc_getcd(u8 *cd, struct mmc *mmc)
>> +{
>> + struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
>> +
>> + if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
>> + *cd = readl(GPIO3_BASE_ADDR) & 0x2000;
>
> Please change this. There is mxc_get_gpio() and mxc_set_gpio() accessors.
OK,
>
>> +int board_init(void)
>> +{
>> + system_rev = get_cpu_rev();
>
> I think we can get rid of system_rev. It is not used in this function
> and you can call get_cpu_rev() directly in checkboard() when the value
> is really needed.
OK, currently, it's not used.
>
>> +#ifdef BOARD_LATE_INIT
>> +int board_late_init(void)
>> +{
>> +#ifdef CONFIG_I2C_MXC
>
> Is it the board working if the pmic is not configured ? As I remember
> from mx51evk, the network did not work if the PMIC via SPI was not
> configured. If this is the case even for mx53evk, setting CONFIG_I2C_MXC
> is a must, else a lot of thing cannot work. Then I would prefer to
> remove these #ifdef and producing an error if it is not set at the
> beginning of the file.
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.
>
>> +int checkboard(void)
>> +{
>> + puts("Board: MX53EVK [");
>> +
>> + switch (__REG(SRC_BASE_ADDR + 0x8)) {
>
> Again, there is a "src" structure for i.MX51. If it is not correct for
> i.MX53, you have to adapt it in imx-regs.h, but you cannot access
> directly to registers. Please use always the correct structure or define
> newer ones if they do not exist.
Yes, OK,
>
>> +/*
>> + * Disabled for now due to build problems under Debian and a significant
>> + * increase in the final file size: 144260 vs. 109536 Bytes.
>> + */
>
> I see the same comment in mx51evk.h, but does it make sense ?
No, I think. We need remove it from mx51 too.
>
>> +/*
>> + * I2C Configs
>> + */
>> +#define CONFIG_CMD_I2C 1
>> +#define CONFIG_HARD_I2C 1
>> +#define CONFIG_I2C_MXC 1
>> +#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.
Thanks,
BR,
Jason
>
> 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
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
More information about the U-Boot
mailing list