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
