Hi Wolfgang,
Wolfgang Denk wrote:
+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.
This will be actually converted to an integer at the compile time.
+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?
Ok, I really like using accessor calls instead of pointer accesses but I don't really understand the reason for using C structs here... I remember I've already asked you about that and you told me that it's for type safety... But we loose this type-safety when we are using I/O accessor functions! All pointers are just silently converted to the needed type... On the other hand Linux uses offsets for registers definitions so converting them to C structs makes porting and maintaining ported drivers harder...
You probably want to use the respective clrbits*() / setbits*() macros here?
I can see these macros defined only for ppc arch... And I really prefer generic writel(readl() | something) here... The reason is the same: to preserve as much code as it possible in drivers ported from Linux.
+#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.
These are actually base addresses. I don't think we can make use of C structs here.
Regards, Ilya.