* Hiremath, Vaibhav <hvaibhav@xxxxxx> [120426 23:40]: > 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. Just to summarize, I guess it's pretty obvious that we need cpu_is_am33xx here. In general work on getting rid of the cpu_is_xxxx checks as they are not safe to use with single zImage in initcalls. Tony -- 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
