OMAP2xxx clock/CPUFreq: fix compilation errors; clean up
Paul Walmsley
maintainercover OMAP2xxx clock/CPUFreq: fix compilation errors; clean up
Hello,
This series fixes several compilation errors for OMAP2xxx chips when CONFIG_CPU_FREQ is set. These were reported and partially fixed by Felipe Balbi <[email protected]>.
Considerable work is still needed to fully support dynamic frequency changes on OMAP2xxx-series chips. Readers interested in such a project are encouraged to review the Maemo Diablo RX-34 and RX-44 kernel source at:
http://repository.maemo.org/pool/diablo/free/k/kernel-source-diablo/
Boot-tested on N800 with n8x0_defconfig with CONFIG_CPU_FREQ; compile-tested for 2430SDP.
- Paul
---
Felipe Balbi (1):
OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
Paul Walmsley (3):
OMAP2xxx OPP: clean up comments in OPP data
OMAP2 clock: dynamically allocate CPUFreq frequency table OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
arch/arm/mach-omap2/clock2xxx.c | 60 ++++++++++++++++++++++++++----- arch/arm/mach-omap2/opp2420_data.c | 38 ++++++++++---------- arch/arm/mach-omap2/opp2430_data.c | 30 ++++++++-------- arch/arm/plat-omap/clock.c | 10 +++++ arch/arm/plat-omap/cpu-omap.c | 1 + arch/arm/plat-omap/include/plat/clock.h | 2 + 6 files changed, 99 insertions(+), 42 deletions(-)
Paul Walmsley
maintainerPATCH 1/4 OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
From: Felipe Balbi <[email protected]>
if we enable CPUFREQ we can't build omap2 for two reasons, one of them is fixed by the patch below.
It's failing because the __must_be_array() check in ARRAY_SIZE() is failing and printing the following message:
arch/arm/mach-omap2/clock2xxx.c:453: error: negative width in bit-field '<anonymous>'
--- arch/arm/mach-omap2/clock2xxx.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clock2xxx.c +4 -1
@@ -455,12 +455,15 @@ static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)];void omap2_clk_init_cpufreq_table (struct cpufreq_frequency_table **table) {struct prcm_config *prcm; +long rate;int i =0 ; + sys_ck_rate =clk_get_rate (sclk); +for (prcm = rate_table; prcm->mpu_speed; prcm++) {if (!(prcm->flags & cpu_mask))continue ; -if (prcm->xtal_speed != sys_ck.rate) +if (prcm->xtal_speed != sys_ck_rate)continue ;/* don't put bypass rates in table */
Re: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
Hi,
Quoted 5 lines · ext Paul Walmsley
From: Felipe Balbi <[email protected]>
if we enable CPUFREQ we can't build omap2 for two reasons, one of them is fixed by the patch below.
Quoted 7 lines
It's failing because the
__must_be_array()check inARRAY_SIZE()is failing and printing the following message:
arch/arm/mach-omap2/clock2xxx.c:453: error: negative width in bit-field '<anonymous>'Signed-off-by Felipe Balbi <[email protected]>[[email protected]: commit message updated; changed rate variable name]
and you forgot to change the variable definition...
Quoted 10 lines
Signed-off-by Paul Walmsley <[email protected]>---
arch/arm/mach-omap2/clock2xxx.c| 5 ++++-1 files changed, 4 insertions(+), 1 deletions(-)arch/arm/mach-omap2/clock2xxx.c +0
-0
arch/arm/mach-omap2/clock2xxx.c static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)]; · line 461
Quoted 7 lines
arch/arm/mach-omap2/clock2xxx.c +2
-0@@ -455,12 +455,15 @@ static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)];void omap2_clk_init_cpufreq_table (struct cpufreq_frequency_table **table) {struct prcm_config *prcm; +long rate;int i =0 ; + sys_ck_rate =clk_get_rate (sclk);
The variable defined is called rate but you use sys_ck_rate. I used rate, because that's what other functions are using and I wanted to keep the code consistent. Anyways, as is now this patch won't compile again :-p
Signature · 1 line
balbi
Paul Walmsley
maintainerRe: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
Quoted 1 line · Felipe Balbi
and you forgot to change the variable definition...
Thanks, looks like I sent an older version of the series. I will resend.
Quoted 3 lines
The variable defined is called rate but you use
sys_ck_rate. I used rate, because that's what other functions are using and I wanted to keep the code consistent.
Did you examine the other functions omap2_round_to_table_rate() and omap2_select_table_rate()?
What advantage is there to keeping this variable named "rate", which is less specific given that this function also deals with DPLL rates, MPU rates, and crystal rates?
- Paul
Paul Walmsley
maintainerPATCH 2/4 OMAP2xxx OPP: clean up comments in OPP data
In reply to Paul WalmsleyRevise some of the comments in the OMAP2xxx OPP data for clarity.
--- arch/arm/mach-omap2/opp2420_data.c | 38 +++++++++++++++++++----------------- arch/arm/mach-omap2/opp2430_data.c | 30 ++++++++++++++-------------- 2 files changed, 35 insertions(+), 33 deletions(-)
arch/arm/mach-omap2/opp2420_data.c +20 -18
@@ -9,45 +9,47 @@* The OMAP2 processor can be run at several discrete 'PRCM configurations'. * These configurations are characterized by voltage and speed for clocks. * The device is only validated for certain combinations. One way to express -* these combinations is via the 'ratio's' which the clocks operate with +* these combinations is via the 'ratios' which the clocks operate with * respect to each other. These ratio sets are for a given voltage/DPLL -* setting. All configurations can be described by a DPLL setting and a ratio -* There are 3 ratio sets for the 2430 and X ratio sets for 2420. -* -* 2430 differs from 2420 in that there are no more phase synchronizers used. -* They both have a slightly different clock domain setup. 2420(iva1,dsp) vs -* 2430 (iva2.1, NOdsp, mdm) +* setting. All configurations can be described by a DPLL setting and a ratio. * * XXX Missing voltage data. +* XXX Missing 19.2MHz sys_clk rate sets (needed for N800/N810) * * THe format described in this file is deprecated. Once a reasonable * OPP API exists, the data in this file should be converted to use it. * * This is technically part of the OMAP2xxx clock code. +* +* Considerable work is still needed to fully support dynamic frequency +* changes on OMAP2xxx-series chips. Readers interested in such a +* project are encouraged to review the Maemo Diablo RX-34 and RX-44 +* kernel source at: +* http://repository.maemo.org/pool/diablo/free/k/kernel-source-diablo/ */ #include "opp2xxx.h" #include "sdrc.h" #include "clock.h" -/*------------------------------------------------------------------------- -* Key dividers which make up a PRCM set. Ratio's for a PRCM are mandated. +/* +* Key dividers which make up a PRCM set. Ratios for a PRCM are mandated. * xtal_speed, dpll_speed, mpu_speed, CM_CLKSEL_MPU, * CM_CLKSEL_DSP, CM_CLKSEL_GFX, CM_CLKSEL1_CORE, CM_CLKSEL1_PLL, * CM_CLKSEL2_PLL, CM_CLKSEL_MDM * -* Filling in table based on H4 boards and 2430-SDPs variants available. -* There are quite a few more rates combinations which could be defined. +* Filling in table based on H4 boards available. There are quite a +* few more rate combinations which could be defined. * -* When multiple values are defined the start up will try and choose the -* fastest one. If a 'fast' value is defined, then automatically, the /2 -* one should be included as it can be used. Generally having more that -* one fast set does not make sense, as static timings need to be changed -* to change the set. The exception is the bypass setting which is -* availble for low power bypass. +* When multiple values are defined the start up will try and choose +* the fastest one. If a 'fast' value is defined, then automatically, +* the /2 one should be included as it can be used. Generally having +* more than one fast set does not make sense, as static timings need +* to be changed to change the set. The exception is the bypass +* setting which is available for low power bypass. * * Note: This table needs to be sorted, fastest to slowest. -*-------------------------------------------------------------------------*/ +**/ const struct prcm_config omap2420_rate_table[] = {/* PRCM I - FAST */ {S12M ,S660M ,S330M ,RI_CM_CLKSEL_MPU_VAL ,/* 330MHz ARM */
arch/arm/mach-omap2/opp2430_data.c +15 -15
@@ -1,5 +1,5 @@/* -* opp2420_data.c - old-style "OPP" table for OMAP2420 +* opp2430_data.c - old-style "OPP" table for OMAP2430 * * Copyright (C) 2005-2009 Texas Instruments, Inc. * Copyright (C) 2004-2009 Nokia Corporation
@@ -9,16 +9,16 @@* The OMAP2 processor can be run at several discrete 'PRCM configurations'. * These configurations are characterized by voltage and speed for clocks. * The device is only validated for certain combinations. One way to express -* these combinations is via the 'ratio's' which the clocks operate with +* these combinations is via the 'ratios' which the clocks operate with * respect to each other. These ratio sets are for a given voltage/DPLL -* setting. All configurations can be described by a DPLL setting and a ratio -* There are 3 ratio sets for the 2430 and X ratio sets for 2420. +* setting. All configurations can be described by a DPLL setting and a ratio. * * 2430 differs from 2420 in that there are no more phase synchronizers used. * They both have a slightly different clock domain setup. 2420(iva1,dsp) vs * 2430 (iva2.1, NOdsp, mdm) * * XXX Missing voltage data. +* XXX Missing 19.2MHz sys_clk rate sets. * * THe format described in this file is deprecated. Once a reasonable * OPP API exists, the data in this file should be converted to use it.
@@ -30,24 +30,24 @@#include "sdrc.h" #include "clock.h" -/*------------------------------------------------------------------------- -* Key dividers which make up a PRCM set. Ratio's for a PRCM are mandated. +/* +* Key dividers which make up a PRCM set. Ratios for a PRCM are mandated. * xtal_speed, dpll_speed, mpu_speed, CM_CLKSEL_MPU, * CM_CLKSEL_DSP, CM_CLKSEL_GFX, CM_CLKSEL1_CORE, CM_CLKSEL1_PLL, * CM_CLKSEL2_PLL, CM_CLKSEL_MDM * -* Filling in table based on H4 boards and 2430-SDPs variants available. -* There are quite a few more rates combinations which could be defined. +* Filling in table based on 2430-SDPs variants available. There are +* quite a few more rate combinations which could be defined. * -* When multiple values are defined the start up will try and choose the -* fastest one. If a 'fast' value is defined, then automatically, the /2 -* one should be included as it can be used. Generally having more that -* one fast set does not make sense, as static timings need to be changed -* to change the set. The exception is the bypass setting which is -* availble for low power bypass. +* When multiple values are defined the start up will try and choose +* the fastest one. If a 'fast' value is defined, then automatically, +* the /2 one should be included as it can be used. Generally having +* more than one fast set does not make sense, as static timings need +* to be changed to change the set. The exception is the bypass +* setting which is available for low power bypass. * * Note: This table needs to be sorted, fastest to slowest. -*-------------------------------------------------------------------------*/ +*/ const struct prcm_config omap2430_rate_table[] = {/* PRCM #4 - ratio2 (ES2.1) - FAST */ {S13M ,S798M ,S399M ,R2_CM_CLKSEL_MPU_VAL ,/* 399MHz ARM */
Paul Walmsley
maintainerPATCH 3/4 OMAP2 clock: dynamically allocate CPUFreq frequency table
In reply to Paul WalmsleyDynamically allocate the CPUFreq frequency table on OMAP2xxx chips. This fixes some compilation problems, since the kernel may not know what chip it is running on until boot-time. This also reduces the size of the CPUFreq frequency table.
Problem reported by Felipe Balbi <[email protected]>.
--- arch/arm/mach-omap2/clock2xxx.c | 50 ++++++++++++++++++++++++++++++++------- 1 files changed, 41 insertions(+), 9 deletions(-)
arch/arm/mach-omap2/clock2xxx.c +41 -9
@@ -449,14 +449,16 @@ int omap2_select_table_rate(struct clk *clk, unsigned long rate)#ifdef CONFIG_CPU_FREQ /* * Walk PRCM rate table and fillout cpufreq freq_table +* XXX This should be replaced by an OPP layer in the near future */ -static struct cpufreq_frequency_table freq_table[ARRAY_SIZE (rate_table)]; +static struct cpufreq_frequency_table *freq_table;void omap2_clk_init_cpufreq_table (struct cpufreq_frequency_table **table) { -struct prcm_config *prcm; -long rate; +const struct prcm_config *prcm; +long sys_ck_rate;int i =0 ; +int tbl_sz =0 ; sys_ck_rate =clk_get_rate (sclk);
@@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)if (prcm->dpll_speed == prcm->xtal_speed)continue ; + tbl_sz++; + } + +/* +* XXX Ensure that we're doing what CPUFreq expects for this error +* case and the following one +*/ +if (tbl_sz ==0 ) { +pr_warning ("%s: no matching entries in rate_table\n" , + __func__); + goto ocict_exit; + } + +/* Include the CPUFREQ_TABLE_END terminator entry */ + tbl_sz++; + + freq_table =kzalloc (sizeof (struct cpufreq_frequency_table ) * tbl_sz, +GFP_KERNEL ); +if (!freq_table) { +pr_err ("%s: could not kzalloc frequency table\n" , __func__); + goto ocict_exit; + } + +for (prcm = rate_table; prcm->mpu_speed; prcm++) { +if (!(prcm->flags & cpu_mask)) +continue ; +if (prcm->xtal_speed != sys_ck_rate) +continue ; + +/* don't put bypass rates in table */ +if (prcm->dpll_speed == prcm->xtal_speed) +continue ; + freq_table[i].index = i; freq_table[i].frequency = prcm->mpu_speed /1000 ; i++; } -if (i ==0 ) { -printk (KERN_WARNING "%s: failed to initialize frequency " -"table\n" , __func__); -return ; - } - freq_table[i].index = i; freq_table[i].frequency =CPUFREQ_TABLE_END ; *table = &freq_table[0 ]; + +ocict_exit: +return ; }#endif
Re: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
Hi,
Quoted 5 lines · ext Paul Walmsley
arch/arm/mach-omap2/clock2xxx.c +1
-0@@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)if (prcm->dpll_speed == prcm->xtal_speed)continue ; + tbl_sz++;
arch/arm/mach-omap2/clock2xxx.c void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table) · line 485
Quoted 10 lines
arch/arm/mach-omap2/clock2xxx.c +10
-0+ } + +/* +* XXX Ensure that we're doing what CPUFreq expects for this error +* case and the following one +*/ +if (tbl_sz ==0 ) { +pr_warning ("%s: no matching entries in rate_table\n" , + __func__); + goto ocict_exit;
you only use this ocict_exit here, so how about only return instead of a goto to a return ?
Signature · 1 line
balbi
Paul Walmsley
maintainerRe: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
Quoted 6 lines · Felipe Balbi
Quoted 5 lines · ext Paul Walmsley
arch/arm/mach-omap2/clock2xxx.c +0
-0@@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)if (prcm->dpll_speed == prcm->xtal_speed)continue ;
Quoted 14 lines
arch/arm/mach-omap2/clock2xxx.c void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table) · line 485
Quoted 11 lines
arch/arm/mach-omap2/clock2xxx.c +11
-0+ tbl_sz++; + } + +/* +* XXX Ensure that we're doing what CPUFreq expects for this error +* case and the following one +*/ +if (tbl_sz ==0 ) { +pr_warning ("%s: no matching entries in rate_table\n" , + __func__); + goto ocict_exit;you only use this
ocict_exithere, so how about only return instead of a goto to a return ?
It's used twice. But your latter point is a good one. I will change that.
- Paul
Paul Walmsley
maintainerPATCH 4/4 OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
In reply to Paul WalmsleyOMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array, so this patch ensures that it is freed if CPUFreq terminates.
--- arch/arm/mach-omap2/clock2xxx.c | 7 +++++++ arch/arm/plat-omap/clock.c | 10 ++++++++++ arch/arm/plat-omap/cpu-omap.c | 1 + arch/arm/plat-omap/include/plat/clock.h | 2 ++ 4 files changed, 20 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock2xxx.c +7 -0
@@ -518,6 +518,12 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table) ocict_exit:return ; } + +void omap2_clk_exit_cpufreq_table (struct cpufreq_frequency_table **table) +{ +kfree (freq_table); +} +#endif struct clk_functions omap2_clk_functions = {
@@ -529,6 +535,7 @@ struct clk_functions omap2_clk_functions = { .clk_disable_unused = omap2_clk_disable_unused, #ifdef CONFIG_CPU_FREQ .clk_init_cpufreq_table = omap2_clk_init_cpufreq_table, + .clk_exit_cpufreq_table = omap2_clk_exit_cpufreq_table,#endif };
arch/arm/plat-omap/clock.c +10 -0
@@ -329,6 +329,16 @@ void clk_init_cpufreq_table(struct cpufreq_frequency_table **table) arch_clock->clk_init_cpufreq_table (table); spin_unlock_irqrestore(&clockfw_lock, flags); } + +void clk_exit_cpufreq_table (struct cpufreq_frequency_table **table) +{ +unsigned long flags; + +spin_lock_irqsave (&clockfw_lock, flags); +if (arch_clock->clk_exit_cpufreq_table) + arch_clock->clk_exit_cpufreq_table (table); +spin_unlock_irqrestore (&clockfw_lock, flags); +}#endif /*-------------------------------------------------------------------------*/
arch/arm/plat-omap/cpu-omap.c +1 -0
@@ -134,6 +134,7 @@ static int __init omap_cpu_init(struct cpufreq_policy *policy)static int omap_cpu_exit (struct cpufreq_policy *policy) { +clk_exit_cpufreq_table (&freq_table);clk_put (mpu_clk);return 0 ; }
arch/arm/plat-omap/include/plat/clock.h +2 -0
@@ -119,6 +119,7 @@ struct clk_functions {void (*clk_disable_unused)(struct clk *clk);#ifdef CONFIG_CPU_FREQ void (*clk_init_cpufreq_table)(struct cpufreq_frequency_table **); +void (*clk_exit_cpufreq_table)(struct cpufreq_frequency_table **);#endif };
@@ -135,6 +136,7 @@ extern unsigned long followparent_recalc(struct clk *clk);extern void clk_enable_init_clocks (void );#ifdef CONFIG_CPU_FREQ extern void clk_init_cpufreq_table (struct cpufreq_frequency_table **table); +extern void clk_exit_cpufreq_table (struct cpufreq_frequency_table **table);#endif extern const struct clkops clkops_null;
Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
Quoted 2 lines · ext Paul Walmsley
OMAP2xxx uses a dynamically-allocated
cpufreq_frequency_tablearray, so this patch ensures that it is freed if CPUFreq terminates.
wasn't this introduced by the previous patch ?
Signature · 1 line
balbi
Paul Walmsley
maintainerRe: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
Quoted 5 lines · Felipe Balbi
Quoted 2 lines · ext Paul Walmsley
OMAP2xxx uses a dynamically-allocated
cpufreq_frequency_tablearray, so this patch ensures that it is freed if CPUFreq terminates.wasn't this introduced by the previous patch ?
The dynamically-allocated array? Yes. Your point being?
- Paul
Paul Walmsley
maintainerRe: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
Hi Felipe,
Quoted 8 lines · Paul Walmsley
Quoted 5 lines · Felipe Balbi
Quoted 2 lines · ext Paul Walmsley
OMAP2xxx uses a dynamically-allocated
cpufreq_frequency_tablearray, so this patch ensures that it is freed if CPUFreq terminates.wasn't this introduced by the previous patch ?
The dynamically-allocated array? Yes. Your point being?
By the way, I am sorry for this - it was too brusque for how I believe collaborative development should be. Next time I will be more clear about the question...
regards,
- Paul