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