| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTi=3pek3+Vv+c1TCxBH2RJcoSXq8TL8CyK9ABMeu@mail.gmail.com>
Date: Wed, 5 Jan 2011 12:05:18 +0100
From: Jean Pihet <jean.pihet@...oldbits.com>
To: Paul Walmsley <paul@...an.com>
Cc: mingo@...e.hu, trenn@...e.de,
Kevin Hilman <khilman@...prootsystems.com>,
Tony Lindgren <tony@...mide.com>,
Russell King <linux@....linux.org.uk>,
linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Arjan van de Ven <arjan@...ux.intel.com>,
linux-perf-users@...r.kernel.org, rjw@...k.pl,
Jean Pihet <j-pihet@...com>
Subject: Re: [PATCH 2/3] perf: add OMAP support for the new power events
Hi Paul,
On Tue, Jan 4, 2011 at 7:48 PM, Paul Walmsley <paul@...an.com> wrote:
> Hello Jean,
>
> On Tue, 4 Jan 2011, jean.pihet@...oldbits.com wrote:
>
>> From: Jean Pihet <j-pihet@...com>
>>
>> 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@...com>
>> ---
>> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists