diff mbox

[RFC] ARM: OMAP2+: clockdomain: Reintroduce SW_SLEEP Support (fwd)

Message ID alpine.DEB.2.02.1402191824260.8777@utopia.booyaka.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Walmsley Feb. 19, 2014, 6:25 p.m. UTC
Just FYI.  Queued for v3.15 unless someone complains.

- 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(-)

Comments

Vaibhav Bedia Feb. 19, 2014, 7:17 p.m. UTC | #1
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
Paul Walmsley Feb. 19, 2014, 7:22 p.m. UTC | #2
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
Vaibhav Bedia Feb. 19, 2014, 7:52 p.m. UTC | #3
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
Paul Walmsley Feb. 19, 2014, 8:13 p.m. UTC | #4
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 mbox

Patch

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;
 }