Dear Chander Kashyap, In message <1311914519-10531-2-git-send-email-chander.kash...@linaro.org> you wrote: > Origen board is based upon S5PV310 SoC which is similiar to > S5PC210 SoC. > > Signed-off-by: Chander Kashyap <chander.kash...@linaro.org> ... > + /* APLL(1), MPLL(1), CORE(0), HPM(0) */ > + ldr r1, =0x0101 > + ldr r2, =0x14200 @CLK_SRC_CPU ... > + mov r1, #0x10000 ... > + ldr r1, =0x00 > + ldr r2, =0x0C210 @CLK_SRC_TOP0 ... > + ldr r1, =0x00 > + ldr r2, =0x0C214 @CLK_SRC_TOP1_OFFSET ... > + ldr r1, =0x10000 > + ldr r2, =0x10200 @CLK_SRC_DMC_OFFSET ...
.. > + /* > + * CLK_DIV_DMC0: > + * > + * CORE_TIMERS_RATIO[28] 0x1 > + * COPY2_RATIO[24] 0x3 > + * DMCP_RATIO[20] 0x1 > + * DMCD_RATIO[16] 0x1 > + * DMC_RATIO[12] 0x1 > + * DPHY_RATIO[8] 0x1 > + * ACP_PCLK_RATIO[4] 0x1 > + * ACP_RATIO[0] 0x3 > + */ > + ldr r1, =0x13111113 > + ldr r2, =0x010500 @CLK_DIV_DMC0_OFFSET > + str r1, [r0, r2] lease get rid of all these magic hard coded constants. Use symbolic names instead. If needed, auto-generate these from the respective C structs. If needed, create the C structs. ... > --- /dev/null > +++ b/board/samsung/origen/mem_setup.S > @@ -0,0 +1,359 @@ What exactly is the reeason for not coding this in C ? ... > +#ifdef CONFIG_MIU_1BIT_12_INTERLEAVED > + ldr r1, =0x0000000c > + str r1, [r0, #0x400] > + ldr r1, =0x00000001 > + str r1, [r0, #0xc00] > +#elif defined(CONFIG_MIU_1BIT_7_INTERLEAVED) > + ldr r1, =0x00000007 > + str r1, [r0, #0x400] > + ldr r1, =0x00000001 > + str r1, [r0, #0xc00] > +#elif defined(CONFIG_MIU_2BIT_21_12_INTERLEAVED) > + ldr r1, =0x2000150c > + str r1, [r0, #0x400] > + ldr r1, =0x00000001 > + str r1, [r0, #0xc00] > +#elif defined(CONFIG_MIU_2BIT_21_7_INTERLEAVED) > + ldr r1, =0x20001507 > + str r1, [r0, #0x400] > + ldr r1, =0x00000001 > + str r1, [r0, #0xc00] > +#elif defined(CONFIG_MIU_2BIT_31_INTERLEAVED) > + ldr r1, =0x0000001d > + str r1, [r0, #0x400] > + ldr r1, =0x00000001 > + str r1, [r0, #0xc00] See above. Please get rid of all these hard-ciooded magic numbers. [Eventually this happens automativcally when you rewrite this in C.] > +int dram_init(void) > +{ > + gd->ram_size = get_ram_size((long *)PHYS_SDRAM_1, PHYS_SDRAM_1_SIZE) > + + get_ram_size((long *)PHYS_SDRAM_2, PHYS_SDRAM_2_SIZE) > + + get_ram_size((long *)PHYS_SDRAM_3, PHYS_SDRAM_3_SIZE) > + + get_ram_size((long *)PHYS_SDRAM_4, PHYS_SDRAM_4_SIZE); > + > + return 0; > +} > + > +void dram_init_banksize(void) > +{ > + gd->bd->bi_dram[0].start = PHYS_SDRAM_1; > + gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE; > + gd->bd->bi_dram[1].start = PHYS_SDRAM_2; > + gd->bd->bi_dram[1].size = PHYS_SDRAM_2_SIZE; > + gd->bd->bi_dram[2].start = PHYS_SDRAM_3; > + gd->bd->bi_dram[2].size = PHYS_SDRAM_3_SIZE; > + gd->bd->bi_dram[3].start = PHYS_SDRAM_4; > + gd->bd->bi_dram[3].size = PHYS_SDRAM_4_SIZE; > +} How do you detect memory issues in one of the memory banks, say, a memory error in bank 3? > diff --git a/include/configs/origen.h b/include/configs/origen.h > new file mode 100644 > index 0000000..3cd9bfe > --- /dev/null > +++ b/include/configs/origen.h ... > +#define COPY_BL2_SIZE 0x80000 > +#define BL2_START_OFFSET ((CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE)/512) > +#define BL2_SIZE_BLOC_COUNT (COPY_BL2_SIZE/512) Some of the defines in this file are not used anywhere in the code (for example, COPY_BL2_SIZE, COPY_BL2_SIZEBL2_START_OFFSET or BL2_SIZE_BLOC_COUNT. Please remove unused defines from this patch. 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: w...@denx.de Don't hit a man when he's down - kick him; it's easier. _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev