| Message ID | 1342189185-5306-8-git-send-email-t-kristo@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)
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) >
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
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?
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
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 > >
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
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
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)
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(-)