> -----Original Message----- > From: "Högander" Jouni [mailto:jouni.hogander@xxxxxxxxx] > Sent: Wednesday, July 09, 2008 12:29 PM > To: ext Rajendra Nayak > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] OMAP3: CPUIDLE & PM: check_bm fix. > > "ext Rajendra Nayak" <rnayak@xxxxxx> writes: > > > > >> -----Original Message----- > >> From: Jouni Hogander [mailto:jouni.hogander@xxxxxxxxx] > >> Sent: Wednesday, July 09, 2008 12:00 PM > >> To: linux-omap@xxxxxxxxxxxxxxx; rnayak@xxxxxx > >> 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 <jouni.hogander@xxxxxxxxx> > >> --- > >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
