On 10 Oct 11, Vincent Guittot wrote:
On 11 October 2010 10:58, Amit Kucheria amit.kucheria@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@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
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@linaro.org wrote:
On 10 Oct 11, Vincent Guittot wrote:
On 11 October 2010 10:58, Amit Kucheria amit.kucheria@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@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
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.guittot@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.kucheria@linaro.org wrote:
On 10 Oct 11, Vincent Guittot wrote:
On 11 October 2010 10:58, Amit Kucheria amit.kucheria@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@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
-
Amit Kucheria -
Vincent Guittot