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? "ext Rajendra Nayak" <rnayak@xxxxxx> writes: >> -----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 > > -- 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
