From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Wed, 06 May 2009 23:16:04 +0200 Subject: [U-Boot] [PATCH 01/10] mx27: basic cpu support In-Reply-To: <1241634633-13917-2-git-send-email-yanok@emcraft.com> References: <1241634633-13917-1-git-send-email-yanok@emcraft.com> <1241634633-13917-2-git-send-email-yanok@emcraft.com> Message-ID: <20090506211604.289C483420E8@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Ilya, in message <1241634633-13917-2-git-send-email-yanok@emcraft.com> you wrote: > This patch adds generic code to support Freescale's i.MX27 SoCs. ... > +static ulong clk_in_26m(void) > +{ > + if (CSCR & CSCR_OSC26M_DIV1P5) { > + /* divide by 1.5 */ > + return 26000000 / 1.5; We definitely do not allow any FP use in U-Boot. > + } else { > + /* divide by 1 */ > + return 26000000; divide by 1 ??? > +#if defined(CONFIG_DISPLAY_CPUINFO) > +int print_cpuinfo (void) > +{ > + printf("CPU: Freescale i.MX27 at %llu MHz\n", > + lldiv(imx_get_mpllclk(), 1000000)); Please use strmhz() to print frequencies. > + printf("\n"); No need to waste another function call - just add this to the previous format element. > +void imx_gpio_mode(int gpio_mode) > +{ > + unsigned int pin = gpio_mode & GPIO_PIN_MASK; > + unsigned int port = (gpio_mode & GPIO_PORT_MASK) >> GPIO_PORT_SHIFT; > + unsigned int ocr = (gpio_mode & GPIO_OCR_MASK) >> GPIO_OCR_SHIFT; > + unsigned int aout = (gpio_mode & GPIO_AOUT_MASK) >> GPIO_AOUT_SHIFT; > + unsigned int bout = (gpio_mode & GPIO_BOUT_MASK) >> GPIO_BOUT_SHIFT; > + unsigned int tmp; > + > + /* Pullup enable */ > + if(gpio_mode & GPIO_PUEN) > + PUEN(port) |= (1 << pin); > + else > + PUEN(port) &= ~(1 << pin); This smells as if these were pointer accesses using register offsets instead of I/O accessor calls using C structs? You probably want to use the respective clrbits*() / setbits*() macros here? ... > +void reset_cpu (ulong ignored) > +{ > + /* Disable watchdog and set Time-Out field to 0 */ > + WCR = 0x00000000; > + > + /* Write Service Sequence */ > + WSR = 0x00005555; > + WSR = 0x0000AAAA; Again: please use I/O accessor calls; also for the rest of the file and the other patches. ... > diff --git a/include/asm-arm/arch-mx27/imx-regs.h b/include/asm-arm/arch-mx27/imx-regs.h > new file mode 100644 > index 0000000..a856f7e ... > +# ifndef __ASSEMBLY__ > +# define __REG(x) (*((volatile u32 *)(x))) > +# define __REG16(x) (*(volatile u16 *)(x)) > +# define __REG2(x,y) (*(volatile u32 *)((u32)&__REG(x) + (y))) > + > +extern void imx_gpio_mode (int gpio_mode); > + > +# else > +# define __REG(x) (x) > +# define __REG16(x) (x) > +# define __REG2(x,y) ((x)+(y)) > +#endif > + > +#define IMX_IO_BASE 0x10000000 > + > +#define IMX_AIPI1_BASE (0x00000 + IMX_IO_BASE) > +#define IMX_WDT_BASE (0x02000 + IMX_IO_BASE) > +#define IMX_TIM1_BASE (0x03000 + IMX_IO_BASE) > +#define IMX_TIM2_BASE (0x04000 + IMX_IO_BASE) > +#define IMX_TIM3_BASE (0x05000 + IMX_IO_BASE) > +#define IMX_UART1_BASE (0x0a000 + IMX_IO_BASE) > +#define IMX_UART2_BASE (0x0b000 + IMX_IO_BASE) > +#define IMX_UART3_BASE (0x0c000 + IMX_IO_BASE) > +#define IMX_UART4_BASE (0x0d000 + IMX_IO_BASE) > +#define IMX_I2C1_BASE (0x12000 + IMX_IO_BASE) > +#define IMX_GPIO_BASE (0x15000 + IMX_IO_BASE) > +#define IMX_TIM4_BASE (0x19000 + IMX_IO_BASE) > +#define IMX_TIM5_BASE (0x1a000 + IMX_IO_BASE) > +#define IMX_UART5_BASE (0x1b000 + IMX_IO_BASE) > +#define IMX_UART6_BASE (0x1c000 + IMX_IO_BASE) > +#define IMX_I2C2_BASE (0x1D000 + IMX_IO_BASE) > +#define IMX_TIM6_BASE (0x1f000 + IMX_IO_BASE) > +#define IMX_AIPI2_BASE (0x20000 + IMX_IO_BASE) > +#define IMX_PLL_BASE (0x27000 + IMX_IO_BASE) > +#define IMX_SYSTEM_CTL_BASE (0x27800 + IMX_IO_BASE) > +#define IMX_IIM_BASE (0x28000 + IMX_IO_BASE) > +#define IMX_FEC_BASE (0x2b000 + IMX_IO_BASE) NAK. We do not accept device I/O through pointers; please use C structs to describe the hardware and use I/O accessor calls. > +/* AIPI */ > +#define AIPI1_PSR0 __REG(IMX_AIPI1_BASE + 0x00) > +#define AIPI1_PSR1 __REG(IMX_AIPI1_BASE + 0x04) > +#define AIPI2_PSR0 __REG(IMX_AIPI2_BASE + 0x00) > +#define AIPI2_PSR1 __REG(IMX_AIPI2_BASE + 0x04) > + > +/* System Control */ > +#define FMCR __REG(IMX_SYSTEM_CTL_BASE + 0x14) > +#define GPCR __REG(IMX_SYSTEM_CTL_BASE + 0x18) > +#define WBCR __REG(IMX_SYSTEM_CTL_BASE + 0x1C) NAK, for this and all similar code. 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: wd at denx.de It is better to marry than to burn. - Bible ``I Corinthians'' ch. 7, v. 9