diff mbox

[PATCHv4,7/8] ARM: OMAP: clockdomain: add support for preventing autodep delete

Message ID 1342189185-5306-8-git-send-email-t-kristo@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tero Kristo July 13, 2012, 2:19 p.m. UTC
Some clockdomains bug out if their autodeps are deleted before idle.
This happens namely with OMAP3 PER domain, it will bug out if it
doesn't have wakedeps enabled when it enters off-mode. This patch
adds support for new flag 'CLKDM_NO_AUTODEP_DISABLE' which does this.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c |    3 +++
 arch/arm/mach-omap2/clockdomain.h |    4 ++++
 2 files changed, 7 insertions(+), 0 deletions(-)

Comments

Rajendra Nayak July 16, 2012, 11 a.m. UTC | #1
Hi Tero,

On Friday 13 July 2012 07:49 PM, Tero Kristo wrote:
> Some clockdomains bug out if their autodeps are deleted before idle.
> This happens namely with OMAP3 PER domain, it will bug out if it
> doesn't have wakedeps enabled when it enters off-mode. This patch
> adds support for new flag 'CLKDM_NO_AUTODEP_DISABLE' which does this.

I had one more thought on how we could handle this (without adding a new
flag :-))
How about marking OMAP3 PER with a CLKDM_NO_AUTODEPS (already existing
flag) and setting a sleep/wakeup dependency of OMAP3 PER with MPU and
IVA one time sometime during late PM init. Because thats what we intent
to do, which is have a sleep/wakeup dependency set *always* and never
try to remove it, right?

regards,
Rajendra

>
> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> ---
>   arch/arm/mach-omap2/clockdomain.c |    3 +++
>   arch/arm/mach-omap2/clockdomain.h |    4 ++++
>   2 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index 7f5423e..56cef58 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -201,6 +201,9 @@ void _clkdm_del_autodeps(struct clockdomain *clkdm)
>   	if (!autodeps || clkdm->flags&  CLKDM_NO_AUTODEPS)
>   		return;
>
> +	if (clkdm->flags&  CLKDM_NO_AUTODEP_DISABLE)
> +		return;
> +
>   	for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
>   		if (IS_ERR(autodep->clkdm.ptr))
>   			continue;
> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
> index 373399a..1fc5314 100644
> --- a/arch/arm/mach-omap2/clockdomain.h
> +++ b/arch/arm/mach-omap2/clockdomain.h
> @@ -31,12 +31,16 @@
>    *
>    * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this
>    *     clockdomain.  (Currently, this applies to OMAP3 clockdomains only.)
> + * CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps.
> + *     Needed for PER domain on omap3, as it will bug out with off-mode if
> + *     wakedeps are removed.
>    */
>   #define CLKDM_CAN_FORCE_SLEEP			(1<<  0)
>   #define CLKDM_CAN_FORCE_WAKEUP			(1<<  1)
>   #define CLKDM_CAN_ENABLE_AUTO			(1<<  2)
>   #define CLKDM_CAN_DISABLE_AUTO			(1<<  3)
>   #define CLKDM_NO_AUTODEPS			(1<<  4)
> +#define CLKDM_NO_AUTODEP_DISABLE		(1<<  5)
>
>   #define CLKDM_CAN_HWSUP		(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)
>   #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo July 17, 2012, 2:56 p.m. UTC | #2
On Mon, 2012-07-16 at 16:30 +0530, Rajendra Nayak wrote:
> Hi Tero,
> 
> On Friday 13 July 2012 07:49 PM, Tero Kristo wrote:
> > Some clockdomains bug out if their autodeps are deleted before idle.
> > This happens namely with OMAP3 PER domain, it will bug out if it
> > doesn't have wakedeps enabled when it enters off-mode. This patch
> > adds support for new flag 'CLKDM_NO_AUTODEP_DISABLE' which does this.
> 
> I had one more thought on how we could handle this (without adding a new
> flag :-))
> How about marking OMAP3 PER with a CLKDM_NO_AUTODEPS (already existing
> flag) and setting a sleep/wakeup dependency of OMAP3 PER with MPU and
> IVA one time sometime during late PM init. Because thats what we intent
> to do, which is have a sleep/wakeup dependency set *always* and never
> try to remove it, right?

I did some extra investigation on this, sorry for the delay. What is
enough, is to just add a wakedep from wkup_clkdm to per_clkdm, as the
wakedeps have usecounting so once this is done, the autodep handling
can't remove the wakedep.

Anyway, it also looks like this fix is no longer needed with the latest
kernel, something has changed with the gpio code / or latencies and it
doesn't crash anymore. Thus, it looks like patches 7 & 8 can be dropped
from this set for now. This is the behavior with beagleboard at least,
if someone can verify this with some other omap3 hw that would be nice.

The underlying issue still remains, we have errata i582 which doesn't
have any workarounds in the kernel yet. We should probably resurrect
something like this:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg38834.html

... or just pull the part which adds the dynamic wakedep add / remove
for the per domain when attempting per OFF.

-Tero


> 
> regards,
> Rajendra
> 
> >
> > Signed-off-by: Tero Kristo<t-kristo@ti.com>
> > ---
> >   arch/arm/mach-omap2/clockdomain.c |    3 +++
> >   arch/arm/mach-omap2/clockdomain.h |    4 ++++
> >   2 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> > index 7f5423e..56cef58 100644
> > --- a/arch/arm/mach-omap2/clockdomain.c
> > +++ b/arch/arm/mach-omap2/clockdomain.c
> > @@ -201,6 +201,9 @@ void _clkdm_del_autodeps(struct clockdomain *clkdm)
> >   	if (!autodeps || clkdm->flags&  CLKDM_NO_AUTODEPS)
> >   		return;
> >
> > +	if (clkdm->flags&  CLKDM_NO_AUTODEP_DISABLE)
> > +		return;
> > +
> >   	for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
> >   		if (IS_ERR(autodep->clkdm.ptr))
> >   			continue;
> > diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
> > index 373399a..1fc5314 100644
> > --- a/arch/arm/mach-omap2/clockdomain.h
> > +++ b/arch/arm/mach-omap2/clockdomain.h
> > @@ -31,12 +31,16 @@
> >    *
> >    * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this
> >    *     clockdomain.  (Currently, this applies to OMAP3 clockdomains only.)
> > + * CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps.
> > + *     Needed for PER domain on omap3, as it will bug out with off-mode if
> > + *     wakedeps are removed.
> >    */
> >   #define CLKDM_CAN_FORCE_SLEEP			(1<<  0)
> >   #define CLKDM_CAN_FORCE_WAKEUP			(1<<  1)
> >   #define CLKDM_CAN_ENABLE_AUTO			(1<<  2)
> >   #define CLKDM_CAN_DISABLE_AUTO			(1<<  3)
> >   #define CLKDM_NO_AUTODEPS			(1<<  4)
> > +#define CLKDM_NO_AUTODEP_DISABLE		(1<<  5)
> >
> >   #define CLKDM_CAN_HWSUP		(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)
> >   #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley July 17, 2012, 9:31 p.m. UTC | #3
Hi

On Tue, 17 Jul 2012, Tero Kristo wrote:

> The underlying issue still remains, we have errata i582 which doesn't
> have any workarounds in the kernel yet. We should probably resurrect
> something like this:
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg38834.html
> 
> ... or just pull the part which adds the dynamic wakedep add / remove
> for the per domain when attempting per OFF.

Yep looks like Kevin had some comments on that patch that no one followed 
up on.  Guess we need to figure out who will have time to revise and 
update it.

A few comments on that patch.

1. Looks to me like the patch needs to be split into several smaller 
patches.  One patch should deal with the serial changes.  Another should 
deal with the pm34xx.c changes.

2. Looks like we also need a patch to run the McBSP2/3 sidetone 
loopback test.  Then the pm34xx.c test code would be something like:

if (omap_uart_test_erratum_i582() || omap_mcbsp_test_erratum_i582()) {
 pr_err("%s: erratum i582 encountered; applying workaround\n", __func__);
 .. etc.
}

3. When the erratum is encountered, shouldn't the code schedule a CORE OFF 
transition to occur at the earliest possible moment, rather than just 
emitting a message?

4. The bug is only a problem when the PER serial ports/McBSP sidetone 
devices are in use, right?  So if those devices aren't in use then we can 
defer the device tests until right before one of those devices is brought 
into use, no?

5. There needs to be a better way of determining if a device is affected 
by this than by testing uart->num.  Adding a hwmod dev_attr flag would be 
my first inclination.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rajendra Nayak July 18, 2012, 7:15 a.m. UTC | #4
On Tuesday 17 July 2012 08:26 PM, Tero Kristo wrote:
> Anyway, it also looks like this fix is no longer needed with the latest
> kernel, something has changed with the gpio code / or latencies and it
> doesn't crash anymore. Thus, it looks like patches 7&  8 can be dropped
> from this set for now. This is the behavior with beagleboard at least,
> if someone can verify this with some other omap3 hw that would be nice.

I can test it on a omap3 SDP. What do you want me to test?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo July 18, 2012, 8:05 a.m. UTC | #5
On Wed, 2012-07-18 at 12:45 +0530, Rajendra Nayak wrote:
> On Tuesday 17 July 2012 08:26 PM, Tero Kristo wrote:
> > Anyway, it also looks like this fix is no longer needed with the latest
> > kernel, something has changed with the gpio code / or latencies and it
> > doesn't crash anymore. Thus, it looks like patches 7&  8 can be dropped
> > from this set for now. This is the behavior with beagleboard at least,
> > if someone can verify this with some other omap3 hw that would be nice.
> 
> I can test it on a omap3 SDP. What do you want me to test?

Just try suspend + cpuidle with and without off-mode enabled and see if
there are any problems. I've usually seen problems with off-mode myself.

-Tero


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rajendra Nayak July 18, 2012, 9:04 a.m. UTC | #6
On Wednesday 18 July 2012 01:35 PM, Tero Kristo wrote:
> On Wed, 2012-07-18 at 12:45 +0530, Rajendra Nayak wrote:
>> On Tuesday 17 July 2012 08:26 PM, Tero Kristo wrote:
>>> Anyway, it also looks like this fix is no longer needed with the latest
>>> kernel, something has changed with the gpio code / or latencies and it
>>> doesn't crash anymore. Thus, it looks like patches 7&   8 can be dropped
>>> from this set for now. This is the behavior with beagleboard at least,
>>> if someone can verify this with some other omap3 hw that would be nice.
>>
>> I can test it on a omap3 SDP. What do you want me to test?
>
> Just try suspend + cpuidle with and without off-mode enabled and see if
> there are any problems. I've usually seen problems with off-mode myself.

So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm-
changes-v4' and tested on my 3430 SDP.

I was able to hit RET and OFF in both suspend and cpuidle. Did not see
any issues.

>
> -Tero
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo July 18, 2012, 9:16 a.m. UTC | #7
On Wed, 2012-07-18 at 14:34 +0530, Rajendra Nayak wrote:
> On Wednesday 18 July 2012 01:35 PM, Tero Kristo wrote:
> > On Wed, 2012-07-18 at 12:45 +0530, Rajendra Nayak wrote:
> >> On Tuesday 17 July 2012 08:26 PM, Tero Kristo wrote:
> >>> Anyway, it also looks like this fix is no longer needed with the latest
> >>> kernel, something has changed with the gpio code / or latencies and it
> >>> doesn't crash anymore. Thus, it looks like patches 7&   8 can be dropped
> >>> from this set for now. This is the behavior with beagleboard at least,
> >>> if someone can verify this with some other omap3 hw that would be nice.
> >>
> >> I can test it on a omap3 SDP. What do you want me to test?
> >
> > Just try suspend + cpuidle with and without off-mode enabled and see if
> > there are any problems. I've usually seen problems with off-mode myself.
> 
> So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm-
> changes-v4' and tested on my 3430 SDP.
> 
> I was able to hit RET and OFF in both suspend and cpuidle. Did not see
> any issues.

Okay thanks, so it looks pretty safe then.

-Tero

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman July 27, 2012, 8:12 p.m. UTC | #8
Rajendra Nayak <rnayak@ti.com> writes:

> On Wednesday 18 July 2012 01:35 PM, Tero Kristo wrote:
>> On Wed, 2012-07-18 at 12:45 +0530, Rajendra Nayak wrote:
>>> On Tuesday 17 July 2012 08:26 PM, Tero Kristo wrote:
>>>> Anyway, it also looks like this fix is no longer needed with the latest
>>>> kernel, something has changed with the gpio code / or latencies and it
>>>> doesn't crash anymore. Thus, it looks like patches 7&   8 can be dropped
>>>> from this set for now. This is the behavior with beagleboard at least,
>>>> if someone can verify this with some other omap3 hw that would be nice.
>>>
>>> I can test it on a omap3 SDP. What do you want me to test?
>>
>> Just try suspend + cpuidle with and without off-mode enabled and see if
>> there are any problems. I've usually seen problems with off-mode myself.
>
> So I just knocked off the last 2 patches from 'mainline-3.5-rc6-pwrdm-
> changes-v4' and tested on my 3430 SDP.
>
> I was able to hit RET and OFF in both suspend and cpuidle. Did not see
> any issues.
>

FYI...

I tested this series without the last 2 patches on top of v3.5 and
suspend/resume hangs on 3430/n900, 3530/Overo, 3730/OveroSTORM, but
suspend/resume and cpuidle tests for retention and idle seem to work
fine on 3730/Beagle-xM.

It works fine on all the platforms with the last 2 applied (note 6/8
needs a minor update to apply cleanly to mainline v3.5.)

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 7f5423e..56cef58 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -201,6 +201,9 @@  void _clkdm_del_autodeps(struct clockdomain *clkdm)
 	if (!autodeps || clkdm->flags & CLKDM_NO_AUTODEPS)
 		return;
 
+	if (clkdm->flags & CLKDM_NO_AUTODEP_DISABLE)
+		return;
+
 	for (autodep = autodeps; autodep->clkdm.ptr; autodep++) {
 		if (IS_ERR(autodep->clkdm.ptr))
 			continue;
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 373399a..1fc5314 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -31,12 +31,16 @@ 
  *
  * CLKDM_NO_AUTODEPS: Prevent "autodeps" from being added/removed from this
  *     clockdomain.  (Currently, this applies to OMAP3 clockdomains only.)
+ * CLKDM_NO_AUTODEP_DISABLE: Prevent clockdomain code from deleting autodeps.
+ *     Needed for PER domain on omap3, as it will bug out with off-mode if
+ *     wakedeps are removed.
  */
 #define CLKDM_CAN_FORCE_SLEEP			(1 << 0)
 #define CLKDM_CAN_FORCE_WAKEUP			(1 << 1)
 #define CLKDM_CAN_ENABLE_AUTO			(1 << 2)
 #define CLKDM_CAN_DISABLE_AUTO			(1 << 3)
 #define CLKDM_NO_AUTODEPS			(1 << 4)
+#define CLKDM_NO_AUTODEP_DISABLE		(1 << 5)
 
 #define CLKDM_CAN_HWSUP		(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)
 #define CLKDM_CAN_SWSUP		(CLKDM_CAN_FORCE_SLEEP | CLKDM_CAN_FORCE_WAKEUP)