Dear Matthias Weisser,
In message 1247039227-17910-2-git-send-email-matthias.weisser@graf-syteco.de you wrote:
This patch adds support for MB86R01 'Jade' SoC from Fujitsu
...
+/* nothing really to do with interrupts, just starts up a counter. */ +int timer_init(void) +{
- *(volatile ulong *)(TIMER_BASE + 0) = TIMER_LOAD_VAL;
- *(volatile ulong *)(TIMER_BASE + 8) = 0x86;
Here and everyhere else: please use accessor functions instead of (volatile) register assignments.
+unsigned long long get_ticks(void) +{
- ulong now = READ_TIMER;
- if (now <= lastdec) /* normal mode (non roll) */
/* move stamp forward with absolut diff ticks */timestamp += (lastdec - now);- else /* we have rollover of incrementer */
Please add braces for multiline branches.
+void udelay(unsigned long usec) +{
- unsigned long long tmp;
- ulong tmo;
- tmo = usec_to_tick(usec);
- tmp = get_ticks() + tmo; /* get current timestamp */
- while (get_ticks() < tmp) /* loop till event */
/*NOP*/;
What about the timestamp wrapping around?
+ulong get_tbclk(void) +{
- ulong tbclk;
- tbclk = CONFIG_SYS_HZ;
- return tbclk;
+}
Why not simply "return CONFIG_SYS_HZ;" ?
diff --git a/include/asm-arm/arch-jade/jade.h b/include/asm-arm/arch-jade/jade.h new file mode 100755 index 0000000..c2b28a2 --- /dev/null +++ b/include/asm-arm/arch-jade/jade.h
...
+#ifndef JADE_H +#define JADE_H
+typedef volatile unsigned int JREG; /* Hardware register definition */
This is not needed / allowed/ Please use accessor functions.
+/*
- Physical Address Defines
- */
+#define JADE_GDC_PHYS_BASE 0xf1fc0000 /* GDC phys */ +#define JADE_GDC_PHYS_DISP_BASE 0xf1fd0000 /* GDC DisplayBase phys */ +#define JADE_CCNT_PHYS_BASE 0xfff42000 /* Chip Control Module */ +#define JADE_CAN0_PHYS_BASE 0xfff54000 /* CAN 0 phys */ +#define JADE_CAN1_PHYS_BASE 0xfff55000 /* CAN 1 phys */ +#define JADE_I2C0_PHYS_BASE 0xfff56000 /* I2C 0 phys */ +#define JADE_I2C1_PHYS_BASE 0xfff57000 /* I2C 1 phys */ +#define JADE_EHCI_PHYS_BASE 0xfff80000 /* EHCI phys */ +#define JADE_OHCI_PHYS_BASE 0xfff81000 /* OHCI phys */ +#define JADE_IRC1_PHYS_BASE 0xfffb0000 /* Jade cascaded Interrupt Controller phys */
Line too long (many more too long lines, please check globally).
+#define JADE_TIMER_PHYS_BASE 0xfffe0000 /* Counter/Timers JADE phys */ +#define JADE_UART0_PHYS_BASE 0xfffe1000 /* UART 0 phys */ +#define JADE_UART1_PHYS_BASE 0xfffe2000 /* UART 1 phys */ +#define JADE_IRCE_PHYS_BASE 0xfffe4000 /* Extended Interrupt Controller */ +#define JADE_CRG_PHYS_BASE 0xfffe7000 /* Clock Reset Generator */ +#define JADE_IRC0_PHYS_BASE 0xfffe8000 /* Jade Interrupt Controller phys */ +#define JADE_GPIO_PHYS_BASE 0xfffe9000 /* GPIO phys */
Please do not use register addresses directly - declare a C structure, and use typed variables. This applies not only here, but globally.
Best regards,
Wolfgang Denk