[U-Boot] [PATCH-V2 2/4] omap3: Consolidate SDRC related operations

Wolfgang Denk
2010-05-05T20:07:52Z

Dear hvaibhav at ti.com,

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)'

> 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?

> 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.

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 at denx.de
People seldom know what they want until you give them what they ask
for.