
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 : {
arch/arm/cpu/armv7/start.o (.text) *(.text) }*(.ivt)
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.
+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 ?
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@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.
+#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.
+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.
- 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.
+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.
+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.
+#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.
+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.
+/*
- 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 ?
+/*
- 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.
Best regards, Stefano Babic