Re: [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
HI Kevin, On Thu, Sep 13, 2012 at 1:47 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Jean Pihet jean.pi...@newoldbits.com writes: Trace the power domain transitions using the functional power states, which include the power and logic states. Just to be clear, this means that a trace will only contain functional power state changes, not logical ones, correct? Correct! The trace reports functional states, while pr_err and pr_debug statements (added by patch 6/7) are present for hardcore debugging on the functional and internal states. While at it, fix the trace in the case a power domain did not hit the desired state, as reported by Paul Walmsley. What was broken here? needs a bit more description. Ok will do To me it sounds like a fix that should be a separate patch. I kept the fix in this patch since it matches $SUBJECT. Can be split if needed though. Thanks for reviewing! Jean Reported-by: Paul Walmsley p...@pwsan.com Signed-off-by: Jean Pihet j-pi...@ti.com --- arch/arm/mach-omap2/powerdomain.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 267241f..2277ad3 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm) static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) { - int prev, state, trace_state = 0; + int prev, next, state, trace_state; if (pwrdm == NULL) return -EINVAL; @@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) * If the power domain did not hit the desired state, * generate a trace event with both the desired and hit states */ - if (state != prev) { + next = pwrdm_read_next_fpwrst(pwrdm); + if (next != prev) { trace_state = (PWRDM_TRACE_STATES_FLAG | -((state OMAP_POWERSTATE_MASK) 8) | -((prev OMAP_POWERSTATE_MASK) 0)); +(next 8) | (prev 0)); trace_power_domain_target(pwrdm-name, trace_state, smp_processor_id()); } @@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst) } } + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, next_fpwrst, + smp_processor_id()); Use of smp_processor_id() here will require the same care as pointed out by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of smp_processor_id. Kevin if (logic != pwrdm_read_logic_retst(pwrdm)) pwrdm_set_logic_retst(pwrdm, logic); @@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm, spin_lock_irqsave(pwrdm-lock, flags); + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, fpwrst, + smp_processor_id()); + ret = pwrdm_set_logic_retst(pwrdm, logic); if (ret) pr_err(%s: unable to set logic state %0x of powerdomain: %s\n, @@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) pr_debug(powerdomain: setting next powerstate for %s to %0x\n, pwrdm-name, pwrst); - if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) { - /* Trace the pwrdm desired target state */ - trace_power_domain_target(pwrdm-name, pwrst, - smp_processor_id()); - /* Program the pwrdm desired target state */ + if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) ret = arch_pwrdm-pwrdm_set_next_pwrst(pwrdm, pwrst); - } return ret; } -- 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
Re: [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
On Thu, Sep 13, 2012 at 1:47 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Use of smp_processor_id() here will require the same care as pointed out by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of smp_processor_id. BTW it looks like get_cpu and put_cpu is the way to go, as pointed out by Russell. Jean -- 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
[PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
Trace the power domain transitions using the functional power states, which include the power and logic states. While at it, fix the trace in the case a power domain did not hit the desired state, as reported by Paul Walmsley. Reported-by: Paul Walmsley p...@pwsan.com Signed-off-by: Jean Pihet j-pi...@ti.com --- arch/arm/mach-omap2/powerdomain.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 267241f..2277ad3 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm) static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) { - int prev, state, trace_state = 0; + int prev, next, state, trace_state; if (pwrdm == NULL) return -EINVAL; @@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) * If the power domain did not hit the desired state, * generate a trace event with both the desired and hit states */ - if (state != prev) { + next = pwrdm_read_next_fpwrst(pwrdm); + if (next != prev) { trace_state = (PWRDM_TRACE_STATES_FLAG | - ((state OMAP_POWERSTATE_MASK) 8) | - ((prev OMAP_POWERSTATE_MASK) 0)); + (next 8) | (prev 0)); trace_power_domain_target(pwrdm-name, trace_state, smp_processor_id()); } @@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst) } } + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, next_fpwrst, + smp_processor_id()); + if (logic != pwrdm_read_logic_retst(pwrdm)) pwrdm_set_logic_retst(pwrdm, logic); @@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm, spin_lock_irqsave(pwrdm-lock, flags); + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, fpwrst, + smp_processor_id()); + ret = pwrdm_set_logic_retst(pwrdm, logic); if (ret) pr_err(%s: unable to set logic state %0x of powerdomain: %s\n, @@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) pr_debug(powerdomain: setting next powerstate for %s to %0x\n, pwrdm-name, pwrst); - if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) { - /* Trace the pwrdm desired target state */ - trace_power_domain_target(pwrdm-name, pwrst, - smp_processor_id()); - /* Program the pwrdm desired target state */ + if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) ret = arch_pwrdm-pwrdm_set_next_pwrst(pwrdm, pwrst); - } return ret; } -- 1.7.7.6 -- 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
Re: [PATCH 5/7] ARM: OMAP2+: PM debug: trace the functional power domains states
Jean Pihet jean.pi...@newoldbits.com writes: Trace the power domain transitions using the functional power states, which include the power and logic states. Just to be clear, this means that a trace will only contain functional power state changes, not logical ones, correct? While at it, fix the trace in the case a power domain did not hit the desired state, as reported by Paul Walmsley. What was broken here? needs a bit more description. To me it sounds like a fix that should be a separate patch. Reported-by: Paul Walmsley p...@pwsan.com Signed-off-by: Jean Pihet j-pi...@ti.com --- arch/arm/mach-omap2/powerdomain.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 267241f..2277ad3 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -144,7 +144,7 @@ static void _update_logic_membank_counters(struct powerdomain *pwrdm) static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) { - int prev, state, trace_state = 0; + int prev, next, state, trace_state; if (pwrdm == NULL) return -EINVAL; @@ -165,10 +165,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag) * If the power domain did not hit the desired state, * generate a trace event with both the desired and hit states */ - if (state != prev) { + next = pwrdm_read_next_fpwrst(pwrdm); + if (next != prev) { trace_state = (PWRDM_TRACE_STATES_FLAG | -((state OMAP_POWERSTATE_MASK) 8) | -((prev OMAP_POWERSTATE_MASK) 0)); +(next 8) | (prev 0)); trace_power_domain_target(pwrdm-name, trace_state, smp_processor_id()); } @@ -723,6 +723,10 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst) } } + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, next_fpwrst, + smp_processor_id()); Use of smp_processor_id() here will require the same care as pointed out by Roger Quadros in [PATCH] perf: Use raw_smp_processor_id insted of smp_processor_id. Kevin if (logic != pwrdm_read_logic_retst(pwrdm)) pwrdm_set_logic_retst(pwrdm, logic); @@ -776,6 +780,10 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm, spin_lock_irqsave(pwrdm-lock, flags); + /* Trace the pwrdm desired target state */ + trace_power_domain_target(pwrdm-name, fpwrst, + smp_processor_id()); + ret = pwrdm_set_logic_retst(pwrdm, logic); if (ret) pr_err(%s: unable to set logic state %0x of powerdomain: %s\n, @@ -821,13 +829,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst) pr_debug(powerdomain: setting next powerstate for %s to %0x\n, pwrdm-name, pwrst); - if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) { - /* Trace the pwrdm desired target state */ - trace_power_domain_target(pwrdm-name, pwrst, - smp_processor_id()); - /* Program the pwrdm desired target state */ + if (arch_pwrdm arch_pwrdm-pwrdm_set_next_pwrst) ret = arch_pwrdm-pwrdm_set_next_pwrst(pwrdm, pwrst); - } return ret; } -- 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