* [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up
@ 2010-01-05 2:42 Paul Walmsley
2010-01-05 2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 2:42 UTC (permalink / raw)
To: linux-omap
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 <felipe.balbi@nokia.com>.
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(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
2010-01-05 2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
@ 2010-01-05 2:42 ` Paul Walmsley
2010-01-05 10:51 ` Felipe Balbi
2010-01-05 2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 2:42 UTC (permalink / raw)
To: linux-omap; +Cc: Felipe Balbi
From: Felipe Balbi <felipe.balbi@nokia.com>
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>'
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
[paul@pwsan.com: commit message updated; changed rate variable name]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/clock2xxx.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index d0e3fb7..2f3c3ba 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -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 */
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data
2010-01-05 2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
2010-01-05 2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
@ 2010-01-05 2:42 ` Paul Walmsley
2010-01-05 2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
2010-01-05 2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
3 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 2:42 UTC (permalink / raw)
To: linux-omap; +Cc: Richard Woodruff
Revise some of the comments in the OMAP2xxx OPP data for clarity.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
---
arch/arm/mach-omap2/opp2420_data.c | 38 +++++++++++++++++++-----------------
arch/arm/mach-omap2/opp2430_data.c | 30 ++++++++++++++--------------
2 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/arch/arm/mach-omap2/opp2420_data.c b/arch/arm/mach-omap2/opp2420_data.c
index 126a939..e6dda69 100644
--- a/arch/arm/mach-omap2/opp2420_data.c
+++ b/arch/arm/mach-omap2/opp2420_data.c
@@ -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 */
diff --git a/arch/arm/mach-omap2/opp2430_data.c b/arch/arm/mach-omap2/opp2430_data.c
index edb8167..1b9596a 100644
--- a/arch/arm/mach-omap2/opp2430_data.c
+++ b/arch/arm/mach-omap2/opp2430_data.c
@@ -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 */
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
2010-01-05 2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
2010-01-05 2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
2010-01-05 2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
@ 2010-01-05 2:42 ` Paul Walmsley
2010-01-05 10:42 ` Felipe Balbi
2010-01-05 2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 2:42 UTC (permalink / raw)
To: linux-omap; +Cc: Felipe Balbi
Dynamically 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 <felipe.balbi@nokia.com>.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Felipe Balbi <felipe.balbi@nokia.com>
---
arch/arm/mach-omap2/clock2xxx.c | 50 ++++++++++++++++++++++++++++++++-------
1 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index 2f3c3ba..5c21afe 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
2010-01-05 2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
` (2 preceding siblings ...)
2010-01-05 2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
@ 2010-01-05 2:42 ` Paul Walmsley
2010-01-05 10:46 ` Felipe Balbi
3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 2:42 UTC (permalink / raw)
To: linux-omap
OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
so this patch ensures that it is freed if CPUFreq terminates.
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
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(-)
diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index 5c21afe..ec57927 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -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
};
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index a64d84c..d9f8c84 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -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
/*-------------------------------------------------------------------------*/
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index f8ddbdd..6d3d333 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -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;
}
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index 309b6d1..94fe2a0 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -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;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
2010-01-05 2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
@ 2010-01-05 10:42 ` Felipe Balbi
2010-01-05 16:22 ` Paul Walmsley
0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:42 UTC (permalink / raw)
To: ext Paul Walmsley
Cc: linux-omap@vger.kernel.org, Balbi Felipe (Nokia-D/Helsinki)
Hi,
On Tue, Jan 05, 2010 at 03:42:07AM +0100, ext Paul Walmsley wrote:
>@@ -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;
you only use this ocict_exit here, so how about only return instead of a
goto to a return ?
--
balbi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
2010-01-05 2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
@ 2010-01-05 10:46 ` Felipe Balbi
2010-01-05 15:58 ` Paul Walmsley
0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:46 UTC (permalink / raw)
To: ext Paul Walmsley; +Cc: linux-omap@vger.kernel.org
On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
>OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
>so this patch ensures that it is freed if CPUFreq terminates.
wasn't this introduced by the previous patch ?
--
balbi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
2010-01-05 2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
@ 2010-01-05 10:51 ` Felipe Balbi
2010-01-05 16:03 ` Paul Walmsley
0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:51 UTC (permalink / raw)
To: ext Paul Walmsley
Cc: linux-omap@vger.kernel.org, Balbi Felipe (Nokia-D/Helsinki)
Hi,
On Tue, Jan 05, 2010 at 03:42:05AM +0100, ext Paul Walmsley wrote:
>From: Felipe Balbi <felipe.balbi@nokia.com>
>
>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>'
>
>Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
>[paul@pwsan.com: commit message updated; changed rate variable name]
and you forgot to change the variable definition...
>Signed-off-by: Paul Walmsley <paul@pwsan.com>
>---
> arch/arm/mach-omap2/clock2xxx.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
>index d0e3fb7..2f3c3ba 100644
>--- a/arch/arm/mach-omap2/clock2xxx.c
>+++ b/arch/arm/mach-omap2/clock2xxx.c
>@@ -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
--
balbi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
2010-01-05 10:46 ` Felipe Balbi
@ 2010-01-05 15:58 ` Paul Walmsley
2010-01-19 19:26 ` Paul Walmsley
0 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 15:58 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org
On Tue, 5 Jan 2010, Felipe Balbi wrote:
> On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
> > OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
> > 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
2010-01-05 10:51 ` Felipe Balbi
@ 2010-01-05 16:03 ` Paul Walmsley
0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 16:03 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org
On Tue, 5 Jan 2010, Felipe Balbi wrote:
> and you forgot to change the variable definition...
Thanks, looks like I sent an older version of the series. I will resend.
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
2010-01-05 10:42 ` Felipe Balbi
@ 2010-01-05 16:22 ` Paul Walmsley
0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 16:22 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org
On Tue, 5 Jan 2010, Felipe Balbi wrote:
> On Tue, Jan 05, 2010 at 03:42:07AM +0100, ext Paul Walmsley wrote:
> > @@ -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;
>
> you only use this ocict_exit here, 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
2010-01-05 15:58 ` Paul Walmsley
@ 2010-01-19 19:26 ` Paul Walmsley
0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-19 19:26 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org
Hi Felipe,
On Tue, 5 Jan 2010, Paul Walmsley wrote:
> On Tue, 5 Jan 2010, Felipe Balbi wrote:
> > On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
> > > OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
> > > 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-01-19 19:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-05 2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
2010-01-05 2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
2010-01-05 10:51 ` Felipe Balbi
2010-01-05 16:03 ` Paul Walmsley
2010-01-05 2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
2010-01-05 2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
2010-01-05 10:42 ` Felipe Balbi
2010-01-05 16:22 ` Paul Walmsley
2010-01-05 2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
2010-01-05 10:46 ` Felipe Balbi
2010-01-05 15:58 ` Paul Walmsley
2010-01-19 19:26 ` Paul Walmsley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox