Dear Stefano Babic, In message <1263212760-27272-10-git-send-email-sba...@denx.de> you wrote: > The patch adds initial support for the Freescale mx51evk board. > Network (FEC) and SD controller (fsl_esdhc) are supported.
> --- a/Makefile > +++ b/Makefile > @@ -324,6 +324,10 @@ $(obj)u-boot.img: $(obj)u-boot.bin > sed -e 's/"[ ]*$$/ for $(BOARD) board"/') \ > -d $< $@ > > +$(obj)u-boot.imx: $(obj)u-boot.bin > + $(obj)tools/mkimage -n $(IMX_CONFIG) -T imximage \ > + -e $(TEXT_BASE) -d $< $@ This actually belongs into the patch that adds the imx image format support. > +int dram_init(void) > +{ > + gd->bd->bi_dram[0].start = PHYS_SDRAM_1; > + gd->bd->bi_dram[0].size = get_ram_size((long *)PHYS_SDRAM_1, > PHYS_SDRAM_1_SIZE); Line too long. Please check globally. > + return 0; > +} > + > +static void setup_uart(void) > +{ > + unsigned int pad = PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | > + PAD_CTL_PUE_PULL | PAD_CTL_DRV_HIGH; Indentation by TAB, please. Please check globally. > +static void setup_expio(void) > +{ > + u32 reg; > + > + /* CS5 setup */ > + mxc_request_iomux(MX51_PIN_EIM_CS5, IOMUX_CONFIG_ALT0); > + writel(0x00410089, WEIM_BASE_ADDR + 0x78 + CSGCR1); > + writel(0x00000002, WEIM_BASE_ADDR + 0x78 + CSGCR2); > + /* RWSC=50, RADVA=2, RADVN=6, OEA=0, OEN=0, RCSA=0, RCSN=0 */ > + writel(0x32260000, WEIM_BASE_ADDR + 0x78 + CSRCR1); > + /* APR = 0 */ > + writel(0x00000000, WEIM_BASE_ADDR + 0x78 + CSRCR2); What is this magic offset 0x78 here? Please get rid of using base address + offset notation, use C struncts instead (see previous review comments about this). Please fix globally. > + /* Reset the XUART and Ethernet controllers */ > + reg = readw(&(mx51_io_board->sw_reset)); > + reg |= 0x9; > + writew(reg, &(mx51_io_board->sw_reset)); > + reg &= ~0x9; > + writew(reg, &(mx51_io_board->sw_reset)); > +} > + > +static void setup_fec(void) > +{ FEC should only be set up (and eventually only be reset, too), if there is any network support at all on this board. ... > +int board_mmc_init(bd_t *bis) > +{ > + u32 interface_esdhc = 0; > + s32 status = 0; > + u32 esdhc_base_pointer; > + > + for (interface_esdhc = 0; interface_esdhc < CONFIG_SYS_FSL_ESDHC_NUM; > interface_esdhc++) { > + switch (interface_esdhc) { > + case 0: Incorrect indent. And line too loing above, of course. ... > + break; > + case 1: ... > + break; > + } Incorrect indent. Default case? > diff --git a/board/freescale/mx51evk/mx51evk.h > b/board/freescale/mx51evk/mx51evk.h > new file mode 100644 > index 0000000..fec886d > --- /dev/null > +++ b/board/freescale/mx51evk/mx51evk.h > @@ -0,0 +1,49 @@ > +/* > + * Copyright 2009 Freescale Semiconductor, Inc. All Rights Reserved. > + */ Please get rid off the "All Rights Reserved." > +/* > + * The code contained herein is licensed under the GNU General Public > + * License. You may obtain a copy of the GNU General Public License > + * Version 2 or later at the following locations: As mentioned before, this clause is not acceptable. > diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h > new file mode 100644 > index 0000000..c8b2970 > --- /dev/null > +++ b/include/configs/mx51evk.h ... > + /* High Level Configuration Options */ > +#define CONFIG_SYS_APCS_GNU What's this? It seems to be not used anywhere, nor documented? > +#define CONFIG_L2_OFF Is this a "High Level Configuration Option"? Move down! > +#define CONFIG_SYS_MEMTEST_START CSD0_BASE_ADDR > +#define CONFIG_SYS_MEMTEST_END 0x10000 > + > +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR Here and everywhere else: please use TABs for vertical alignment. > +#define CONFIG_ENV_SECT_SIZE (128 * 1024) > +#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE > +#define CONFIG_ENV_IS_NOWHERE Seems strange to me to define an environment sector size and an environment size and then to say there is no environment at all? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Nothing ever becomes real until it is experienced. - John Keats _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot