Hi Wolfgang,

On Monday 08 June 2009 12:38:18 Wolfgang Denk wrote:
> > +int eeprom_write_enable(unsigned dev_addr, int state)
> > +{
> > +   volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
> > +
> > +        if (dev_addr != CONFIG_SYS_I2C_EEPROM_ADDR)
> > +           return -1;
> > +
> > +        if (state == 0)
> > +           setbits_be32(&im->gpio.gpdat, 0x00100000);
> > +   else
> > +           clrbits_be32(&im->gpio.gpdat, 0x00100000);
> > +
> > +        return 0;
> > +}
>
> Please fix indentation - indentation by TAB only, please.

OK.

> ...
>
> > +int board_early_init_f(void)
> > +{
>
> ...
>
> > +   /* Initialize DDR Priority Manager */
> > +   out_be32(&im->mddrc.prioman_config1, CONFIG_SYS_MDDRCGRP_PM_CFG1);
> > +   out_be32(&im->mddrc.prioman_config2, CONFIG_SYS_MDDRCGRP_PM_CFG2);
> > +   out_be32(&im->mddrc.hiprio_config, CONFIG_SYS_MDDRCGRP_HIPRIO_CFG);
> > +   out_be32(&im->mddrc.lut_table0_main_upper,
> > CONFIG_SYS_MDDRCGRP_LUT0_MU);
> > +   out_be32(&im->mddrc.lut_table0_main_lower,
> > CONFIG_SYS_MDDRCGRP_LUT0_ML);
> > +   out_be32(&im->mddrc.lut_table1_main_upper,
> > CONFIG_SYS_MDDRCGRP_LUT1_MU);
> > +   out_be32(&im->mddrc.lut_table1_main_lower,
> > CONFIG_SYS_MDDRCGRP_LUT1_ML);
> > +   out_be32(&im->mddrc.lut_table2_main_upper,
> > CONFIG_SYS_MDDRCGRP_LUT2_MU);
> > +   out_be32(&im->mddrc.lut_table2_main_lower,
> > CONFIG_SYS_MDDRCGRP_LUT2_ML);
> > +   out_be32(&im->mddrc.lut_table3_main_upper,
> > CONFIG_SYS_MDDRCGRP_LUT3_MU);
> > +   out_be32(&im->mddrc.lut_table3_main_lower,
> > CONFIG_SYS_MDDRCGRP_LUT3_ML);
> > +   out_be32(&im->mddrc.lut_table4_main_upper,
> > CONFIG_SYS_MDDRCGRP_LUT4_MU);
> > +   out_be32(&im->mddrc.lut_table4_main_lower,
> > CONFIG_SYS_MDDRCGRP_LUT4_ML);
> > +   out_be32(&im->mddrc.lut_table0_alternate_upper,
> > CONFIG_SYS_MDDRCGRP_LUT0_AU);
> > +   out_be32(&im->mddrc.lut_table0_alternate_lower,
> > CONFIG_SYS_MDDRCGRP_LUT0_AL);
> > +   out_be32(&im->mddrc.lut_table1_alternate_upper,
> > CONFIG_SYS_MDDRCGRP_LUT1_AU);
> > +   out_be32(&im->mddrc.lut_table1_alternate_lower,
> > CONFIG_SYS_MDDRCGRP_LUT1_AL);
> > +   out_be32(&im->mddrc.lut_table2_alternate_upper,
> > CONFIG_SYS_MDDRCGRP_LUT2_AU);
> > +   out_be32(&im->mddrc.lut_table2_alternate_lower,
> > CONFIG_SYS_MDDRCGRP_LUT2_AL);
> > +   out_be32(&im->mddrc.lut_table3_alternate_upper,
> > CONFIG_SYS_MDDRCGRP_LUT3_AU);
> > +   out_be32(&im->mddrc.lut_table3_alternate_lower,
> > CONFIG_SYS_MDDRCGRP_LUT3_AL);
> > +   out_be32(&im->mddrc.lut_table4_alternate_upper,
> > CONFIG_SYS_MDDRCGRP_LUT4_AU);
> > +   out_be32(&im->mddrc.lut_table4_alternate_lower,
> > CONFIG_SYS_MDDRCGRP_LUT4_AL);
>
> Lines too long.

OK. Some is true for aria and mcp5121ads btw. But I'll change it in the next 
patch version.

> > +phys_size_t initdram(int board_type)
> > +{
> > +   return fixed_sdram();
> > +}
>
> Use auto-sizing?

You refer to using get_ram_size()? I'm no big fan of using this routine in a 
fixed environment (only one soldering possibility). But ok, I can change this 
too.

BTW, again this is the same code as in aria.

> > +int misc_init_r(void)
> > +{
> > +   volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR;
> > +
> > +   clrbits_be32(&im->clk.scfr[0], 0x00003800);
> > +   setbits_be32(&im->clk.scfr[0], 0x01 << 11);
> > +   clrbits_be32(&im->lpc.cs_dccr, 0x000000ff);
> > +   setbits_be32(&im->lpc.cs_dccr, (0x00 << 4) | (0x01 << 0));
> > +   clrbits_be32(&im->lpc.cs_hccr, 0x000000ff);
> > +   setbits_be32(&im->lpc.cs_hccr, (0x00 << 4) | (0x01 << 0));
>
> Please add some comments what you are doing here, and eventually
> #define the magic constants used.

OK.

> > --- /dev/null
> > +++ b/include/configs/mecp5123.h
> > @@ -0,0 +1,455 @@
> > +/*
> > + * (C) Copyright 2007, 2008 DENX Software Engineering
>
> 2009?

OK.

> > +/*
> > + * NOR FLASH on the Local Bus
> > + */
> > +#define CONFIG_SYS_FLASH_CFI                               /* use the 
> > Common 
Flash Interface */
> > +#define CONFIG_FLASH_CFI_DRIVER                            /* use the CFI 
driver */
> > +
> > +#define CONFIG_SYS_FLASH_BASE              0xFFC00000      /* start of 
> > FLASH   
*/
> > +#define CONFIG_SYS_FLASH_SIZE              0x00400000      /* max flash 
> > size 
in bytes */
>
> Lines too long (check everywhere!)

OK.

> > +#define    CONFIG_EXTRA_ENV_SETTINGS                                       
> > \
> > +   "u-boot_addr_r=200000\0"                                        \
> > +   "kernel_addr_r=600000\0"                                        \
> > +   "fdt_addr_r=880000\0"                                           \
> > +   "ramdisk_addr_r=900000\0"                                       \
> > +   "u-boot_addr=FFF00000\0"                                        \
> > +   "kernel_addr=FFC40000\0"                                        \
> > +   "fdt_addr=FFEC0000\0"                                           \
> > +   "ramdisk_addr=FC040000\0"                                       \
> > +   "ramdiskfile=/tftpboot/mecp512x/uRamdisk\0"                     \
> > +   "u-boot=/tftpboot/mecp512x/u-boot.bin\0"                        \
> > +   "bootfile=/tftpboot/mecp512x/uImage\0"                          \
> > +   "fdtfile=/tftpboot/mecp512x/mecp512x.dtb\0"                     \
> > +   "rootpath=/tftpboot/mecp512x/target_root\n"                     \
> > +   "netdev=eth0\0"                                                 \
> > +   "consdev=ttyPSC0\0"                                             \
> > +   "nfsargs=setenv bootargs root=/dev/nfs rw "                     \
> > +           "nfsroot=${serverip}:${rootpath}\0"                     \
> > +   "ramargs=setenv bootargs root=/dev/ram rw\0"                    \
> > +   "addip=setenv bootargs ${bootargs} "                            \
> > +           "ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}"      \
> > +           ":${hostname}:${netdev}:off panic=1\0"                  \
> > +   "addtty=setenv bootargs ${bootargs} "                           \
> > +           "console=${consdev},${baudrate}\0"                      \
> > +   "flash_nfs=run nfsargs addip addtty;"                           \
> > +           "bootm ${kernel_addr} - ${fdt_addr}\0"                  \
> > +   "flash_self=run ramargs addip addtty;"                          \
> > +           "bootm ${kernel_addr} ${ramdisk_addr} ${fdt_addr}\0"    \
> > +   "net_nfs=tftp ${kernel_addr_r} ${bootfile};"                    \
> > +           "tftp ${fdt_addr_r} ${fdtfile};"                        \
> > +           "run nfsargs addip addtty;"                             \
> > +           "bootm ${kernel_addr_r} - ${fdt_addr_r}\0"              \
> > +   "net_self=tftp ${kernel_addr_r} ${bootfile};"                   \
> > +           "tftp ${ramdisk_addr_r} ${ramdiskfile};"                \
> > +           "tftp ${fdt_addr_r} ${fdtfile};"                        \
> > +           "run ramargs addip addtty;"                             \
> > +           "bootm ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}\0"\
> > +   "load=tftp ${u-boot_addr_r} ${u-boot}\0"                        \
> > +   "update=protect off ${u-boot_addr} +${filesize};"               \
> > +           "era ${u-boot_addr} +${filesize};"                      \
> > +           "cp.b ${u-boot_addr_r} ${u-boot_addr} ${filesize}\0"    \
> > +   "upd=run load update\0"                                         \
> > +   ""
>
> How long does it to boot with such an environment in EEPROM?

Quite long. About 10 seconds to the U-Boot prompt. 

> > +#define CONFIG_OF_LIBFDT   1
> > +#define CONFIG_OF_BOARD_SETUP      1
> > +#define CONFIG_OF_SUPPORT_OLD_DEVICE_TREES 1
>
> Do you really need this on a new board?

Probably not. Again this is just a copy from aria. I'll remove it in the next 
patch version.

Thanks.

Best regards,
Stefan

=====================================================================
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: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to