[PATCH-V4 4/4] ARM: OMAP3+: am33xx: Add powerdomain & PRM support

[Hiremath, Vaibhav]

On Fri, Apr 27, 2012 at 06:19:02, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav at ti.com> writes:
> 
> > As far as PRM/CM/PRCM modules are concerned, AM33XX device is
> > different than OMAP3 and OMAP4 architectures; so we need to
> > handle it separately.
> > This patch adds support for, Powerdomain, Powerdomain data,
> > PRM api's required for AM33XX device.
> >
> > And also, hooks up AM33XX powerdomain to existing OMAP framework.
> 
> [...]
> 
> > @@ -1288,7 +1289,15 @@ static int _assert_hardreset(struct omap_hwmod *oh, const char *name)
> >  	if (IS_ERR_VALUE(ret))
> >  		return ret;
> >
> > -	if (cpu_is_omap24xx() || cpu_is_omap34xx())
> > +	/*
> > +	 * cpu_is_omap34xx() is true for am33xx device as well, so
> > +	 * fist check for cpu_is_am33xx().
> > +	 */
> > +	if (cpu_is_am33xx())
> > +		return am33xx_prm_assert_hardreset(ohri.rst_shift,
> > +				oh->clkdm->pwrdm.ptr->prcm_offs,
> > +				oh->prcm.omap4.rstctrl_offs);
> 
> This still troubles me.  I *really* don't like that we have a dependence
> on cpu_is* call ordering.  This is very fragile and error prone.
> 
> I also don't like all the cpu_is* checking currently in omap_hwmod.c
> (which is here before you added this) and have an idea on how to clean
> it up, I should have a patch by tomorrow for this.  That should help
> adding am33xx support here without needing all the cpu_is* checking.
> 
> That being said, I just did a simple experiment[1] to see why
> cpu_is_omap34xx() needs to be true for AM33xx in the first place.  Based
> on my quick experiment, it does not appear to be needed.  I think our
> lives will be much simpler if cpu_is_omap34xx() is not true for the
> AM335x family. 
> 
> Can you have a look at my test branch[1] and see what you think?  I
> changed the omap_revision for AM335x so that cpu_is_omap34xx() is no
> longer true on this platform.  Then, I only needed to fixup the SRAM
> init, and it boots just fine on my BeagleBone.
> 
> I really think we need to go this route, because having
> cpu_is_omap34xx() true on AM335x is causing more headaches than it is
> solving problems.  This will require you to rework a little bit these
> clock/power/voltage domain patches, but I belive it will greatly
> simplify the maintainability of the end result.
> 

Let me spend some time, and explore your changes; I think getting system to boot should not be only criteria here.
But honestly, I fully agree with you that, we are creating more issue, 
adding more cpu_is_xxx() checks, with cpu_is_34xx() true for AM33xx. 

As I have done in the past initially, I recommend and vote for,

 1. Creating separate family cpu_is_am33xx() for AM33xx device.
OR
 2. Bring it to omap44xx family, since prcm block is closer to omap4
    and not with omap3. Also, 


Tony,
I will let tony make a decision on this. By the time, Tony makes his 
decision, I will spend time to explore your (Kevin's below) branch.

Thanks,
Vaibhav

> Kevin
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git tmp/am335x-cpu-is-hacking
> 
>