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