Patch looks OK. I'm not sure who is working to integrate the ST-E
kernel into the common Linaro tree though. We want these 3 patches
included there, and if relevant sent to mainline.

To repeat my question since I'm still not clear, is there
cpufreq/cpuidle support for ux500 in mainline?

Regards,
Amit

On Tue, Oct 12, 2010 at 10:35 AM, Vincent Guittot
<vincent.guit...@linaro.org> wrote:
> I have updated the cpufreq_powertop_V2.patch with an enum for frequency index.
>
> Vincent
>
> On 11 October 2010 14:30, Amit Kucheria <amit.kuche...@linaro.org> wrote:
>> On 10 Oct 11, Vincent Guittot wrote:
>>> On 11 October 2010 10:58, Amit Kucheria <amit.kuche...@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.guit...@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
>>
>>
>

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to