[PATCH] cpufreq, cpuidle & clock for MOP500 hrefp

[Vincent Guittot]

I have updated the cpufreq_powertop_V2.patch with an enum for frequency index.

Vincent

On 11 October 2010 14:30, Amit Kucheria <amit.kucheria at linaro.org> wrote:
> On 10 Oct 11, Vincent Guittot wrote:
>> On 11 October 2010 10:58, Amit Kucheria <amit.kucheria at linaro.org> wrote:
>> > On 10 Oct 11, Vincent Guittot wrote:
>> >> Hi,
>> >>
>> >> Please find attached 3 patches for :
>> >> - enabling cpuidle feature on MOP500 hrefp
>> >> - making cpufreq stat available for powertop
>> >> - adding debugfs clock tree for powerdebug
>> >>
>> >> These patches have been tested on the latest
>> >> //git.linaro.org/bsp/st-ericsson/linux-2.6.34-ux500
>> >>
>> >> Vincent
>> >
>> > Thanks Vincent. So with these patches, the ux500 platform can be used with
>> > cpufreq and cpuidle and works correctly with the powertop and powerdebug tools
>> > we have?
>> >
>>
>> Yes it is.
>>
>> > Is the same true for the ux500 code in mainline? Does it support cpufreq,
>> > cpuidle?
>> >
>>
>> Not yet. it's on going
>>
>> > Just a brief comment below, regarding the patch.
>> >
>> >> From 5bd1f1a5ecc7ce6d812215a474869fcf2e10c1e4 Mon Sep 17 00:00:00 2001
>> >> From: Vincent Guittot <vincent.guittot at stericsson.com>
>> >> Date: Mon, 11 Oct 2010 09:23:18 +0200
>> >> Subject: [PATCH] cpufreq_getspeed can't return negative value
>> >>
>> >> ---
>> >>  arch/arm/mach-ux500/cpufreq.c |    4 +++-
>> >>  1 files changed, 3 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mach-ux500/cpufreq.c b/arch/arm/mach-ux500/cpufreq.c
>> >> index 4454a08..ea01240 100755
>> >> --- a/arch/arm/mach-ux500/cpufreq.c
>> >> +++ b/arch/arm/mach-ux500/cpufreq.c
>> >> @@ -100,7 +100,9 @@ unsigned int u8500_getspeed(unsigned int cpu)
>> >>       case ARM_50_OPP: return freq_table[1].frequency;
>> >>       case ARM_100_OPP: return freq_table[2].frequency;
>> >>       default:
>> >> -                       return -EINVAL;
>> >> +             /* During boot, the returned value is undefined */
>> >> +             /* In this case, we set the max frequency */
>> >> +             return freq_table[2].frequency;
>> >
>> > freq_table[2] will break if another frequency is added to the table. I
>> > recommend defining something like a MAX_NUM_FREQ for the table and using that
>> > in the driver.
>> >
>>
>> The idea was to return the same value than ARM_100_OPP. I could map
>> the default use case to the ARM_100_OPP one instead ?
>
> Perhaps I'm being pedantic, but I prefer using names to hard-coded numbers.
>
> So, something like a
>
>        enum freq {
>          ARM_50_OPP
>          ARM_100_OPP
>        }
>
> used with
>
>        return freq_table[ARM_100_OPP].frequency
>
> will continue to work even if you add ARM_25_OPP and ARM_75_OPP to the enum.
>
> /Amit
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpufreq_powertop_V2.patch
Type: text/x-patch
Size: 1527 bytes
Desc: not available
URL: <http://lists.linaro.org/pipermail/linaro-dev/attachments/20101012/83e4cb23/attachment.bin>