Dear Stefano Babic,
In message 1295012124-15551-1-git-send-email-sbabic@denx.de you wrote:
The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.
Signed-off-by: Stefano Babic sbabic@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