Hi Paul,
On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@xxxxxxxxx> wrote:
> Hello Jean,
>
> On Tue, 4 Jan 2011, jean.pihet@xxxxxxxxxxxxxx wrote:
>
>> From: Jean Pihet <j-pihet@xxxxxx>
>>
>> The patch adds the new power management trace points for
>> the OMAP architecture.
>>
>> The trace points are for:
>> - default idle handler. Since the cpuidle framework is
>> instrumented in the generic way there is no need to
>> add trace points in the OMAP specific cpuidle handler;
>> - cpufreq (DVFS),
>> - clocks changes (enable, disable, set_rate),
>
> A question about these. Are these only meant to track calls to these
> functions from outside the clock code? Or meant to track actual hardware
> clock changes?
The former: this is used to track the clock requests from outside the
clock framework.
> If the latter, then it might make sense to put these
> trace points into the functions that actually change the hardware
> registers, e.g., omap2_dflt_clk_{enable,disable}(), etc., since a
> clk_enable() on a leaf clock may result in many internal system clocks
> being enabled up the clock tree.
I agree with you it is better to track the actual clock changes instead.
I propose to move the tracepoints to omap2_clk_{enable...} which
enables all the clocks irrespectively of the installed handler.
Note about the clock handlers: omap2_dflt_clk_enable happens to be the
handler for all controllable clocks but could that change in the
future?
>
>
> - Paul
Thanks,
Jean
>
>> - change of power domains next power states.
>>
>> Signed-off-by: Jean Pihet <j-pihet@xxxxxx>
>> ---
>> arch/arm/mach-omap2/pm34xx.c | 7 +++++++
>> arch/arm/mach-omap2/powerdomain.c | 3 +++
>> arch/arm/plat-omap/clock.c | 13 ++++++++++---
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 0ec8a04..0ee0b0e 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -29,6 +29,7 @@
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> #include <linux/console.h>
>> +#include <trace/events/power.h>
>>
>> #include <plat/sram.h>
>> #include <plat/clockdomain.h>
>> @@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
>> if (omap_irq_pending() || need_resched())
>> goto out;
>>
>> + trace_power_start(POWER_CSTATE, 1, smp_processor_id());
>> + trace_cpu_idle(1, smp_processor_id());
>> +
>> omap_sram_idle();
>>
>> + trace_power_end(smp_processor_id());
>> + trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
>> +
>> out:
>> local_fiq_enable();
>> local_irq_enable();
>> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
>> index 6527ec3..73cbe9a 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -23,6 +23,7 @@
>> #include <linux/errno.h>
>> #include <linux/err.h>
>> #include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> #include <asm/atomic.h>
>>
>> @@ -440,6 +441,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);
>>
>> + trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
>> +
>> prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
>> (pwrst << OMAP_POWERSTATE_SHIFT),
>> pwrdm->prcm_offs, pwrstctrl_reg_offs);
>> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
>> index fc62fb5..7cbb09b 100644
>> --- a/arch/arm/plat-omap/clock.c
>> +++ b/arch/arm/plat-omap/clock.c
>> @@ -21,6 +21,7 @@
>> #include <linux/cpufreq.h>
>> #include <linux/debugfs.h>
>> #include <linux/io.h>
>> +#include <trace/events/power.h>
>>
>> #include <plat/clock.h>
>>
>> @@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
>> return -EINVAL;
>>
>> spin_lock_irqsave(&clockfw_lock, flags);
>> - if (arch_clock->clk_enable)
>> + if (arch_clock->clk_enable) {
>> + trace_clock_enable(clk->name, 1, smp_processor_id());
>> ret = arch_clock->clk_enable(clk);
>> + }
>> spin_unlock_irqrestore(&clockfw_lock, flags);
>>
>> return ret;
>> @@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
>> goto out;
>> }
>>
>> - if (arch_clock->clk_disable)
>> + if (arch_clock->clk_disable) {
>> + trace_clock_disable(clk->name, 0, smp_processor_id());
>> arch_clock->clk_disable(clk);
>> + }
>>
>> out:
>> spin_unlock_irqrestore(&clockfw_lock, flags);
>> @@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
>> return ret;
>>
>> spin_lock_irqsave(&clockfw_lock, flags);
>> - if (arch_clock->clk_set_rate)
>> + if (arch_clock->clk_set_rate) {
>> + trace_clock_set_rate(clk->name, rate, smp_processor_id());
>> ret = arch_clock->clk_set_rate(clk, rate);
>> + }
>> if (ret == 0) {
>> if (clk->recalc)
>> clk->rate = clk->recalc(clk);
>> --
>> 1.7.2.3
>>
>
>
> - Paul
>
--
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
