Re: [PATCH-V5 2/3] arm:omap:am33xx: Add AM335XEVM machine support

[Tony Lindgren <tony@atomide.com>]

* Hiremath, Vaibhav <hvaibhav@ti.com> [120503 23:04]:
> 
> Tony, seems to be against adding new ARCH_OMAPAM33XX, but which _ARCH_ we need to follow for AM33XX?
> I have to choose between ARCH_OMAP3 or ARCH_OMAP4 and what should I choose 
> here?

I think you're getting confused now :) I'm against ARCH_XXX but I'm OK with
adding SOC_XXX.

We should only need ARCH_OMAP2PLUS + SOC_XXX, there should not be any need
to add new ARCH_XXX under mach-omap2. Whatever we have left for ARCH_OMAP
in mach-omap2 will be eventually converted to SOC_OMAP.
 
> Does it make sense to follow ARCH_OMAPx but not follow cpu_is_omapxxx()?
> OR

No

> Should we create ARCH_AMXXXX, assuming that all AM devices have similar 

No

> memory map layout, interrupt mapping, etc...
> OR
> Should I just add SOC_OMAPAM33XX, wherever required?

Yes, but how about just use SOC_AM33XX? 
 
> Also, there are lot of thing wrapped under ARCH_OMAP3 || ARCH_OMAP4 option, which is required for AM33XX, how should we handle this?
> 
> For example,
> 
> "arch/arm/plat-omap/include/plat/clock.h"
> struct dpll_data {
> #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> 	<dpll related variables>
> #endif
> };
> 
> "arch/arm/mach-omap2/clock.c"
> 
> #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> 
> const struct clkops clkops_omap3_noncore_dpll_ops = {
> };
> const struct clkops clkops_omap3_core_dpll_ops = {
> }

I suggest doing some clean-up patches before adding SOC_AM33XX where
you just convert those to be

#if defined(CONFIG_ARCH_OMAP2PLUS) && !defined(CONFIG_ARCH_OMAP2)

or something similar depending if they already are inside mach-omap2
directory. This will make them future proof for adding new SoCs
without having to patch all over the place.

Regards,

Tony