-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Thursday, May 06, 2010 1:38 AM To: Hiremath, Vaibhav Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH-V2 2/4] omap3: Consolidate SDRC related operations
Dear hvaibhav@ti.com,
[Hiremath, Vaibhav] Thanks Denk for your comments, see my response below -
In message 1272034546-26041-4-git-send-email-hvaibhav@ti.com you wrote:
From: Vaibhav Hiremath hvaibhav@ti.com
Consolidated SDRC related functions into one file - sdrc.c
And also replaced sdrc_init with generic memory init function (mem_init), this generalization of omap memory setup is necessary to support the new emif4 interface introduced in AM3517.
Signed-off-by: Vaibhav Hiremath hvaibhav@ti.com Signed-off-by: Sanjeev Premi premi@ti.com
diff --git a/arch/arm/cpu/arm_cortexa8/omap3/Makefile
b/arch/arm/cpu/arm_cortexa8/omap3/Makefile
index 136b163..8cc7802 100644 --- a/arch/arm/cpu/arm_cortexa8/omap3/Makefile +++ b/arch/arm/cpu/arm_cortexa8/omap3/Makefile @@ -33,6 +33,9 @@ COBJS += board.o COBJS += clock.o COBJS += gpio.o COBJS += mem.o +ifdef CONFIG_SDRC +COBJS += sdrc.o +endif
Please don't use 'ifdef" here; instead, use `COBJS-$(CONFIG_SDRC)'
[Hiremath, Vaibhav] ok, will incorporate in next version.
diff --git a/arch/arm/include/asm/arch-omap3/cpu.h
b/arch/arm/include/asm/arch-omap3/cpu.h
index aa8de32..a49af10 100644 --- a/arch/arm/include/asm/arch-omap3/cpu.h +++ b/arch/arm/include/asm/arch-omap3/cpu.h @@ -183,6 +183,7 @@ struct sms { /* SDRC */ #ifndef __KERNEL_STRICT_NAMES #ifndef __ASSEMBLY__ +#if defined(CONFIG_SDRC) struct sdrc_cs { u32 mcfg; /* 0x80 || 0xB0 */ u32 mr; /* 0x84 || 0xB4 */ @@ -215,6 +216,8 @@ struct sdrc { u8 res4[0xC]; struct sdrc_cs cs[2]; /* 0x80 || 0xB0 */ };
+#endif /* CONFIG_SDRC */
I don't like such a #ifdef here - it is absolutely necessary? Why?
[Hiremath, Vaibhav] Denk,
This is common file being used for all OMAP series of devices (OMAP2, OMAP3 and AM35x family) and OMAP2/3 family supports SDRC controller and AM35x family support EMIF4.
And due to this difference we need to add this #ifdef.
diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h
b/arch/arm/include/asm/arch-omap3/sys_proto.h
index 34bd515..34e4e0d 100644 --- a/arch/arm/include/asm/arch-omap3/sys_proto.h +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h @@ -31,8 +31,10 @@ void prcm_init(void); void per_clocks_enable(void);
void memif_init(void); +#if defined(CONFIG_SDRC) void sdrc_init(void); void do_sdrc_init(u32, u32); +#endif
Ditto - please drop this #ifdef.
[Hiremath, Vaibhav] Same as above.
Thanks, Vaibhav
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 People seldom know what they want until you give them what they ask for.