Dear Stefano Babic, In message <1295012124-15551-1-git-send-email-sba...@denx.de> you wrote: > The patch adds basic support for the Freescale's i.MX35 > (arm1136 based) processor. > > Signed-off-by: Stefano Babic <sba...@denx.de> ... > +u32 get_cpu_rev(void) > +{ > + int reg; > + struct iim_regs *iim = > + (struct iim_regs *)IIM_BASE_ADDR; > + reg = readl(&iim->iim_srev); > + if (!reg) { > + reg = __REG(ROMPATCH_REV);
__REG()? NAK. Please use I/O accessors. Please fix globally. > +void imx_get_mac_from_fuse(unsigned char *mac) > +{ > + int i; > + > + for (i = 0; i < 6; i++) > + mac[i] = 0; memset(mac, 0, 6); ? > +/* > + * IOMUX register (base) addresses > + */ > +enum iomux_reg_addr { > + IOMUXGPR = IOMUXC_BASE_ADDR, > + /*!< General purpose */ > + IOMUXSW_MUX_CTL = IOMUXC_BASE_ADDR + 4, > + /*!< MUX control */ > + IOMUXSW_MUX_END = IOMUXC_BASE_ADDR + 0x324, > + /*!< last MUX control register */ > + IOMUXSW_PAD_CTL = IOMUXC_BASE_ADDR + 0x328, > + /*!< Pad control */ > + IOMUXSW_PAD_END = IOMUXC_BASE_ADDR + 0x794, > + /*!< last Pad control register */ > + IOMUXSW_INPUT_CTL = IOMUXC_BASE_ADDR + 0x7AC, > + /*!< input select register */ > + IOMUXSW_INPUT_END = IOMUXC_BASE_ADDR + 0x9F4, > + /*!< last input select register */ > +}; What do these "!<" markers mean? > +/* > + * This function is used to configure a pin through the IOMUX module. > + * FIXED ME: for backward compatible. Will be static function! > + * @param pin a pin number as defined in \b #iomux_pin_name_t > + * @param cfg an output function as defined in \b > #iomux_pin_cfg_t > + * > + * @return 0 if successful; Non-zero otherwise When does the "otherwise" happen? > + */ > +static int iomux_config_mux(iomux_pin_name_t pin, iomux_pin_cfg_t cfg) > +{ > + u32 mux_reg = PIN_TO_IOMUX_MUX(pin); > + > + if (mux_reg != NON_MUX_I) { > + mux_reg += IOMUXGPR; > + __REG(mux_reg) = cfg; > + } > + > + return 0; > +} Should we make this function return "void" ? > +/* delay x useconds AND perserve advance timstamp value */ > +/* GPTCNT is now supposed to tick 1 by 1 us. */ s/perserve/preserve/ ? Incorrect multiline comment style. > diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h > b/arch/arm/include/asm/arch-mx35/imx-regs.h > new file mode 100644 > index 0000000..f382960 > --- /dev/null > +++ b/arch/arm/include/asm/arch-mx35/imx-regs.h ... > +#define __REG(x) (*((volatile u32 *)(x))) > +#define __REG16(x) (*((volatile u16 *)(x))) > +#define __REG8(x) (*((volatile u8 *)(x))) NAK!! Please use I/O accessors. Please fix globally. > +/* CCM */ > +#define CLKCTL_CCMR 0x00 > +#define CLKCTL_PDR0 0x04 > +#define CLKCTL_PDR1 0x08 > +#define CLKCTL_PDR2 0x0C > +#define CLKCTL_PDR3 0x10 > +#define CLKCTL_PDR4 0x14 > +#define CLKCTL_RCSR 0x18 > +#define CLKCTL_MPCTL 0x1C > +#define CLKCTL_PPCTL 0x20 > +#define CLKCTL_ACMR 0x24 > +#define CLKCTL_COSR 0x28 > +#define CLKCTL_CGR0 0x2C > +#define CLKCTL_CGR1 0x30 > +#define CLKCTL_CGR2 0x34 > +#define CLKCTL_CGR3 0x38 NAK!! Please use C struct. Please fix globally. > +#if 0 > +extern unsigned int mxc_get_clock(enum mxc_clock clk); > +#endif Please remove such dead code. Please fix globally. > +extern unsigned int get_board_rev(void); > +extern int is_soc_rev(int rev); > +extern int sdhc_init(void); > + > +#define fixup_before_linux \ > + { \ > + volatile unsigned long *l2cc_ctl = (unsigned long *)0x30000100;\ 0x30000100 ? Don't we have a #define for it? > +/* > + * 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: Please fix these license headers - all of these. They say where you can _get_ License Version 2 or later, but they don't say which version actually applies. Please make clear that License v2+ applies. 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 By the way, ALL software projects are done by iterative prototyping. Some companies call their prototypes "releases", that's all. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot