Dear Wolfgang Denk,
On 31 July 2011 15:30, Wolfgang Denk wd@denx.de wrote:
Dear Chander Kashyap,
In message 1311914519-10531-2-git-send-email-chander.kashyap@linaro.org you wrote:
Origen board is based upon S5PV310 SoC which is similiar to S5PC210 SoC.
Signed-off-by: Chander Kashyap chander.kashyap@linaro.org
...
/* APLL(1), MPLL(1), CORE(0), HPM(0) */ldr r1, =0x0101ldr r2, =0x14200 @CLK_SRC_CPU...
mov r1, #0x10000...
ldr r1, =0x00ldr r2, =0x0C210 @CLK_SRC_TOP0...
ldr r1, =0x00ldr r2, =0x0C214 @CLK_SRC_TOP1_OFFSET...
ldr r1, =0x10000ldr 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, =0x13111113ldr r2, =0x010500 @CLK_DIV_DMC0_OFFSETstr 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.
I will change hard coded values to symbolic names.
...
--- /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, =0x0000000cstr r1, [r0, #0x400]ldr r1, =0x00000001str r1, [r0, #0xc00]+#elif defined(CONFIG_MIU_1BIT_7_INTERLEAVED)
ldr r1, =0x00000007str r1, [r0, #0x400]ldr r1, =0x00000001str r1, [r0, #0xc00]+#elif defined(CONFIG_MIU_2BIT_21_12_INTERLEAVED)
ldr r1, =0x2000150cstr r1, [r0, #0x400]ldr r1, =0x00000001str r1, [r0, #0xc00]+#elif defined(CONFIG_MIU_2BIT_21_7_INTERLEAVED)
ldr r1, =0x20001507str r1, [r0, #0x400]ldr r1, =0x00000001str r1, [r0, #0xc00]+#elif defined(CONFIG_MIU_2BIT_31_INTERLEAVED)
ldr r1, =0x0000001dstr r1, [r0, #0x400]ldr r1, =0x00000001str 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.]
I will use symbolic names.
+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?
get_ram_size will take care for it.
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.
These are used in spl code. I will move them to spl 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: wd@denx.de Don't hit a man when he's down - kick him; it's easier.