| Message ID | alpine.DEB.2.02.1402191824260.8777@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 19, 2014 at 1:25 PM, Paul Walmsley <paul@pwsan.com> wrote: > > Just FYI. Queued for v3.15 unless someone complains. > No complains but i wanted to point out that with some additional changes it's possible to consolidate AM335x (and AM437x) very nicely with the rest of OMAP4+ PRM/CM code. I had posted a series (which had a similar patch) a while back [1] but that got never got any feedback :/ If one looks hard enough AM335x/AM437x/TI81xx are not that different from OMAP4+ :) Regards, Vaibhav [1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100474 > - Paul > > ---------- Forwarded message ---------- > Date: Fri, 7 Feb 2014 16:20:59 -0600 > From: Dave Gerlach <d-gerlach@ti.com> > To: linux-omap@vger.kernel.org > Cc: Tony Lindgren <tony@atomide.com>, Paul Walmsley <paul@pwsan.com>, > Rajendra Nayak <rnayak@ti.com>, Nishanth Menon <nm@ti.com>, > Dave Gerlach <d-gerlach@ti.com> > Subject: [RFC PATCH] ARM: OMAP2+: clockdomain: Reintroduce SW_SLEEP Support > > Since commit 65aa94b204d (ARM: OMAP4: clockdomain/CM code: Update supported > transition modes), on OMAP4, all CLKDMs support HW_AUTO so this is used > instead of SW_SLEEP for the idling of clockdomains. However, additional > SoCs now leverage the OMAP4 clockdomain code so update it to use SW_SLEEP > if the clockdomain data specifies that the CLKDM has the > CLKDM_CAN_FORCE_SLEEP flag set rather than using HW_AUTO for both cases. > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > --- > arch/arm/mach-omap2/cminst44xx.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c > index 731ca13..f5c4731 100644 > --- a/arch/arm/mach-omap2/cminst44xx.c > +++ b/arch/arm/mach-omap2/cminst44xx.c > @@ -254,6 +254,11 @@ void omap4_cminst_clkdm_force_wakeup(u8 part, u16 inst, u16 cdoffs) > * > */ > > +void omap4_cminst_clkdm_force_sleep(u8 part, u16 inst, u16 cdoffs) > +{ > + _clktrctrl_write(OMAP34XX_CLKSTCTRL_FORCE_SLEEP, part, inst, cdoffs); > +} > + > /** > * omap4_cminst_wait_module_ready - wait for a module to be in 'func' state > * @part: PRCM partition ID that the CM_CLKCTRL register exists in > @@ -404,8 +409,17 @@ static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain *clkdm) > > static int omap4_clkdm_sleep(struct clockdomain *clkdm) > { > - omap4_cminst_clkdm_enable_hwsup(clkdm->prcm_partition, > - clkdm->cm_inst, clkdm->clkdm_offs); > + if (clkdm->flags & CLKDM_CAN_HWSUP) > + omap4_cminst_clkdm_enable_hwsup(clkdm->prcm_partition, > + clkdm->cm_inst, > + clkdm->clkdm_offs); > + else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP) > + omap4_cminst_clkdm_force_sleep(clkdm->prcm_partition, > + clkdm->cm_inst, > + clkdm->clkdm_offs); > + else > + return -EINVAL; > + > return 0; > } > > -- > 1.7.9.5 > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, 19 Feb 2014, Vaibhav Bedia wrote: > On Wed, Feb 19, 2014 at 1:25 PM, Paul Walmsley <paul@pwsan.com> wrote: > > > > Just FYI. Queued for v3.15 unless someone complains. > > > > No complains but i wanted to point out that with some additional > changes it's possible > to consolidate AM335x (and AM437x) very nicely with the rest of OMAP4+ > PRM/CM code. > > I had posted a series (which had a similar patch) a while back [1] but > that got never > got any feedback :/ Yeah, to be honest I don't like the consolidation approach. The reason why is because the hardware seems to be quite different underneath, so the risk of breaking another chip is high. > If one looks hard enough AM335x/AM437x/TI81xx are not that different > from OMAP4+ :) Hmm, it's my understanding that the hardware implementations are quite different between the Wireless and the Catalog parts. Certainly the Wireless engineers didn't have any knowledge at all of what Catalog folks were doing, and many implementation assumptions that held true across several Wireless parts did not hold up at all when extended to Catalog parts. - Paul
On Wed, Feb 19, 2014 at 2:22 PM, Paul Walmsley <paul@pwsan.com> wrote: > On Wed, 19 Feb 2014, Vaibhav Bedia wrote: > >> On Wed, Feb 19, 2014 at 1:25 PM, Paul Walmsley <paul@pwsan.com> wrote: >> > >> > Just FYI. Queued for v3.15 unless someone complains. >> > >> >> No complains but i wanted to point out that with some additional >> changes it's possible >> to consolidate AM335x (and AM437x) very nicely with the rest of OMAP4+ >> PRM/CM code. >> >> I had posted a series (which had a similar patch) a while back [1] but >> that got never >> got any feedback :/ > > Yeah, to be honest I don't like the consolidation approach. The reason > why is because the hardware seems to be quite different underneath, so the > risk of breaking another chip is high. > >> If one looks hard enough AM335x/AM437x/TI81xx are not that different >> from OMAP4+ :) > > Hmm, it's my understanding that the hardware implementations are quite > different between the Wireless and the Catalog parts. Certainly the > Wireless engineers didn't have any knowledge at all of what Catalog folks > were doing, and many implementation assumptions that held true across > several Wireless parts did not hold up at all when extended to Catalog > parts. > After spending a lot of time looking at the code and PRCM specs for the different chips i had come to the conclusion that in reality the Catalog parts had the same hardware minus intelligence ;) The control bits are the same with just the AUTO modes in the PWRDM, CLKDM, CLKCTRL never being supported. The series touched less than 100 lines of OMAP4 code and from what i can see it was just making it generic in a couple of places. Most of the changes were just getting rid of AM335x specific pieces. FWIW i had even suspend/resume also working on AM335x on top of that series so a breakage, if any, should be simple to fix up. On the contrary i found it easier to fix any breakage on AM335x due to common code. But i'll leave the final decision to you :) Regards, Vaibhav
On Wed, 19 Feb 2014, Vaibhav Bedia wrote: > After spending a lot of time looking at the code and PRCM specs for the > different chips i had come to the conclusion that in reality the Catalog > parts had the same hardware minus intelligence ;) Did you look at the RTL? > The control bits are the same with just the AUTO modes in the PWRDM, > CLKDM, CLKCTRL never being supported. I'm not sure it's that simple. For example PWRSTCTRL/PWRSTST offsets are handled very differently between AM335x and the other OMAPs. Also the LOGICRETSTATE mask bits are different. These unnecessary deltas suggest to me that the underlying hardware implementation is different. I spent some time a few years ago trying to find some hardware guy to give some straight answers about the implementation history for the underlying blocks, but was never able to find that person. - Paul
diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c index 731ca13..f5c4731 100644 --- a/arch/arm/mach-omap2/cminst44xx.c +++ b/arch/arm/mach-omap2/cminst44xx.c @@ -254,6 +254,11 @@ void omap4_cminst_clkdm_force_wakeup(u8 part, u16 inst, u16 cdoffs) * */ +void omap4_cminst_clkdm_force_sleep(u8 part, u16 inst, u16 cdoffs) +{ + _clktrctrl_write(OMAP34XX_CLKSTCTRL_FORCE_SLEEP, part, inst, cdoffs); +} + /** * omap4_cminst_wait_module_ready - wait for a module to be in 'func' state * @part: PRCM partition ID that the CM_CLKCTRL register exists in @@ -404,8 +409,17 @@ static int omap4_clkdm_clear_all_wkup_sleep_deps(struct clockdomain *clkdm) static int omap4_clkdm_sleep(struct clockdomain *clkdm) { - omap4_cminst_clkdm_enable_hwsup(clkdm->prcm_partition, - clkdm->cm_inst, clkdm->clkdm_offs); + if (clkdm->flags & CLKDM_CAN_HWSUP) + omap4_cminst_clkdm_enable_hwsup(clkdm->prcm_partition, + clkdm->cm_inst, + clkdm->clkdm_offs); + else if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP) + omap4_cminst_clkdm_force_sleep(clkdm->prcm_partition, + clkdm->cm_inst, + clkdm->clkdm_offs); + else + return -EINVAL; + return 0; }