Hi Jean,

On Wednesday 20 June 2012 02:16 PM, Rajendra Nayak wrote:
On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote:
Hi Rajendra,

On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayak<rna...@ti.com> wrote:
Hi Jean,


On Friday 01 June 2012 08:41 PM, Jean Pihet wrote:

For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.


How much do we really save doing this? I understand what you are doing
by looking at the patch but the changelog seems very confusing.
The gain is on the registers accesses and the internal PRCM state
machine.
If needed the changelog can be updated.

Can you explain a bit more on which register accesses are you talking
about? and some more on the PRCM state machine.

never mind, I looked at the patch again and then the cpuidle code and
figured what you are doing. Makes sense to me now :-)

regards,
Rajendra


What the patch does is get rid of a few function indirection
and assumes there is only *one* clkdm for mpu and core. Is that right?
Not exactly. The patch does not assume that there is only one clkdm
per power domain. It just allows or denies only one clkdm to idle,
which has the same effect at the power domain level.

Ok, so lets assume mpu on OMAP3 has 2 clkdms, and you allow only one
of them to idle. Will that have the same effect at the power domain
level?
The first line of the change log says "For a power domain to idle *all*
the clock domains in it must idle" and now you say allowing only *one*
clkdm to idle should have the same effect at the power domain level.
I am confused.


Thanks for reviewing,
Jean


regards,
Rajendra



The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.

Signed-off-by: Jean Pihet<j-pi...@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata
omap3_idle_data[] =
{

static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;

-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,

/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd->pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd->pwrdm_clkdms[0]);
}

/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,

/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd->pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd->pwrdm_clkdms[0]);
}

return_sleep_time:




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

Reply via email to