Dear angelo,
In message 20121104195901.GA5141@angel3 you wrote:
Add support for freescale coldfire mcf5307 cpu.
...
--- /dev/null +++ b/arch/m68k/cpu/mcf530x/cpu_init.c
...
+#define MCF5307_SP_ERR_FIX(cs_base,mask) \
if((cs_base+(mask&0xffff0000))>=0xffff0000)mask|=0x20
Please never do this. Please ALWAYS use the standard "do { ... } while (0)" construct to make sure such macros can be used savely in any call envionment.
+void init_csm(void) +{
- volatile csm_t *csm = (csm_t *) (MMAP_CSM);
NAK. Please so not use volatile. Hey, did you run your ptches through checkpatch? What did it say?
+#if (defined(CONFIG_SYS_CS0_BASE) && defined(CONFIG_SYS_CS0_MASK) \
&& defined(CONFIG_SYS_CS0_CTRL))- csm->csar0 = (CONFIG_SYS_CS0_BASE>>16);
- csm->cscr0 = CONFIG_SYS_CS0_CTRL;
- csm->csmr0 = CONFIG_SYS_CS0_MASK;
- MCF5307_SP_ERR_FIX(CONFIG_SYS_CS0_BASE,csm->csmr0);
We do not allow accesses to device registers through plain (even volatile) pointers. Please make sure to use proper I/O accessors instead.
...
+/*
- Define the 5249 SIM register set addresses.
- */
+/*
- ***** MBAR *****
- */
+#define MCFSIM_RSR 0x00 /* Reset Status reg (r/w) */ +#define MCFSIM_SYPCR 0x01 /* System Protection reg (r/w) */ +#define MCFSIM_SWIVR 0x02 /* SW Watchdog intr reg (r/w) */ +#define MCFSIM_SWSR 0x03 /* SW Watchdog service (r/w) */ +#define MCFSIM_PAR 0x04 /* Parallel pin assignment reg */ +#define MCFSIM_PLLCR 0x08 /* PLL Control register */ +#define MCFSIM_MPARK 0x0c /* Bus master park register (r/w) */
+#define MCFSIM_SIMR 0x00 /* SIM Config reg (r/w) */ +#define MCFSIM_ICR0 0x4c /* Intr Ctrl reg 0 (r/w) */ +#define MCFSIM_ICR1 0x4d /* Intr Ctrl reg 1 (r/w) */ +#define MCFSIM_ICR2 0x4e /* Intr Ctrl reg 2 (r/w) */ +#define MCFSIM_ICR3 0x4f /* Intr Ctrl reg 3 (r/w) */ +#define MCFSIM_ICR4 0x50 /* Intr Ctrl reg 4 (r/w) */ +#define MCFSIM_ICR5 0x51 /* Intr Ctrl reg 5 (r/w) */ +#define MCFSIM_ICR6 0x52 /* Intr Ctrl reg 6 (r/w) */ +#define MCFSIM_ICR7 0x53 /* Intr Ctrl reg 7 (r/w) */ +#define MCFSIM_ICR8 0x54 /* Intr Ctrl reg 8 (r/w) */ +#define MCFSIM_ICR9 0x55 /* Intr Ctrl reg 9 (r/w) */ +#define MCFSIM_ICR10 0x56 /* Intr Ctrl reg 10 (r/w) */ +#define MCFSIM_ICR11 0x57 /* Intr Ctrl reg 11 (r/w) */
+#define MCFSIM_IPR 0x40 /* Interrupt Pend reg (r/w) */ +#define MCFSIM_IMR 0x44 /* Interrupt Mask reg (r/w) */
+#define MCFSIM_DCR 0x100 /* DRAM Control reg (r/w) */ +#define MCFSIM_DACR0 0x108 /* DRAM 0 Addr and Ctrl (r/w) */ +#define MCFSIM_DMR0 0x10c /* DRAM 0 Mask reg (r/w) */ +#define MCFSIM_DACR1 0x110 /* DRAM 1 Addr and Ctrl (r/w) */ +#define MCFSIM_DMR1 0x114 /* DRAM 1 Mask reg (r/w) */
+#define MCFSIM_PADDR 0x244 /* Parallel data direction reg */ +#define MCFSIM_PADAT 0x248 /* Parallel data direction reg */
We don't allow any base address + offset notation. Please define a proper C structure instead.
Please fix globally.
-#if defined(CONFIG_M5249) || defined(CONFIG_M5253) || defined(CONFIG_M5272) +#if defined(CONFIG_M5307) || defined(CONFIG_M5249) || \
- defined(CONFIG_M5253) || defined(CONFIG_M5272)
Please keep the list sorted.
Best regards,
Wolfgang Denk