Hi Wolfgang, On Saturday 16 May 2009 10:47:46 Wolfgang Denk wrote: > ARIA is a MPC5121E based COM Express module by Dave/DENX. > > Signed-off-by: Wolfgang Denk <w...@denx.de> > Cc: John Rigby <jcri...@gmail.com>
Please find some mostly nitpicking comments below. (Sorry about the late review - I just stumbled over a few issue while using this port as basis for a port for an MPC5123 board from esd). <snip> > diff --git a/board/davedenx/aria/Makefile b/board/davedenx/aria/Makefile > new file mode 100644 > index 0000000..48c2a83 > --- /dev/null > +++ b/board/davedenx/aria/Makefile > @@ -0,0 +1,53 @@ > +# > +# (C) Copyright 2009 Wolfgang Denk <w...@denx.de> > +# > +# See file CREDITS for list of people who contributed to this > +# project. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, > +# MA 02111-1307 USA > +# > + > +include $(TOPDIR)/config.mk > + > +$(shell mkdir -p $(OBJTREE)/board/freescale/common) Is this really needed? > + > +LIB = $(obj)lib$(BOARD).a > + > +COBJS-y := $(BOARD).o > + > +COBJS := $(COBJS-y) > +SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) > +OBJS := $(addprefix $(obj),$(COBJS)) > +SOBJS := $(addprefix $(obj),$(SOBJS)) > + > +$(LIB): $(obj).depend $(OBJS) > + > + $(AR) $(ARFLAGS) $@ $(OBJS) Please remove this empty line above. > + > +clean: > + rm -f $(SOBJS) $(OBJS) > + > +distclean: clean > + rm -f $(LIB) core *.bak $(obj).depend > + > +######################################################################### > + > +# defines $(obj).depend target > +include $(SRCTREE)/rules.mk > + > +sinclude $(obj).depend > + > +######################################################################### > diff --git a/board/davedenx/aria/aria.c b/board/davedenx/aria/aria.c > new file mode 100644 > index 0000000..4d26713 > --- /dev/null > +++ b/board/davedenx/aria/aria.c > @@ -0,0 +1,317 @@ > +/* > + * (C) Copyright 2009 Wolfgang Denk <w...@denx.de> > + * (C) Copyright 2009 Dave Srl www.dave.eu > + * > + * See file CREDITS for list of people who contributed to this > + * project. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > + * MA 02111-1307 USA > + * > + */ > + > +#include <common.h> > +#include <asm/bitops.h> > +#include <command.h> > +#include <asm/io.h> > +#include <asm/processor.h> > +#include <fdt_support.h> > +#ifdef CONFIG_MISC_INIT_R > +#include <i2c.h> > +#endif > + > +DECLARE_GLOBAL_DATA_PTR; > + > +extern void ide_set_reset(int idereset); > + > +/* Clocks in use */ > +#define SCCR1_CLOCKS_EN (CLOCK_SCCR1_CFG_EN | > \ > + CLOCK_SCCR1_LPC_EN | \ > + CLOCK_SCCR1_PSC_EN(CONFIG_PSC_CONSOLE) | \ > + CLOCK_SCCR1_PSCFIFO_EN | \ > + CLOCK_SCCR1_DDR_EN | \ > + CLOCK_SCCR1_FEC_EN | \ > + CLOCK_SCCR1_PATA_EN | \ > + CLOCK_SCCR1_PCI_EN | \ > + CLOCK_SCCR1_TPR_EN) > + > +#define SCCR2_CLOCKS_EN (CLOCK_SCCR2_MEM_EN | \ > + CLOCK_SCCR2_SPDIF_EN | \ > + CLOCK_SCCR2_DIU_EN | \ > + CLOCK_SCCR2_I2C_EN) > + > +#define CSAW_START(start) ((start) & 0xFFFF0000) > +#define CSAW_STOP(start, size) (((start) + (size) - 1) >> 16) > + > +long int fixed_sdram(void); > + > +int board_early_init_f(void) > +{ > + volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR; > + u32 spridr; > + > + /* > + * Initialize Local Window for the On Board FPGA access > + */ > + out_be32(&im->sysconf.lpcs2aw, > + CSAW_START(CONFIG_SYS_ARIA_FPGA_BASE) | > + CSAW_STOP(CONFIG_SYS_ARIA_FPGA_BASE, CONFIG_SYS_ARIA_FPGA_SIZE) > + ); > + out_be32(&im->lpc.cs_cfg[2], CONFIG_SYS_CS2_CFG); > + > + /* > + * According to MPC5121e RM, configuring local access windows should > + * be followed by a dummy read of the config register that was > + * modified last and an isync > + */ > + in_be32(&im->sysconf.lpcs2aw); > + __asm__ __volatile__ ("isync"); > + > + /* > + * Initialize Local Window for the On Board SRAM access > + */ > + out_be32(&im->sysconf.lpcs6aw, > + CSAW_START(CONFIG_SYS_ARIA_SRAM_BASE) | > + CSAW_STOP(CONFIG_SYS_ARIA_SRAM_BASE, CONFIG_SYS_ARIA_SRAM_SIZE) > + ); > + out_be32(&im->lpc.cs_cfg[6], CONFIG_SYS_CS6_CFG); > + > + /* > + * According to MPC5121e RM, configuring local access windows should > + * be followed by a dummy read of the config register that was > + * modified last and an isync > + */ > + in_be32(&im->sysconf.lpcs6aw); > + __asm__ __volatile__ ("isync"); > + > + /* > + * Configure Flash Speed > + */ > + out_be32(&im->lpc.cs_cfg[0], CONFIG_SYS_CS0_CFG); > + > + spridr = in_be32(&im->sysconf.spridr); > + > + if (SVR_MJREV(spridr) >= 2) { > + out_be32(&im->lpc.altr, CONFIG_SYS_CS_ALETIMING); > + } Curly braces can be removed. And I suggest to add an empty line here. > + /* > + * Enable clocks > + */ > + out_be32(&im->clk.sccr[0], SCCR1_CLOCKS_EN); > + out_be32(&im->clk.sccr[1], SCCR2_CLOCKS_EN); > +#if defined(CONFIG_IIM) || defined(CONFIG_CMD_FUSE) > + setbits_be32(&im->clk.sccr[1], CLOCK_SCCR2_IIM_EN); > +#endif > + > + return 0; > +} > + > +phys_size_t initdram (int board_type) > +{ > + return fixed_sdram(); > +} > + > +/* > + * fixed sdram init: > + * The board doesn't use memory modules that have serial presence > + * detect or similar mechanism for discovery of the DRAM settings > + */ > +long int fixed_sdram (void) > +{ > + volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR; > + u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024; > + u32 msize_log2 = __ilog2(msize); > + u32 i; > + > + /* Initialize IO Control */ > + out_be32(&im->io_ctrl.io_control_mem, IOCTRL_MUX_DDR); > + > + /* Initialize DDR Local Window */ > + out_be32(&im->sysconf.ddrlaw.bar, CONFIG_SYS_DDR_BASE & 0xFFFFF000); > + out_be32(&im->sysconf.ddrlaw.ar, msize_log2 - 1); > + > + /* > + * According to MPC5121e RM, configuring local access windows should > + * be followed by a dummy read of the config register that was > + * modified last and an isync > + */ > + in_be32(&im->sysconf.ddrlaw.ar); > + __asm__ __volatile__ ("isync"); > + > + /* Enable DDR */ > + out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG_EN); > + > + /* 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); + > + /* Initialize MDDRC */ > + out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG); > + out_be32(&im->mddrc.ddr_time_config0, CONFIG_SYS_MDDRC_TIME_CFG0); > + out_be32(&im->mddrc.ddr_time_config1, CONFIG_SYS_MDDRC_TIME_CFG1); > + out_be32(&im->mddrc.ddr_time_config2, CONFIG_SYS_MDDRC_TIME_CFG2); > + > + /* Initialize DDR */ > + for (i = 0; i < 10; i++) > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM3); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EN_DLL); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_OCD_DEFAULT); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL); > + out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP); > + > + /* Start MDDRC */ > + out_be32(&im->mddrc.ddr_time_config0, CONFIG_SYS_MDDRC_TIME_CFG0_RUN); > + out_be32(&im->mddrc.ddr_sys_config, CONFIG_SYS_MDDRC_SYS_CFG_RUN); > + > + return msize; > +} > + > +int misc_init_r(void) > +{ > + u32 tmp; > + extern int mpc5121_diu_init(void); Please move prototype declaration to top of file or to some header. > + > + /* we use I2C-2 for on-board eeprom */ > + i2c_set_bus_num(2); > + > + tmp = in_be32((u32*)CONFIG_SYS_ARIA_FPGA_BASE); > + printf("FPGA: %u-%u.%u.%u\n", > + (tmp & 0xFF000000) >> 24, > + (tmp & 0x00FF0000) >> 16, > + (tmp & 0x0000FF00) >> 8, > + tmp & 0x000000FF > + ); > + > +#ifdef CONFIG_FSL_DIU_FB > +#if !(defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)) > + mpc5121_diu_init(); > +#endif > +#endif > + > + return 0; > +} Insert empty line here. > +static iopin_t ioregs_init[] = { > + /* > + * FEC > + */ > + > + /* FEC on PSCx_x*/ > + { > + offsetof(struct ioctrl512x, io_control_psc0_0), 5, 0, > + IO_PIN_FMUX(1) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(0) | IO_PIN_DS(3) > + }, > + { > + offsetof(struct ioctrl512x, io_control_psc1_0), 10, 0, > + IO_PIN_FMUX(1) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(0) | IO_PIN_DS(3) > + }, > + { > + offsetof(struct ioctrl512x, io_control_spdif_txclk), 3, 0, > + IO_PIN_FMUX(1) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(0) | IO_PIN_DS(3) > + }, > + > + /* > + * DIU > + */ > + /* FUNC2=DIU CLK */ > + { > + offsetof(struct ioctrl512x, io_control_psc6_0), 1, 0, > + IO_PIN_FMUX(2) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(1) | IO_PIN_DS(3) > + }, > + /* FUNC2=DIU_HSYNC */ > + { > + offsetof(struct ioctrl512x, io_control_psc6_1), 1, 0, > + IO_PIN_FMUX(2) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(0) | IO_PIN_DS(3) > + }, > + /* FUNC2=DIUVSYNC Sets Next 26 to DIU Pads */ > + { > + offsetof(struct ioctrl512x, io_control_psc6_4), 26, 0, > + IO_PIN_FMUX(2) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(0) | IO_PIN_ST(0) | IO_PIN_DS(3) > + }, > + /* > + * On board SRAM > + */ > + /* FUNC2=/LPC CS6 */ > + { > + offsetof(struct ioctrl512x, io_control_j1850_rx), 1, 0, > + IO_PIN_FMUX(2) | IO_PIN_HOLD(0) | IO_PIN_PUD(0) | > + IO_PIN_PUE(1) | IO_PIN_ST(1) | IO_PIN_DS(3) > + }, > +}; > + > + > +int checkboard (void) > +{ > + puts("Board: ARIA\n"); > + > + /* initialize function mux & slew rate IO inter alia on IO Pins */ > + > + iopin_initialize(ioregs_init, > + sizeof(ioregs_init) / sizeof(ioregs_init[0])); Please use ARRAY_SIZE(ioregs_init) here. 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