> -----Original Message-----
> From: "Högander" Jouni [mailto:[EMAIL PROTECTED] 
> Sent: Wednesday, July 09, 2008 2:05 PM
> To: ext Rajendra Nayak
> Cc: [email protected]
> Subject: Re: [PATCH] OMAP3: CPUIDLE & PM: check_bm fix.
> 
> Hi, Rajendra
> 
> I think we have now all the pieces here to get stable cpuidle +
> pm_idle + suspend + off mode with minimal configuration on SDP
> board. Could you Rajendra now gather these together and prepare new
> patch set in sensible format?

Yes, I will do that. Will re-send the complete patch set with all the fixes in 
place.

> 
> "ext Rajendra Nayak" <[EMAIL PROTECTED]> writes:
> 
> >> -----Original Message-----
> >> From: "Högander" Jouni [mailto:[EMAIL PROTECTED] 
> >> Sent: Wednesday, July 09, 2008 12:29 PM
> >> To: ext Rajendra Nayak
> >> Cc: [email protected]
> >> Subject: Re: [PATCH] OMAP3: CPUIDLE & PM: check_bm fix.
> >> 
> >> "ext Rajendra Nayak" <[EMAIL PROTECTED]> writes:
> >> 
> >> >  
> >> >> -----Original Message-----
> >> >> From: Jouni Hogander [mailto:[EMAIL PROTECTED] 
> >> >> Sent: Wednesday, July 09, 2008 12:00 PM
> >> >> To: [email protected]; [EMAIL PROTECTED]
> >> >> Subject: [PATCH] OMAP3: CPUIDLE & PM: check_bm fix.
> >> >> 
> >> >> This patch fixes problems with uart usage. 
> omap3_enter_idle_bm was
> >> >> select C5 and C6 states even if there was "bus activity.
> >> >
> >> > Am not very clear on what was the issue with the previous 
> >> logic that it would 
> >> > attempt a C5/C6 even with bus activity.
> >> 
> >> If you try it you will notice the difference;) What is the 
> meaning of
> >> this bm check if C5/C6 is used even when there is 
> >> "acitivity".
> >
> > No, What I meant to say was that the previous logic I 
> thought was good enough to
> > *prevent* C5/C6 or any other state with 
> CPUIDLE_FLAG_CHECK_BM flag set, while there is 
> > bus activity and then go ahead and select the highest 
> possible state with 
> > CPUIDLE_FLAG_CHECK_BM _not_ set.
> >
> >  Anyway if
> >> you want to have bm check code in your cpuidle driver you need to
> >> rewrite the check code. I have seen that omap3_can_sleep 
> partially as
> >> a workaround. Just to make sure PM doesn't break any 
> driver as long as
> >> they are not all configured properly what comes to 
> interconnect. Just
> >> like what happened here as cpuidle was trying to enter C5/C6.
> >> 
> >> >
> >> >> 
> >> >> Signed-off-by: Jouni Hogander <[EMAIL PROTECTED]>
> >> >> ---
> >> >>  arch/arm/mach-omap2/cpuidle34xx.c |   23 
> +++++++----------------
> >> >>  arch/arm/mach-omap2/pm34xx.c      |    2 +-
> >> >>  2 files changed, 8 insertions(+), 17 deletions(-)
> >> >> 
> >> >> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
> >> >> b/arch/arm/mach-omap2/cpuidle34xx.c
> >> >> index c14152f..a636edb 100644
> >> >> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> >> >> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> >> >> @@ -473,31 +473,22 @@ static int omap3_enter_idle_bm(struct 
> >> >> cpuidle_device *dev,
> >> >>                                struct cpuidle_state *state)
> >> >>  {
> >> >>         struct cpuidle_state *new_state = NULL;
> >> >> -       int i, j;
> >> >> -
> >> >> -       if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && 
> >> >> omap3_idle_bm_check()) {
> >> >> -
> >> >> -               /* Find current state in list */
> >> >> -               for (i = 0; i < OMAP3_MAX_STATES; i++)
> >> >> -                       if (state == &dev->states[i])
> >> >> -                               break;
> >> >> -               BUG_ON(i == OMAP3_MAX_STATES);
> >> >> -
> >> >> -               /* Back up to non 'CHECK_BM' state */
> >> >> -               for (j = i - 1;  j > 0; j--) {
> >> >> -                       struct cpuidle_state *s = 
> &dev->states[j];
> >> >> +       int i;
> >> >>  
> >> >> +       if (omap3_idle_bm_check()) {
> >> >> +               for (i = 0; i < OMAP3_MAX_STATES; i++) {
> >> >> +                       struct cpuidle_state *s = 
> &dev->states[i];
> >> >
> >> > So now a C0 is attempted every time there is bus activity?
> >> 
> >> Yes you are right here. This code is not very effective. It should
> >> select deepest sleep state without CPUIDLE_FLAG_CHECK_BM.
> >> 
> >> >
> >> >>                         if (!(s->flags & 
> CPUIDLE_FLAG_CHECK_BM)) {
> >> >>                                 new_state = s;
> >> >>                                 break;
> >> >>                         }
> >> >>                 }
> >> >> -
> >> >> +               BUG_ON(i == OMAP3_MAX_STATES);
> >> >>                 pr_debug("%s: Bus activity: Entering %s 
> >> >> (instead of %s)\n",
> >> >> -                       __FUNCTION__, new_state->name, 
> state->name);
> >> >> +                        __FUNCTION__, new_state->name, 
> state->name);
> >> >>         }
> >> >>  
> >> >> -       return omap3_enter_idle(dev, new_state ? : state);
> >> >> +       return omap3_enter_idle(dev, new_state ? 
> new_state : state);
> >> >>  }
> >> >>  
> >> >>  DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
> >> >> diff --git a/arch/arm/mach-omap2/pm34xx.c 
> >> >> b/arch/arm/mach-omap2/pm34xx.c
> >> >> index 9f73e5c..ca0600a 100644
> >> >> --- a/arch/arm/mach-omap2/pm34xx.c
> >> >> +++ b/arch/arm/mach-omap2/pm34xx.c
> >> >> @@ -734,7 +734,7 @@ static int __init pwrdms_setup(struct 
> >> >> powerdomain *pwrdm)
> >> >>                 pwrdm_enable_hdwr_sar(pwrdm);
> >> >>  
> >> >>         if (!strcmp(pwrst->pwrdm->name, "core_pwrdm") || 
> >> >> !strcmp(pwrst->pwrdm->name, "mpu_pwrdm") ||
> >> >> -           !strcmp(pwrst->pwrdm->name, "mpu_pwrdm"))
> >> >> +           !strcmp(pwrst->pwrdm->name, "neon_pwrdm"))
> >> >>                 return set_pwrdm_state(pwrst->pwrdm, 
> PWRDM_POWER_ON);
> >> >>         else
> >> >>                 return set_pwrdm_state(pwrst->pwrdm, 
> pwrst->next_state);
> >> >> -- 
> >> >> 1.5.5
> >> >> 
> >> >> 
> >> >
> >> >
> >> >
> >> 
> >> -- 
> >> Jouni Högander
> >> 
> >> 
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> > the body of a message to [EMAIL PROTECTED]
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> 
> -- 
> Jouni Högander
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to