[PATCH] perf: add OMAP support for the new power events

[Jean Pihet]

Hi Kevin,

Can you please check this patch? From the previous discussions I
understood it was OK.
This one has been submitted to and l-o and l-a-k MLs. How will this
one be merged in?

Thanks,
Jean

On Mon, Feb 21, 2011 at 9:53 AM, Santosh Shilimkar
<santosh.shilimkar at ti.com> wrote:
>> -----Original Message-----
>> From: Jean Pihet [mailto:jean.pihet at newoldbits.com]
>> Sent: Monday, February 21, 2011 2:14 PM
>> To: Santosh Shilimkar
>> Cc: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org;
>> linux-arm-kernel at lists.infradead.org; Jean Pihet-XID
>> Subject: Re: [PATCH] perf: add OMAP support for the new power events
>>
>> Hi Santosh,
>>
>
> [...]
>
>> >> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-
>> >> omap2/pm34xx.c
>> >> index 2f864e4..d1cc3f4 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 "clockdomain.h"
>> >> @@ -519,8 +520,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());
>> >> +
>> >
>> > This default idle code won't be used when you enable the
>> > CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets
>> > registered.
>> That is correct. OMAP has a default idle handler (omap3_pm_idle) and
>> a
>> cpuidle handler (omap3_enter_idle in
>> arch/arm/mach-omap2/cpuidle34xx.c).
>>
>> > Shouldn't you patch that code instead? This is more or less
>> > dead code and it is just like default idle code when idle
>> > drivers isn't registered.
>> The cpuidle framework already is instrumented in a generic way. This
>> code adds the instrumentation to the default idle handler so that
>> all
>> cases are covered. BTW the patch description gives that information.
>>
>> If there is dead code then it is not only the code from this patch
>> but
>> all the code for the default idle handler.
>>
> I read your change log. It says.
>
>>> 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;
> Now code in cpuilde34xx.c is also OMAP specific and hence the
> confusion at least for me.
> Regarding dead code, I meant existing code of default handler.
>
> Thanks for clarification.
>
> Regards,
> Santosh
>