On Fri, Apr 27, 2012 at 06:19:02, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@xxxxxx> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
